-
Notifications
You must be signed in to change notification settings - Fork 4
Add ~ rule for dep-check #113
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,18 +3,12 @@ | |
| """ | ||
|
|
||
| import re | ||
| from typing import List, Optional | ||
| from typing import List | ||
|
|
||
| from ordered_set import OrderedSet | ||
|
|
||
| from dep_check.dependency_finder import IParser | ||
| from dep_check.models import ( | ||
| Dependency, | ||
| MatchingRule, | ||
| MatchingRules, | ||
| Module, | ||
| ModuleWildcard, | ||
| ) | ||
| from dep_check.models import Dependency, MatchingRules, Module, ModuleWildcard | ||
|
|
||
|
|
||
| class NotAllowedDependencyException(Exception): | ||
|
|
@@ -32,44 +26,82 @@ def __init__( | |
| self.authorized_modules = authorized_modules | ||
|
|
||
|
|
||
| def _raise_on_forbidden_rules( | ||
| parser: IParser, dependency: Dependency, matching_rules: MatchingRules | ||
| ) -> MatchingRules: | ||
| # pylint: disable=too-many-nested-blocks | ||
| used_rules: MatchingRules = OrderedSet() | ||
| imports = [ | ||
| dependency.main_import, | ||
| *[ | ||
| Module(f"{dependency.main_import}.{sub_import}") | ||
| for sub_import in dependency.sub_imports | ||
| ], | ||
| ] | ||
| for module in imports: | ||
| for matching_rule in matching_rules: | ||
| regex_rule = parser.wildcard_to_regex(matching_rule.specific_rule_wildcard) | ||
| if regex_rule.raise_if_found: | ||
| if re.match(f"{regex_rule.regex}$", module): | ||
| raise NotAllowedDependencyException( | ||
| module, [r.specific_rule_wildcard for r in matching_rules] | ||
| ) | ||
| used_rules.add(matching_rule) | ||
|
|
||
| return used_rules | ||
|
|
||
|
|
||
| def _find_matching_rules( | ||
| parser: IParser, matching_rules: MatchingRules, dotted_import: Module | ||
| ) -> MatchingRules: | ||
| used_rules: MatchingRules = OrderedSet() | ||
|
|
||
| for matching_rule in matching_rules: | ||
| regex_rule = parser.wildcard_to_regex(matching_rule.specific_rule_wildcard) | ||
| if regex_rule.raise_if_found: | ||
| # Don't want to handle if here, it should have been done earlier in the flow | ||
| continue | ||
| if re.match(f"{regex_rule.regex}$", dotted_import): | ||
| used_rules.add(matching_rule) | ||
|
|
||
| return used_rules | ||
|
|
||
|
|
||
| def check_dependency( | ||
| parser: IParser, dependency: Dependency, matching_rules: MatchingRules | ||
| ) -> MatchingRules: | ||
| """ | ||
| Check that dependencies match a given set of rules. | ||
| """ | ||
| used_rule: Optional[MatchingRule] = None | ||
| for matching_rule in matching_rules: | ||
| if re.match( | ||
| f"{parser.wildcard_to_regex(matching_rule.specific_rule_wildcard)}$", | ||
| dependency.main_import, | ||
| ): | ||
| used_rule = matching_rule | ||
| return OrderedSet((used_rule,)) | ||
|
|
||
| forbidden_rules = _raise_on_forbidden_rules(parser, dependency, matching_rules) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this imply that forbidden rules will suppress the output of unused rules ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, it means that the forbidden rules cannot be handled as the "normal" rules as they need to NOT raise "UnusedRule" (as we expect them to be unused, technically) and they need their own handling as we might miss imports per modules otherwise. |
||
| used_rules = _find_matching_rules(parser, matching_rules, dependency.main_import) | ||
| if used_rules: | ||
| return OrderedSet([*used_rules, *forbidden_rules]) | ||
|
|
||
| if not dependency.sub_imports: | ||
| raise NotAllowedDependencyException( | ||
| dependency.main_import, [r.specific_rule_wildcard for r in matching_rules] | ||
| ) | ||
|
|
||
| return check_import_from_dependency(parser, dependency, matching_rules) | ||
| return OrderedSet( | ||
| [ | ||
| *check_import_from_dependency(parser, dependency, matching_rules), | ||
| *forbidden_rules, | ||
| ] | ||
| ) | ||
|
|
||
|
|
||
| def check_import_from_dependency( | ||
| parser: IParser, dependency: Dependency, matching_rules: MatchingRules | ||
| ) -> MatchingRules: | ||
| used_rules: MatchingRules = OrderedSet() | ||
| for import_module in dependency.sub_imports: | ||
| used_rule = None | ||
| for matching_rule in matching_rules: | ||
| if re.match( | ||
| f"{parser.wildcard_to_regex(matching_rule.specific_rule_wildcard)}$", | ||
| f"{dependency.main_import}.{import_module}", | ||
| ): | ||
| used_rule = matching_rule | ||
| used_rules.add(used_rule) | ||
| if not used_rule: | ||
| module = Module(f"{dependency.main_import}.{import_module}") | ||
| matched_rules = _find_matching_rules(parser, matching_rules, module) | ||
| used_rules.update(matched_rules) | ||
| if not matched_rules: | ||
| raise NotAllowedDependencyException( | ||
| Module(f"{dependency.main_import}.{import_module}"), | ||
| [r.specific_rule_wildcard for r in matching_rules], | ||
| module, [r.specific_rule_wildcard for r in matching_rules] | ||
| ) | ||
| return used_rules | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unclear what you're trying to mean here, do you imply that
raise_if_foundshould have already been handled at this point ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, exactly