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

linter: unicorn/filename-case does not match behavior of original rule #6459

Closed
camchenry opened this issue Oct 11, 2024 · 1 comment · Fixed by #6463
Closed

linter: unicorn/filename-case does not match behavior of original rule #6459

camchenry opened this issue Oct 11, 2024 · 1 comment · Fixed by #6463
Assignees
Labels
A-linter Area - Linter C-bug Category - Bug

Comments

@camchenry
Copy link
Collaborator

  • original rule only enables kebab case by default where we disable by default
  • original rule only looks at name up to first dot, we look at the whole file name

there are also some configuration bugs here and a general lack of testing

@camchenry camchenry added the C-bug Category - Bug label Oct 11, 2024
@camchenry camchenry self-assigned this Oct 11, 2024
@camchenry camchenry added the A-linter Area - Linter label Oct 11, 2024
Boshen pushed a commit that referenced this issue Oct 13, 2024
…e` (#6463)

- closes #6459

Currently, the `unicorn/filename-case` doesn't match the behavior of the original rule. There are several issues which this PR fixes:

- The defaults cases were incorrect: only kebab case should be enabled by default, according to the original rule docs.
- Setting a single case or multiple cases did not remove the default cases as it should.
- Leading/trailing underscores were not ignored by default.
- We did not provide a clear diagnostic message indicating which cases are allowed.
- We did not try to parse out multiple file parts (separated by `.`).
  - TODO: We should also support multiple file part checking (which the original rule supports via a config option), for file names such as `someTest.fileName.js`

I have also added many of the original test cases to ensure we are more closely compatible.

This also improves the performance of just running the `unicorn/filename-case` alone by 4% (20ms total) on the `vscode` codebase:

```
Benchmark 1: ./oxlint-main -A all -W unicorn/filename-case --silent vscode
  Time (mean ± σ):     489.4 ms ±  24.0 ms    [User: 2623.8 ms, System: 447.2 ms]
  Range (min … max):   474.7 ms … 622.9 ms    100 runs

  Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options.

Benchmark 2: ./oxlint-new-filename-case -A all -W unicorn/filename-case --silent vscode
  Time (mean ± σ):     470.8 ms ±  22.9 ms    [User: 2478.8 ms, System: 463.5 ms]
  Range (min … max):   455.6 ms … 599.3 ms    100 runs

  Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options.

Summary
  ./oxlint-new-filename-case -A all -W unicorn/filename-case --silent vscode ran
    1.04 ± 0.07 times faster than ./oxlint-main -A all -W unicorn/filename-case --silent vscode
```

Perplexingly, it seems like it might actually be even faster on the `vscode` repository, saving ~5% (70ms) with the default ruleset enabled as well. Maybe my laptop was just running a bit faster.

```
Benchmark 1: ./oxlint-main -W unicorn/filename-case --silent vscode
  Time (mean ± σ):      1.402 s ±  0.096 s    [User: 8.863 s, System: 0.505 s]
  Range (min … max):    1.318 s …  1.920 s    100 runs

  Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options.

Benchmark 2: ./oxlint-new-filename-case -W unicorn/filename-case --silent vscode
  Time (mean ± σ):      1.339 s ±  0.042 s    [User: 8.582 s, System: 0.511 s]
  Range (min … max):    1.266 s …  1.506 s    100 runs

Summary
  ./oxlint-new-filename-case -W unicorn/filename-case --silent vscode ran
    1.05 ± 0.08 times faster than ./oxlint-main -W unicorn/filename-case --silent vscode
```
@camchenry
Copy link
Collaborator Author

mostly addressed in #6463, still some config options to support but it is much closer to the correct behavior now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linter Area - Linter C-bug Category - Bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant