kotcrab / callsuper-annotation Goto Github PK

Hi,
is there a reason why Java 8 is required to use this lib?

Fatal error compiling: java.lang.EnumConstantNotPresentException: javax.lang.model.SourceVersion.RELEASE_8

Calling getBody() on abstract method returns null.

I would fix it myself, but there is a decision to make. When there is an abstract method in chain of overriding methods, the upper half the chain is not accessible from bottom. So @CallSuper makes sense for all overriding methods until you hit abstract one. Maybe I would simply issue a warning for any overriding method below the abstract method, or you may want to handle it as compilation error.

If you handle it as compilation error, then @OverrideCallSuper on the abstract method would be good way to suppress the error.

class A {
    void doSomething() {
    }
}

abstract class B extends A {
    @Override
    abstract void doSomething();
}

class C extends B {
    @Override
    void doSomething() {
        super.doSomething(); // you cant call abstract methods
    }
}

If the type parameter in checkSuperTypes is generic it will have erased supertype and thus type.supertype_field == null. Luckily we can still access the non-erased type thru tsym.

public boolean checkSuperTypes (ClassType type) {
    // generic types have the goodies elsewhere
    if (type.hasErasedSupertypes())
        type=(ClassType)type.tsym.type

    if (type.supertype_field != null && type.supertype_field.tsym != null) {
        if (checkScope(type.supertype_field.tsym.members()))
            return true;
        else
            return checkSuperTypes((ClassType) type.supertype_field);
    }

    return false;
}

Note that this problem affect generic types only if there is at least three or more in the inheritance chain. If there are only two generic classes, then only checkScopes is called and its not affected. Only checkSuperTypes is affected. I believe it boils down to that checkScopes is taking members from JCIdent and the checkSuperTypes from ClassType object.

Testing the presence of super call expects the expression to end with semicolon, but thats not the case in Java 7. I am not sure what would other versions of Java return, especially when I know you probably tested this on Java 8.

exprString.endsWith(");")) expects string like super.onSuccess(o);, but I actually got super.onSuccess(o). The lack of semicolon was constant, and there were no cases where it would be present.

Java Annontation Processing does not process anonymous classes at all. Hence their are not part of getElementsAnnotatedWith().

You can't fix this, but could you please list this as known issue on homepage?

Looking at your code I noticed the @CallSuper check is really easy to fool.
Consider this:

import com.kotcrab.annotation.CallSuper;

public class SuperClass {
    @CallSuper
    public void callMe () {
    }

    public void someOtherMethod () {}
}

And then this:

abstract class TestClass extends SuperClass {
    @Override //overriding method must be annotated with @Override
    public void callMe () {
        super.someOtherMethod(); // this is not the right super method but there is no compile error
    }
}

I agree this is scenario won't be common, but it's perfectly acceptable Java. So I'm afraid this check may be a bit too naive: https://github.com/kotcrab/callsuper-annotation/blob/master/src/com/kotcrab/annotation/callsuper/CallSuperProcessor.java#L74

It makes me wonder how Google implemented their version of the @CallSuper annotation. I haven't been able to find source code for their processor, have you?

The javax.annotation.processing.Processor file is missing in all your maven builds. I would prefer triggering your annotation processor via this file instead of configuring my pom.

Great project! But I do have one comment...

I agree that, in general, it is a good idea to annotate any overriding method with @Override. But was it really necessary to make the enforcing of the @CallSuper annotation depend on the overriding method having the @Override annotation?
What if I'm so sloppy that I forget both the @Override annotation AND the super call? Currently I wouldn't not get any error or warning in such cases, which seems less than ideal.

So if possible I'd remove the dependency on there being an @Override annotation. After all you already provide a more explicit mechanism to disable the @CallSuper check, namely the @OverrideCallSuper annotation.

Thanks for taking this into consideration. If you think it is possible but don't have the time I can have a try myself, but I've never played with implementing annotations so it might take me a while to figure it out.