Skip to content

Update scripts to add and sort rules#3747

Closed
JonathanPlasse wants to merge 47 commits intoastral-sh:mainfrom
JonathanPlasse:sort-rules
Closed

Update scripts to add and sort rules#3747
JonathanPlasse wants to merge 47 commits intoastral-sh:mainfrom
JonathanPlasse:sort-rules

Conversation

@JonathanPlasse
Copy link
Copy Markdown
Contributor

@JonathanPlasse JonathanPlasse commented Mar 26, 2023 •
edited
Loading

The aim of this PR is to reintroduce the use of add_rule.py and add_plugin.py in CONTRIBUTING.md.
This PR also fixes the sorting of rules, this means that all new code will be sorted correctly.
The CI job to test these scripts has been improved and should test all cases.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 26, 2023 •
edited
Loading

PR Check Results

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00     10.5±0.35ms     3.9 MB/sec    1.00     10.5±0.38ms     3.9 MB/sec
formatter/numpy/ctypeslib.py               1.00      2.1±0.09ms     8.0 MB/sec    1.00      2.1±0.12ms     8.0 MB/sec
formatter/numpy/globals.py                 1.00   239.6±10.95µs    12.3 MB/sec    1.02   244.2±12.60µs    12.1 MB/sec
formatter/pydantic/types.py                1.00      4.5±0.18ms     5.6 MB/sec    1.02      4.6±0.21ms     5.5 MB/sec
linter/all-rules/large/dataset.py          1.00     15.2±0.43ms     2.7 MB/sec    1.00     15.2±0.39ms     2.7 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.01      3.9±0.15ms     4.3 MB/sec    1.00      3.9±0.15ms     4.3 MB/sec
linter/all-rules/numpy/globals.py          1.01   526.3±19.10µs     5.6 MB/sec    1.00   520.7±20.62µs     5.7 MB/sec
linter/all-rules/pydantic/types.py         1.00      7.0±0.25ms     3.7 MB/sec    1.00      7.0±0.22ms     3.6 MB/sec
linter/default-rules/large/dataset.py      1.01      7.7±0.34ms     5.3 MB/sec    1.00      7.6±0.21ms     5.3 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1639.9±65.66µs    10.2 MB/sec    1.00  1643.8±56.36µs    10.1 MB/sec
linter/default-rules/numpy/globals.py      1.00    195.9±9.24µs    15.1 MB/sec    1.00    196.6±7.92µs    15.0 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.5±0.11ms     7.3 MB/sec    1.00      3.5±0.12ms     7.3 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00     11.4±0.15ms     3.6 MB/sec    1.04     11.9±0.14ms     3.4 MB/sec
formatter/numpy/ctypeslib.py               1.00      2.2±0.05ms     7.5 MB/sec    1.03      2.3±0.03ms     7.3 MB/sec
formatter/numpy/globals.py                 1.00    243.0±6.01µs    12.1 MB/sec    1.05   254.4±18.02µs    11.6 MB/sec
formatter/pydantic/types.py                1.00      4.9±0.07ms     5.2 MB/sec    1.04      5.1±0.07ms     5.0 MB/sec
linter/all-rules/large/dataset.py          1.00     16.0±0.16ms     2.5 MB/sec    1.00     15.9±0.16ms     2.6 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      4.1±0.06ms     4.0 MB/sec    1.00      4.1±0.06ms     4.0 MB/sec
linter/all-rules/numpy/globals.py          1.01    500.4±7.94µs     5.9 MB/sec    1.00    497.2±7.57µs     5.9 MB/sec
linter/all-rules/pydantic/types.py         1.00      7.2±0.08ms     3.5 MB/sec    1.00      7.2±0.10ms     3.5 MB/sec
linter/default-rules/large/dataset.py      1.01      8.3±0.07ms     4.9 MB/sec    1.00      8.2±0.08ms     4.9 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1710.6±17.48µs     9.7 MB/sec    1.00  1703.6±16.06µs     9.8 MB/sec
linter/default-rules/numpy/globals.py      1.00    193.2±4.01µs    15.3 MB/sec    1.00    192.4±3.41µs    15.3 MB/sec
linter/default-rules/pydantic/types.py     1.01      3.7±0.05ms     7.0 MB/sec    1.00      3.6±0.04ms     7.0 MB/sec

@JonathanPlasse
Copy link
Copy Markdown
Contributor Author

JonathanPlasse commented May 20, 2023

CI will now fail if the rules are not sorted.
CI fail
CI success

@JonathanPlasse JonathanPlasse marked this pull request as ready for review May 20, 2023 22:27
@JonathanPlasse
Copy link
Copy Markdown
Contributor Author

JonathanPlasse commented May 21, 2023

Only thing remaining, adding the instructions in CONTRIBUTING.md to use these scripts.

@JonathanPlasse
Copy link
Copy Markdown
Contributor Author

JonathanPlasse commented May 21, 2023 •
edited
Loading

Currently there are 24 plugins that uses the old architecture:

  • flake8-async
  • flake8-builtins
  • flake8-blind-except
  • flake8-debugger
  • mccabe
  • flake8-tidy-imports
  • flake8-return
  • flake8-gettext
  • flake8-implicit-str-concat
  • flake8-quotes
  • flake8-annotations
  • flake8-future-annotations
  • flake8-2020
  • eradicate
  • flake8-boolean-trap
  • flake8-unused-arguments
  • flake8-datetimez
  • flake8-errmsg
  • flake8-pie
  • flake8-commas
  • flake8-no-pep420
  • flake8-use-pathlib
  • flake8-logging-format
  • flake8-todos

Ideally, all plugins should be updated to use the new rules architecture as currently the files will be generated but will need manual tweaking with the old architecture.
The CI test_add_rule only runs with rules with the new architecture.

@JonathanPlasse
Copy link
Copy Markdown
Contributor Author

JonathanPlasse commented May 21, 2023

Ready for review.

@JonathanPlasse
Copy link
Copy Markdown
Contributor Author

JonathanPlasse commented May 21, 2023 •
edited
Loading

This is ready to merge as is.
Following PRs will be needed to:

  • Sort all rules
  • Enable keeping rules in CI
  • JonathanPlasse/ruff#2

@JonathanPlasse
Copy link
Copy Markdown
Contributor Author

JonathanPlasse commented May 22, 2023

I finished converting all old architectures to the new one.
I would like both PRs to be merged to avoid having to rebase them.

@JonathanPlasse JonathanPlasse mentioned this pull request May 22, 2023
Merged
@MichaReiser MichaReiser requested a review from charliermarsh May 24, 2023 06:35
@JonathanPlasse
Copy link
Copy Markdown
Contributor Author

JonathanPlasse commented May 24, 2023

This was referenced May 24, 2023
Closed
Closed
@JonathanPlasse JonathanPlasse marked this pull request as draft May 30, 2023 14:45
@JonathanPlasse JonathanPlasse marked this pull request as ready for review May 30, 2023 20:54
@JonathanPlasse JonathanPlasse mentioned this pull request Jun 2, 2023
Merged
@JonathanPlasse
Copy link
Copy Markdown
Contributor Author

JonathanPlasse commented Jun 2, 2023

JonathanPlasse
JonathanPlasse commented Jun 2, 2023
View reviewed changes
Comment thread scripts/_utils.py
Comment on lines +44 to +45
r"(?s)Rule::(.*?),.*?Path::new\("
r'(?:"(?:.*?)?([A-Z]+)([0-9]+)?|.+?)(?:.*?)?(_[0-9]+)?(?:.(?:pyi?|txt))?"'
Copy link
Copy Markdown
Contributor Author

@JonathanPlasse JonathanPlasse Jun 2, 2023

Choose a reason for hiding this comment

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

Would you like some documentation for the regex?

JonathanPlasse referenced this pull request Jun 2, 2023
@JonathanPlasse
Fix flake8-fixme architecture (#4807)
03ee603
@JonathanPlasse
Fix pandas-vet test case sorting
3e00c6c
@JonathanPlasse
Fix pygrep-hooks fixture path
d69c3fe
@JonathanPlasse
Fix sort_exports()
d33c540
@JonathanPlasse
Create rules folder if needed
690bb41
@JonathanPlasse
Fix sort_exports() for ruff specific rules
4d39039
@JonathanPlasse
Fix pascal_case() for pep8-naming
c974654
@JonathanPlasse
Fix sort_exports() for ruff specific rules
acdf1b5
@JonathanPlasse
Add test add_rule on each linter in CI
4ca82f4
@JonathanPlasse
Update CONTRIBUTING.md
642b82d
@JonathanPlasse
Use should_create instead
3ecfaab
@JonathanPlasse
Disable git diff in CI
6053303
@JonathanPlasse
Add _utils.get_linters_with_deprecated_architecture()
69d01c2
@JonathanPlasse
Fix pascal_case for flake8-errmsg
5a7d436
@JonathanPlasse
Fix pascal_case() for some linters
2bdd5eb
@JonathanPlasse
Remove old architecture from scripts
c22322b
@JonathanPlasse
Remove remnant rules.rs
7c00a7f
@JonathanPlasse
Fix pep8-naming sorting
7be40dd
@JonathanPlasse
Fix pandas-vet sorting
8b619d7
@JonathanPlasse
Handle muti-line test cases
5c89fe8
@JonathanPlasse
Improve regex to find more rule prefix and code
ebf0a4d
@JonathanPlasse
Fix add_code_to_rule_pair()
3793049
@JonathanPlasse
Fix ruff/rules/mod.rs
1d2c525
charliermarsh added a commit that referenced this pull request Jul 21, 2023
@charliermarsh
Port over some fixes from #3747
1ed4d2e
charliermarsh added a commit that referenced this pull request Jul 21, 2023
@charliermarsh
Port over some fixes from #3747 (#5940)
0301889
@charliermarsh
Copy link
Copy Markdown
Member

charliermarsh commented Jul 21, 2023

Hey @JonathanPlasse - sorry to let this one sit for so long. I really appreciate the work here, but after discussion with some other maintainers, we've opted not to merge this. The main concern is that we're signing up for ongoing maintenance of these Python scripts which are getting increasingly complex and will need to change whenever we change implementation details of the rule files. In other words: they're parsing a lot of Rust code via regex and otherwise, and that Python parsing code will likely need modifications over time. As one example: the current version doesn't handle feature flags, and so is broken in the Ruff rules. I'm sure we could fix that bug, but it's just an example of the kind of thing we'll run into over time. Again, thank you for putting it together and sorry for the extensive delay.

@JonathanPlasse JonathanPlasse deleted the sort-rules branch October 17, 2023 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@charliermarsh charliermarsh Awaiting requested review from charliermarsh

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@JonathanPlasse @charliermarsh