fix: fixed #3870 Function names with a prefix shared with a built-in … #3871
Conversation
…a built-in global are confused for a global Don't exclude a built-in global (e.g. `eval`) followed by an identifier , but exclude built-in global followed by an open paren (i.e. `eval(`) See highlightjs#3870 for an example.
| /\b/, | ||
| noneOf([ | ||
| ...ECMAScript.BUILT_IN_GLOBALS, | ||
| ...ECMAScript.BUILT_IN_GLOBALS.map((x) => x + '\\('), |
There was a problem hiding this comment.
Don't we need this same protection for super and import also? And wouldn't just adding a \b on the other side of noneOf get the job done here without explicitly looking for a (?
There was a problem hiding this comment.
yes, there might be more bugs in the way the parser handles keywords and other keywords affected. I haven't tested that and I'm not sure of what the logic behind the original code was, but your suggestion seems reasonable.
Same thing for using \b instead of (, again I'm not sure of what the original logic was so I tried to find a minimal solution that would fix the test case I encountered.
There was a problem hiding this comment.
Happy to review again after you update.
There was a problem hiding this comment.
That PR was just a suggestion. I'll let someone with a better understanding of the codebase to contribute a fix.
…global are confused for a global
Don't exclude a built-in global (e.g.
eval) followed by an identifier , but exclude built-in global followed by an open paren (i.e.eval()See #3870 for an example.