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

Support module wildcards everywhere #235

Merged
merged 17 commits into from
Sep 11, 2024
Merged

Support module wildcards everywhere #235

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
d5de725
Introduce ModulExpression and refactor ImportExpression accodingly
fbinz Aug 30, 2024
42af972
Make forbidden contract accept wildcards in source modules
fbinz Aug 30, 2024
8df31fb
Make forbidden contract accept wildcards in forbidden modules
fbinz Aug 30, 2024
d996763
Make indepdence contract accept wildcards in modules
fbinz Sep 1, 2024
bcdf15a
Return set of Modules when resolving module expressions
fbinz Sep 4, 2024
a2f108e
Rename resolve_module_expressions to module_expressions_to_modules
fbinz Sep 4, 2024
337902b
Add tests for wildcards in forbidden and independence contracts
fbinz Sep 4, 2024
c632021
Adjust docs wrt wildcards
fbinz Sep 4, 2024
3edda0e
Change argument order of module_expressions_to_modules to be more con…
fbinz Sep 6, 2024
1f61f78
Revert release information
fbinz Sep 6, 2024
04276a1
Add link to wildcards
fbinz Sep 6, 2024
eb59109
Revert version in conf.py
fbinz Sep 6, 2024
4d0f9bf
Fix wrong argument order in module_expression_to_modules calls
fbinz Sep 6, 2024
b1488fb
Fix typing wrt python 3.8 compatibility
fbinz Sep 8, 2024
59bb745
Remove dead code, fix linting errors
fbinz Sep 10, 2024
f2fdeba
Sort modules in forbidden contract to ensure stable output
fbinz Sep 11, 2024
44d5503
Fix typings
fbinz Sep 11, 2024
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
7 changes: 6 additions & 1 deletion src/importlinter/contracts/forbidden.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,13 @@ def check(self, graph: ImportGraph, verbose: bool) -> ContractCheck:
f"in {timer.duration_in_s}s.",
)

# Sorting by upstream and downstream module ensures that the output is deterministic
# and that the same upstream and downstream modules are always adjacent in the output.
def chain_sort_key(chain_data):
return (chain_data["upstream_module"], chain_data["downstream_module"])

return ContractCheck(
kept=is_kept, warnings=warnings, metadata={"invalid_chains": invalid_chains}
kept=is_kept, warnings=warnings, metadata={"invalid_chains": sorted(invalid_chains, key=chain_sort_key)}
)

def render_broken_contract(self, check: "ContractCheck") -> None:
Expand Down
10 changes: 6 additions & 4 deletions src/importlinter/domain/helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,16 +123,18 @@ def resolve_import_expressions(

def resolve_module_expressions(
fbinz marked this conversation as resolved.
Show resolved Hide resolved
expressions: Iterable[ModuleExpression], graph: ImportGraph
) -> Iterable[Module]:
) -> set[Module]:
modules = set()
for expression in expressions:
yield from resolve_module_expression(expression, graph)
modules |= resolve_module_expression(expression, graph)
return modules


def resolve_module_expression(
expression: ModuleExpression, graph: ImportGraph
) -> Iterable[Module]:
) -> set[Module]:
if not expression.has_wildcard_expression():
return [Module(expression.expression)]
return set([Module(expression.expression)])
fbinz marked this conversation as resolved.
Show resolved Hide resolved

pattern = _to_pattern(expression.expression)

Expand Down
14 changes: 14 additions & 0 deletions tests/adapters/printing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,17 @@ def pop_and_assert(self, expected_string):
self._buffer = ""

assert popped_string == modified_expected_string

def pop_and_assert_lines(self, expected_string):
Copy link
Owner

Choose a reason for hiding this comment

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

Not convinced we should be changing the tests - if anything it's a smell that the Import Linter is not behaving deterministically, which I think should at least be an aspiration.

I think a better way to address this is just to move the sort up the stack, so while module_expressions_to_modules returns a set, the check methods that use it sort the results.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, good idea!
But I guess as soon as I introduce any kind of ordering, I'll have to adjust the order in the assertions aswell, right? Or what do you mean by "changing the tests"?

Copy link
Owner

Choose a reason for hiding this comment

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

Oh I see what you mean. Yes I don't mind if the assertions change if the ordering becomes alphabetical.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, I'm not able to reproduce the original issue currently. I.e. all tests now pass on Python 3.12 at least.

If I understand correctly, tox should run the tests etc. for several python versions.
However, if I try to run tox locally, I get the following error:

    eps = importlib_metadata.entry_points().get(self.namespace, ())
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'EntryPoints' object has no attribute 'get'
check: exit 1 (0.11 seconds) /home/binz/Repositories/import-linter> flake8 src tests pid=763946

This seems to be related to importlib_metadata changing stuff, but installing a different version (the proposed solution found by googling) does not work.

How did you setup tox locally?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

While I have still not run tox locally, I fixed the typing error related to Set[...] vs set[...]. So hopefully, everything passes now

Copy link
Owner

Choose a reason for hiding this comment

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

Sorry for the delay in replying. If you create a new virtual environment and then pip install tox into it, you should be able to then run it. The only thing is you'll need to make sure you have Python versions accessible for each version of Python you want to run it under. I would suggest just picking one and running that (I use pyenv for that).

E.g. running tox on 3.10 you can do tox -epy310.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I see. I somehow expected tox to switch python versions for. No idea why. :D
So, now the output should be stable, since I'm ordering both source and forbidden modules of a forbidden contract.

"""
Like pop_and_assert, but compares the lines of the buffer to the lines of the expected string, ignoring order.
"""

expected_lines = textwrap.dedent(expected_string.strip("\n")).splitlines()
buffer_lines = self._buffer.splitlines()

self._buffer = ""

assert set(buffer_lines) == set(expected_lines)


40 changes: 20 additions & 20 deletions tests/unit/contracts/test_forbidden.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,19 @@ def test_is_broken_when_forbidden_modules_imported(self):
],
],
},
{
"upstream_module": "mypackage.green",
"downstream_module": "mypackage.three",
"chains": [
[
{
"importer": "mypackage.three",
"imported": "mypackage.green",
"line_numbers": (4,),
}
]
],
},
{
"upstream_module": "mypackage.purple",
"downstream_module": "mypackage.two",
Expand All @@ -77,19 +90,6 @@ def test_is_broken_when_forbidden_modules_imported(self):
]
],
},
{
"upstream_module": "mypackage.green",
"downstream_module": "mypackage.three",
"chains": [
[
{
"importer": "mypackage.three",
"imported": "mypackage.green",
"line_numbers": (4,),
}
]
],
},
]
}

Expand Down Expand Up @@ -703,31 +703,31 @@ def test_verbose(self):

contract.check(graph=graph, verbose=True)

settings.PRINTER.pop_and_assert(
settings.PRINTER.pop_and_assert_lines(
"""
Searching for import chains from mypackage.one to mypackage.blue...
Found 0 illegal chains in 10s.
Searching for import chains from mypackage.one to mypackage.green...
Found 1 illegal chain in 10s.
Searching for import chains from mypackage.one to mypackage.yellow...
Found 0 illegal chains in 10s.
Searching for import chains from mypackage.one to mypackage.purple...
Found 0 illegal chains in 10s.
Searching for import chains from mypackage.one to mypackage.yellow...
Found 0 illegal chains in 10s.
Searching for import chains from mypackage.two to mypackage.blue...
Found 0 illegal chains in 10s.
Searching for import chains from mypackage.two to mypackage.green...
Found 0 illegal chains in 10s.
Searching for import chains from mypackage.two to mypackage.yellow...
Found 0 illegal chains in 10s.
Searching for import chains from mypackage.two to mypackage.purple...
Found 1 illegal chain in 10s.
Searching for import chains from mypackage.two to mypackage.yellow...
Found 0 illegal chains in 10s.
Searching for import chains from mypackage.three to mypackage.blue...
Found 0 illegal chains in 10s.
Searching for import chains from mypackage.three to mypackage.green...
Found 1 illegal chain in 10s.
Searching for import chains from mypackage.three to mypackage.yellow...
Found 0 illegal chains in 10s.
Searching for import chains from mypackage.three to mypackage.purple...
Found 0 illegal chains in 10s.
Searching for import chains from mypackage.three to mypackage.yellow...
Found 0 illegal chains in 10s.
"""
)