Fix: improve comment indentation (fixes #514) (#676)

* Fix: improve comment indentation (fixes #514)

* 📝 fix JSDoc comment

Author: mysticatea

lib/utils/indent-common.js

@@ -18,6 +18,7 @@ const KNOWN_NODES = new Set(['ArrayExpression', 'ArrayPattern', 'ArrowFunctionEx
 const LT_CHAR = /[\r\n\u2028\u2029]/
 const LINES = /[^\r\n\u2028\u2029]+(?:$|\r\n|[\r\n\u2028\u2029])/g
 const BLOCK_COMMENT_PREFIX = /^\s*\*/
+const ITERATION_OPTS = Object.freeze({ includeComments: true, filter: isNotWhitespace })
 
 /**
  * Normalize options.
@@ -194,6 +195,15 @@ function isNotComment (token) {
   return token != null && token.type !== 'Block' && token.type !== 'Line' && token.type !== 'Shebang' && !token.type.endsWith('Comment')
 }
 
+/**
+ * Check whether the given node is not an empty text node.
+ * @param {Node} node The node to check.
+ * @returns {boolean} `false` if the token is empty text node.
+ */
+function isNotEmptyTextNode (node) {
+  return !(node.type === 'VText' && node.value.trim() === '')
+}
+
 /**
  * Get the last element.
  * @param {Array} xs The array to get the last element.
@@ -295,7 +305,7 @@ module.exports.defineVisitor = function create (context, tokenStore, defaultOpti
    * @param {Token} token The token to set.
    * @returns {void}
    */
-  function setBaseline (token, hardTabAdditional) {
+  function setBaseline (token) {
     const offsetInfo = offsets.get(token)
     if (offsetInfo != null) {
       offsetInfo.baseline = true
@@ -353,17 +363,21 @@ module.exports.defineVisitor = function create (context, tokenStore, defaultOpti
    * The first node is offsetted from the given left token.
    * Rest nodes are adjusted to the first node.
    * @param {Node[]} nodeList The node to process.
-   * @param {Node|null} leftToken The left parenthesis token.
-   * @param {Node|null} rightToken The right parenthesis token.
+   * @param {Node|Token|null} left The left parenthesis token.
+   * @param {Node|Token|null} right The right parenthesis token.
    * @param {number} offset The offset to set.
-   * @param {Node} [alignVertically=true] The flag to align vertically. If `false`, this doesn't align vertically even if the first node is not at beginning of line.
+   * @param {boolean} [alignVertically=true] The flag to align vertically. If `false`, this doesn't align vertically even if the first node is not at beginning of line.
    * @returns {void}
    */
-  function processNodeList (nodeList, leftToken, rightToken, offset, alignVertically) {
+  function processNodeList (nodeList, left, right, offset, alignVertically) {
     let t
+    const leftToken = (left && tokenStore.getFirstToken(left)) || left
+    const rightToken = (right && tokenStore.getFirstToken(right)) || right
 
     if (nodeList.length >= 1) {
-      let lastToken = leftToken
+      let baseToken = null
+      let lastToken = left
+      const alignTokensBeforeBaseToken = []
       const alignTokens = []
 
       for (let i = 0; i < nodeList.length; ++i) {
@@ -374,30 +388,50 @@ module.exports.defineVisitor = function create (context, tokenStore, defaultOpti
         }
         const elementTokens = getFirstAndLastTokens(node, lastToken != null ? lastToken.range[1] : 0)
 
-        // Collect related tokens.
-        // Commas between this and the previous, and the first token of this node.
+        // Collect comma/comment tokens between the last token of the previous node and the first token of this node.
         if (lastToken != null) {
           t = lastToken
-          while ((t = tokenStore.getTokenAfter(t)) != null && t.range[1] <= elementTokens.firstToken.range[0]) {
-            alignTokens.push(t)
+          while (
+            (t = tokenStore.getTokenAfter(t, ITERATION_OPTS)) != null &&
+            t.range[1] <= elementTokens.firstToken.range[0]
+          ) {
+            if (baseToken == null) {
+              alignTokensBeforeBaseToken.push(t)
+            } else {
+              alignTokens.push(t)
+            }
           }
         }
-        alignTokens.push(elementTokens.firstToken)
 
-        // Save the last token to find tokens between the next token.
+        if (baseToken == null) {
+          baseToken = elementTokens.firstToken
+        } else {
+          alignTokens.push(elementTokens.firstToken)
+        }
+
+        // Save the last token to find tokens between this node and the next node.
         lastToken = elementTokens.lastToken
       }
 
-      // Check trailing commas.
+      // Check trailing commas and comments.
       if (rightToken != null && lastToken != null) {
         t = lastToken
-        while ((t = tokenStore.getTokenAfter(t)) != null && t.range[1] <= rightToken.range[0]) {
-          alignTokens.push(t)
+        while (
+          (t = tokenStore.getTokenAfter(t, ITERATION_OPTS)) != null &&
+          t.range[1] <= rightToken.range[0]
+        ) {
+          if (baseToken == null) {
+            alignTokensBeforeBaseToken.push(t)
+          } else {
+            alignTokens.push(t)
+          }
         }
       }
 
       // Set offsets.
-      const baseToken = alignTokens.shift()
+      if (leftToken != null) {
+        setOffset(alignTokensBeforeBaseToken, offset, leftToken)
+      }
       if (baseToken != null) {
         // Set offset to the first token.
         if (leftToken != null) {
@@ -409,7 +443,7 @@ module.exports.defineVisitor = function create (context, tokenStore, defaultOpti
           setBaseline(baseToken)
         }
 
-        if (alignVertically === false) {
+        if (alignVertically === false && leftToken != null) {
           // Align tokens relatively to the left token.
           setOffset(alignTokens, offset, leftToken)
         } else {
@@ -657,10 +691,10 @@ module.exports.defineVisitor = function create (context, tokenStore, defaultOpti
    * Validate the given token with the pre-calculated expected indentation.
    * @param {Token} token The token to validate.
    * @param {number} expectedIndent The expected indentation.
-   * @param {number|undefined} optionalExpectedIndent The optional expected indentation.
+   * @param {number[]|undefined} optionalExpectedIndents The optional expected indentation.
    * @returns {void}
    */
-  function validateCore (token, expectedIndent, optionalExpectedIndent) {
+  function validateCore (token, expectedIndent, optionalExpectedIndents) {
     const line = token.loc.start.line
     const indentText = getIndentText(token)
 
@@ -692,7 +726,7 @@ module.exports.defineVisitor = function create (context, tokenStore, defaultOpti
       }
     }
 
-    if (actualIndent !== expectedIndent && (optionalExpectedIndent === undefined || actualIndent !== optionalExpectedIndent)) {
+    if (actualIndent !== expectedIndent && (optionalExpectedIndents == null || !optionalExpectedIndents.includes(actualIndent))) {
       context.report({
         loc: {
           start: { line, column: 0 },
@@ -716,7 +750,7 @@ module.exports.defineVisitor = function create (context, tokenStore, defaultOpti
    * @param {Token|null} nextToken The next token of comments.
    * @param {number|undefined} nextExpectedIndent The expected indent of the next token.
    * @param {number|undefined} lastExpectedIndent The expected indent of the last token.
-   * @returns {{primary:number|undefined,secondary:number|undefined}}
+   * @returns {number[]}
    */
   function getCommentExpectedIndents (nextToken, nextExpectedIndent, lastExpectedIndent) {
     if (typeof lastExpectedIndent === 'number' && isClosingToken(nextToken)) {
@@ -725,26 +759,23 @@ module.exports.defineVisitor = function create (context, tokenStore, defaultOpti
         // <div>
         //    <!-- comment -->
         // </div>
-        return {
-          primary: nextExpectedIndent + options.indentSize,
-          secondary: undefined
-        }
+        return [nextExpectedIndent + options.indentSize, nextExpectedIndent]
       }
 
       // For last comment. E.g.,
       // <div>
       //    <div></div>
       //    <!-- comment -->
       // </div>
-      return { primary: lastExpectedIndent, secondary: nextExpectedIndent }
+      return [lastExpectedIndent, nextExpectedIndent]
     }
 
     // Adjust to next normally. E.g.,
     // <div>
     //    <!-- comment -->
     //    <div></div>
     // </div>
-    return { primary: nextExpectedIndent, secondary: undefined }
+    return [nextExpectedIndent]
   }
 
   /**
@@ -815,11 +846,17 @@ module.exports.defineVisitor = function create (context, tokenStore, defaultOpti
     // It allows the same indent level with the previous line.
     const lastOffsetInfo = offsets.get(lastToken)
     const lastExpectedIndent = lastOffsetInfo && lastOffsetInfo.expectedIndent
-    const commentExpectedIndents = getCommentExpectedIndents(firstToken, expectedIndent, lastExpectedIndent)
+    const commentOptionalExpectedIndents = getCommentExpectedIndents(firstToken, expectedIndent, lastExpectedIndent)
 
     // Validate.
     for (const comment of comments) {
-      validateCore(comment, commentExpectedIndents.primary, commentExpectedIndents.secondary)
+      const commentExpectedIndents = getExpectedIndents([comment])
+      const commentExpectedIndent =
+        commentExpectedIndents
+          ? commentExpectedIndents.expectedIndent
+          : commentOptionalExpectedIndents[0]
+
+      validateCore(comment, commentExpectedIndent, commentOptionalExpectedIndents)
     }
     validateCore(firstToken, expectedIndent)
   }
@@ -844,16 +881,14 @@ module.exports.defineVisitor = function create (context, tokenStore, defaultOpti
     },
 
     VElement (node) {
-      const startTagToken = tokenStore.getFirstToken(node)
-      const endTagToken = node.endTag && tokenStore.getFirstToken(node.endTag)
-
       if (node.name !== 'pre') {
-        const childTokens = node.children.map(n => tokenStore.getFirstToken(n))
-        setOffset(childTokens, 1, startTagToken)
+        processNodeList(node.children.filter(isNotEmptyTextNode), node.startTag, node.endTag, 1)
       } else {
+        const startTagToken = tokenStore.getFirstToken(node)
+        const endTagToken = node.endTag && tokenStore.getFirstToken(node.endTag)
+        setOffset(endTagToken, 0, startTagToken)
         setPreformattedTokens(node)
       }
-      setOffset(endTagToken, 0, startTagToken)
     },
 
     VEndTag (node) {
@@ -1116,7 +1151,6 @@ module.exports.defineVisitor = function create (context, tokenStore, defaultOpti
 
       setOffset(leftParenToken, 1, forToken)
       processNodeList([node.init, node.test, node.update], leftParenToken, rightParenToken, 1)
-      setOffset(rightParenToken, 0, leftParenToken)
       processMaybeBlock(node.body, forToken)
     },
 
@@ -1544,7 +1578,7 @@ module.exports.defineVisitor = function create (context, tokenStore, defaultOpti
       let lastValidatedToken = null
 
       // Validate indentation of tokens.
-      for (const token of tokenStore.getTokens(node, { includeComments: true, filter: isNotWhitespace })) {
+      for (const token of tokenStore.getTokens(node, ITERATION_OPTS)) {
         if (tokensOnSameLine.length === 0 || tokensOnSameLine[0].loc.start.line === token.loc.start.line) {
           // This is on the same line (or the first token).
           tokensOnSameLine.push(token)

tests/fixtures/html-indent/issue514.vue

@@ -0,0 +1,8 @@
+<!--{}-->
+<template>
+  <div>
+    <input type="text"
+           value="foo">
+    <!-- comment -->
+  </div>
+</template>