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
674 stars 46 forks source link

Disallow imports not specified through __all__ #240

Open charettes opened 5 days ago

charettes commented 5 days ago

👋 there, thanks for that awesome project!

We've been using import-linter to define interfaces between packages in our application but one thing that keeps coming is preventing re-exports from violating our contracts.

For example, say that we allow module foo to only import from bar.interface (and nothing else in bar.*) when we define bar.interface we might imports a few things from bar.* to define the interface.

We'd like for a way to prevent imports from bar.* in bar.interface from being re-exported and we were hoping we could use bar.interface.__all__ for that through some flags at least instead of doing function level imports in bar.interface.

It might be easier to reason about this with code. Today if we want to prevent re-exports of bar.* in bar.interface we have to do

# bar.interface

def update_bar(bar_id: int, updates: dict) -> bool:
    from bar.models import Bar
    return Bar.objects.get(bar_id).update(**updates)

As otherwise doing

# bar.interface
from bar.models import Bar

def update_bar(bar_id: int, updates: dict) -> bool:
    return Bar.objects.get(bar_id).update(**updates)

re-exports Bar as bar.interface.Bar. We'd like to be able to do

# bar.interface
from bar.models import Bar

__all__ = ('update_bar',)

def update_bar(bar_id: int, updates: dict) -> bool:
    return Bar.objects.get(bar_id).update(**updates)

and have import linter prevent bar.interface.Bar imports.

Thank you for your consideration!

seddonym commented 3 days ago

Interesting one this, thanks for raising. It's actually something we're also grappling with at work at the moment - broadly speaking I would describe this as wanting to achieve encapsulation - that is, a separation between the public interface and the private internals of a given subpackage. And it looks like you're wanting to make Django models a private concern - again, that's something we're working on.

In terms of your specific feature request, I suspect adding support for this in Import Linter would be a substantial amount of work. Currently it relies on building a directed graph of all the imports between modules. Supporting your use case would require more granularity about the objects exposed in each module.

I wonder if this might be relatively straightforward to lint for without building an import graph - perhaps a regular expression might be enough, as the concern is quite localized. Or you could write a flake8 plugin, or request an addition to Ruff.

Another option would be to import the model with a private alias, like this?

# bar.interface
from bar.models import Bar as _Bar

__all__ = ('update_bar',)

def update_bar(bar_id: int, updates: dict) -> bool:
    return _Bar.objects.get(bar_id).update(**updates)

At the very least, that will give people second thoughts before importing bar.interface._Bar, and you could probably use a linter already available to enforce that, such as this one from ruff.

Incidentally, the convention we currently follow is to prefix private modules with an underscore. So you could move to a structure like this:

bar/
   __init__.py
   interface.py
   _models.py

Of course, that would break Django's autodiscovery of models - because really, models have to be public as far as Django is concerned. But there's probably a way around it, such as running an autodiscovery of _models modules during application bootstrap.

I anticipate that Import Linter will support enforcing the rules based on underscores in the not too distant future.

Another tactic could be to use the Repository Pattern, and structure your application so that the implementation of the repositories is in a layer above the code that interacts with the repository abstraction. That's something Import Linter currently supports (via the layers contract) but I imagine that would be a big rearchitecting job!

Let me know how you get on!