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

Use of implicitly imported module namespace not considered correctly #180

Open
Tracked by #272
cb109 opened this issue Apr 23, 2021 · 2 comments
Open
Tracked by #272

Use of implicitly imported module namespace not considered correctly #180

cb109 opened this issue Apr 23, 2021 · 2 comments
Assignees
Labels
bug Something isn't working changelog Changes should be written to the changelog file.
Milestone

Comments

@cb109
Copy link

cb109 commented Apr 23, 2021

Problem

An import like import module.submodule is removed when module.submodule is not used 1:1. However, we may use module.other_submodule or module.some_other_name in the file, which then becomes invalid and as such our codebase is broken.

Example

# my_module.py
import urllib.request

def parse_url(url):
    return urllib.parse.urlparse(url)

print(parse_url("http://google.com"))

The urllib.parse.urlparse(url) works here (somewhat surprisingly), because import urllib.request implicitly imports urllib, allowing us to access urllib.parse as well.


Running unimport against the file will detect import urllib.request as unused and remove it:

$ unimport my_module.py --remove
diff --git a/my_module.py b/my_module.py
-import urllib.request
 
 def parse_url(url):

As a result, the module is now broken:

$ python my_module.py
Traceback (most recent call last):
  File "my_module.py", line 7, in <module>
    print(parse_url("http://google.com"))
  File "my_module.py", line 4, in parse_url
    return urllib.parse.urlparse(url)
NameError: name 'urllib' is not defined

Proposed Solution

unimport should be able to detect these cases and either prefer to not remove anything here or at least let us configure the behaviour to be more or less forgiving (maybe via --strict defaulting to True). I do see unimport as a cleanup mechanism to keep the codebase tidy, but it should never break anything, I'd much prefer to have a weird import lingering around if I had to choose.

Thank you for your time!

@cb109
Copy link
Author

cb109 commented Apr 23, 2021

Okay I am a bit confused since this seems to have been mentioned before in #127 and should have been fixed in v0.7.1, but I am still experiencing this on v0.8.3 right now.

@hakancelikdev hakancelikdev added bug Something isn't working changelog Changes should be written to the changelog file. needs test labels Apr 24, 2021
@hakancelikdev
Copy link
Owner

Hello, thank you for reporting this issue. You don't need to get confused because this seems like our fault, I'll fix it soon.

I also plan to add a feature like this

if the Unimport code breaks after removing unused imports, it will cancel the remove and report it.

In this way, we will prevent cases such as code-breaking.

@hakancelikdev hakancelikdev self-assigned this Dec 5, 2022
@hakancelikdev hakancelikdev added this to the 0.1.0 milestone Dec 5, 2022
@hakancelikdev hakancelikdev mentioned this issue Dec 5, 2022
9 tasks
@hakancelikdev hakancelikdev modified the milestones: 1.0.0, 1.1.0 Jul 7, 2023
@hakancelikdev hakancelikdev mentioned this issue Jun 13, 2023
7 tasks
@polar-sh polar-sh bot added polar and removed polar labels Jul 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working changelog Changes should be written to the changelog file.
Projects
Status: 📋 Backlog
Development

No branches or pull requests

2 participants