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

Larger clippy::pedantic and code quality pass on whole repo #5945

Merged
merged 15 commits into from
May 4, 2024

Conversation

JoshuaBatty
Copy link
Member

@JoshuaBatty JoshuaBatty commented May 2, 2024

Description

There's still a ton more to do but going to open this to get merged to avoid conflicts. We can possibly add in some of these rules into CI in a future PR.

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.

@JoshuaBatty JoshuaBatty marked this pull request as draft May 2, 2024 07:04
@JoshuaBatty JoshuaBatty self-assigned this May 2, 2024
Copy link

github-actions bot commented May 2, 2024

Benchmark for 17605d3

Click to view benchmark
Test Base PR %
code_action 5.4±0.08ms 5.6±0.11ms +3.70%
code_lens 343.2±43.66ns 290.1±7.59ns -15.47%
compile 6.5±0.05s 6.5±0.04s 0.00%
completion 4.9±0.10ms 5.1±0.12ms +4.08%
did_change_with_caching 6.5±0.06s 6.4±0.11s -1.54%
document_symbol 1003.8±84.19µs 946.6±18.72µs -5.70%
format 88.0±2.74ms 90.9±1.45ms +3.30%
goto_definition 368.6±5.26µs 369.9±8.53µs +0.35%
highlight 8.7±0.26ms 9.1±0.15ms +4.60%
hover 608.2±17.99µs 625.6±28.17µs +2.86%
idents_at_position 123.4±0.33µs 124.3±1.13µs +0.73%
inlay_hints 657.1±24.70µs 669.3±12.86µs +1.86%
on_enter 484.1±17.79ns 495.0±71.75ns +2.25%
parent_decl_at_position 3.6±0.04ms 3.8±0.03ms +5.56%
prepare_rename 365.0±5.74µs 370.1±9.69µs +1.40%
rename 9.4±0.19ms 9.7±0.17ms +3.19%
semantic_tokens 1030.7±20.11µs 1034.1±20.67µs +0.33%
token_at_position 355.6±3.29µs 364.9±2.86µs +2.62%
tokens_at_position 3.6±0.04ms 3.7±0.03ms +2.78%
tokens_for_file 432.4±2.08µs 429.3±3.90µs -0.72%
traverse 52.8±1.67ms 52.7±1.51ms -0.19%

kayagokalp
kayagokalp previously approved these changes May 2, 2024
Copy link
Member

@kayagokalp kayagokalp left a comment

Choose a reason for hiding this comment

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

Nice! should we also ensure this in CI?

Copy link

github-actions bot commented May 3, 2024

Benchmark for 63bf457

Click to view benchmark
Test Base PR %
code_action 5.4±0.04ms 5.4±0.15ms 0.00%
code_lens 285.9±9.32ns 284.2±18.54ns -0.59%
compile 6.3±0.05s 6.2±0.08s -1.59%
completion 5.0±0.06ms 4.8±0.05ms -4.00%
did_change_with_caching 6.3±0.06s 6.3±0.09s 0.00%
document_symbol 927.8±23.39µs 1008.6±33.52µs +8.71%
format 87.0±1.12ms 91.0±1.41ms +4.60%
goto_definition 368.3±27.69µs 372.3±2.70µs +1.09%
highlight 9.0±0.11ms 8.9±0.26ms -1.11%
hover 610.1±18.23µs 653.9±23.03µs +7.18%
idents_at_position 122.0±0.32µs 123.7±0.70µs +1.39%
inlay_hints 660.6±28.55µs 712.6±22.24µs +7.87%
on_enter 492.0±16.72ns 479.8±7.96ns -2.48%
parent_decl_at_position 3.7±0.04ms 3.6±0.04ms -2.70%
prepare_rename 364.4±8.22µs 393.0±7.73µs +7.85%
rename 9.6±0.13ms 9.3±0.24ms -3.12%
semantic_tokens 1021.3±36.81µs 1052.8±18.01µs +3.08%
token_at_position 356.1±3.55µs 363.8±2.43µs +2.16%
tokens_at_position 3.7±0.05ms 3.6±0.06ms -2.70%
tokens_for_file 428.1±2.76µs 431.0±2.21µs +0.68%
traverse 50.9±1.57ms 50.1±1.92ms -1.57%

Copy link

