From 4564c92b605d583104674eb608f27196aead4e07 Mon Sep 17 00:00:00 2001 From: Nathan Walters Date: Tue, 10 Aug 2021 12:21:20 -0700 Subject: [PATCH 1/8] Add test to reproduce error --- test/parser/comments.test.js | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/test/parser/comments.test.js b/test/parser/comments.test.js index e3be6dc..5751a83 100644 --- a/test/parser/comments.test.js +++ b/test/parser/comments.test.js @@ -179,3 +179,20 @@ test('inline comments with asterisk are persisted (#135)', (t) => { t.is(first.text, '*batman'); t.is(nodeToString(root), less); }); + +test.only('handles single quotes in comments', (t) => { + const less = `.a {\n // '\n outline-width: 0 !important;\n}\n\n/** ' */`; + + const root = parse(less); + const { first } = root; + + const [commentNode, declarationNode] = first.nodes; + + t.is(commentNode.type, 'comment'); + + t.is(declarationNode.type, 'decl'); + t.is(declarationNode.source.start.line, 3); + t.is(declarationNode.source.start.column, 3); + t.is(declarationNode.source.end.line, 3); + t.is(declarationNode.source.end.column, 30); +}); From 420f90ecc8331e23e36a1cab49d12b84a3862aeb Mon Sep 17 00:00:00 2001 From: Nathan Walters Date: Tue, 10 Aug 2021 12:25:22 -0700 Subject: [PATCH 2/8] Enhance test --- test/parser/comments.test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/parser/comments.test.js b/test/parser/comments.test.js index 5751a83..1f74c1d 100644 --- a/test/parser/comments.test.js +++ b/test/parser/comments.test.js @@ -195,4 +195,6 @@ test.only('handles single quotes in comments', (t) => { t.is(declarationNode.source.start.column, 3); t.is(declarationNode.source.end.line, 3); t.is(declarationNode.source.end.column, 30); + + t.is(nodeToString(root), less); }); From 98f17f6932b7b94d28a61307f24d0acf7b6738af Mon Sep 17 00:00:00 2001 From: Nathan Walters Date: Tue, 10 Aug 2021 12:25:54 -0700 Subject: [PATCH 3/8] Drop .only --- test/parser/comments.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parser/comments.test.js b/test/parser/comments.test.js index 1f74c1d..1933038 100644 --- a/test/parser/comments.test.js +++ b/test/parser/comments.test.js @@ -180,7 +180,7 @@ test('inline comments with asterisk are persisted (#135)', (t) => { t.is(nodeToString(root), less); }); -test.only('handles single quotes in comments', (t) => { +test('handles single quotes in comments', (t) => { const less = `.a {\n // '\n outline-width: 0 !important;\n}\n\n/** ' */`; const root = parse(less); From 0dca2bbd279de6837c91c530141c478142f6ee1d Mon Sep 17 00:00:00 2001 From: Nathan Walters Date: Tue, 10 Aug 2021 14:02:18 -0700 Subject: [PATCH 4/8] Work on post-processing tree --- lib/index.js | 40 ++++++++++++++++++++++++++++++++++++ test/parser/comments.test.js | 14 ++++++++----- 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/lib/index.js b/lib/index.js index 6b247b0..9713a15 100644 --- a/lib/index.js +++ b/lib/index.js @@ -12,6 +12,46 @@ module.exports = { parser.parse(); + parser.root.walk((node, index) => { + // TODO: post-process here + console.log('========='); + const offset = input.css.lastIndexOf(node.source.input.css); + console.log('computed offset:', offset); + console.log('input.css:', input.css); + console.log('node.source.input.css:', node.source.input.css); + console.log('original source:', node.source); + + // Sanity check + if (offset + node.source.input.css.length !== input.css.length) { + console.log('input.css:', input.css); + console.log('node.source.input.css:', node.source.input.css); + throw new Error('TODO handle me'); + } + + const newStartOffset = offset + node.source.start.offset; + const newEndOffset = offset + node.source.end.offset; + const newStartPosition = input.fromOffset(offset + node.source.start.offset); + const newEndPosition = input.fromOffset(offset + node.source.end.offset); + + // eslint-disable-next-line no-param-reassign + node.source.start = { + offset: newStartOffset, + line: newStartPosition.line, + column: newStartPosition.col + }; + + // eslint-disable-next-line no-param-reassign + node.source.end = { + offset: newEndOffset, + line: newEndPosition.line, + column: newEndPosition.col + } + + console.log(node.source.start); + node.source.end = input.fromOffset(offset + node.source.end.offset); + console.log(node.source.end); + }); + return parser.root; }, diff --git a/test/parser/comments.test.js b/test/parser/comments.test.js index 1933038..d31beb6 100644 --- a/test/parser/comments.test.js +++ b/test/parser/comments.test.js @@ -180,21 +180,25 @@ test('inline comments with asterisk are persisted (#135)', (t) => { t.is(nodeToString(root), less); }); -test('handles single quotes in comments', (t) => { - const less = `.a {\n // '\n outline-width: 0 !important;\n}\n\n/** ' */`; +test.only('handles single quotes in comments', (t) => { + const less = `a {\n // '\n color: pink;\n}\n\n/** ' */`; const root = parse(less); - const { first } = root; - const [commentNode, declarationNode] = first.nodes; + const [ruleNode, commentNode] = root.nodes; + t.is(ruleNode.type, 'rule'); t.is(commentNode.type, 'comment'); + const [innerCommentNode, declarationNode] = ruleNode.nodes; + + t.is(innerCommentNode.type, 'comment'); t.is(declarationNode.type, 'decl'); + t.is(declarationNode.source.start.line, 3); t.is(declarationNode.source.start.column, 3); t.is(declarationNode.source.end.line, 3); - t.is(declarationNode.source.end.column, 30); + t.is(declarationNode.source.end.column, 14); t.is(nodeToString(root), less); }); From e1a0293886205685137e3d85b3101954a305901b Mon Sep 17 00:00:00 2001 From: Nathan Walters Date: Tue, 10 Aug 2021 14:22:46 -0700 Subject: [PATCH 5/8] Enhance code + tests --- lib/index.js | 50 ++++++++++++++++++++---------------- test/parser/comments.test.js | 2 +- 2 files changed, 29 insertions(+), 23 deletions(-) diff --git a/lib/index.js b/lib/index.js index 9713a15..b8aa213 100644 --- a/lib/index.js +++ b/lib/index.js @@ -12,26 +12,30 @@ module.exports = { parser.parse(); - parser.root.walk((node, index) => { - // TODO: post-process here - console.log('========='); + // To handle double-slash comments (`//`) we end up creating a new tokenizer + // in certain cases (see `lib/nodes/inline-comment.js`). However, this means + // that any following node in the AST will have incorrect start/end positions + // on the `source` property. To fix that, we'll walk the AST and compute + // updated positions for all nodes. + parser.root.walk((node) => { const offset = input.css.lastIndexOf(node.source.input.css); - console.log('computed offset:', offset); - console.log('input.css:', input.css); - console.log('node.source.input.css:', node.source.input.css); - console.log('original source:', node.source); - // Sanity check + if (offset === 0) { + // Short circuit - this node was processed with the original tokenizer + // and should therefore have correct position information. + return; + } + + // This ensures that the chunk of source we're processing corresponds + // strictly to a terminal substring of the input CSS. This should always + // be the case, but if it ever isn't, we prefer to fail instead of + // producing potentially invalid output. if (offset + node.source.input.css.length !== input.css.length) { - console.log('input.css:', input.css); - console.log('node.source.input.css:', node.source.input.css); - throw new Error('TODO handle me'); + throw new Error('Invalid state detected in postcss-less'); } const newStartOffset = offset + node.source.start.offset; - const newEndOffset = offset + node.source.end.offset; const newStartPosition = input.fromOffset(offset + node.source.start.offset); - const newEndPosition = input.fromOffset(offset + node.source.end.offset); // eslint-disable-next-line no-param-reassign node.source.start = { @@ -40,16 +44,18 @@ module.exports = { column: newStartPosition.col }; - // eslint-disable-next-line no-param-reassign - node.source.end = { - offset: newEndOffset, - line: newEndPosition.line, - column: newEndPosition.col - } + // Not all nodes have an `end` property. + if (node.source.end) { + const newEndOffset = offset + node.source.end.offset; + const newEndPosition = input.fromOffset(offset + node.source.end.offset); - console.log(node.source.start); - node.source.end = input.fromOffset(offset + node.source.end.offset); - console.log(node.source.end); + // eslint-disable-next-line no-param-reassign + node.source.end = { + offset: newEndOffset, + line: newEndPosition.line, + column: newEndPosition.col + }; + } }); return parser.root; diff --git a/test/parser/comments.test.js b/test/parser/comments.test.js index d31beb6..97ea464 100644 --- a/test/parser/comments.test.js +++ b/test/parser/comments.test.js @@ -180,7 +180,7 @@ test('inline comments with asterisk are persisted (#135)', (t) => { t.is(nodeToString(root), less); }); -test.only('handles single quotes in comments', (t) => { +test('handles single quotes in comments (#163)', (t) => { const less = `a {\n // '\n color: pink;\n}\n\n/** ' */`; const root = parse(less); From 01dcca9fb73925323f6861a73782184e1ce3154c Mon Sep 17 00:00:00 2001 From: Nathan Walters Date: Tue, 10 Aug 2021 14:53:45 -0700 Subject: [PATCH 6/8] Add additional test assertions --- test/parser/comments.test.js | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/test/parser/comments.test.js b/test/parser/comments.test.js index 97ea464..4824e7e 100644 --- a/test/parser/comments.test.js +++ b/test/parser/comments.test.js @@ -180,7 +180,7 @@ test('inline comments with asterisk are persisted (#135)', (t) => { t.is(nodeToString(root), less); }); -test('handles single quotes in comments (#163)', (t) => { +test.only('handles single quotes in comments (#163)', (t) => { const less = `a {\n // '\n color: pink;\n}\n\n/** ' */`; const root = parse(less); @@ -190,11 +190,22 @@ test('handles single quotes in comments (#163)', (t) => { t.is(ruleNode.type, 'rule'); t.is(commentNode.type, 'comment'); + t.is(commentNode.source.start.line, 6); + t.is(commentNode.source.start.column, 1); + t.is(commentNode.source.end.line, 6); + t.is(commentNode.source.end.column, 8); + const [innerCommentNode, declarationNode] = ruleNode.nodes; t.is(innerCommentNode.type, 'comment'); t.is(declarationNode.type, 'decl'); + t.is(innerCommentNode.source.start.line, 2); + t.is(innerCommentNode.source.start.column, 3); + t.is(innerCommentNode.source.end.line, 2); + // TODO: this test is failing + // t.is(innerCommentNode.source.end.column, 6); + t.is(declarationNode.source.start.line, 3); t.is(declarationNode.source.start.column, 3); t.is(declarationNode.source.end.line, 3); From a8649993a54ad3cd92eafd7a2729b98be6fe73b1 Mon Sep 17 00:00:00 2001 From: Nathan Walters Date: Tue, 10 Aug 2021 15:32:11 -0700 Subject: [PATCH 7/8] Fix computation of end offset of comment --- lib/nodes/inline-comment.js | 14 ++++++++++---- test/parser/comments.test.js | 5 ++--- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/lib/nodes/inline-comment.js b/lib/nodes/inline-comment.js index 6e5bc04..b933949 100644 --- a/lib/nodes/inline-comment.js +++ b/lib/nodes/inline-comment.js @@ -8,7 +8,7 @@ module.exports = { if (token[0] === 'word' && token[1].slice(0, 2) === '//') { const first = token; const bits = []; - let last; + let endOffset; let remainingInput; while (token) { @@ -20,7 +20,12 @@ module.exports = { // Get remaining input and retokenize remainingInput = token[1].substring(token[1].indexOf('\n')); - remainingInput += this.input.css.valueOf().substring(this.tokenizer.position()); + const untokenizedRemainingInput = this.input.css + .valueOf() + .substring(this.tokenizer.position()); + remainingInput += untokenizedRemainingInput; + + endOffset = token[3] + untokenizedRemainingInput.length - remainingInput.length; } else { // If the tokenizer went to the next line go back this.tokenizer.back(token); @@ -29,11 +34,12 @@ module.exports = { } bits.push(token[1]); - last = token; + // eslint-disable-next-line prefer-destructuring + endOffset = token[2]; token = this.tokenizer.nextToken({ ignoreUnclosed: true }); } - const newToken = ['comment', bits.join(''), first[2], last[2]]; + const newToken = ['comment', bits.join(''), first[2], endOffset]; this.inlineComment(newToken); // Replace tokenizer to retokenize the rest of the string diff --git a/test/parser/comments.test.js b/test/parser/comments.test.js index 4824e7e..6f7fdc1 100644 --- a/test/parser/comments.test.js +++ b/test/parser/comments.test.js @@ -180,7 +180,7 @@ test('inline comments with asterisk are persisted (#135)', (t) => { t.is(nodeToString(root), less); }); -test.only('handles single quotes in comments (#163)', (t) => { +test('handles single quotes in comments (#163)', (t) => { const less = `a {\n // '\n color: pink;\n}\n\n/** ' */`; const root = parse(less); @@ -203,8 +203,7 @@ test.only('handles single quotes in comments (#163)', (t) => { t.is(innerCommentNode.source.start.line, 2); t.is(innerCommentNode.source.start.column, 3); t.is(innerCommentNode.source.end.line, 2); - // TODO: this test is failing - // t.is(innerCommentNode.source.end.column, 6); + t.is(innerCommentNode.source.end.column, 6); t.is(declarationNode.source.start.line, 3); t.is(declarationNode.source.start.column, 3); From 83f09860a9a39eede0619c7b446a973d3c6b5c55 Mon Sep 17 00:00:00 2001 From: Nathan Walters Date: Tue, 10 Aug 2021 15:43:58 -0700 Subject: [PATCH 8/8] Add istanbul ignore comment --- lib/index.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/index.js b/lib/index.js index b8aa213..d083211 100644 --- a/lib/index.js +++ b/lib/index.js @@ -30,6 +30,7 @@ module.exports = { // strictly to a terminal substring of the input CSS. This should always // be the case, but if it ever isn't, we prefer to fail instead of // producing potentially invalid output. + // istanbul ignore next if (offset + node.source.input.css.length !== input.css.length) { throw new Error('Invalid state detected in postcss-less'); }