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 771df0615f382bfd1e5e560b1834c60c9f62089d, 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();
    }