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

Migrate some rules from Fix::unspecified #4587

Merged
merged 3 commits into from
May 24, 2023

Conversation

sladyn98
Copy link
Contributor

@sladyn98 sladyn98 commented May 22, 2023

This PR aims to migrate unspecified code to safe or maybe_correct

Refers: #4184

@@ -94,7 +94,7 @@ fn duplicate_handler_exceptions<'a>(
);
if checker.patch(diagnostic.kind.rule()) {
#[allow(deprecated)]
diagnostic.set_fix(Fix::unspecified(Edit::range_replacement(
diagnostic.set_fix(Fix::maybe_correct(Edit::range_replacement(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a specific rule to follow when changing to safe or maybe_correct, right now I just went off the issue description where it might introduce semantic changes

Copy link
Member

Choose a reason for hiding this comment

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

fyi the names have changed per #4184 (comment)

The meaning for each applicability level is documented in the Applicability enum

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah it makes sense now.

@sladyn98 sladyn98 marked this pull request as draft May 22, 2023 21:21
@zanieb
Copy link
Member

zanieb commented May 22, 2023

Could you remove the Closes #4184? The issue covers many more cases where the applicability needs to be specified and shouldn't be closed yet. Generally we're expecting a pull request per modified rule so that there can be proper discussion about the correct applicability level.

@sladyn98 sladyn98 requested a review from zanieb May 22, 2023 23:15
@github-actions
Copy link
Contributor

github-actions bot commented May 22, 2023

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
linter/all-rules/large/dataset.py          1.00     16.4±0.22ms     2.5 MB/sec    1.01     16.5±0.19ms     2.5 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.01      4.0±0.04ms     4.1 MB/sec    1.00      4.0±0.05ms     4.2 MB/sec
linter/all-rules/numpy/globals.py          1.00    483.7±9.97µs     6.1 MB/sec    1.00    485.7±9.79µs     6.1 MB/sec
linter/all-rules/pydantic/types.py         1.03      6.9±0.08ms     3.7 MB/sec    1.00      6.7±0.16ms     3.8 MB/sec
linter/default-rules/large/dataset.py      1.03      7.7±0.26ms     5.3 MB/sec    1.00      7.5±0.15ms     5.4 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1624.3±49.78µs    10.3 MB/sec    1.02  1660.7±32.13µs    10.0 MB/sec
linter/default-rules/numpy/globals.py      1.03    190.1±2.52µs    15.5 MB/sec    1.00    184.2±3.89µs    16.0 MB/sec
linter/default-rules/pydantic/types.py     1.01      3.5±0.10ms     7.4 MB/sec    1.00      3.4±0.09ms     7.5 MB/sec
parser/large/dataset.py                    1.00      6.0±0.23ms     6.8 MB/sec    1.04      6.2±0.06ms     6.6 MB/sec
parser/numpy/ctypeslib.py                  1.00  1165.0±32.34µs    14.3 MB/sec    1.05   1228.8±6.34µs    13.6 MB/sec
parser/numpy/globals.py                    1.00    124.2±2.38µs    23.8 MB/sec    1.01    125.4±1.14µs    23.5 MB/sec
parser/pydantic/types.py                   1.00      2.6±0.06ms     9.7 MB/sec    1.02      2.7±0.02ms     9.5 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
linter/all-rules/large/dataset.py          1.01     16.8±0.24ms     2.4 MB/sec    1.00     16.6±0.27ms     2.4 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      4.2±0.11ms     3.9 MB/sec    1.01      4.3±0.12ms     3.9 MB/sec
linter/all-rules/numpy/globals.py          1.00    487.2±7.08µs     6.1 MB/sec    1.00    489.0±6.80µs     6.0 MB/sec
linter/all-rules/pydantic/types.py         1.00      7.0±0.14ms     3.6 MB/sec    1.00      7.0±0.16ms     3.6 MB/sec
linter/default-rules/large/dataset.py      1.01      8.2±0.26ms     5.0 MB/sec    1.00      8.1±0.10ms     5.0 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1696.3±26.08µs     9.8 MB/sec    1.00  1696.7±22.16µs     9.8 MB/sec
linter/default-rules/numpy/globals.py      1.00    187.6±3.24µs    15.7 MB/sec    1.02    190.5±4.81µs    15.5 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.6±0.05ms     7.0 MB/sec    1.01      3.7±0.06ms     7.0 MB/sec
parser/large/dataset.py                    1.00      6.5±0.06ms     6.2 MB/sec    1.00      6.5±0.06ms     6.2 MB/sec
parser/numpy/ctypeslib.py                  1.00  1240.4±15.38µs    13.4 MB/sec    1.00  1240.3±15.21µs    13.4 MB/sec
parser/numpy/globals.py                    1.00    126.7±2.43µs    23.3 MB/sec    1.00    127.0±2.16µs    23.2 MB/sec
parser/pydantic/types.py                   1.00      2.8±0.03ms     9.1 MB/sec    1.00      2.8±0.03ms     9.1 MB/sec

@sladyn98 sladyn98 changed the title Migrate Fix::unspecified to Fix::safe or Fix::maybe_correct Migrate Fix::unspecified to Fix::safe or Fix::suggested May 22, 2023
@sladyn98 sladyn98 marked this pull request as ready for review May 23, 2023 19:58
@sladyn98
Copy link
Contributor Author

@MichaReiser Do let me know if the choices made here for safe and suggested are correct. Marked this PR ready for review

@charliermarsh
Copy link
Member

I'll take a look at this tonight.

@charliermarsh charliermarsh requested review from charliermarsh and removed request for zanieb May 23, 2023 22:57
@charliermarsh
Copy link
Member

Reviewed these choices and they look reasonable to me based on the definitions.

@charliermarsh charliermarsh changed the title Migrate Fix::unspecified to Fix::safe or Fix::suggested Migrate some rules from Fix::unspecified May 24, 2023
@charliermarsh charliermarsh merged commit 4e84e8a into astral-sh:main May 24, 2023
@sladyn98 sladyn98 deleted the fix-unspecified branch May 24, 2023 04:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants