seddonym / import-linter

Import Linter allows you to define and enforce rules for the internal and external imports within your Python project.
https://import-linter.readthedocs.io/
BSD 2-Clause "Simplified" License
664 stars 45 forks source link

Rust panic when using root package as a layer #206

Open LewisGaul opened 10 months ago

LewisGaul commented 10 months ago

Using the root package name in the "layers" contract causes a Rust panic.

[[tool.importlinter.contracts]]
name = "pkg layers"
type = "layers"
layers = [
    "minegauler",
    "minegauler.app",
]

Small reproduction at https://github.com/LewisGaul/minegauler/commit/7ee4e627997cc879d883f9a51cfabc2f1287ef3d.

$ PYTHONPATH=src lint-imports
=============
Import Linter
=============

thread '<unnamed>' panicked at src/importgraph.rs:148:63:
no entry found for key
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Traceback (most recent call last):
  File "/mnt/c/Users/legaul/Documents/personal/minegauler/venv/bin/lint-imports", line 8, in <module>
    sys.exit(lint_imports_command())
  File "/mnt/c/Users/legaul/Documents/personal/minegauler/venv/lib/python3.8/site-packages/click/core.py", line 1137, in __call__
    return self.main(*args, **kwargs)
  File "/mnt/c/Users/legaul/Documents/personal/minegauler/venv/lib/python3.8/site-packages/click/core.py", line 1062, in main
    rv = self.invoke(ctx)
  File "/mnt/c/Users/legaul/Documents/personal/minegauler/venv/lib/python3.8/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/mnt/c/Users/legaul/Documents/personal/minegauler/venv/lib/python3.8/site-packages/click/core.py", line 763, in invoke
    return __callback(*args, **kwargs)
  File "/mnt/c/Users/legaul/Documents/personal/minegauler/venv/lib/python3.8/site-packages/importlinter/cli.py", line 52, in lint_imports_command
    exit_code = lint_imports(
  File "/mnt/c/Users/legaul/Documents/personal/minegauler/venv/lib/python3.8/site-packages/importlinter/cli.py", line 99, in lint_imports
    passed = use_cases.lint_imports(
  File "/mnt/c/Users/legaul/Documents/personal/minegauler/venv/lib/python3.8/site-packages/importlinter/application/use_cases.py", line 54, in lint_imports
    report = create_report(user_options, limit_to_contracts, cache_dir, show_timings, verbose)
  File "/mnt/c/Users/legaul/Documents/personal/minegauler/venv/lib/python3.8/site-packages/importlinter/application/use_cases.py", line 122, in create_report
    return _build_report(
  File "/mnt/c/Users/legaul/Documents/personal/minegauler/venv/lib/python3.8/site-packages/importlinter/application/use_cases.py", line 201, in _build_report
    check = contract.check(copy_of_graph, verbose=verbose)
  File "/mnt/c/Users/legaul/Documents/personal/minegauler/venv/lib/python3.8/site-packages/importlinter/contracts/layers.py", line 115, in check
    dependencies = graph.find_illegal_dependencies_for_layers(
  File "/mnt/c/Users/legaul/Documents/personal/minegauler/venv/lib/python3.8/site-packages/grimp/adaptors/graph.py", line 371, in find_illegal_dependencies_for_layers
    return _layers.find_illegal_dependencies(
  File "/mnt/c/Users/legaul/Documents/personal/minegauler/venv/lib/python3.8/site-packages/grimp/adaptors/_layers.py", line 31, in find_illegal_dependencies
    rust_package_dependency_tuple = rust.find_illegal_dependencies(
pyo3_runtime.PanicException: no entry found for key

I'm on WSL1 (Ubuntu 20.04 on Windows 11), although I don't expect this to be relevant.

seddonym commented 10 months ago

Thanks for the bug report - I'll look into it.

Incidentally this should just have better error handling, as using the root package as a layer isn't valid.

tuukkamustonen commented 9 months ago

The same happens also the other way (ie. when more detailed package is declared first):

[importlinter]
root_packages =
    root

[importlinter:contract:layers]
name = Layers
type = layers
layers =
    root.subpkg
    root

Is it a bug or just unsupported? Ie.

One example use case would be like (though this may already be a different use case, I'm not sure):

[importlinter:contract:layers]
name = Layers
type = layers
layers =
    root.public_api | root.private_api
    root
    root.util
seddonym commented 9 months ago

Is it a bug or just unsupported?

Unsupported...or, maybe, meaningless. Each layer is (potentially) a subpackage, so a layer shouldn't contain another layer in the same contract.

My expectation with layers has been that they're siblings, though rereading the documentation it doesn't spell that out, and possibly it doesn't need to. I guess as long as layers don't contain each other, they could still be linted in that way.

Possibly this issue is more related to what you want to do.