github-actions bot commented May 3, 2024

Benchmark for f3d1bdf

Click to view benchmark
Test Base PR %
code_action 5.5±0.05ms 5.6±0.16ms +1.82%
code_lens 287.6±4.97ns 334.1±13.48ns +16.17%
compile 6.4±0.10s 6.4±0.06s 0.00%
completion 4.9±0.09ms 5.1±0.19ms +4.08%
did_change_with_caching 6.4±0.07s 6.4±0.06s 0.00%
document_symbol 975.6±33.59µs 1004.9±18.96µs +3.00%
format 87.1±1.31ms 88.7±0.66ms +1.84%
goto_definition 361.3±16.26µs 374.7±8.58µs +3.71%
highlight 9.0±0.05ms 9.2±0.74ms +2.22%
hover 607.0±18.91µs 615.8±23.63µs +1.45%
idents_at_position 122.6±0.43µs 123.1±0.76µs +0.41%
inlay_hints 657.7±9.58µs 672.2±17.66µs +2.20%
on_enter 487.2±13.22ns 501.9±14.77ns +3.02%
parent_decl_at_position 3.7±0.05ms 3.7±0.04ms 0.00%
prepare_rename 353.0±6.27µs 371.2±9.07µs +5.16%
rename 9.5±0.04ms 9.7±0.28ms +2.11%
semantic_tokens 1017.6±25.06µs 1129.1±23.59µs +10.96%
token_at_position 351.2±3.76µs 366.5±4.29µs +4.36%
tokens_at_position 3.7±0.05ms 3.7±0.07ms 0.00%
tokens_for_file 417.9±4.01µs 423.1±5.02µs +1.24%
traverse 52.0±2.17ms 51.9±2.65ms -0.19%

@JoshuaBatty
Copy link
Member Author

Nice! should we also ensure this in CI?

Thanks, yeah I think it would be helpful to add a few more checks above what vanilla clippy gives us. Will try and find a reasonable set of suggestions to add in.

@JoshuaBatty JoshuaBatty marked this pull request as ready for review May 3, 2024 23:16
@JoshuaBatty JoshuaBatty requested review from a team May 3, 2024 23:17
Copy link

github-actions bot commented May 3, 2024

Benchmark for 79dcce9

Click to view benchmark
Test Base PR %
code_action 5.5±0.10ms 5.6±0.13ms +1.82%
code_lens 292.5±6.20ns 328.0±7.83ns +12.14%
compile 6.3±0.06s 6.4±0.07s +1.59%
completion 5.0±0.14ms 5.1±0.62ms +2.00%
did_change_with_caching 6.6±0.09s 6.5±0.10s -1.52%
document_symbol 984.1±24.46µs 1018.6±47.65µs +3.51%
format 88.0±1.40ms 91.0±1.41ms +3.41%
goto_definition 370.0±5.65µs 367.3±6.15µs -0.73%
highlight 9.0±0.14ms 9.0±0.15ms 0.00%
hover 603.9±23.28µs 611.8±18.83µs +1.31%
idents_at_position 122.6±0.34µs 123.6±0.61µs +0.82%
inlay_hints 665.5±18.70µs 658.8±25.69µs -1.01%
on_enter 492.6±17.66ns 502.5±22.80ns +2.01%
parent_decl_at_position 3.7±0.06ms 4.0±0.12ms +8.11%
prepare_rename 363.9±8.58µs 367.9±11.04µs +1.10%
rename 9.7±0.18ms 9.7±0.20ms 0.00%
semantic_tokens 1025.3±13.24µs 1015.0±15.97µs -1.00%
token_at_position 356.2±1.82µs 530.3±1.23µs +48.88%
tokens_at_position 3.7±0.05ms 3.9±0.05ms +5.41%
tokens_for_file 418.6±13.48µs 653.1±6.96µs +56.02%
traverse 52.4±1.27ms 51.0±1.86ms -2.67%

@JoshuaBatty JoshuaBatty requested a review from a team May 4, 2024 10:09
@JoshuaBatty JoshuaBatty enabled auto-merge (squash) May 4, 2024 10:10
@JoshuaBatty JoshuaBatty merged commit 297de57 into master May 4, 2024
37 checks passed
@JoshuaBatty JoshuaBatty deleted the josh/clippy branch May 4, 2024 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants