Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
kosciuszkopiotr opened this issue Jan 18, 2024 · 3 comments

Comments

@kosciuszkopiotr
Copy link

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
Copy link
Contributor

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
Copy link
Author

This works! Thanks a lot :)

@codecholeric
Copy link
Collaborator

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...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants