>From 1d404e0c9256b6e18ca4ac9e8e663a611ab7e256 Mon Sep 17 00:00:00 2001 From: Jackson Ray Hamilton
Date: Sat, 9 Feb 2019 20:06:29 -0800 Subject: [PATCH 02/19] Refactor JSX indentation code to improve enclosing JSX discovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix a number of bugs reported for JSX indentation (caused by poor JSX detection): - https://github.com/mooz/js2-mode/issues/140#issuecomment-166250016 - https://github.com/mooz/js2-mode/issues/490 - Bug#24896 / https://github.com/mooz/js2-mode/issues/389 (with respect to comments) - Bug#26001 / https://github.com/mooz/js2-mode/issues/389#issuecomment-271869380 - https://github.com/mooz/js2-mode/issues/411 / Bug#27000 / https://github.com/mooz/js2-mode/issues/451 Potentially manifest some new bugs (due to false positives with â<â and â>â and SGML detection). Slow down indentation a fair bit. * list/progmodes/js.el (js-jsx-syntax, js--jsx-start-tag-re) (js--looking-at-jsx-start-tag-p, js--looking-back-at-jsx-end-tag-p): New variables and functions. (js--jsx-find-before-tag, js--jsx-after-tag-re): Deleted. (js--looking-at-operator-p): Donât mistake a JSXOpeningElement for the â<â operator. (js--continued-expression-p): Donât mistake a JSXClosingElement as a fragment of a continued expression including the â>â operator. (js--as-sgml): Simplify. Probably neednât bind forward-sexp-function to nil (sgml-mode already does) and probably shouldnât bind parse-sexp-lookup-properties to nil either (see Bug#24896). (js--outermost-enclosing-jsx-tag-pos): Find enclosing JSX more accurately than js--jsx-find-before-tag. Use sgml-modeâs parsing logic, rather than unreliable heuristics like paren-wrapping. This implementation is much slower; the previous implementation was fast, but at the expense of accuracy. To make up for all the grief weâve caused users, we will prefer accuracy over speed from now on. That said, this can still probably be optimized a lot. (js--jsx-indented-element-p): Rename to js--jsx-indentation, since it doesnât just return a boolean. (js--jsx-indentation): Refactor js--jsx-indented-element-p to simplify the implementation as the improved accuracy of other code allows (and to repent for some awful stylistic choices I made earlier). (js--expression-in-sgml-indent-line): Rename to js--indent-line-in-jsx-expression, since itâs a private function and we can give it a name that reads more like English. (js--indent-line-in-jsx-expression): Restructure point adjustment logic more like js-indent-line. (js--indent-n+1th-jsx-line): New function to complement js--indent-line-in-jsx-expression. (js-jsx-indent-line): Refactor. Donât bind js--continued-expression-p to ignore any more; instead, rely on the improved accuracy of js--continued-expression-p. (js-jsx-mode): Set js-jsx-syntax to t. For now, this will be the flag we use to determine whether âJSX is enabled.â (Maybe later, we will refactor the code to use this variable instead of requiring js-jsx-mode to be enabled, thus rendering the mode obsolete.) --- lisp/progmodes/js.el | 337 +++++++++++++++++++++------------------------------ 1 file changed, 141 insertions(+), 196 deletions(-) diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el index 4d91da7334..5b992535a8 100644 --- a/lisp/progmodes/js.el +++ b/lisp/progmodes/js.el @@ -572,6 +572,15 @@ js-chain-indent :safe 'booleanp :group 'js) +(defcustom js-jsx-syntax nil + "When non-nil, parse JavaScript with consideration for JSX syntax. +This fixes indentation of JSX code in some cases. It is set to +be buffer-local when in `js-jsx-mode'." + :version "27.1" + :type 'boolean + :safe 'booleanp + :group 'js) + ;;; KeyMap (defvar js-mode-map @@ -1774,6 +1783,14 @@ js--indent-operator-re (js--regexp-opt-symbol '("in" "instanceof"))) "Regexp matching operators that affect indentation of continued expressions.") +(defconst js--jsx-start-tag-re + (concat "<" sgml-name-re) + "Regexp matching code that looks like a JSXOpeningElement.") + +(defun js--looking-at-jsx-start-tag-p () + "Non-nil if a JSXOpeningElement immediately follows point." + (looking-at js--jsx-start-tag-re)) + (defun js--looking-at-operator-p () "Return non-nil if point is on a JavaScript operator, other than a comma." (save-match-data @@ -1796,7 +1813,9 @@ js--looking-at-operator-p (js--backward-syntactic-ws) ;; We might misindent some expressions that would ;; return NaN anyway. Shouldn't be a problem. - (memq (char-before) '(?, ?} ?{)))))))) + (memq (char-before) '(?, ?} ?{))))) + ;; â<â isnât necessarily an operator in JSX. + (not (and js-jsx-syntax (js--looking-at-jsx-start-tag-p)))))) (defun js--find-newline-backward () "Move backward to the nearest newline that is not in a block comment." @@ -1816,6 +1835,14 @@ js--find-newline-backward (setq result nil))) result)) +(defconst js--jsx-end-tag-re + (concat "" sgml-name-re ">\\|/>") + "Regexp matching a JSXClosingElement.") + +(defun js--looking-back-at-jsx-end-tag-p () + "Non-nil if a JSXClosingElement immediately precedes point." + (looking-back js--jsx-end-tag-re (point-at-bol))) + (defun js--continued-expression-p () "Return non-nil if the current line continues an expression." (save-excursion @@ -1833,12 +1860,19 @@ js--continued-expression-p (and (js--find-newline-backward) (progn (skip-chars-backward " \t") - (or (bobp) (backward-char)) - (and (> (point) (point-min)) - (save-excursion (backward-char) (not (looking-at "[/*]/\\|=>"))) - (js--looking-at-operator-p) - (and (progn (backward-char) - (not (looking-at "\\+\\+\\|--\\|/[/*]")))))))))) + (and + ;; The â>â at the end of any JSXBoundaryElement isnât + ;; part of a continued expression. + (not (and js-jsx-syntax (js--looking-back-at-jsx-end-tag-p))) + (progn + (or (bobp) (backward-char)) + (and (> (point) (point-min)) + (save-excursion + (backward-char) + (not (looking-at "[/*]/\\|=>"))) + (js--looking-at-operator-p) + (and (progn (backward-char) + (not (looking-at "\\+\\+\\|--\\|/[/*]")))))))))))) (defun js--skip-term-backward () "Skip a term before point; return t if a term was skipped." @@ -2153,190 +2187,108 @@ js--proper-indentation ;;; JSX Indentation -(defsubst js--jsx-find-before-tag () - "Find where JSX starts. - -Assume JSX appears in the following instances: -- Inside parentheses, when returned or as the first argument - to a function, and after a newline -- When assigned to variables or object properties, but only - on a single line -- As the N+1th argument to a function - -This is an optimized version of (re-search-backward \"[(,]\n\" -nil t), except set point to the end of the match. This logic -executes up to the number of lines in the file, so it should be -really fast to reduce that impact." - (let (pos) - (while (and (> (point) (point-min)) - (not (progn - (end-of-line 0) - (when (or (eq (char-before) 40) ; ( - (eq (char-before) 44)) ; , - (setq pos (1- (point)))))))) - pos)) - -(defconst js--jsx-end-tag-re - (concat "" sgml-name-re ">\\|/>") - "Find the end of a JSX element.") - -(defconst js--jsx-after-tag-re "[),]" - "Find where JSX ends. -This complements the assumption of where JSX appears from -`js--jsx-before-tag-re', which see.") - -(defun js--jsx-indented-element-p () +(defmacro js--as-sgml (&rest body) + "Execute BODY as if in sgml-mode." + `(with-syntax-table sgml-mode-syntax-table + ,@body)) + +(defun js--outermost-enclosing-jsx-tag-pos () + (let (context tag-pos last-tag-pos parse-status parens paren-pos curly-pos) + (js--as-sgml + ;; Search until we reach the top or encounter the start of a + ;; JSXExpressionContainer (implying nested JSX). + (while (and (setq context (sgml-get-context)) + (progn + (setq tag-pos (sgml-tag-start (car (last context)))) + (or (not curly-pos) + ;; Stop before curly brackets (start of a + ;; JSXExpressionContainer). + (> tag-pos curly-pos)))) + ;; Record this position so it can potentially be returned. + (setq last-tag-pos tag-pos) + ;; Always parse sexps / search for the next context from the + ;; immediately enclosing tag (sgml-get-context may not leave + ;; point there). + (goto-char tag-pos) + (unless parse-status ; Donât needlessly reparse. + ;; Search upward for an enclosing starting curly bracket. + (setq parse-status (syntax-ppss)) + (setq parens (reverse (nth 9 parse-status))) + (while (and (setq paren-pos (car parens)) + (not (when (= (char-after paren-pos) ?{) + (setq curly-pos paren-pos)))) + (setq parens (cdr parens))) + ;; Always search for the next context from the immediately + ;; enclosing tag (calling syntax-ppss in the above loop + ;; may move point from there). + (goto-char tag-pos)))) + last-tag-pos)) + +(defun js--jsx-indentation () "Determine if/how the current line should be indented as JSX. -Return `first' for the first JSXElement on its own line. -Return `nth' for subsequent lines of the first JSXElement. -Return `expression' for an embedded JS expression. -Return `after' for anything after the last JSXElement. -Return nil for non-JSX lines. - -Currently, JSX indentation supports the following styles: - -- Single-line elements (indented like normal JS): - - var element = ; - -- Multi-line elements (enclosed in parentheses): - - function () { - return ( -