Is there a way to detect unused methods and fields in Spring Application?

[adavid9]

Hey, I try to write some rules that would detect unused classes, methods and fields in a Spring Application.

I already found some examples on GitHub, but it doesn't work for me.

For instance to find unused methods I have this code:

private static Predicate<JavaMethod> hasMatchingNameAndParameters(JavaMethod input) {
    return m -> m.getName().equals(input.getName())
            && m.getRawParameterTypes().size() == input.getRawParameterTypes().size()
            && (m.getDescriptor().equals(input.getDescriptor())
            || namesOf(m.getRawParameterTypes()).containsAll(namesOf(input.getRawParameterTypes())));
}

private static DescribedPredicate<JavaMethod> methodHasAnnotationThatEndsWith(String suffix) {
    return describe(String.format("has annotation that ends with '%s'", suffix),
            method -> method.getAnnotations().stream()
                    .anyMatch(annotation -> annotation.getRawType().getFullName().endsWith(suffix)));
}

private static ArchCondition<JavaMethod> shouldBeReferencedMethod = new ArchCondition<>("not be unreferenced") {
    @Override
    public void check(JavaMethod javaMethod, ConditionEvents events) {
        Set<JavaCodeUnitAccess<?>> accesses = new HashSet<>(javaMethod.getAccessesToSelf());
        accesses.removeAll(javaMethod.getAccessesFromSelf());
        if (accesses.isEmpty()) {
            events.add(new SimpleConditionEvent(javaMethod, false, String.format("%s is unreferenced in %s",
                    javaMethod.getDescription(), javaMethod.getSourceCodeLocation())));
        }
    }
};

@ArchTest
static final ArchRule methodsShouldNotBeUnused = methods()
        .that(describe("are not declared in super type", input -> input.getOwner().getAllRawSuperclasses().stream()
                .flatMap(c -> c.getMethods().stream()).noneMatch(hasMatchingNameAndParameters(input))))
        .and(describe("are not declared in interface", input -> input.getOwner().getAllRawInterfaces().stream()
                .flatMap(i -> i.getMethods().stream()).noneMatch(hasMatchingNameAndParameters(input))))
        .and().doNotHaveName("main")
        .and(not(methodHasAnnotationThatEndsWith("Handler")
                .or(methodHasAnnotationThatEndsWith("Listener"))
                .or(methodHasAnnotationThatEndsWith("Scheduled"))
                .and(declaredIn(describe("component", clazz -> clazz.isMetaAnnotatedWith(Component.class))))))
        .should(shouldBeReferencedMethod)
        .as("should use all methods")
        .because("unused methods should be removed");

but unfortunately it searches for getters & setters and enum methods such as values() and valueOf(), and I am not sure how to avoid that.

As for the fields I use this code:

@ArchTest
static final ArchRule fieldsShouldNotBeUnreferenced =
        fields().should(new ArchCondition<>("not be unreferenced") {
            @Override
            public void check(JavaField field, ConditionEvents events) {
                Set<JavaFieldAccess> accesses = field.getAccessesToSelf();
                if(accesses.isEmpty()) {
                    events.add(new SimpleConditionEvent(field, false, String.format("%s is unreferenced in %s", field.getDescription(), field.getSourceCodeLocation())));
                }
            }
        });

but it detects fields that are used within the same class in some specific method. The method should find all fields that are not used anywhere, but are only declared.

[hankem]

Ah, it seems that you already know timtebeek/archunit-unreferenced... 😉

it searches for getters & setters

If you want to allow for unused getters & setters, you could probably exclude them explicitly in your methods().that selection, e.g. for getters:

            // ...
            .and(not(rawParameterTypes(/* none */).and(nameStartingWith("get"))))
            // ...

with a predicate like this:

    static DescribedPredicate<JavaCodeUnit> rawParameterTypes(Class<?>... expectedTypes) {
        return describe("parameters", codeUnit -> {
            List<JavaClass> parameterTypes = codeUnit.getRawParameterTypes();
            if (parameterTypes.size() != expectedTypes.length) {
                return false;
            }
            for (int i = 0; i < expectedTypes.length; i++) {
                if (!parameterTypes.get(i).isEquivalentTo(expectedTypes[i])) {
                    return false;
                }
            }
            return true;
        });
    }

and enum methods such as values() and valueOf()

You can likewise exclude them by adding

            // ...
            .and(not(rawParameterTypes(String.class).and(name("valueOf")).and(declaredIn(assignableTo(Enum.class))))
            .and(not(rawParameterTypes(/* none */).and(name("values")).and(declaredIn(assignableTo(Enum.class))))))
            // ...

All of those checks can be made more specific by also checking the return parameter type, but this might already give you an idea.

[adavid9]

@hankem Yes, I do :) Thank you for the answer, it helps me a lot!

[codecholeric]

Just a little addition: There is already HasParameterTypes.Predicates#rawParameterTypes(java.lang.Class<?>...), so no reason to define that yourself...

[adavid9]

However, I see one limitation with this. It will still find methods that have been named manually and not automatically generated by the IDE. Let's say someone has a boolean field installConfiguration in the Java Class, so the method could be named shouldInstallConfiguration. I wonder if there is a way to ignore all methods within the same class that references the fields, so you will always ignore any setters & getters, even the ones that are named differently.

[adavid9]

I know that some people does not follow conventions and may name boolean getters differently, and in that case I do not want to find them as unreferenced.

[adavid9]

@hankem I do wonder if there is a better solution for setters than this one.

private static DescribedPredicate<JavaMethod> setter() {
    return describe("setter", method -> {
        Set<String> fields = method.getOwner().getFields().stream()
                .map(JavaMember::getName)
                .map(e -> "set" + capitalize(e))
                .collect(Collectors.toSet());
        return fields.contains(method.getName());
    });
}

[adavid9]

Ah, it seems that you already know timtebeek/archunit-unreferenced...

As for this repository, I think that I have found a problem. If you have an interface that is implemented by any class, and then you create an instance of that class and call the interface method it will show this method as unreferenced.

package org.example;

class ExampleRunner {
	void example() {
		Example example = new Example();
		example.example();
	}
}

class Example implements ExampleInterface{
	@Override
	public void example() {
		
	}
}

interface ExampleInterface {
	void example();
}

I personally feel the lack of documentation and I think that writing rules that are more advanced is quite difficult, especially for a person who is starting with ArchUnit 🤔

[hankem]

ArchUnit doesn't yet offer a direct API to find out whether a supertype's method is overridden. @timtebeek himself asked for it in #982.

[hankem]

writing rules that are more advanced is quite difficult, especially for a person who is starting with ArchUnit

Any suggestion how to improve documentation etc. is highly appreciated – preferably (but not necessarily! 😉) in form of a pull request.