Skip to content

Commit

Permalink
ignore invalid trailing pure annotations (#4068)
Browse files Browse the repository at this point in the history
that were incorrectly associated with subsequent calls

Co-authored-by: Lukas Taegert-Atkinson <lukastaegert@users.noreply.github.com>
  • Loading branch information
kzc and lukastaegert committed May 14, 2021
1 parent 3345267 commit c40d206
Show file tree
Hide file tree
Showing 6 changed files with 113 additions and 6 deletions.
2 changes: 1 addition & 1 deletion src/Graph.ts
Expand Up @@ -136,7 +136,7 @@ export default class Graph {

options.onComment = onCommentOrig;

markPureCallExpressions(comments, ast);
markPureCallExpressions(comments, ast, code);

return ast;
}
Expand Down
21 changes: 19 additions & 2 deletions src/utils/pureComments.ts
Expand Up @@ -19,18 +19,34 @@ basicWalker.PropertyDefinition = function (node: any, st: any, c: any) {
};

interface CommentState {
code: string;
commentIndex: number;
commentNodes: acorn.Comment[];
}

function isOnlyWhitespaceOrComments(code: string) {
// streamline the typical case
if (/^\s*$/.test(code)) return true;
try {
// successful only if it's a valid Program without statements
const ast = acorn.parse(code, { ecmaVersion: 'latest' }) as any;
return ast.body && ast.body.length === 0;
} catch {
// should never be reached -
// the entire module was previously successfully parsed
}
return false;
}

function handlePureAnnotationsOfNode(
node: acorn.Node,
state: CommentState,
type: string = node.type
) {
let commentNode = state.commentNodes[state.commentIndex];
while (commentNode && node.start >= commentNode.end) {
markPureNode(node, commentNode);
const between = state.code.substring(commentNode.end, node.start);
if (isOnlyWhitespaceOrComments(between)) markPureNode(node, commentNode);
commentNode = state.commentNodes[++state.commentIndex];
}
if (commentNode && commentNode.end <= node.end) {
Expand Down Expand Up @@ -62,8 +78,9 @@ function markPureNode(
const pureCommentRegex = /[@#]__PURE__/;
const isPureComment = (comment: acorn.Comment) => pureCommentRegex.test(comment.value);

export function markPureCallExpressions(comments: acorn.Comment[], esTreeAst: acorn.Node) {
export function markPureCallExpressions(comments: acorn.Comment[], esTreeAst: acorn.Node, code: string) {
handlePureAnnotationsOfNode(esTreeAst, {
code,
commentIndex: 0,
commentNodes: comments.filter(isPureComment)
});
Expand Down
22 changes: 20 additions & 2 deletions test/form/samples/pure-comment-line-break/_expected.js
Expand Up @@ -6,7 +6,7 @@ console.log('should remain impure');
console.log('code');
console.log('should remain impure');

console.log('code');
console.log('code')/*@__PURE__*/;
console.log('should remain impure');
console.log('should remain impure');

Expand All @@ -16,7 +16,7 @@ console.log('should remain impure');
console.log('code'),
console.log('should remain impure');

console.log('code'),
console.log('code')/*@__PURE__*/,
console.log('should remain impure');

console.log('should remain impure');
Expand All @@ -35,3 +35,21 @@ console.log('should remain impure', x);
{
console.log('should remain impure');
}
keep1() /*@__PURE__*/ ; keep2();
keep3() ;
keep4() /*@__PURE__*/ ; /* other comment */ keep5();
keep6() /*@__PURE__*/ ; // other comment
keep7();
keep8() /*@__PURE__*/ && keep9();

/*@__PURE__*/ Drop1(), // FIXME: unrelated issue
Keep1() /*@__PURE__*/ , Keep2(),
Keep3() ,
Keep4() /*@__PURE__*/ , /* other comment */ Keep5(),
Keep6() /*@__PURE__*/ , // other comment
Keep7(),
Keep8() /*@__PURE__*/ && Keep9();

// FIXME: unrelated issue
/*@__PURE__*/ Drop10(),
/*@__PURE__*/ Drop13();
28 changes: 28 additions & 0 deletions test/form/samples/pure-comment-line-break/main.js
Expand Up @@ -42,3 +42,31 @@ console.log('should remain impure', code);
if (true) {
console.log('should remain impure');
}

/*@__PURE__*/ drop1();
/*@__PURE__*/
drop2();
keep1() /*@__PURE__*/ ; keep2();
keep3() ; /*@__PURE__*/
drop3();
keep4() /*@__PURE__*/ ; /* other comment */ keep5();
keep6() /*@__PURE__*/ ; // other comment
keep7();
keep8() /*@__PURE__*/ && keep9();

/*@__PURE__*/ Drop1(), // FIXME: unrelated issue
/*@__PURE__*/
Drop2(),
Keep1() /*@__PURE__*/ , Keep2(),
Keep3() , /*@__PURE__*/
Drop3(),
Keep4() /*@__PURE__*/ , /* other comment */ Keep5(),
Keep6() /*@__PURE__*/ , // other comment
Keep7(),
Keep8() /*@__PURE__*/ && Keep9();

// FIXME: unrelated issue
/*@__PURE__*/ Drop10(),
/*@__PURE__*/ Drop11(),
/*@__PURE__*/ Drop12(),
/*@__PURE__*/ Drop13();
24 changes: 23 additions & 1 deletion test/form/samples/pure-comments-disabled/_expected.js
Expand Up @@ -2,5 +2,27 @@
/*@__PURE__*/ a();
/*@__PURE__*/ new a();

console.log('code');
console.log('code')/*@__PURE__*/;
console.log('should remain impure');

/*@__PURE__*/ drop1();
/*@__PURE__*/
drop2();
keep1() /*@__PURE__*/ ; keep2();
keep3() ; /*@__PURE__*/
drop3();
keep4() /*@__PURE__*/ ; /* other comment */ keep5();
keep6() /*@__PURE__*/ ; // other comment
keep7();
keep8() /*@__PURE__*/ && keep9();

/*@__PURE__*/ Drop1(),
/*@__PURE__*/
Drop2(),
Keep1() /*@__PURE__*/ , Keep2(),
Keep3() , /*@__PURE__*/
Drop3(),
Keep4() /*@__PURE__*/ , /* other comment */ Keep5(),
Keep6() /*@__PURE__*/ , // other comment
Keep7(),
Keep8() /*@__PURE__*/ && Keep9();
22 changes: 22 additions & 0 deletions test/form/samples/pure-comments-disabled/main.js
Expand Up @@ -5,3 +5,25 @@
console.log('code')/*@__PURE__*/;
/*@__PURE__*/(() => {})();
console.log('should remain impure');

/*@__PURE__*/ drop1();
/*@__PURE__*/
drop2();
keep1() /*@__PURE__*/ ; keep2();
keep3() ; /*@__PURE__*/
drop3();
keep4() /*@__PURE__*/ ; /* other comment */ keep5();
keep6() /*@__PURE__*/ ; // other comment
keep7();
keep8() /*@__PURE__*/ && keep9();

/*@__PURE__*/ Drop1(),
/*@__PURE__*/
Drop2(),
Keep1() /*@__PURE__*/ , Keep2(),
Keep3() , /*@__PURE__*/
Drop3(),
Keep4() /*@__PURE__*/ , /* other comment */ Keep5(),
Keep6() /*@__PURE__*/ , // other comment
Keep7(),
Keep8() /*@__PURE__*/ && Keep9();

0 comments on commit c40d206

Please sign in to comment.