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.

Sign up for GitHub

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

feat: detect conditional imports #117

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

feat: detect conditional imports #117

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions z3c/dependencychecker/modules.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

FOLDERS_TO_IGNORE = ("node_modules", "__pycache__", "venv")

HAS_REPORTED_CONDITIONAL_IMPORTS = False
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?


class BaseModule:
def __init__(self, package_path, full_path):
Expand Down Expand Up @@ -117,11 +118,31 @@ def _process_ast_node(self, node):
file_path=self.path,
is_test=self.testing,
)
elif isinstance(node, ast.Try):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

if not HAS_REPORTED_CONDITIONAL_IMPORTS:
self._is_a_conditional_import(node)

@staticmethod
def _is_relative_import(import_node):
return import_node.level > 0

def _is_a_conditional_import(self, node):
global HAS_REPORTED_CONDITIONAL_IMPORTS
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

for handler in node.handlers:
type_info = getattr(handler, 'type', False)
if type_info:
exception_id = getattr(type_info, 'id', False)
if exception_id == 'ImportError':
print('This distribution has conditional imports')
HAS_REPORTED_CONDITIONAL_IMPORTS = True
Comment on lines +135 to +137
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The basic idea was to just report it, right? Just for information?

Is it easy to at a filename+linenumber at this point?

break

value = getattr(type_info, 'value', False)
if value and value.id == 'pkg_resources' and type_info.attr == 'DistributionNotFound':
print('This distribution has conditional imports')
HAS_REPORTED_CONDITIONAL_IMPORTS = True
break


class ZCMLFile(BaseModule):
"""Extract imports from .zcml files
Expand Down
Loading