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

🐛 Including smart-quote with escape \ confuses the JS lexer #1941

Closed
1 task done
danthedeckie opened this issue Feb 29, 2024 · 5 comments · Fixed by #2044
Closed
1 task done

🐛 Including smart-quote with escape \ confuses the JS lexer #1941

danthedeckie opened this issue Feb 29, 2024 · 5 comments · Fixed by #2044
Assignees

Comments

@danthedeckie
Copy link

Environment information

CLI:
  Version:                      1.5.3
  Color support:                true

Platform:
  CPU Architecture:             x86_64
  OS:                           macos

Environment:
  BIOME_LOG_DIR:                unset
  NO_COLOR:                     unset
  TERM:                         "xterm-256color"
  JS_RUNTIME_VERSION:           "v20.7.0"
  JS_RUNTIME_NAME:              "node"
  NODE_PACKAGE_MANAGER:         unset

Biome Configuration:
  Status:                       Loaded successfully
  Formatter disabled:           false
  Linter disabled:              false
  Organize imports disabled:    false
  VCS disabled:                 true

Workspace:
  Open Documents:               0

What happened?

  1. In the playground, paste in
"hello world".replace(/\’/, "")

or put that in a .js file and run biome lint against it.


The biome lint gives this output:

src/js/test.js:1:28 parse ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ✖ unterminated regex literal

  > 1 │ "hello".replace(/\‘/, "'");
      │
    2 │

  ℹ ...but the line ends here

  > 1 │ "hello".replace(/\‘/, "'");
      │
    2 │

  ℹ a regex literal starts there...

  > 1 │ "hello".replace(/\‘/, "'");
      │                 ^
    2 │


src/js/test.js:2:1 parse ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ✖ expected `)` but instead the file ends

    1 │ "hello".replace(/\‘/, "'");
  > 2 │
      │

  ℹ the file ends here

    1 │ "hello".replace(/\‘/, "'");
  > 2 │
      │


src/js/test.js lint ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ✖ The file contains diagnostics that needs to be addressed.


Checked 1 file(s) in 15ms
Found 3 error(s)
lint ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ✖ Some errors were emitted while running checks.

The playground crashes completely, and shows this in the browser console:

client.LpXbfsP5.js:24 RangeError: Invalid position 33 in document of length 31
    at qe.lineAt (PlaygroundLoader.O4NTypk8.js:1:4066)
    at SA (PlaygroundLoader.O4NTypk8.js:10:41341)
    at li.update [as updateF] (PlaygroundLoader.O4NTypk8.js:10:41717)
    at Object.update (PlaygroundLoader.O4NTypk8.js:4:17201)
    at ke.computeSlot (PlaygroundLoader.O4NTypk8.js:4:25884)
    at _c (PlaygroundLoader.O4NTypk8.js:4:20008)
    at new ke (PlaygroundLoader.O4NTypk8.js:4:25071)
    at ke.applyTransaction (PlaygroundLoader.O4NTypk8.js:4:25853)
    at get state (PlaygroundLoader.O4NTypk8.js:4:21482)
    at Rt.update (PlaygroundLoader.O4NTypk8.js:9:14200)
pi @ client.LpXbfsP5.js:24
t.callback @ client.LpXbfsP5.js:24
eo @ client.LpXbfsP5.js:22
go @ client.LpXbfsP5.js:24
ha @ client.LpXbfsP5.js:24
kf @ client.LpXbfsP5.js:24
_f @ client.LpXbfsP5.js:24
yn @ client.LpXbfsP5.js:24
So @ client.LpXbfsP5.js:24
pn @ client.LpXbfsP5.js:22
Xn @ client.LpXbfsP5.js:24
(anonymous) @ client.LpXbfsP5.js:24
E @ client.LpXbfsP5.js:9
lt @ client.LpXbfsP5.js:9
client.LpXbfsP5.js:22 Uncaught RangeError: Invalid position 33 in document of length 31
    at qe.lineAt (PlaygroundLoader.O4NTypk8.js:1:4066)
    at SA (PlaygroundLoader.O4NTypk8.js:10:41341)
    at li.update [as updateF] (PlaygroundLoader.O4NTypk8.js:10:41717)
    at Object.update (PlaygroundLoader.O4NTypk8.js:4:17201)
    at ke.computeSlot (PlaygroundLoader.O4NTypk8.js:4:25884)
    at _c (PlaygroundLoader.O4NTypk8.js:4:20008)
    at new ke (PlaygroundLoader.O4NTypk8.js:4:25071)
    at ke.applyTransaction (PlaygroundLoader.O4NTypk8.js:4:25853)
    at get state (PlaygroundLoader.O4NTypk8.js:4:21482)
    at Rt.update (PlaygroundLoader.O4NTypk8.js:9:14200)

Trying to run biome format on it gives:

./node_modules/@biomejs/biome/bin/biome format --verbose  src/js/test.js
src/js/test.js format ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ✖ Format with errors is disabled.


This also happens with other similar types of "smart" quotes,

"hello world".replace(/\‟/, "")

for instance.

Expected result

It should work just fine, the same as

"hello world".replace(/\'/, "")

Code of Conduct

  • I agree to follow Biome's Code of Conduct
@Sec-ant
Copy link
Member

Sec-ant commented Mar 2, 2024

An observation: the crash of the playground seems to be the result of an out of range position acquired from the diagnostics. So it should be automatically fixed if the root cause is fixed.

And apart from smart quotes, Chinese characters will also trigger the similar problem.

Just type (or copy paste) 一' into the playground and it will crash immediately.

@Sec-ant
Copy link
Member

Sec-ant commented Mar 3, 2024

This issue seems to be the same as #1788.

@arendjr
Copy link
Contributor

arendjr commented Mar 6, 2024

Thanks for investigating, @Sec-ant. Closing this one as duplicate.

@arendjr arendjr closed this as not planned Won't fix, can't repro, duplicate, stale Mar 6, 2024
@arendjr arendjr changed the title 🐛 Including smart-quote with escape \ crashes the playground, or confuses the terminal literal reading 🐛 Including smart-quote with escape \ confuses the JS lexer Mar 11, 2024
@arendjr
Copy link
Contributor

arendjr commented Mar 11, 2024

Reopening, since there may be two separate issues at play. This one regarding the lexer (may be specific to Unicode characters inside regex literals), and possibly another about compatibility of character offsets in diagnostics (#1385).

@arendjr arendjr reopened this Mar 11, 2024
@Sec-ant
Copy link
Member

Sec-ant commented Mar 11, 2024

FWIW, running this in a debug build yields:

echo "/\’/" | ../target/debug/biome lint --stdin-file-path=a.js
Biome encountered an unexpected error

This is a bug in Biome, not an error in your code, and we would appreciate it if you could report it to https://github.com/biomejs/biome/issues/ along with the following information to help us fixing the issue:

Source Location: crates/biome_js_parser/src/lexer/mod.rs:538:9
Thread Name: main
Message: assertion failed: self.source.is_char_boundary(self.position)

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

Successfully merging a pull request may close this issue.

3 participants