jraska/modules-graph-assert

`assertModuleGraph` doesn't take into account baselined `restricted` violations through `allowed`

saifc opened this issue · 5 comments

saifc commented

assertModuleGraph doesn't take into account allowing specific violations as an exception to a broader restricted rule.
For example, trying to allow :services:dialogs to depend on libraries:shared-prefs:impl through the rules below fails with the following message

Execution failed for task ':assertRestrictions'.
> Dependency '':services:dialogs' -> ':libraries:shared-prefs:impl' violates: ':services:\S* -X> :libraries:[a-zA-Z0-9:\-]*:impl'

Rules:

restricted = arrayOf(":services:\\S* -X> :libraries:[a-zA-Z0-9:\\-]*:impl")
allowed = arrayOf(":services:dialogs -> :libraries:shared-prefs:impl")

I have the same issue/feature request

Example:
:network:api-base (response/error wrapper)
:network:api-imdb (explicit)
:network:api-trackt (explicit)

allowed = ":network:api-.* -> :network:api-base" // should be allowed
restricted = ":network:.* -X> :network:.*" // network modules shouldn't depends on each other

As a temporary solution, I do
allowed = ":network-api-imdb -> :network:api-base", ":network-api-track -> :network:api-base"
which I am not a big fan as it requires to whitelist manually at each module creation.

Allowed being eval after the restricted would be a nice addition

Hi, sorry for late response - missed the notifications 😞

Thanks for raising an issues and using the plugin 👍 @saifc @johnLegasse

This is expected behaviour

The restricted/allowed checks are completely independent from each other - therefore what you are experiencing now is a feature by design :)

It was a design choice when discussing API for version 2.0 with conclusion of these checks to be independent.

The reasoning behind that is that it brings a circular problem - is allowed what is restricted or is restricted what is allowed - which one wins?
Therefore the checks are treated independently and they don't know about each other on purpose

  • If some restricted rule finds match - assertions fails regardless allowed
  • If some module dependency does not match the allowed rules - assertion fails

Talking about the cases above in next messages ...

Rules:

restricted = arrayOf(":services:\S* -X> :libraries:[a-zA-Z0-9:\-]*:impl")
allowed = arrayOf(":services:dialogs -> :libraries:shared-prefs:impl")

@saifc - I would remove the restricted completely and stick only to allowed + add your other allowed rules - what is not allowed is restricted by the allowed rules.

@johnLegasse - I would continue with the allowed rules as this is how the rules were intended - what is not allowed is restricted. I can imagine that whitelisting would be more convenient for your use case, but perhaps different strategies like naming change or moduleNameAlias could work for you as well 🤔

Hey, I will close the issue for now - please if there is more to follow up, reopen it.

Thanks