ArchUnit icon indicating copy to clipboard operation
ArchUnit copied to clipboard

ArchUnit thinks "Switch with arrows" produces non-final fields

Open lasselindqvist opened this issue 1 year ago • 4 comments

Example code:

having some method with a call inside it like:

			var status = service.waitForResult(taskId, Duration.ofMinutes(1));
			switch (status) {
			case COMPLETED -> result.setStatus(StatusEnum.COMPLETED);
			case FAILED -> result.setStatus(StatusEnum.FAILED);
			default -> result.setStatus(StatusEnum.PENDING);
			}

causes a violation like Field <com.controller.Controller.$SWITCH_TABLE$...$StatusEnum> is not final in (Controller.java:0)

with a test

    @ArchTest
    public static final ArchRule rule = ArchRuleDefinition.classes()
        .that()
        .areAnnotatedWith(Controller.class)
        .should().haveOnlyFinalFields();

Maybe this is the fault of the compiler used (Eclipse), but I wonder if ArchUnit can ignore these switch tables when checking the rule?

lasselindqvist avatar May 31 '24 11:05 lasselindqvist

I guess this can be handled by skipping fields with JavaModifier.SYNTHETIC. Even if we don't want to do that in the default haveOnlyFinalFields method, maybe it would be good to show/document this workaround somehow.

I suppose one way is to check this using ArchRuleDefinition::fields, filtering with ‎MembersThatInternal::doNotHaveModifier(JavaModifier.SYNTHETIC) and then check for the final modifier.

lasselindqvist avatar Jun 08 '24 17:06 lasselindqvist

Is this going to be fixed in some version or is there a workaround? We are having this issue with java 21.

geoludbit avatar Sep 09 '24 13:09 geoludbit

Here is a proper example of the workaround example for anyone wondering:

    @ArchTest
    public static final ArchRule rule = 
    	ArchRuleDefinition.fields()
        .that()
        .doNotHaveModifier(JavaModifier.SYNTHETIC)
        .and()
        .areDeclaredInClassesThat(CanBeAnnotated.Predicates.annotatedWith(Controller.class))
        .should(ArchConditions.beFinal());

lasselindqvist avatar Oct 11 '24 10:10 lasselindqvist

Sorry for the late reply and thanks for the workaround! It might make sense to ignore this by default in reasonable predicates. This is a good case where it likely never makes sense to consider synthetic fields. I've tried to exclude all synthetic members and classes by default at some point, but that actually caused a ton of problems back then. But for specific predefined predicates we could likely do this without any danger to overlook something 🤔

codecholeric avatar Nov 10 '24 10:11 codecholeric