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

Fix range OOB bug in the language server #6314

Merged
merged 14 commits into from
Jul 31, 2024
Merged

Conversation

JoshuaBatty
Copy link
Member

@JoshuaBatty JoshuaBatty commented Jul 30, 2024

Description

This PR fixes a crash in the language server caused by an out-of-bounds error when using rope. It replaces the ropey crate with a custom implementation using String and Vec<usize> for line offsets. This change improves robustness and prevents panics when handling rapid text changes.

Closes #6313

Checklist

  • I have linked to any relevant issues.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation where relevant (API docs, the reference, and the Sway book).
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added (or requested a maintainer to add) the necessary Breaking* or New Feature labels where relevant.
  • I have done my best to ensure that my PR adheres to the Fuel Labs Code Review Standards.
  • I have requested a review from the relevant team or maintainers.

Sorry, something went wrong.

@JoshuaBatty JoshuaBatty requested a review from a team as a code owner July 30, 2024 00:15
@JoshuaBatty JoshuaBatty requested a review from a team as a code owner July 30, 2024 00:19
@JoshuaBatty JoshuaBatty requested a review from a team July 30, 2024 00:20
@JoshuaBatty JoshuaBatty self-assigned this Jul 30, 2024
@JoshuaBatty JoshuaBatty added bug Something isn't working language server LSP server labels Jul 30, 2024
fmt
Copy link

Benchmark for ecb2644

Click to view benchmark
Test Base PR %
code_action 5.0±0.12ms 5.2±0.04ms +4.00%
code_lens 303.6±14.08ns 285.0±12.04ns -6.13%
compile 2.6±0.03s 2.6±0.05s 0.00%
completion 4.5±0.08ms 4.7±0.05ms +4.44%
did_change_with_caching 2.6±0.06s 2.5±0.06s -3.85%
document_symbol 917.7±10.22µs 931.9±40.49µs +1.55%
format 69.4±1.08ms 67.8±0.79ms -2.31%
goto_definition 337.2±6.34µs 343.1±5.25µs +1.75%
highlight 8.6±0.01ms 9.0±0.10ms +4.65%
hover 495.2±7.41µs 501.6±5.35µs +1.29%
idents_at_position 122.5±4.75µs 118.0±0.43µs -3.67%
inlay_hints 624.7±23.97µs 638.4±29.37µs +2.19%
on_enter 474.9±12.68ns 1991.5±86.51ns +319.35%
parent_decl_at_position 3.6±0.03ms 3.7±0.02ms +2.78%
prepare_rename 333.6±8.38µs 343.9±5.71µs +3.09%
rename 8.9±0.17ms 9.3±0.02ms +4.49%
semantic_tokens 1280.3±14.00µs 1281.0±14.83µs +0.05%
token_at_position 338.9±2.83µs 335.3±3.63µs -1.06%
tokens_at_position 3.6±0.04ms 3.7±0.02ms +2.78%
tokens_for_file 397.0±1.87µs 400.5±3.76µs +0.88%
traverse 37.7±0.89ms 37.1±0.71ms -1.59%

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Copy link

Benchmark for 5b2b5a2

Click to view benchmark
Test Base PR %
code_action 5.0±0.10ms 5.2±0.08ms +4.00%
code_lens 286.9±5.90ns 326.2±10.39ns +13.70%
compile 2.6±0.04s 2.7±0.04s +3.85%
completion 4.6±0.39ms 4.8±0.16ms +4.35%
did_change_with_caching 2.6±0.05s 2.6±0.04s 0.00%
document_symbol 853.0±43.03µs 896.4±40.84µs +5.09%
format 68.9±0.69ms 70.7±1.16ms +2.61%
goto_definition 335.4±7.00µs 347.2±7.08µs +3.52%
highlight 8.6±0.02ms 9.0±0.09ms +4.65%
hover 492.9±8.37µs 502.2±11.80µs +1.89%
idents_at_position 120.2±0.50µs 118.6±1.47µs -1.33%
inlay_hints 616.7±9.25µs 650.5±24.53µs +5.48%
on_enter 471.6±13.02ns 2.1±0.11µs +345.29%
parent_decl_at_position 3.6±0.08ms 3.8±0.06ms +5.56%
prepare_rename 333.3±6.89µs 345.6±7.77µs +3.69%
rename 8.9±0.06ms 9.8±0.14ms +10.11%
semantic_tokens 1275.9±7.06µs 1311.3±10.20µs +2.77%
token_at_position 334.7±4.89µs 339.9±2.31µs +1.55%
tokens_at_position 3.6±0.04ms 3.7±0.01ms +2.78%
tokens_for_file 400.4±5.23µs 399.0±2.24µs -0.35%
traverse 38.2±0.79ms 38.7±1.40ms +1.31%

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Copy link

