Warn only for unmatched ignored expressions

[expobrain]

Current behaviour

Consider a simple forbidden contract like this:

[importlinter]
root_package = mypackage

[importlinter:contract:1]
name = My forbidden contract (internal packages only)
type = forbidden
source_modules =
    mypackage.one
forbidden_modules =
    mypackage.two
    mypackage.three
ignore_imports =
    mypackage.one.* -> mypackage.*.mymodule

This will forbid to any module in mypackage.one to import import the module mymodule from any of the mypackage.two or mypackage.three packages.

However if the mymodule module doesn't exists the current version of import-linter will terminate with an error like:

Ignored import expression mypackage.one -> mypackage.*.mymodule didn't match anything in the graph.

Proposed behaviour

There are two options to improve this case:

1. instead of failing because the ignored expression doesn't match we can just print a warning and continue. The cons of this approach is that this will change the behaviour of import-linter and a new backward incompatible release will be necessary; also I don't have the context of why it fails for that condition as well.
2. add a new ignore_unmatched_expressions with a default value of false that, if set to true, will ignore the unmatched expression and continue to lint the imports. This case is preferred because doesn't change the current behaviour of import-linter and whoever needs it can enabled it when a where is necessary

Notes

I can prepare a PR for this for any of the two cases mentioned above (but also open to any third option).

[seddonym]

Hi Daniele, thanks for opening this issue!

It's interesting, the original intention of ignore_imports was to provide a list of temporary exceptions that constitute technical debt - so they're meant to be a list that is eventually burned down. For that use case, erroring makes sense.

But I get the impression that you're using ignore_imports as an integral part of the contract, for imports that are seen as okay (not just silenced). Is that correct? I have a feeling others are also using them in that way. While it's not how ignore_imports was originally envisaged, I think you're right that for that use case, errors don't make sense. Indeed, possibly warnings are too strong too?

With that in mind, I prefer option 2. ignore_unmatched_expressions is probably too general though - I think it should refer specifically to the behaviour of the ignore_imports field. What about a contract-specific field called unmatched_ignore_imports_alerting, with the choices none, warn, error? This should default to the existing behaviour of error.

I look forward to your PR! Feel free to reach out with any questions about a proposed implementation. A good place to start is to consider how we'll test the new behaviour.

[expobrain]

@seddonym got it, and yes, I'm using it probably in a different way of how it was intended.

My use case is that instead of explicit all the disallowed imports I prefer to disallow all the imports and allow only a few of them, it makes the rule easier to read and more maintainable (not all the source module exists and also creating a new module which is not listed in the forbidden contract will bypass it completely).

[seddonym]

This should now be addressed with the unmatched_ignore_imports_alerting flag.