lemon24 / reader

A Python feed reader library.
https://reader.readthedocs.io
BSD 3-Clause "New" or "Revised" License
438 stars 36 forks source link

Consider using Import Linter #274

Closed lemon24 closed 2 years ago

lemon24 commented 2 years ago

https://import-linter.readthedocs.io/en/stable/readme.html

lemon24 commented 2 years ago

Played with it; nice idea, but it seems like too much work.

... especially because in-module-component wildcards aren't supported (e.g. can't match reader.*_utils).

For reader a list of human-readable rules would likely be enough; I expect it might be useful for larger projects with lots of contributors of different experience levels.

Some things I played with:

[importlinter]
root_package = reader
contract_types =
    forbidden: contracts.ForbiddenWildcardContract
    independence: contracts.IndependenceWildcardContract

[importlinter:contract:api-no-private-modules]
name = reader does not import from private modules
type = forbidden
importer = reader
imported = reader._*

[importlinter:contract:utils-independent]
name = reader.*_utils should be independent
type = independence
modules =
    reader.*_utils

[importlinter:contract:xxx]
name = xxx
type = independence
modules =
    reader._parser
    reader._storage
    reader._update
contracts.py ```python from fnmatch import fnmatchcase from itertools import permutations from importlinter import Contract, ContractCheck, fields, output class ForbiddenWildcardContract(Contract): importer = fields.StringField() imported = fields.StringField() def check(self, graph): all_imports = graph.find_modules_directly_imported_by(self.importer) forbidden_imports = [ module for module in all_imports if fnmatchcase(module, self.imported) ] return ContractCheck( kept=not forbidden_imports, metadata={'forbidden_imports': forbidden_imports} ) def render_broken_contract(self, check): output.print_error( f'{self.importer} is not allowed to import {self.imported}:', bold=True, ) output.new_line() for imported in check.metadata['forbidden_imports']: output.indent_cursor() output.print_error(f'{self.importer} -> {imported}') class IndependenceWildcardContract(Contract): modules = fields.ListField(subfield=fields.StringField()) def check(self, graph): modules = [ module for pattern in self.modules for module in graph.modules if fnmatchcase(module, pattern) ] dependent = [] for importer, imported in permutations(modules, r=2): if graph.direct_import_exists(importer=importer, imported=imported): dependent.append((importer, imported)) return ContractCheck(kept=not dependent, metadata={'dependent': dependent}) def render_broken_contract(self, check): output.print_error( f'({", ".join(self.modules)}) are not allowed to import one another:', bold=True, ) output.new_line() for importer, imported in check.metadata['dependent']: output.indent_cursor() output.print_error(f'{importer} -> {imported}') ```

Also, the _parser/_storage/_update one creates what I think is a false alarm: _update depends on the other two for type checking purposes only (if TYPE_CHECKING: import ...); it can be fixed in code with Protocols/ABCs, but that way lies Java (interfaces with exactly one class implementing them) ... I guess I would have to do that if I was type checking tests.