Benchmark for 1ba0fbb

Click to view benchmark
Test Base PR %
code_action 5.0±0.09ms 5.2±0.01ms +4.00%
code_lens 286.5±4.89ns 333.2±10.58ns +16.30%
compile 2.7±0.03s 2.6±0.05s -3.70%
completion 4.5±0.08ms 4.7±0.05ms +4.44%
did_change_with_caching 2.6±0.06s 2.6±0.02s 0.00%
document_symbol 937.8±56.21µs 852.8±18.25µs -9.06%
format 68.5±1.10ms 68.8±1.15ms +0.44%
goto_definition 345.0±6.50µs 345.3±6.05µs +0.09%
highlight 8.7±0.23ms 9.0±0.01ms +3.45%
hover 499.3±6.58µs 497.8±4.87µs -0.30%
idents_at_position 121.0±0.97µs 118.8±0.38µs -1.82%
inlay_hints 633.8±35.67µs 648.4±28.08µs +2.30%
on_enter 467.5±13.23ns 2.0±0.06µs +327.81%
parent_decl_at_position 3.6±0.02ms 3.7±0.04ms +2.78%
prepare_rename 344.4±5.99µs 348.5±5.33µs +1.19%
rename 8.9±0.14ms 9.3±0.02ms +4.49%
semantic_tokens 1260.2±22.46µs 1243.9±6.63µs -1.29%
token_at_position 340.5±2.76µs 337.9±4.06µs -0.76%
tokens_at_position 3.5±0.03ms 3.7±0.08ms +5.71%
tokens_for_file 403.0±5.29µs 394.9±2.07µs -2.01%
traverse 37.2±0.83ms 37.8±0.60ms +1.61%

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Copy link

Benchmark for 1349a26

Click to view benchmark
Test Base PR %
code_action 5.0±0.12ms 5.2±0.21ms +4.00%
code_lens 288.0±5.47ns 327.7±11.19ns +13.78%
compile 2.6±0.08s 2.6±0.04s 0.00%
completion 4.6±0.09ms 4.7±0.03ms +2.17%
did_change_with_caching 2.6±0.05s 2.6±0.07s 0.00%
document_symbol 893.0±22.40µs 933.1±23.79µs +4.49%
format 69.1±0.83ms 68.3±0.76ms -1.16%
goto_definition 353.3±6.29µs 349.5±8.71µs -1.08%
highlight 8.7±0.17ms 9.0±0.10ms +3.45%
hover 511.2±18.77µs 501.1±8.04µs -1.98%
idents_at_position 118.7±0.63µs 118.2±1.19µs -0.42%
inlay_hints 631.4±23.39µs 642.9±26.55µs +1.82%
on_enter 481.4±12.34ns 2.0±0.05µs +315.45%
parent_decl_at_position 3.6±0.13ms 3.7±0.02ms +2.78%
prepare_rename 352.3±10.00µs 342.8±5.85µs -2.70%
rename 8.9±0.15ms 9.3±0.12ms +4.49%
semantic_tokens 1269.8±14.95µs 1245.0±9.17µs -1.95%
token_at_position 338.6±3.11µs 332.2±5.01µs -1.89%
tokens_at_position 3.6±0.04ms 3.7±0.03ms +2.78%
tokens_for_file 399.6±2.42µs 394.8±2.17µs -1.20%
traverse 37.9±0.97ms 37.2±0.93ms -1.85%

Copy link
Member

@sdankel sdankel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a 300% increase in the benchmark for on_enter, but it's still only 2 microseconds total, which is acceptable. Looks good to me!

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Copy link

