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
679 stars 48 forks source link

Docs request: a richer custom contract example #143

Closed gwerbin closed 1 year ago

gwerbin commented 1 year ago

Thanks a lot for this very clever and useful tool, and for providing reasonably thorough documentation.

I was looking into custom contracts (as many of the feature requests here are essentially custom contract requests for implementation), and I was a little disappointed to find that the example is very close to a trivial example: https://import-linter.readthedocs.io/en/stable/custom_contract_types.html.

I next tried looking at the built-in contracts such as independence, but those are very complicated compared to the example, and it would take a really long time to figure out how it works and how to put my own together: https://github.com/seddonym/import-linter/blob/master/src/importlinter/contracts/independence.py

I understand that these are docs for Import Linter and not for Grimp, but given that the custom contracts design is almost entirely dependent on Grimp functionality, I think it would be helpful to have a bit more of a thorough example. At minimum, it might be very helpful to pick a feature request here and use that as an example, especially one that involves walking up and down the import graph, like #123 or #124.

Maybe an examples/ or contrib/ directory could also be useful, containing contract types that have not been as thoroughly tested as the ones in the core library, but might still be useful to people, or might at least provide a good starting point.

Maybe an ideal example would make a little more use of Grimp features than the current one, while keeping complexity below the level of the built-in contract types.

seddonym commented 1 year ago

Hi! Thanks so much for opening the issue, this is useful feedback.

It's an interesting one this...there is obviously a balance to be struck between a simple example and a complex example, and it sounds like you want something in between.

My immediate reaction is that it's quite subjective what examples will click for a particular individual, so it's not going to be that straightforward to deliver on.

Incidentally, the reason the forbidden and layered contracts are so complex is that they have been optimized quite heavily for speed. For something in between you might want to have a look at the forbidden contract (as implemented in the code base) which is definitely somewhere in between the trivial example in the docs and the layers/independence contracts.

There's also a lot of docs for using Grimp.

Do you have a concrete thing you're trying to achieve and are not sure how? I might be able to give you some pointers.

gwerbin commented 1 year ago

Thanks for the reply!

Do you have a concrete thing you're trying to achieve and are not sure how? I might be able to give you some pointers.

I do indeed. My use case is that I frequently have one the following two import graphs in mind:

  1. foo (i.e. foo/__init__.py) is the "base" from which all foo.* submodules import various items. foo never imports from foo.*. I believe this is the use case of #123.
  2. foo is the "composition root", importing items from foo.*. The foo.* submodules never import from foo. I believe this is the use case of #124.
  3. __main__ (foo/__main__.py) may only import from foo.cli. Currently I don't think you can work with __main__ modules at all.
  4. a may only import from b or c but not anything else. I think layers cover many possible cases here, but see below for some thoughts on this.

I think case 2 in particular is very common in Python development, so I think a recipe for that case in particular would be very much appreciated.


Regarding case 4 above, I often want to enforce other, more-complicated graphs, such as this one:

graph TD
    foo._base --> foo._utils
    foo.a --> foo._base
    foo.b --> foo._base
    foo.c --> foo._base
    foo.a --> foo._utils
    foo.b --> foo._utils
    foo.c --> foo._utils
    foo -->|imports from| foo.a
    foo --> foo.b
    foo --> foo.c

Or, generalized to an arbitrary number of submodules:

graph TD
    foo._base --> foo._utils
    foo.* --> foo._base
    foo.* --> foo._utils
    foo -->|imports from| foo.*["foo.* (excluding foo.{_base,_utils})"]

I think my ideal tool would allow me to declaratively specify a graph, perhaps using a Mermaid-like syntax (as above), and maybe using wildcards. Here is one off-the-cuff proposal for how you might declare such a thing:

[[tool.importlinter.contracts]]
name = 'Foo module'
type = 'graph'
wildcard_exclude = ['_base', '_utils']
graph = [
  'foo._utils <- foo._base',
  'foo._base <- foo.*',
  'foo.* <- foo'
]

The above is essentially a collection of "allow only" rules, so maybe what I am looking for is the inverse of forbidden, which would indeed allow users to construct arbitrary dependency graphs in the form of adjacency lists:

[[tool.importlinter.contracts]]
name = 'Foo base and utils'
type = 'allow-only'
source_modules = ['foo._base']
allowed_modules = ['foo._utils']

[[tool.importlinter.contracts]]
name = 'Foo submodules and utils, base'
type = 'allow-only'
source_modules = ['foo.*']
allowed_modules = ['foo._base', 'foo._utils']

[[tool.importlinter.contracts]]
name = 'Foo top-level and submodules'
type = 'allow-only'
source_modules = ['foo']
allowed_modules = ['foo.*']

# To strictly comply with the Mermaid diagrams, but most users won't need this:
wildcard_exclude = ['_base', '_utils']

Perhaps implementing this allow-only contract as a core type would provide a much more powerful basic primitive that will obviate many uses of custom contracts. I'm not really sure if these are all equivalent to layers. Maybe layers is just missing a wildcard option?

seddonym commented 1 year ago

Thanks for taking the trouble to write such a detailed reply, and apologies for the delay. Really interesting!

Here's some tips for how I'd implement the 'allow-only' contract.

  1. Start by writing failing tests. This can really help to define exactly what a particular contract means. You could take the tests for the forbidden contract as inspiration.
  2. Use Import Linter fields to define the contracts. In this case, to allow wildcards, the source modules and allowed modules would probably be sets of expressions that would need to be turned into concrete modules somehow. You could take inspiration from ignored imports for this, which uses ImportExpression objects.
  3. You could iterate over each source module and call (say) graph.find_modules_directly_imported_by to figure if it's importing the modules it's allowed to. If you want to drill down into the children too, you could call graph.find_children. There are other things you can do with the graph documented in the Grimp docs.
  4. If there are any problems, put them in the contract check's metadata, and separately implement render_broken_contract to display the problems.
  5. I would suggest doing this as a custom contract (rather than need to rely on it being merged into the main code base).

Hope that helps!