TNG / ArchUnit

A Java architecture test library, to specify and assert architecture rules in plain Java
http://archunit.org
Apache License 2.0
3.17k stars 287 forks source link

Enum classes not recognized as enums and with with wrong access modifiers #1233

Open kosciuszkopiotr opened 7 months ago

kosciuszkopiotr commented 7 months ago

Hello, I'm using following code for rule:

           classes()
                    .that()
                    .haveSimpleNameContaining("Command")
                    .and()
                    .doNotHaveModifier(JavaModifier.ABSTRACT)
                    .should(new ArchCondition<>("In command package only result, request and command can be public") {
                        @Override
                        public void check(JavaClass item, ConditionEvents events) {
                            JavaPackage aPackage = item.getPackage();
                            long publicClasses = aPackage.getClasses().stream()
                                    .filter(clazz -> clazz.getModifiers().contains(JavaModifier.PUBLIC))
                                    .filter(clazz-> !clazz.isNestedClass())
                                    .filter(clazz -> Stream.of("Result", "Request", "Command")
                                            .noneMatch(suffix -> clazz.getName().endsWith(suffix)))
                                    .count();
                            if (publicClasses > 0) {
                                events.add(SimpleConditionEvent.violated(item.getPackage(), item.getPackage() + ": package warning - only result, request and command classes should be public "));
                            }
                        }
                    })
                    .as("In command package only result, request and command can be public")

As per description, the objective of the rule is to keep only selected public classes in packages with "command" in name. The issue is that this rule fails for package-private access enums and even shows those enums with wrong descriptors (Array): Screenshot 2024-01-18 121657 If I make one of the enums public (I have two cases as per screenshot), then this enum will pop up twice in the condition check. Could you please advise? Best regards!

rweisleder commented 7 months ago

This looks like a bug in JavaPackage.getClasses(), because the method returns the type and the corresponding array type.

class GH1233 {

    @Test
    void test() {
        JavaClass javaClass = new ClassFileImporter().importClass(GH1233.class);
        Set<JavaClass> classesInPackage = javaClass.getPackage().getClasses();
        assertThat(classesInPackage).extracting(JavaClass::getSimpleName).containsExactly("GH1233");
    }

    void foo(GH1233[] bar) {
    }
}

This test fails, because classesInPackage is ["GH1233", "GH1233[]"]

@kosciuszkopiotr: maybe you can rewrite your rule like this:

classes()
    .that(describe("reside in command package", javaClass -> javaClass.getPackage().getClasses().stream().anyMatch(simpleNameContaining("Command"))))
    .and().areTopLevelClasses()
    .and(not(have(modifier(ABSTRACT))))
    .and(not(have(simpleNameEndingWith("Result").or(simpleNameEndingWith("Request")).or(simpleNameEndingWith("Command")))))
    .should().notBePublic();
kosciuszkopiotr commented 7 months ago

This works! Thanks a lot :)

codecholeric commented 5 months ago

This looks like a bug in JavaPackage.getClasses()

AFAIR this is by design, you can argue if it's the best design of course 🤔 Cause at some point we decided that for most purposes an array should be considered to have the same package as the corresponding component type (as opposed to the Reflection API) and on the other hand all types found during the import are sorted into their respective package. So if you cause an import of an array type, like GH1233[] in your case, then this will be sorted into the same JavaPackage as GH1233. I'm not sure what would be a better behavior though 🤷 Every option seems possibly confusing depending on the circumstances...