Benchmark for e475a13

Click to view benchmark
Test Base PR %
code_action 5.1±0.09ms 5.3±0.03ms +3.92%
code_lens 287.3±7.85ns 326.6±10.89ns +13.68%
compile 2.7±0.05s 2.7±0.05s 0.00%
completion 4.6±0.03ms 4.7±0.09ms +2.17%
did_change_with_caching 2.6±0.06s 2.7±0.04s +3.85%
document_symbol 848.6±17.61µs 945.8±25.72µs +11.45%
format 69.5±0.84ms 69.2±0.53ms -0.43%
goto_definition 342.7±9.34µs 338.7±6.63µs -1.17%
highlight 8.7±0.19ms 9.0±0.03ms +3.45%
hover 497.0±5.44µs 496.4±28.34µs -0.12%
idents_at_position 122.6±0.26µs 120.5±0.56µs -1.71%
inlay_hints 627.4±27.59µs 642.8±22.25µs +2.45%
on_enter 475.2±30.56ns 1900.4±37.45ns +299.92%
parent_decl_at_position 3.6±0.03ms 3.7±0.03ms +2.78%
prepare_rename 341.4±8.78µs 342.6±8.90µs +0.35%
rename 9.0±0.17ms 9.6±0.71ms +6.67%
semantic_tokens 1259.3±11.65µs 1278.2±14.52µs +1.50%
token_at_position 346.1±11.79µs 338.5±11.94µs -2.20%
tokens_at_position 3.6±0.07ms 3.7±0.14ms +2.78%
tokens_for_file 401.1±2.62µs 400.4±2.75µs -0.17%
traverse 38.4±1.03ms 38.5±1.06ms +0.26%

@JoshuaBatty JoshuaBatty enabled auto-merge (squash) July 31, 2024 09:44

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Copy link

Benchmark for 98bca02

Click to view benchmark
Test Base PR %
code_action 5.0±0.11ms 5.2±0.04ms +4.00%
code_lens 346.9±9.95ns 292.1±7.84ns -15.80%
compile 2.6±0.05s 2.6±0.04s 0.00%
completion 4.6±0.06ms 4.7±0.19ms +2.17%
did_change_with_caching 2.5±0.05s 2.6±0.09s +4.00%
document_symbol 859.1±23.62µs 863.9±21.26µs +0.56%
format 70.6±0.53ms 70.4±2.35ms -0.28%
goto_definition 347.0±6.80µs 340.5±11.75µs -1.87%
highlight 8.7±0.12ms 9.1±0.11ms +4.60%
hover 505.6±10.36µs 498.0±7.14µs -1.50%
idents_at_position 121.9±0.32µs 117.4±0.27µs -3.69%
inlay_hints 635.9±11.61µs 645.1±36.23µs +1.45%
on_enter 471.3±11.63ns 2.1±0.06µs +345.58%
parent_decl_at_position 3.6±0.06ms 3.7±0.06ms +2.78%
prepare_rename 345.2±6.64µs 337.1±5.78µs -2.35%
rename 8.9±0.18ms 9.4±0.48ms +5.62%
semantic_tokens 1309.6±9.83µs 1264.2±19.82µs -3.47%
token_at_position 340.1±2.48µs 335.0±2.51µs -1.50%
tokens_at_position 3.6±0.05ms 3.7±0.05ms +2.78%
tokens_for_file 402.0±2.52µs 403.3±3.69µs +0.32%
traverse 38.5±0.81ms 37.9±1.37ms -1.56%

@JoshuaBatty JoshuaBatty merged commit 0245223 into master Jul 31, 2024
39 checks passed
@JoshuaBatty JoshuaBatty deleted the josh/text_document branch July 31, 2024 14:17
esdrubal pushed a commit that referenced this pull request Aug 13, 2024
## Description
This PR fixes a crash in the language server caused by an out-of-bounds
error when using `rope`. It replaces the `ropey` crate with a custom
implementation using `String` and `Vec<usize>` for line offsets. This
change improves robustness and prevents panics when handling rapid text
changes.

Closes #6313

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [x] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working language server LSP server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Language server crashes with an OOB error when using rope in the Documents
4 participants