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

Division with comment containing '/' - SyntaxError #9300

Closed
dlangerenken opened this issue Oct 26, 2016 · 6 comments · Fixed by #10103
Closed

Division with comment containing '/' - SyntaxError #9300

dlangerenken opened this issue Oct 26, 2016 · 6 comments · Fixed by #10103
Labels
confirmed-bug Issues with confirmed bugs. repl Issues and PRs related to the REPL subsystem.

Comments

@dlangerenken
Copy link

dlangerenken commented Oct 26, 2016

  • Version: 6.9.1, 7.0.0
  • Platform: Linux
  • Subsystem: Mint 17, Ubuntu 16

Following function fails parsing:

function foo(){
    var bar = 1 / 1; // '/'
}

SyntaxError: Unexpected end of input

tested with:

  • docker run -it node:6.9.1,
  • docker run -it node:7.0.0,
  • local installation

This code parses correctly in some node versions 5.X

@dlangerenken dlangerenken changed the title Division with comment containing '/' faio Division with comment containing '/' fails Oct 26, 2016
@dlangerenken dlangerenken changed the title Division with comment containing '/' fails Division with comment containing '/' - SyntaxError Oct 26, 2016
@jasnell
Copy link
Member

jasnell commented Oct 26, 2016

Interesting... I'm able to reproduce only in the REPL and only when the line var bar = 1 / 1; // '/' occurs within a function.

@dlangerenken
Copy link
Author

dlangerenken commented Oct 26, 2016

Yes, that's what I found out too. It also does not fail if you use more than one slash. However, it fails even if the comment is not in the same line.

@mscdex
Copy link
Contributor

mscdex commented Oct 26, 2016

It works for me with node v6.9.1, even with strict mode.

It looks like this is indeed REPL-specific.

/cc @princejwesley ?

@mscdex mscdex added confirmed-bug Issues with confirmed bugs. repl Issues and PRs related to the REPL subsystem. labels Oct 26, 2016
@princejwesley
Copy link
Contributor

princejwesley commented Oct 26, 2016

It's a LineParser bug. Looking into it.

Update: With our ad-hoc parser, first /(division operator) in var bar = 1 / 1; // '/' is treated as start of regex literal

@princejwesley
Copy link
Contributor

Without proper state machine, Its not easy to deduce whether '/' is a division operator or start of regex literal.

@dlangerenken Worst case, type/paste in .editor mode.

@not-an-aardvark
Copy link
Contributor

not-an-aardvark commented Dec 3, 2016

I'm investigating this issue now. I think it should be possible to correctly distinguish between division and regex literals in almost all cases, with only a single lookbehind. A regex literal is almost always1 preceded by (, {, [, ;, }, or the beginning of input. A division operator is almost always2 preceded by another character.

1 An exception is code like this:

function foo() {
  return
  /regex/ // not preceded by ;
}

2 An exception is code like this:

({ ok: true } / 3)

Both of these exceptional cases are sort of obselete (they either require dead code, or dividing an object literal by something), so I think using a single-character lookbehind would be an improvement over the current behavior.

not-an-aardvark added a commit to not-an-aardvark/node that referenced this issue Dec 6, 2016
This improves the heuristic used in multiline-prompt mode to determine
whether a given slash character is at the beginning of a regular
expression.

PR-URL: nodejs#10103
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: James M Snell <jasnell@keybase.io>
Fixes: nodejs#9300
Fishrock123 pushed a commit that referenced this issue Dec 6, 2016
This improves the heuristic used in multiline-prompt mode to determine
whether a given slash character is at the beginning of a regular
expression.

PR-URL: #10103
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: James M Snell <jasnell@keybase.io>
Fixes: #9300
jmdarling pushed a commit to jmdarling/node that referenced this issue Dec 8, 2016
This improves the heuristic used in multiline-prompt mode to determine
whether a given slash character is at the beginning of a regular
expression.

PR-URL: nodejs#10103
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: James M Snell <jasnell@keybase.io>
Fixes: nodejs#9300
MylesBorins pushed a commit that referenced this issue Dec 20, 2016
This improves the heuristic used in multiline-prompt mode to determine
whether a given slash character is at the beginning of a regular
expression.

PR-URL: #10103
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: James M Snell <jasnell@keybase.io>
Fixes: #9300
MylesBorins pushed a commit that referenced this issue Dec 21, 2016
This improves the heuristic used in multiline-prompt mode to determine
whether a given slash character is at the beginning of a regular
expression.

PR-URL: #10103
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: James M Snell <jasnell@keybase.io>
Fixes: #9300
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. repl Issues and PRs related to the REPL subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants