python-rope / rope

a python refactoring library
GNU Lesser General Public License v3.0
1.96k stars 164 forks source link

AutoImport's `get_modules` is case insensitive #715

Closed niqodea closed 1 year ago

niqodea commented 1 year ago

Describe the bug

AutoImport's get_modules is case insensitive. Not sure if exactly a bug, but I don't think this behavior fits considering the method is used for full names and not for autocompletion purposes.

To Reproduce

from pathlib import Path
from rope.base.project import Project
from rope.contrib.autoimport.sqlite import AutoImport

autoimport = AutoImport(Project(Path()), memory=False)
autoimport.generate_cache()
autoimport.generate_modules_cache()
print(autoimport.get_modules("C"))

The code above prints ['calendar'] but from calendar import C is not a valid import. In fact, the correct symbol is c (lower-case).

The same happens with the pickle version of AutoImport.

Editor information:

lieryan commented 1 year ago

IIUC, this is likely intentional because one of the way autoimport can be used is for adding imports automatically when autocompleting names that aren't in the current scope, this is how autoimport is implemented in pylsp's builtin autoimport for example.

For the use case of autoimport of the symbol under cursor, this behavior might not necessarily be desirable, so it might make sense to make this behavior configurable.

@bagel897 do you have any thoughts on this?

bagel897 commented 1 year ago

The method is kept for compatibility with the old API, using exact matches would make sense for this. I cannot reproduce the same behavior in pickle, only sqllite. This method isn't used in the pylsp completion engine, so I'll change it to match pickle.

lieryan commented 1 year ago

@niqodea does #721 fix your issue? If it doesn't, please feel free to re-open the issue.

niqodea commented 1 year ago

It works! Thank you so much @bagel897. Looking forward to the new release containing this fix.