Fix template literal comments (#1296)

The implementation was checking if the comment was inside of the expression range, which seems like a good idea. Unfortunately, the expression range is not what's inside of `${}` but the actual AST node, which incidentally doesn't include comments. So the logic was off and returned `undefined` which threw afterwards.

Another solution is to find the first quasi where start is > comment start. This means that the comment appeared between the quasi before and this one... therefore in the expression before!

The flow parser has issues with unicode where it makes node location invalid, there are likely other places where node locations are off. So instead of throwing with a weird error, let's attach it to the first one if it doesn't work.

Fixes #1293
master
Christopher Chedeau 2017-04-17 08:35:12 -07:00 committed by James Long
parent 8f2c20872b
commit 9cd4517a64
3 changed files with 51 additions and 15 deletions

View File

@ -505,7 +505,7 @@ function handleObjectPropertyAssignment(enclosingNode, precedingNode, comment) {
function handleTemplateLiteralComments(enclosingNode, comment) {
if (enclosingNode && enclosingNode.type === "TemplateLiteral") {
const expressionIndex = findExpressionIndexForComment(
enclosingNode.expressions,
enclosingNode.quasis,
comment
);
// Enforce all comments to be leading block comments.
@ -780,27 +780,21 @@ function printComment(commentPath) {
}
}
function findExpressionIndexForComment(expressions, comment) {
let match;
function findExpressionIndexForComment(quasis, comment) {
const startPos = locStart(comment) - 1;
const endPos = locEnd(comment) + 1;
for (let i = 0; i < expressions.length; ++i) {
const range = getExpressionRange(expressions[i]);
if (
(startPos >= range.start && startPos <= range.end) ||
(endPos >= range.start && endPos <= range.end)
) {
match = i;
break;
for (let i = 1; i < quasis.length; ++i) {
if (startPos < getQuasiRange(quasis[i]).start) {
return i - 1;
}
}
return match;
// We haven't found it, it probably means that some of the locations are off.
// Let's just return the first one.
return 0;
}
function getExpressionRange(expr) {
function getQuasiRange(expr) {
if (expr.start !== undefined) {
// Babylon
return { start: expr.start, end: expr.end };

View File

@ -1,5 +1,35 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`comment.js 1`] = `
\`
(?:\${escapeChar}[\\\\S\\\\s]|(?:(?!\${// Using \`XRegExp.union\` safely rewrites backreferences in \`left\` and \`right\`.
// Intentionally not passing \`basicFlags\` to \`XRegExp.union\` since any syntax
// transformation resulting from those flags was already applied to \`left\` and
// \`right\` when they were passed through the XRegExp constructor above.
XRegExp.union([left, right], '', {conjunction: 'or'}).source})[^\${escapeChar}])+)+
\`;
\`a\${/* b */c/* d */}e\${// f
g
// h
}\`;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
\`
(?:\${escapeChar}[\\\\S\\\\s]|(?:(?!\${/* Using \`XRegExp.union\` safely rewrites backreferences in \`left\` and \`right\`.*/
// Intentionally not passing \`basicFlags\` to \`XRegExp.union\` since any syntax
// transformation resulting from those flags was already applied to \`left\` and
// \`right\` when they were passed through the XRegExp constructor above.
XRegExp.union([left, right], "", {
conjunction: "or"
}).source})[^\${escapeChar}])+)+
\`;
\`a\${/* b */ /* d */ c}e\${/* f*/
g}// h
\`;
`;
exports[`faulty-locations.js 1`] = `
var o = {
[\`key\`]: () => {

12
tests/template/comment.js Normal file
View File

@ -0,0 +1,12 @@
`
(?:${escapeChar}[\\S\\s]|(?:(?!${// Using `XRegExp.union` safely rewrites backreferences in `left` and `right`.
// Intentionally not passing `basicFlags` to `XRegExp.union` since any syntax
// transformation resulting from those flags was already applied to `left` and
// `right` when they were passed through the XRegExp constructor above.
XRegExp.union([left, right], '', {conjunction: 'or'}).source})[^${escapeChar}])+)+
`;
`a${/* b */c/* d */}e${// f
g
// h
}`;