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

Split UP035 (deprecated-import) for aliases #7436

Open
auscompgeek opened this issue Sep 16, 2023 · 3 comments
Open

Split UP035 (deprecated-import) for aliases #7436

auscompgeek opened this issue Sep 16, 2023 · 3 comments
Labels
help wanted Contributions especially welcome rule Implementing or modifying a lint rule

Comments

@auscompgeek
Copy link

I'd like to request the deprecated-import rule be split in two. Currently UP035 flags two different kinds of deprecations:

  • Names that have been moved, with a deprecated alias. Examples:
    • collections.* => collections.abc.*
    • {pipes => shlex}.quote
  • Types that are deprecated, considered equivalent by type checkers, but are different runtime values.
    • This is everything in the typing, typing_extensions and mypy_extensions modules.

For the former group, fixing them is significantly safer, since the only visible side-effects would be that a module wasn't imported (in the case of pipes), and a lack of a DeprecationWarning. It'd be rare for code to depend on those side-effects. The latter group can potentially be an unsafe change if something tries to observe the types directly at runtime though.

Ultimately I'd like to be able to autofix the moved names, whilst being able to separately flag (with a suggested fix) the typing changes that would be observable at runtime.

@charliermarsh charliermarsh added rule Implementing or modifying a lint rule needs-decision Awaiting a decision from a maintainer labels Sep 19, 2023
@charliermarsh
Copy link
Member

Once we have #4181, I could see us making the former rules automatic fixes and the latter suggested. I'm hesitant to split these into two rules since the difference between them is so subtle, and more rules means more configuration burden for users.

@charliermarsh charliermarsh removed the needs-decision Awaiting a decision from a maintainer label Sep 19, 2023
zanieb added a commit that referenced this issue Oct 6, 2023
Rebase of #5119 authored by
@evanrittenhouse with additional refinements.

## Changes

- Adds `--unsafe-fixes` / `--no-unsafe-fixes` flags to `ruff check`
- Violations with unsafe fixes are not shown as fixable unless opted-in
- Fix applicability is respected now
    - `Applicability::Never` fixes are no longer applied
    - `Applicability::Sometimes` fixes require opt-in
    - `Applicability::Always` fixes are unchanged
- Hints for availability of `--unsafe-fixes` added to `ruff check`
output

## Examples

Check hints at hidden unsafe fixes
```
❯ ruff check example.py --no-cache --select F601,W292
example.py:1:14: F601 Dictionary key literal `'a'` repeated
example.py:2:15: W292 [*] No newline at end of file
Found 2 errors.
[*] 1 fixable with the `--fix` option (1 hidden fix can be enabled with the `--unsafe-fixes` option).
```

We could add an indicator for which violations have hidden fixes in the
future.

Check treats unsafe fixes as applicable with opt-in
```
❯ ruff check example.py --no-cache --select F601,W292 --unsafe-fixes
example.py:1:14: F601 [*] Dictionary key literal `'a'` repeated
example.py:2:15: W292 [*] No newline at end of file
Found 2 errors.
[*] 2 fixable with the --fix option.
```

Also can be enabled in the config file

```
❯ cat ruff.toml
unsafe-fixes = true
```

And opted-out per invocation

```
❯ ruff check example.py --no-cache --select F601,W292 --no-unsafe-fixes
example.py:1:14: F601 Dictionary key literal `'a'` repeated
example.py:2:15: W292 [*] No newline at end of file
Found 2 errors.
[*] 1 fixable with the `--fix` option (1 hidden fix can be enabled with the `--unsafe-fixes` option).
```

Diff does not include unsafe fixes
```
❯ ruff check example.py --no-cache --select F601,W292 --diff
--- example.py
+++ example.py
@@ -1,2 +1,2 @@
 x = {'a': 1, 'a': 1}
-print(('foo'))
+print(('foo'))
\ No newline at end of file

Would fix 1 error.
```

Unless there is opt-in
```
❯ ruff check example.py --no-cache --select F601,W292 --diff --unsafe-fixes
--- example.py
+++ example.py
@@ -1,2 +1,2 @@
-x = {'a': 1}
-print(('foo'))
+x = {'a': 1, 'a': 1}
+print(('foo'))
\ No newline at end of file

Would fix 2 errors.
```

#7790 will improve the diff
messages following this pull request

Similarly, `--fix` and `--fix-only` require the `--unsafe-fixes` flag to
apply unsafe fixes.

## Related

Replaces #5119
Closes #4185
Closes #7214
Closes #4845
Closes #3863
Addresses #6835
Addresses #7019
Needs follow-up #6962
Needs follow-up #4845
Needs follow-up #7436
Needs follow-up #7025
Needs follow-up #6434
Follow-up #7790 
Follow-up #7792

---------

Co-authored-by: Evan Rittenhouse <evanrittenhouse@gmail.com>
@zanieb
Copy link
Member

zanieb commented Oct 6, 2023

We now have support for applicability but this looks like a significant project to update — contributions welcome here!

@zanieb zanieb added the help wanted Contributions especially welcome label Oct 6, 2023
konstin pushed a commit that referenced this issue Oct 11, 2023
Rebase of #5119 authored by
@evanrittenhouse with additional refinements.

## Changes

- Adds `--unsafe-fixes` / `--no-unsafe-fixes` flags to `ruff check`
- Violations with unsafe fixes are not shown as fixable unless opted-in
- Fix applicability is respected now
    - `Applicability::Never` fixes are no longer applied
    - `Applicability::Sometimes` fixes require opt-in
    - `Applicability::Always` fixes are unchanged
- Hints for availability of `--unsafe-fixes` added to `ruff check`
output

## Examples

Check hints at hidden unsafe fixes
```
❯ ruff check example.py --no-cache --select F601,W292
example.py:1:14: F601 Dictionary key literal `'a'` repeated
example.py:2:15: W292 [*] No newline at end of file
Found 2 errors.
[*] 1 fixable with the `--fix` option (1 hidden fix can be enabled with the `--unsafe-fixes` option).
```

We could add an indicator for which violations have hidden fixes in the
future.

Check treats unsafe fixes as applicable with opt-in
```
❯ ruff check example.py --no-cache --select F601,W292 --unsafe-fixes
example.py:1:14: F601 [*] Dictionary key literal `'a'` repeated
example.py:2:15: W292 [*] No newline at end of file
Found 2 errors.
[*] 2 fixable with the --fix option.
```

Also can be enabled in the config file

```
❯ cat ruff.toml
unsafe-fixes = true
```

And opted-out per invocation

```
❯ ruff check example.py --no-cache --select F601,W292 --no-unsafe-fixes
example.py:1:14: F601 Dictionary key literal `'a'` repeated
example.py:2:15: W292 [*] No newline at end of file
Found 2 errors.
[*] 1 fixable with the `--fix` option (1 hidden fix can be enabled with the `--unsafe-fixes` option).
```

Diff does not include unsafe fixes
```
❯ ruff check example.py --no-cache --select F601,W292 --diff
--- example.py
+++ example.py
@@ -1,2 +1,2 @@
 x = {'a': 1, 'a': 1}
-print(('foo'))
+print(('foo'))
\ No newline at end of file

Would fix 1 error.
```

Unless there is opt-in
```
❯ ruff check example.py --no-cache --select F601,W292 --diff --unsafe-fixes
--- example.py
+++ example.py
@@ -1,2 +1,2 @@
-x = {'a': 1}
-print(('foo'))
+x = {'a': 1, 'a': 1}
+print(('foo'))
\ No newline at end of file

Would fix 2 errors.
```

#7790 will improve the diff
messages following this pull request

Similarly, `--fix` and `--fix-only` require the `--unsafe-fixes` flag to
apply unsafe fixes.

## Related

Replaces #5119
Closes #4185
Closes #7214
Closes #4845
Closes #3863
Addresses #6835
Addresses #7019
Needs follow-up #6962
Needs follow-up #4845
Needs follow-up #7436
Needs follow-up #7025
Needs follow-up #6434
Follow-up #7790 
Follow-up #7792

---------

Co-authored-by: Evan Rittenhouse <evanrittenhouse@gmail.com>
@tjkuson
Copy link
Contributor

tjkuson commented Nov 17, 2023

I'm having a go at this, should have a PR this weekend (depending on how long it takes to determine the safety of each fix)!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Contributions especially welcome rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants