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.

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 "forbidden" contracts within packages (again) #199

Open
benthorner opened this issue Oct 13, 2023 · 3 comments · May be fixed by #229
Open

Support "forbidden" contracts within packages (again) #199

benthorner opened this issue Oct 13, 2023 · 3 comments · May be fixed by #229

Comments

@benthorner
Copy link

benthorner commented Oct 13, 2023

Previously it was possible to write a contract like this:

[importlinter:contract:core-factories]
name = "Factories should be split into subpackages"
type = forbidden
allow_indirect_imports = true
source_modules =
    tests.factories
forbidden_modules =
    tests.factories.domain_a
    tests.factories.domain_b

...to prevent the following imports:

# tests/factories/__init__.py
from tests.factories.domain_a import factory_1
...
from tests.factories.domain_b import factory_2

This stopped working in 2cdb36f - essentially, the bug was a feature.

Although this is quite a "dumb" use of import linter - it's little more than a grep - it makes sense to express such contracts alongside other more powerful ones for the same codebase.

Would it be possible to support this again?

Suggestions

  • Introduce a separate contract type for this scenario - awkward as the intent is the same.
  • Introduce a flag to treat the source modules as modules and not packages*.
  • Something more clever where the linter recognises the contract is within the same package.

*Treating modules as packages seems to be the default / intended behaviour.

@seddonym
Copy link
Owner

seddonym commented Oct 20, 2023

Thanks Ben for these suggestions. I agree we should support this somehow.

I'm leaning towards adding an as_packages flag which, if it was set to false, would allow you to do this. That would fit in well with the terminology of Grimp, the underlying static analysis library: many of the methods there allow you to specify whether to treat the modules as_packages.

Open to other ideas though.

@seddonym
Copy link
Owner

seddonym commented Jan 5, 2024

Having had more time to mull this over, I think it would the as_packages flag would be a good addition. Let me know if you would be interested in submitting a pull request!

@NicholasBunn NicholasBunn linked a pull request Nov 8, 2024 that will close this issue
@NicholasBunn
Copy link

I think this PR should re-introduce this functionality

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants