ArchUnit icon indicating copy to clipboard operation
ArchUnit copied to clipboard

JavaMethodCall target owner is sub class instead of super for inherited methods

Open Wolf-64 opened this issue 2 years ago • 5 comments

For my use case and therefore the issue I would like to raise, consider the following basic example:

public abstract class BaseType {
    protected Long id;
    public Long getId() {
        return id;
    }
    public void setId(Long id) {
        this.id = id;
    }
}

public class MyType extends BaseType {
    // ...some other fields...
	
    public void methodToTest() {
        someLocalGetter();
        getId();
    }
}

In this example, MyType.methodToTest() is the subject of my test. Using getMethodCallsFromSelf() on this method will yield all method calls as intended. Now I would like to filter to methods only implemented in MyType itself. Using getTargetOwner() on getId() will return MyType as owning class instead of BaseType, which actually implements this method, so this cannot be used to filter the way I need it.

So my proposal would be to either change the return value of getTargetOwner() to the class in the hierarchy which actually implements the method, or provide another way to get the actual implementing class.

I don't know what the implications are of changing this, or what the reasoning is for how it's currently working, but I hope in your perspective my proposal is feasible and makes sense.

Wolf-64 avatar Sep 01 '22 10:09 Wolf-64

ArchUnit is consistent with the bytecode:

For this example:
class Parent {
    void parentMethod() { }

    static void staticParentMethod() { }
}

class Child extends Parent {
    void childMethod() { }

    static void staticChildMethod() { }

    void call() {
        childMethod();
        parentMethod();
        ((Parent) this).parentMethod();

        staticChildMethod();
        staticParentMethod();
        Parent.staticParentMethod();
    }
}
the bytecode of Child's call() is this:
       0: aload_0
       1: invokevirtual #2                  // Method childMethod:()V
       4: aload_0
       5: invokevirtual #3                  // Method parentMethod:()V
       8: aload_0
       9: invokevirtual #4                  // Method Parent.parentMethod:()V

      12: invokestatic  #5                  // Method staticChildMethod:()V
      15: invokestatic  #6                  // Method staticParentMethod:()V
      18: invokestatic  #7                  // Method Parent.staticParentMethod:()V
      21: return

But even if your call seems to target a subclass method, you can just check whether the target owner actually contains the called method, and recurse to the superclass if it doesn't:

JavaClass getEffectiveTargetOwner(JavaMethodCall methodCall) {
    String[] parameterTypeNames = methodCall.getTarget().getParameterTypes().stream().map(JavaType::getName).toArray(String[]::new);
    JavaClass targetOwner = methodCall.getTargetOwner();
    while (!targetOwner.tryGetMethod(methodCall.getName(), parameterTypeNames).isPresent()) {
        targetOwner = targetOwner.getRawSuperclass().get();  // a called method should be found in the hierarchy!
    }
    return targetOwner;
}

Would you expect to find such a getEffectiveTargetOwner() method on JavaMethodCall?

hankem avatar Sep 01 '22 19:09 hankem

Thanks for the explanation and the example! I am aware that I could solve it this way myself. I was initially just expecting the target owner of method calls to be the actual class the method is implemented in (not knowing the actual bytecode), so my tests in this regard were failing until I checked what getTargetOwner() actually returns. So I would say, at least for the sake of convenience getEffectiveTargetOwner() would be a nice built-in addition.

Wolf-64 avatar Sep 02 '22 07:09 Wolf-64

Note that there already is something like this. You can find more info about the concepts behind this in the javadoc of AccessTarget. As @hankem said, the "target" is derived from the bytecode and also semantically on the type where you call the method on. If you call MyType.getId(), then this is the target, even if getId() is inherited from BaseType. However, what you call getEffectiveTargetOwner() already exists in a way, because there is AccessTarget.resolveMember(). As explained in the javadoc on AccessTarget, this will try to "resolve" the real member that is targeted by the call. You have to take into account however, that this is not always possible. For example, if you import MyType, but you explicitly exclude BaseType from the import. Then ArchUnit only sees from the bytecode that the method MyType.getId() is called, but it can't know from where exactly this is inherited, because the class where it is defined is unknown. In this case you can still do JavaMethodCall.getTarget() fine and you will have the info, but resolveMember() would yield Optional.empty().

If you have any idea how we could improve the docs so that you would have found this by yourself instead of being confused, I would be really happy to hear it! Maybe the docs are in the wrong place?

codecholeric avatar Sep 09 '22 17:09 codecholeric

Hi there,

i ran in the same problem. I want to check that no class implementing an AbstractBaseClass in our projects does access methods of AbstractBaseClass's parent class which is provided by an project-external jar (and yes, inheritance here is itself the problem, but nonetheless ;-))

So i have this rule:

noClasses()
    .orShould().callMethodWhere(target(declaredIn(AbstractBaseClassParent.class)))
    .check(classesUnderTest);

which stays green for calls to AbstractBaseClassParent#method() if the call happens inside a method of any class that itself inherits from AbstractBaseClass as the owner of method() seems to be the calling class itself (same problem as mentioned above).

How could i formulate the predicate to callMethodeWhere(<predicate>) in a way that the effective owner is considered?

Thanks for any help or pointers to the docs!

Argelbargel avatar Oct 20 '22 14:10 Argelbargel

I have a case which might be related:

MyClass.class.isAssignableFrom(someOtherType)

This case:

  • the access target is java.lang.Class.isAssignableFrom(java.lang.Class)
  • the access target resolveMember is JavaMethod{java.lang.Class.isAssignableFrom(java.lang.Class)}

There's no way to know that the effective target is MyClass

Similar scenario when having myObj.getClass(). The target is java.lang.Object.getClass() and we can't know which was the real owner type behind it.

ratoaq2 avatar Mar 18 '23 07:03 ratoaq2