#8735 removes `tmp_bindings` and then calls `Iterator::sorted_unstabled` instead, I thought it ideal but after I saw performance regression, I looked at `sorted_unstabled`'s implementation, and it will cause an allocation every run, but `tmp_bindings` only allocate once, then clear and reuse it in that loop.
closes#8594
This is because previously, oxlintrc.path was determined by
`path.canonicalize().unwrap_or_else(|_| path.to_path_buf())`, but after
#8214, it was changed to `path.to_path_buf()`. As a result,
`\\?\D:\shulaoda` became `D:\shulaoda`. This caused the condition
`uri_path.starts_with(gitignore.path())` to evaluate as `true`,
preventing the bypass of the `is_ignore` check and exposing the original
code issue.
b1499e6711/crates/oxc_language_server/src/main.rs (L531-L533)
Just some low-hanging fruit optimization.
I initially want to dedupe for reference scopes, so that we can avoid calling `scope_tree.ancestors(used_scope_id).take_while(|s_id| *s_id != scope_id)` for duplicate `used_scope_id` but the overhead of calling `unique` or `dedup` is over the improvement.
The previous implementation is a little weird, I think it should be a shortcut of `bindings.iter`, It is infrequent we need to iter all symbols and know its scope ID. Only two cases in tests, and no use case in `Rolldown`
This pull request implements the
[vitest/prefer-lowercase-title](https://github.com/vitest-dev/eslint-plugin-vitest/blob/main/docs/rules/prefer-lowercase-title.md)
rule.
Since there was an existing jest rule with this title, I followed the
existing pattern in
[no-unused-vars](https://github.com/oxc-project/oxc/blob/main/crates/oxc_linter/src/rules/eslint/no_unused_vars/mod.rs)
to group the jest and vitest rules together in a shared module. I used
the existing `jest/prefer-lowercase-title` documentation as a base and
modified it where it seemed appropriate. I added a `jest` and `vitest`
snapshot suffix for each respective test suite.
One item I wasn't 100% about is adding `bench` to the jest test names.
Without this change, the vitest test suite fails because of [this
check](https://github.com/oxc-project/oxc/blob/main/crates/oxc_linter/src/utils/jest/parse_jest_fn.rs#L108)
which validates that we're only parsing valid jest functions from a
detected jest file. The unit tests that are sourced from the vitest
plugin are all read by the linting host as jest-like files, so adding
`bench` as a "valid" jest method allows us to lint a unit test using
this keyword. This seemed to me like the least invasive solution to
accommodate the new rule without breaking any existing code, but I'm
certainly open to alternatives.
This PR fixes#8697 by checking if semantic detects a comment in the
span of an otherwise empty block statement.
I also formatted some of the existing test cases to improve readability.
Fixes broken diagnostic spans for async functions that did not correctly report on the `async` keyword. I changed the diagnostic reporting to look for the `async` keyword itself within a given span which is a little slower but worth it for accuracy I think.
I also updated the diagnostic to be async-specific as we don't report on await yet.
windows will fail, looks like the offset missmatch is because of `\r\n`
vs `\n`.
```
Snapshot file: apps\oxlint\src\snapshots\--format=json test.js@oxlint.snap
Snapshot: --format=json test.js@oxlint
Source: C:\dev\oxc:74
──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────-old snapshot
+new results
────────────┬───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── 0 0 │ ##########
1 1 │ --format=json test.js
2 2 │ ----------
3 3 │ [
4 │- {"message": "`debugger` statement is not allowed","code": "eslint(no-debugger)","severity": "error","causes": [],"url": "https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-debugger.html","help": "Delete this code.","filename": "test.js","labels": [{"span": {"offset": 38,"length": 9}}],"related": []},
4 │+ {"message": "`debugger` statement is not allowed","code": "eslint(no-debugger)","severity": "error","causes": [],"url": "https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-debugger.html","help": "Delete this code.","filename": "test.js","labels": [{"span": {"offset": 42,"length": 9}}],"related": []},
5 5 │ {"message": "Function 'foo' is declared but never used.","code": "eslint(no-unused-vars)","severity": "warning","causes": [],"url": "https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-unused-vars.html","help": "Consider removing this declaration.","filename": "test.js","labels": [{"label": "'foo' is declared here","span": {"offset": 9,"length": 3}}],"related": []},
6 6 │ {"message": "Parameter 'b' is declared but never used. Unused parameters should start with a '_'.","code": "eslint(no-unused-vars)","severity": "warning","causes": [],"url": "https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-unused-vars.html","help": "Consider removing this parameter.","filename": "test.js","labels": [{"label": "'b' is declared here","span": {"offset": 16,"length": 1}}],"related": []}
7 7 │ ]
────────────┴─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────To update snapshots run `cargo insta review`
Stopped on the first failure. Run `cargo insta test` to run all snapshots.
thread 'output_formatter::test::test_output_formatter_diagnostic_json' panicked at C:\Users\sysix\.cargo\registry\src\index.crates.io-6f17d22bba15001f\insta-1.42.0\src\runtime.rs:679:13:
snapshot assertion for '--format=json test.js@oxlint' failed in line 74
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
---- output_formatter::test::test_output_formatter_diagnostic_stylish stdout ----
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Snapshot Summary ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━Snapshot file: apps\oxlint\src\snapshots\--format=stylish test.js@oxlint.snap
Snapshot: --format=stylish test.js@oxlint
Source: C:\dev\oxc:74
──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────-old snapshot
+new results
────────────┬───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── 3 3 │
4 4 │ ␛[4mtest.js␛[0m␊
5 5 │ ␛[2m9:3 ␛[0m ␛[33mwarning␛[0m Function 'foo' is declared but never used. ␛[2meslint(no-unused-vars)␛[0m␊
6 6 │ ␛[2m16:1␛[0m ␛[33mwarning␛[0m Parameter 'b' is declared but never used. Unused parameters should start with a '_'. ␛[2meslint(no-unused-vars)␛[0m␊
7 │- ␛[2m38:9␛[0m ␛[31merror␛[0m `debugger` statement is not allowed ␛[2meslint(no-debugger)␛[0m␊
7 │+ ␛[2m42:9␛[0m ␛[31merror␛[0m `debugger` statement is not allowed ␛[2meslint(no-debugger)␛[0m␊
8 8 │
9 9 │ ␛[31m✖ 3 problems (1 error, 2 warnings)␛[0m
────────────┴─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────To update snapshots run `cargo insta review`
Stopped on the first failure. Run `cargo insta test` to run all snapshots.
thread 'output_formatter::test::test_output_formatter_diagnostic_stylish' panicked at C:\Users\sysix\.cargo\registry\src\index.crates.io-6f17d22bba15001f\insta-1.42.0\src\runtime.rs:679:13:
snapshot assertion for '--format=stylish test.js@oxlint' failed in line 74
failures:
output_formatter::test::test_output_formatter_diagnostic_json
output_formatter::test::test_output_formatter_diagnostic_stylish
test result: FAILED. 85 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.12s
```
Changed the mangler to reuse variable names where possible.
This will reduce the code size as shorter variable names can be used in
more places. But requires global information and limits parallelism in a
single file and requires more memory.
---------
Co-authored-by: Boshen <boshenc@gmail.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
This is needed so we can use a custom `Write` implementation (or just `[u8]`) to make snapshots.
In this step I also updated `OutputFormatter::all_rules` so the Formatter does not need to handle the write-error.
Now every lint output is owned by is right OutputFormatter and his DiagnosticReporter 🥳
Next step is to setup a snapshot Tester, so I can remove the ToDos.
Reorded some lines so the outfor is now for: `cargo run -p oxlint -- test.js --max-warnings=2`
```
Found 4 warnings and 0 errors.
Exceeded maximum number of warnings. Found 4.
Finished in 5ms on 1 file with 97 rules using 24 threads.
```
and for `cargo run -p oxlint -- test.js`
```
Found 4 warnings and 0 errors.
Finished in 5ms on 1 file with 97 rules using 24 threads.
```
The output time and warnings/error count wil be always printed.
