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

Add fix to re-export unused first-party imports in __init__.py files #10391

Closed
zanieb opened this issue Mar 13, 2024 · 0 comments · Fixed by #11314
Closed

Add fix to re-export unused first-party imports in __init__.py files #10391

zanieb opened this issue Mar 13, 2024 · 0 comments · Fixed by #11314
Assignees
Labels
fixes Related to suggested fixes for violations

Comments

@zanieb
Copy link
Member

zanieb commented Mar 13, 2024

See #10365 (comment) and #10390

We currently do not offer any fixes for unused imports in __init__.py files.

This issue is to fix first-party imports by

  1. Adding them to __all__ if __all__ is present
  2. Otherwise, converting them to a redundant alias e.g. import foo as foo.

We can probably label this fix as safe.

@zanieb zanieb added fixes Related to suggested fixes for violations help wanted Contributions especially welcome labels Mar 13, 2024
@AlexWaygood AlexWaygood self-assigned this Mar 13, 2024
@AlexWaygood AlexWaygood removed the help wanted Contributions especially welcome label Mar 13, 2024
plredmond added a commit that referenced this issue Apr 26, 2024
…ability; remove unnecessary pattern match
plredmond added a commit that referenced this issue Apr 26, 2024
…urn a fix if imports were actually given
plredmond added a commit that referenced this issue Apr 26, 2024
… either moves to __all__ or makes imports explicit
plredmond added a commit that referenced this issue Apr 26, 2024
… two groups of import statement bindings; then iterate over those bindings and the fix which applies to them
plredmond added a commit that referenced this issue Apr 27, 2024
plredmond added a commit that referenced this issue Apr 27, 2024
plredmond added a commit that referenced this issue Apr 29, 2024
plredmond added a commit that referenced this issue Apr 29, 2024
plredmond added a commit that referenced this issue Apr 30, 2024
plredmond added a commit that referenced this issue Apr 30, 2024
plredmond added a commit that referenced this issue Apr 30, 2024
…sable F401 based on deprecated flag, but use the preview flag; move the testcases down to the preview testcase section; move the insta-snapshots to the preview paths
plredmond added a commit that referenced this issue Apr 30, 2024
… test and add a test-case for the preview linter rules that references that fixture; move its insta-snapshot to the location used by the new test-case
plredmond added a commit that referenced this issue Apr 30, 2024
plredmond added a commit that referenced this issue Apr 30, 2024
plredmond added a commit that referenced this issue May 2, 2024
…h to make explicit-exports (#11168)

Resolves #10390 and starts to address #10391

# Changes to behavior

* In `__init__.py` we now offer some fixes for unused imports.
* If the import binding is first-party this PR suggests a fix to turn it
into a redundant alias.
* If the import binding is not first-party, this PR suggests a fix to
remove it from the `__init__.py`.
* The fix-titles are specific to these new suggested fixes.
* `checker.settings.ignore_init_module_imports` setting is
deprecated/ignored. There is probably a documentation change to make
that complete which I haven't done.

---

<details><summary>Old description of implementation changes</summary>

# Changes to the implementation

* In the body of the loop over import statements that contain unused
bindings, the bindings are partitioned into `to_reexport` and
`to_remove` (according to how we want to resolve the fact they're
unused) with the following predicate:
  ```rust
in_init && is_first_party(checker, &import.qualified_name().to_string())
// true means make it a reexport
  ```
* Instead of generating a single fix per import statement, we now
generate up to two fixes per import statement:
  ```rust
  (fix_by_removing_imports(checker, node_id, &to_remove, in_init).ok(),
   fix_by_reexporting(checker, node_id, &to_reexport, dunder_all).ok())
  ```
* The `to_remove` fixes are unsafe when `in_init`.
* The `to_explicit` fixes are safe. Currently, until a future PR, we
make them redundant aliases (e.g. `import a` would become `import a as
a`).

## Other changes

* `checker.settings.ignore_init_module_imports` is deprecated/ignored.
Instead, all fixes are gated on `checker.settings.preview.is_enabled()`.
* Got rid of the pattern match on the import-binding bound by the inner
loop because it seemed less readable than referencing fields on the
binding.
* [x] `// FIXME: rename "imports" to "bindings"` if reviewer agrees (see
code)
* [x] `// FIXME: rename "node_id" to "import_statement"` if reviewer
agrees (see code)

<details>
<summary><h2>Scope cut until a future PR</h2></summary>

* (Not implemented) The `to_explicit` fixes will be added to `__all__`
unless it doesn't exist. When `__all__` doesn't exist they're resolved
by converting to redundant aliases (e.g. `import a` would become `import
a as a`).
 
---

</details>

# Test plan

* [x] `crates/ruff_linter/resources/test/fixtures/pyflakes/F401_24`
contains an `__init__.py` with*out* `__all__` that exercises the
features in this PR, but it doesn't pass.
* [x]
`crates/ruff_linter/resources/test/fixtures/pyflakes/F401_25_dunder_all`
contains an `__init__.py` *with* `__all__` that exercises the features
in this PR, but it doesn't pass.
* [x] Write unit tests for the new edit functions in
`fix::edits::make_redundant_alias`.

</details>

---------

Co-authored-by: Micha Reiser <micha@reiser.io>
plredmond added a commit that referenced this issue May 15, 2024
Followup on #11168 and resolve #10391

# User facing changes

* F401 now recommends a fix to add unused import bindings to to
`__all__` if a single `__all__` list or tuple is found in `__init__.py`.
* If there are no `__all__` found in the file, fall back to recommending
redundant-aliases.
* If there are multiple `__all__` or only one but of the wrong type (non
list or tuple) then diagnostics are generated without fixes.
* `fix_title` is updated to reflect what the fix/recommendation is.

Subtlety: For a renamed import such as `import foo as bees`, we can
generate a fix to add `bees` to `__all__` but cannot generate a fix to
produce a redundant import (because that would break uses of the binding
`bees`).

# Implementation changes

* Add `name` field to `ImportBinding` to contain the name of the
_binding_ we want to add to `__all__` (important for the `import foo as
bees` case). It previously only contained the `AnyImport` which can give
us information about the import but not the binding.
* Add `binding` field to `UnusedImport` to contain the same. (Naming
note: the field `name` field already existed on `UnusedImport` and
contains the qualified name of the imported symbol/module)
* Change `fix_by_reexporting` to branch on the size of `dunder_all:
Vec<&Expr>`
* For length 0 call the edit-producing function `make_redundant_alias`.
  * For length 1 call edit-producing function `add_to_dunder_all`.
  * Otherwise, produce no fix.
* Implement the edit-producing function `add_to_dunder_all` and add unit
tests.
* Implement several fixture tests: empty `__all__ = []`, nonempty
`__all__ = ["foo"]`, mis-typed `__all__ = None`, plus-eq `__all__ +=
["foo"]`
* `UnusedImportContext::Init` variant now has two fields: whether the
fix is in `__init__.py` and how many `__all__` were found.

# Other changes

* Remove a spurious pattern match and instead use field lookups b/c the
addition of a field would have required changing the unrelated pattern.
* Tweak input type of `make_redundant_alias`

---------

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixes Related to suggested fixes for violations
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants