Duplicate error messages for noClasses().should().dependOnClassesThat().resideInAPackage(...) when culprit is in finally clause

[ttho]

Since 1.3.0, the following test prints duplicate messages for the incorrect() method:

package test;

import com.tngtech.archunit.core.domain.JavaClasses;
import com.tngtech.archunit.core.importer.ClassFileImporter;
import com.tngtech.archunit.lang.ArchRule;
import org.junit.jupiter.api.Test;

import java.lang.reflect.Field;

import static com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noClasses;

    public static class TestClass {

        public void incorrectReporting() throws Exception {
            try {
                int a=0;
            } finally {
                Field field = null;
                field.setBoolean(null, false);
            }
        }

        public void correctReporting() throws Exception {
            Field field = null;
            field.setBoolean(null, false);
        }

        public void alsoCorrectReporting() throws Exception {
            try {
            } finally {
                Field field = null;
                field.setBoolean(null, false);
            }
        }

    }

    @Test
    public void doNotUseReflection() {
        JavaClasses importedClasses = new ClassFileImporter().importPackages("test");
        ArchRule rule = noClasses().should().dependOnClassesThat().resideInAPackage("java.lang.reflect..");
        rule.check(importedClasses);
    }
}

Actual output:

java.lang.AssertionError: Architecture Violation [Priority: MEDIUM] - Rule 'no classes should depend on classes that reside in a package 'java.lang.reflect..'' was violated (4 times):
Method <test.MyArchitectureTest$TestClass.alsoCorrectReporting()> calls method <java.lang.reflect.Field.setBoolean(java.lang.Object, boolean)> in (MyArchitectureTest.java:42)
Method <test.MyArchitectureTest$TestClass.correctReporting()> calls method <java.lang.reflect.Field.setBoolean(java.lang.Object, boolean)> in (MyArchitectureTest.java:35)
Method <test.MyArchitectureTest$TestClass.incorrectReporting()> calls method <java.lang.reflect.Field.setBoolean(java.lang.Object, boolean)> in (MyArchitectureTest.java:29)
Method <test.MyArchitectureTest$TestClass.incorrectReporting()> calls method <java.lang.reflect.Field.setBoolean(java.lang.Object, boolean)> in (MyArchitectureTest.java:29)

Expected output:

java.lang.AssertionError: Architecture Violation [Priority: MEDIUM] - Rule 'no classes should depend on classes that reside in a package 'java.lang.reflect..'' was violated (4 times):
Method <test.MyArchitectureTest$TestClass.alsoCorrectReporting()> calls method <java.lang.reflect.Field.setBoolean(java.lang.Object, boolean)> in (MyArchitectureTest.java:42)
Method <test.MyArchitectureTest$TestClass.correctReporting()> calls method <java.lang.reflect.Field.setBoolean(java.lang.Object, boolean)> in (MyArchitectureTest.java:35)
Method <test.MyArchitectureTest$TestClass.incorrectReporting()> calls method <java.lang.reflect.Field.setBoolean(java.lang.Object, boolean)> in (MyArchitectureTest.java:29)

[hankem]

[This is just to document my first analysis. I don't have a complete understanding yet.]

The byte code of try-catch blocks can be quite a beast.

The byte code for incorrectReporting() actually has two calls to Field.setBoolean(java.lang.Object, boolean) – one for the case where int a = 0 succeeds, and one for the case where it throws an exception.

With OpenJDK 17 as well as OpenJDK 21 (and essentially the same for JDK 8 and OpenJDK 11):

  public void incorrectReporting() throws java.lang.Exception;
    descriptor: ()V
    flags: (0x0001) ACC_PUBLIC
    Code:
      stack=3, locals=4, args_size=1
         0: iconst_0
         1: istore_1
         2: aconst_null
         3: astore_1
         4: aload_1
         5: aconst_null
         6: iconst_0
         7: invokevirtual #7                  // Method java/lang/reflect/Field.setBoolean:(Ljava/lang/Object;Z)V
        10: goto          24
        13: astore_2
        14: aconst_null
        15: astore_3
        16: aload_3
        17: aconst_null
        18: iconst_0
        19: invokevirtual #7                  // Method java/lang/reflect/Field.setBoolean:(Ljava/lang/Object;Z)V
        22: aload_2
        23: athrow
        24: return
      Exception table:
         from    to  target type
             0     2    13   any

The issue somehow seems to arise with 771df06, where Dependency.tryCreateFromAccess was changed to produce a Dependency.FromAccess instead of a plain Dependency:

    static Set<Dependency> tryCreateFromAccess(JavaAccess<?> access) {
        JavaClass originOwner = access.getOriginOwner();
        JavaClass targetOwner = access.getTargetOwner();
        ImmutableSet.Builder<Dependency> dependencies = ImmutableSet.<Dependency>builder()
                .addAll(createComponentTypeDependencies(originOwner, access.getOrigin().getDescription(), targetOwner, access.getSourceCodeLocation()));
-       dependencies.addAll(asSet(tryCreateDependency(originOwner, targetOwner, access.getDescription(), access.getSourceCodeLocation())));
+       if (!originOwner.equals(targetOwner) && !targetOwner.isPrimitive()) {
+           dependencies.add(new Dependency.FromAccess(access));
+       }
        return dependencies.build();
    }