Do not include line numbers in freezed messages

[wolfs]

I think the feature to freeze existing arch unit violations works great! One thing we have been struggling with is if we add/remove violations and want to accept the changes. The diffs become somewhat big since the line numbers are part of the text in the freeze property files. This makes it much harder to spot what actually changed: https://github.com/gradle/gradle/pull/22897/files#diff-6a12ad9a5a7bfd75e7d6500bd0946255450b1eb5066aef04cb843acf9848fd5eL28-R30

Is there an option to not include the line numbers in the freeze file?

[codecholeric]

Hey, sorry for the late response :see_no_evil: I don't think right now there is a good option for this :thinking: The problem for me is that

a) The line numbers are already not accessible in a structured form anymore, once they reach the ViolationStore interface b) That stored violations without line numbers are treated correctly also depends on the configured ViolationLineMatcher

You could get something like you want right now by using a custom ViolationStore. But again, it's not super pretty, because the default ViolationStore is implementation detail and not obtainable from the outside. So putting aside all morals (:grin:) it could be achieved right now in the following way:

public class LineNumberIgnoringViolationStore implements ViolationStore {
  private final ViolationStore delegate;

  public LineNumberIgnoringViolationStore() throws Exception {
    Class<?> storeFactoryClass = Class.forName("com.tngtech.archunit.library.freeze.ViolationStoreFactory");
    Method createMethod = storeFactoryClass.getDeclaredMethod("create");
    delegate = ArchConfiguration.withThreadLocalScope(configuration -> {
      // temporarily reset store to default
      configuration.setProperty("freeze.store", "com.tngtech.archunit.library.freeze.ViolationStoreFactory$TextFileBasedViolationStore");
      return invokeStaticMethod(createMethod);
    });
  }

  @SuppressWarnings("unchecked")
  private static <T> T invokeStaticMethod(Method method) {
    try {
      method.setAccessible(true);
      return (T) method.invoke(null);
    } catch (Exception e) {
      throw new RuntimeException(e);
    }
  }

  @Override
  public void initialize(Properties properties) {
    delegate.initialize(properties);
  }

  @Override
  public boolean contains(ArchRule rule) {
    return delegate.contains(rule);
  }

  @Override
  public void save(ArchRule rule, List<String> violations) {
    List<String> violationsWithoutLineNumbers = violations.stream()
        .map(it -> it.replaceAll("\\.java:\\d+", ".java:0"))
        .collect(toList());
    delegate.save(rule, violationsWithoutLineNumbers);
  }

  @Override
  public List<String> getViolations(ArchRule rule) {
    return delegate.getViolations(rule);
  }
}

Then configure in your archunit.properties

freeze.store=com.some.pkg.LineNumberIgnoringViolationStore

The store should now replace all line numbers by 0 when saving them and the default ViolationLineMatcher ignores line numbers, so the violations should be accepted against the real violations.

I'm wondering what would be a better API though :thinking: Two possibilities come to mind

1. Adjust the API to enable the above workaround in a less shady way (make ViolationStoreFactory.create() public API, add possibility to remove an ArchConfiguration property so we wouldn't have to set the class name of TextBasedViolationStore)
2. Add another configurable hook like ViolationLineMatcher, but to be plugged into ViolationStore. Like ViolationStoreTransformer that would be called on every violation message to be stored first. That could just house the it.replaceAll("\\.java:\\d+", ".java:0") part and no further tweaks would be necessary, except configuring this little part in the archunit.properties

Any better ideas? Or if not, which of the above ideas sound better for your use case? :thinking: In the end, it could also be a boolean property freeze.store.ignoreLineNumbers and if activated, then no matter the store we call it.replaceAll("\\.java:\\d+", ".java:0"). But the problem there is that e.g. .java depends on the source language again, so getting this really right for all projects is not that trivial. That's why I would try to avoid that and put the project-specific stuff on users as much as possible :thinking: