Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jshint doesn't expect regexp literals in one-liners following if/while/for conditions, and also else #3635

Open
qdirks opened this issue Nov 9, 2022 · 3 comments
Labels

Comments

@qdirks
Copy link

qdirks commented Nov 9, 2022

Trying this in vscode:

if (x) /abc/.exec('abc');
while (x) /abc/.exec('abc');
for (;x;) /abc/.exec('abc');

if (false) ;
else /abc/.exec('abc');

Several error underlines occur, but they shouldn't. Thanks! :)

@qdirks
Copy link
Author

qdirks commented Nov 9, 2022

Also, be sure that regexp literals are not expected after switch (x) and catch (x). A left curly brace should always be expected after those constructs.

@qdirks qdirks changed the title jshint doesn't expect regexp literals in one-liners following if/while/for conditions jshint doesn't expect regexp literals in one-liners following if/while/for conditions, and also else Nov 9, 2022
@jugglinmike
Copy link
Member

Confirmed; thanks for the report!

This looks to be a problem with the tokenizer--JSHint uses a naive (but, to my mind, surprisingly effective) heuristic for disambiguating the solidus. It's getting tripped up on the closing parenthesis, so the consequent bodies of switch and catch are perhaps less interesting than they might appear since they require an interstitial brace.

ASI makes it possible to observe the same faulty behavior for "do/while" loops:

do {} while (0) /./;

Though the real trouble with ASI comes from the closing brace character. JSHint incorrectly rejects each of these lines:

switch (0) {} /./;
try {} catch (e) {} /./;
try {} finally {} /./;
{} /./;
function f() {} /./;
class C {} /./;

There are many ways to reformat the source code to avoid this problem:

-do {} while (0) /./;
+do {} while (0); /./;
-switch (0) {} /./;
+switch (0) {}
+/./;
-try {} catch (e) {} /./;
+try {} catch (e) {} (/./);
-try {} finally {} /./;
+try {} finally {} void /./;

To be sure: JSHint should be fixed, but those are some tips for folks that need a workaround in the mean time.

This a pretty fundamental problem, but because it's rarely triggered in practice (as evidenced by the novelty of this report and JSHint's age), because there are many ways to avoid it, and because the fix will likely require non-trivial effort, I'm considering the fix low-priority.

@jugglinmike jugglinmike added the P2 label Nov 11, 2022
@qdirks
Copy link
Author

qdirks commented Nov 11, 2022

Don't forget you can also use jshint ignore:start with jshint ignore:end. Personally, I have never needed to write a regexp literal like this, but I found the bug because I'm working on my own javascript tokenizer, which I have great ambitions for eventually using in the browser to stop third party scripts from running certain codes, and a code obfuscator might do something silly like trying to put a regexp in a place that might look confusing, and I wouldn't want my tokenizer to fail if that happened. Anyway, jshint is old, and I can abide by your reasoning and your decision. Thanks! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants