feat: detect conditional imports #117
Conversation
| @@ -115,11 +116,31 @@ def _process_ast_node(self, node): | |||
| file_path=self.path, | |||
| is_test=self.testing, | |||
| ) | |||
| elif isinstance(node, ast.Try): | |||
There was a problem hiding this comment.
I don't know the code. Could this condition also be true for a try/except block that does not do any imports? Or is this part only reached for code that tries an import?
|
|
||
| @staticmethod | ||
| def _is_relative_import(import_node): | ||
| return import_node.level > 0 | ||
|
|
||
| def _is_a_conditional_import(self, node): | ||
| global HAS_REPORTED_CONDITIONAL_IMPORTS |
There was a problem hiding this comment.
Normally CONSTANTS are pretty constant and don't change. This one does, which surprised me initially.
A short comment next to the original constant would help (# Gets set to True when xyz is found).
| @@ -29,6 +29,7 @@ | |||
|
|
|||
| FOLDERS_TO_IGNORE = ("node_modules", "__pycache__") | |||
|
|
|||
| HAS_REPORTED_CONDITIONAL_IMPORTS = False | |||
There was a problem hiding this comment.
Why the "reported"? It might have to do with code you'll add later. For now, it sounds like CONDITIONAL_IMPORTS_FOUND could be clearer?
| if exception_id == 'ImportError': | ||
| print('This distribution has conditional imports') | ||
| HAS_REPORTED_CONDITIONAL_IMPORTS = True |
There was a problem hiding this comment.
The basic idea was to just report it, right? Just for information?
Is it easy to at a filename+linenumber at this point?
gforcada
force-pushed
the
conditional-imports
branch
from
562dffa
to
94a7334
Compare
gforcada
force-pushed
the
conditional-imports
branch
from
94a7334
to
61c7e0a
Compare
Closes #106
This is a rough draft, I tested with
Products.CMFPloneand at least the code does not break 😅Wording and code organization, as well as tests are missing/can be improved.
Do you think the direction is the right one? 🤔