TNG / ArchUnit

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

SimpleName-Check failing because of trailing file extension (Kt) #1318

Open darthkali opened 1 month ago

darthkali commented 1 month ago

I have a data class named FooEntity. In this file, besides the data class, I also have a function. This causes the name in the classpath to be FooEntityKt instead of FooEntity. Now my check doesn't pass. Is there a nice solution for this?

Example: FooEntity.kt

@Entity(name = "foo")

data class FooEntity(
    
    @Id

    var id: UUID,
    
    var name: String,
  
)



fun Foo.toBar(): String {
    
    return "Bar is better"
}

Error:

Class <...FooEntityKt> does not have simple name ending with 'Entity' in (FooEntity.kt:0)

ArchUnit Test:

@ArchTest
val `check entities`: ArchRule = classes()
    .that().haveSimpleNameEndingWith("Entity")
    .or().resideInAPackage("..entity..")
   .or().areAnnotatedWith(Entity::class.java)
   .should().haveSimpleNameEndingWith("Entity")
   .andShould().resideInAPackage("..entity..")
   .andShould().beAnnotatedWith(Entity::class.java)

Current used ArchUnit Version: "com.tngtech.archunit:archunit-junit5:1.3.0"

hankem commented 1 month ago

Now my check doesn't pass. Is there a nice solution for this?

Can you share your check? 🙂

darthkali commented 1 month ago

I have added the test in the main post.

hankem commented 1 month ago

I'm not aware of a reliable way to recognize extension functions, which you seem to want to allow?

Maybe the following workaround is good enough for you?

imports ```kotlin import com.tngtech.archunit.base.DescribedPredicate import com.tngtech.archunit.base.DescribedPredicate.describe import com.tngtech.archunit.core.domain.JavaClass import com.tngtech.archunit.core.domain.JavaClass.Predicates.resideInAPackage import com.tngtech.archunit.core.domain.JavaClass.Predicates.simpleNameEndingWith import com.tngtech.archunit.core.domain.JavaModifier import com.tngtech.archunit.core.domain.properties.CanBeAnnotated.Predicates.annotatedWith import com.tngtech.archunit.junit.ArchTest import com.tngtech.archunit.lang.ArchRule import com.tngtech.archunit.lang.conditions.ArchConditions import com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes ```
@ArchTest
val `classes named or annotated as entities should be entities`: ArchRule =
    classes()
        .that().haveSimpleNameEndingWith("Entity").or().areAnnotatedWith(Entity::class.java)
        .should(beAnEntity())

@ArchTest
val `classes in an entity package should be entities`: ArchRule =
    classes()
        .that().resideInAPackage("..entity..")
        .should(beAnEntity()).orShould(haveOnlyExtensionFunctionsForEntities())

private fun beAnEntity() = ArchConditions.be(anEntity())

private fun anEntity(): DescribedPredicate<JavaClass> =
    simpleNameEndingWith("Entity").and(resideInAPackage("..entity..")).and(annotatedWith(Entity::class.java))
        .`as`("be an entity")

private fun haveOnlyExtensionFunctionsForEntities() = ArchConditions.have(
    describe("only extension functions for entities") { javaClass: JavaClass ->
        javaClass.methods.all {
            it.modifiers == setOf(JavaModifier.PUBLIC, JavaModifier.STATIC, JavaModifier.FINAL)
                    && it.parameters.size == 1
                    && anEntity().test(it.rawParameterTypes[0])
        }
    }
)
darthkali commented 1 month ago

I will have a look into your solution. Thanks for this. I will give you a response as soon as possible. :)

codecholeric commented 1 month ago

I think the basic problem is to combine those conditions all in one rule, i.e. what fails is that you say "classes that reside in a package '..entity..' should have simple name ending with 'Entity'". Because once you declare an extension function the Kotlin compiler also declares a class FooEntityKt to house it. It doesn't complain about your declared class FooEntity per se (which is another class in the package that's also imported). You could also check if there is a way to filter out those Kt fake classes in the first place and not check them.

A simple solution could also be to split the rules into several rules where the package rule is adjusted to "classes that reside in a package '..entity..' should have simple name ending with 'Entity' or ending with 'EntityKt'" and ignore the corner case that somebody could also declare an illegal class ending with Kt 🤷 You could still have another one that "annotated with @Entity should have simple name ending with 'Entity'", cause that's true no matter if there is an extension function or not (i.e. the Kt file won't be annotated with that). And vice versa another rule that says "simple name ending with 'Entity' => annotated with @Entity" (again not matching the Kt classes).

I'm not sure the rule that every class that resides in ..entity.. should have simple name ending with "Entity" really makes sense though. At least I could imagine that from time to time you might want a utility class or a nested data class that is just an implementation detail and maybe just meant as a package internal not to be used from the outside 🤷 But not sure how strict you want to play this...

darthkali commented 3 weeks ago

So, I finally had some time to look into the topic. First of all, thank you for the feedback. I want to keep the solution as lightweight as possible so that I can continue to incorporate it into the project.

I had already considered simply saying that ExtensionFunctions should either be placed in a Companion Object, which seems to work, or outsourcing the Extension Functions into a separate file.

@codecholeric Your response definitely gave me a good idea. I believe I can't make the whole thing as generic as I initially did.

I took up your suggestion and split the tests. For the entities, it worked great because I have the annotation as an additional comparison tool. For something like the Port, I kept it a bit stricter. Only ports as interfaces are allowed in that package.