-
-
Notifications
You must be signed in to change notification settings - Fork 484
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
Comments
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 ```
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
there are also some configuration bugs here and a general lack of testing
The text was updated successfully, but these errors were encountered: