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

[BUG] from foo import * imports should be ignored in stub files #152

Closed
AlexWaygood opened this issue Jul 13, 2022 · 6 comments · Fixed by #153
Closed

[BUG] from foo import * imports should be ignored in stub files #152

AlexWaygood opened this issue Jul 13, 2022 · 6 comments · Fixed by #153
Labels
bug Something isn't working
Projects

Comments

@AlexWaygood
Copy link
Contributor

Hi!

I see pycln recently added support for .pyi files. That's awesome!

I'm interested in possibly using pycln in typeshed's CI. However, it seems like there's one part of PEP 484 that pycln isn't quite compliant with when it comes to stub files, namely this:

[...] all objects imported into a stub using from ... import * are considered exported. (This makes it easier to re-export all objects from a given module that may vary by Python version.)

If I run pycln on typeshed at the moment, it erroneously deletes a large number of "unused" from foo import * imports. These imports are all deliberate -- if a type checker sees a from foo import * import in a stub file bar.pyi, it understands all symbols from module foo to be re-exported in module bar.

Playing around with pycln, it looks like this one-line patch solves the problem:

diff --git a/pycln/utils/refactor.py b/pycln/utils/refactor.py
index 4739e72..a241fc8 100644
--- a/pycln/utils/refactor.py
+++ b/pycln/utils/refactor.py
@@ -279,7 +279,7 @@ class Refactor:

                 # Expand any import '*' before checking.
                 node, is_star = self._expand_import_star(node)
-                if is_star is None:
+                if is_star is None or (is_star and self._path.is_stub):
                     continue

                 # Get set of used names.

I'd file a PR, but I'm struggling a little bit with the test suite 😅 (I'm not too familiar with the advanced features of pytest, I'm afraid.)

@AlexWaygood AlexWaygood added the bug Something isn't working label Jul 13, 2022
@hadialqattan
Copy link
Owner

Hi @AlexWaygood, this is a good catch.. actually, I planned to implement this within PR #150 but forgot to do so.

@hadialqattan
Copy link
Owner

@AlexWaygood after doing some investigation, your patch looks good to me, so please open a PR, and I'll help you finalize it even if you are unsure where/how to write the unit tests.

@hadialqattan
Copy link
Owner

hadialqattan commented Jul 13, 2022

+ @AlexWaygood I'm really happy to see you supporting the project. Thanks a lot bro!

@AlexWaygood
Copy link
Contributor Author

  • @AlexWaygood I'm delighted to see you supporting the project. Thanks a lot bro!

Thank you for the very cool tool!

@hadialqattan hadialqattan added this to To do in Bug Fixing via automation Jul 13, 2022
Bug Fixing automation moved this from To do to Done Jul 13, 2022
@hadialqattan
Copy link
Owner

@AlexWaygood I've just published a new release including the fix for this bug: v2.0.2.

@AlexWaygood
Copy link
Contributor Author

@AlexWaygood I've just published a new release including the fix for this bug: v2.0.2.

Awesome, thanks so much for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Bug Fixing
  
Done
Development

Successfully merging a pull request may close this issue.

2 participants