Archunit test to not allow to log the return value of method with specific annotation using Log4j logging methods

Question

Suppose I have below Person class with one getAccountNumber() method having @ShouldNotBeLogged custom annotation.

public class Person {

private String name;
private String accountNumber;

@ShouldNotBeLogged
public String getAccountNumber() {
return accountNumber;
}

}

So, the question is I want to create a archunit which checks that any class (like HelloWorld class below here) having org.apache.logging.log4j.Logger type static field cannot log the return value of method with @ShouldNotBeLogged annotation.

Here, archunit test should report violation in HelloWorld.logMessage method - as it is logging the return value of Person class object - getAccountNumber() method having @ShouldNotBeLogged custom annotation.

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

    public class HelloWorld {
    
        private static final Logger logger = LogManager.getLogger(HelloWorld.class);
    
        public void logMessage(Person person) {
         logger.debug("logging Acc No. - {}", person.getAccountNumber());
        }
    }

Answer 1

ArchUnit (as of the current version 0.23.1) does not analyze parameter values of a method call – but maybe it's good enough to find calls to a logger and to a @ShouldNotBeLogged annotated method in the same line?

• False positives (unrelated calls in the same line) like

  logger.debug("This is okay:");  String accountNumber = person.getAccountNumber();
  

  might already be prevented by code style rules.

• For false negatives (the return value of a @ShouldNotBeLogged annotated method is logged, but the method calls are spread over multiple lines) like

  logger.debug("This is not okay: {}",
      person.getAccountNumber()
  );
  

  , you could easily adapt the rule to require that the method calls have some reasonable™ line number distance (which unfortunately increases the probability of false positives).

Yes, this just a heuristic. But it might be better than nothing. 🤷‍♂️

@ArchTest
ArchRule rule = noClasses().should(new ArchCondition<JavaClass>("log return values of @ShouldNotBeLogged methods") {
    @Override
    public void check(JavaClass javaClass, ConditionEvents events) {
        Set<JavaMethodCall> loggerCalls = new HashSet<>();
        Set<JavaMethodCall> shouldNotBeLoggedCalls = new HashSet<>();
        javaClass.getMethodCallsFromSelf().forEach(methodCall -> {
            Optional<JavaMethod> resolvedTarget = methodCall.getTarget().resolveMember();
            if (resolvedTarget.isPresent()) {
                JavaMethod javaMethod = resolvedTarget.get();
                if (javaMethod.getOwner().isAssignableTo(Logger.class)) {
                    loggerCalls.add(methodCall);
                }
                if (javaMethod.tryGetAnnotationOfType(ShouldNotBeLogged.class).isPresent()) {
                    shouldNotBeLoggedCalls.add(methodCall);
                }
            }
        });
        loggerCalls.forEach(loggerCall ->
            shouldNotBeLoggedCalls.stream()
                .filter(shouldNotBeLoggedCall ->
                        shouldNotBeLoggedCall.getLineNumber() == loggerCall.getLineNumber()
                )
                .forEach(shouldNotBeLoggedCall -> {
                    String message = loggerCall.getOrigin().getDescription() + " logs in the same line as "
                        + shouldNotBeLoggedCall.getTarget().getDescription() + " is called "
                        + shouldNotBeLoggedCall.getSourceCodeLocation();
                    events.add(SimpleConditionEvent.satisfied(javaClass, message));
                })
        );
    }
});

For your example this rule fails with

Architecture Violation [Priority: MEDIUM] - Rule 'no classes should log return values of @ShouldNotBeLogged methods' was violated (1 times): Method <HelloWorld.logMessage(Person)> logs in the same line as method <Person.getAccountNumber()> is called (HelloWorld.java:9)