FeatureRequest: Possibility to define IDs for Rules

[AndreasIgelCC]

Hit at all,

first: archunit is really great and this issue is just a way to improve the way to use it!

When using in unit-test scope, the context and specific rule is easily to be identified. But if you are using it in framework way (for example in big projects) or if you have a 3rd party tool for analysis, there should be an unique ID for a rule.

I would have the following suggestion: add an optional ID for rules. If it is not set, the generic ID from store could be used. In that way the ID generation would be moved from ViolationStore to the rules.

Best Regards, Andreas

PS: If you like and support that change, I would create a PullRequest!

[codecholeric]

At the moment the ViolationStore pretty much considers the rule text as the ID, which obviously has some shortcomings :thinking: Problem so far was that I didn't see any other property to really identify a rule. If I understand you correctly you would propose to add an optional id field that could be set by the user? E.g. something like

classes()...should()...ruleId("someCustomRuleId42")

This could still be too little to disambiguate though, in cases where rules are shared via

@ArchTest
static final ArchTests included = ArchTests.in(OtherClass.class);

Because the same rule with the same id could be included in several other rule suits this way :thinking: So far in the projects where I have used it, the scope of the violation store was always limited enough though that a certain rule would only appear once within a test run :thinking: Or the test engine could prepend the test descriptor path (e.g. MyLibrary:MyTest:someRuleId)

[AndreasIgelCC]

The Idea with the test descriptor sounds good (not used that before). I could have a deeper look on that. That would be really a unique identifier.

But I would have liked to have it really in that way you mentioned:

classes()...should()...withRuleId("customRuleId1")

If a developer specifies the same ID on two tests, that would be a topic to address. But may be only in a way, that the resulting error is understandable.

[codecholeric]

In the end that could already be a problem at the moment :thinking: Because, right now the ID principally is the rule text. So if the same rule is evaluated and stored twice into the same ViolationStore (possibly with different import sets) this would already clash. I think getting a good stable id is really not trivial.

One thing I want to point out is that you could already implement all this for yourself with the current version of ArchUnit:

1) Create your own wrapper of ArchRule

public class ArchRuleWithId implements ArchRule {
    private final String id;
    private final ArchRule delegate;

    private ArchRuleWithId(String id, ArchRule delegate) {
        this.id = id;
        this.delegate = delegate;
    }

    public static ArchRuleWithId of(String id, ArchRule delegate) {
        return new ArchRuleWithId(id, delegate);
    }

    public String getId() {
        return id;
    }
    // all further methods delegated
}

2) Implement a store that checks the custom implementation, e.g.

public class SonaTypeViolationStore implements ViolationStore {
    @Override
    public void save(ArchRule rule, List<String> violations) {
        if (rule instanceof ArchRuleWithId) {
            String id = ((ArchRuleWithId) rule).getId();
            saveWithId(id, violations);
        }
        throw new IllegalArgumentException(
            "SonaTypeViolationStore only supports ArchRules wrapped in ArchRuleWithId");
    }
   // ...
}

3) Wrap the rule to freeze first into your custom ID wrapper

freeze(ArchRuleWithId.of("mySpecialId", classes()....should()...))
    .persistIn(new SonaTypeViolationStore())

Just in case you need to get something on the road quickly in your project, even if it's a little quick&dirty :wink:

[stefanroeck]

An rough implementation of the highlighted approach (wrapping and extending with metadata) can be found here: https://gist.github.com/stefanroeck/0e7b2002eb0e801b8ff619e6738048db

This also uses an XML-based persistence with the provided rule ids as reference which simplifies maintenance of the violation store.

[klu2]

You also might have a look at our implementation:

ArchRuleWithId: https://github.com/cloudflightio/archunit-cleancode-verifier/blob/master/src/main/kotlin/io/cloudflight/cleancode/archunit/ArchRuleWithId.kt Store: https://github.com/cloudflightio/archunit-cleancode-verifier/blob/master/src/main/kotlin/io/cloudflight/cleancode/archunit/IdBasedViolationStore.kt

written in Kotlin