From 375dcc9f12302977fabc07e107efc68d62dfd211 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Sat, 24 Nov 2018 19:22:26 +0900 Subject: [PATCH 1/2] Fix: improve comment indentation (fixes #514) --- lib/utils/indent-common.js | 105 ++++++++++++++++-------- tests/fixtures/html-indent/issue514.vue | 8 ++ 2 files changed, 78 insertions(+), 35 deletions(-) create mode 100644 tests/fixtures/html-indent/issue514.vue diff --git a/lib/utils/indent-common.js b/lib/utils/indent-common.js index e705e26d7..66f9bb0ef 100644 --- a/lib/utils/indent-common.js +++ b/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. @@ -294,7 +304,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 @@ -332,14 +342,18 @@ module.exports.defineVisitor = function create (context, tokenStore, defaultOpti * @param {Node|null} leftToken The left parenthesis token. * @param {Node|null} rightToken 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) { @@ -350,30 +364,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) { @@ -385,7 +419,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 { @@ -633,10 +667,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) @@ -668,7 +702,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 }, @@ -692,7 +726,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)) { @@ -701,10 +735,7 @@ module.exports.defineVisitor = function create (context, tokenStore, defaultOpti //
// //
- return { - primary: nextExpectedIndent + options.indentSize, - secondary: undefined - } + return [nextExpectedIndent + options.indentSize, nextExpectedIndent] } // For last comment. E.g., @@ -712,7 +743,7 @@ module.exports.defineVisitor = function create (context, tokenStore, defaultOpti //
// // - return { primary: lastExpectedIndent, secondary: nextExpectedIndent } + return [lastExpectedIndent, nextExpectedIndent] } // Adjust to next normally. E.g., @@ -720,7 +751,7 @@ module.exports.defineVisitor = function create (context, tokenStore, defaultOpti // //
// - return { primary: nextExpectedIndent, secondary: undefined } + return [nextExpectedIndent] } /** @@ -786,11 +817,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) } @@ -815,14 +852,13 @@ 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) } - setOffset(endTagToken, 0, startTagToken) }, VEndTag (node) { @@ -1081,7 +1117,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) }, @@ -1509,7 +1544,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) diff --git a/tests/fixtures/html-indent/issue514.vue b/tests/fixtures/html-indent/issue514.vue new file mode 100644 index 000000000..ede217b2a --- /dev/null +++ b/tests/fixtures/html-indent/issue514.vue @@ -0,0 +1,8 @@ + + From 3569a650870fce18631b2119f97ac3458e9b01b6 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Sat, 24 Nov 2018 23:27:33 +0900 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=93=9D=20fix=20JSDoc=20comment?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/utils/indent-common.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/utils/indent-common.js b/lib/utils/indent-common.js index 66f9bb0ef..a330a1acf 100644 --- a/lib/utils/indent-common.js +++ b/lib/utils/indent-common.js @@ -339,8 +339,8 @@ 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 {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}