tweag / FawltyDeps

Python dependency checker
Other
200 stars 14 forks source link

Research corner cases of correctly calculating first- vs. third-party modules in `extract_imports.parse_code()` #95

Open jherland opened 1 year ago

jherland commented 1 year ago

In commit 9fa05db56531f5d69048e409cd080466e75aa43e we introduced some basic configuration of isort in order to enable isort.place_module() to correctly differentiate first-party from third-party imports on Python code.

However, there are corner cases that have not yet been fully considered/resolved. From the commit message:

Since we're specifically looking for THIRDPARTY imports, it is therefore
important that this `directory` is correctly configured, otherwise we
will report first-party imports as undeclared external dependencies!

For now, we fix this by setting `directory` to the path that is passed
to parse_dir(), and otherwise defaulting back to the current directory.

TODO/FIXME: There are some scenarios where this is probably not the
correct solution, for example:

- Passing --code as a single file or a stdin stream will bypass
  parse_dir() altogether, and the current directory will be used to find
  FIRSTPARTY imports. This may or may not be what the user intended.
- The project may do trickery with $PYTHONPATH or other mechanisms to
  alter where it is appropriate to look for FIRSTPARTY imports.

We may have to consider adding command-line (and configuration) options
to FawltyDeps for directly specifying this `directory`, and/or for
including/excluding specific names as FIRSTPARTY imports.
mknorps commented 1 year ago

Example of such corner case: https://github.com/tweag/FawltyDeps/issues#issuecomment-1416055168

jherland commented 1 year ago

Yes, the issues we are battling in PR #105 are precisely of the kind I allude to in the issue description above. They were not as clear to me when I first created this issue. One of the things we've learned today is that isort requires some extra hand-holding in its config in order to resolve imports in the same manner that Python itself does (and thus be able to differentiate first- and third-party imports correctly). Once we solve this for our current real_projects, I feel we can drop the https://github.com/tweag/FawltyDeps/milestone/1 milestone for this issue, and reconsider if there are further corner cases to be solved before the https://github.com/tweag/FawltyDeps/milestone/2 milestone

jherland commented 1 year ago

With #116 merged, I am removing the "First prototype" milestone from this issue. Not sure whether to close the issue completely yet, or retarget it for a later milestone?