unimport icon indicating copy to clipboard operation
unimport copied to clipboard

Use of implicitly imported module namespace not considered correctly

Open cb109 opened this issue 4 years ago • 2 comments
trafficstars

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 avatar Apr 23 '21 12:04 cb109

Okay I am a bit confused since this seems to have been mentioned before in https://github.com/hakancelik96/unimport/issues/127 and should have been fixed in v0.7.1, but I am still experiencing this on v0.8.3 right now.

cb109 avatar Apr 23 '21 12:04 cb109

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 avatar Apr 24 '21 17:04 hakancelikdev