Unable to suppress violation in a field declaration

mttjj commented 1 year ago

Summary

In short, we have a class that is mostly (but not all) generated code that uses an API that we have forbidden in a field declaration. We would like to suppress the violation on just the field but have not been able to do so. Adding a suppression annotation to the class declaration works but has the downside of suppressing everything in the class including the non-generated code. This is less than ideal.

Setup

Given signatures.txt:

java.util.Collections#singleton(java.lang.Object)
java.util.Date

and FooBar.java:

public class FooBar {
    private final Set<String> foos = Collections.singleton("A");

    public Set<String> getBars() {
        Set<String> bars = Collections.singleton("Z");
        return bars;
    }

    public void setDate(Date date) { }
}

The following expected failure is the result of running the maven plugin:

[ERROR] Forbidden method invocation: java.util.Collections#singleton(java.lang.Object)
[ERROR]   in forbiddenapis.test.FooBar (FooBar.java:14)
[ERROR] Forbidden method invocation: java.util.Collections#singleton(java.lang.Object)
[ERROR]   in forbiddenapis.test.FooBar (FooBar.java:18)
[ERROR] Forbidden class/interface use: java.util.Date
[ERROR]   in forbiddenapis.test.FooBar (FooBar.java, method declaration of 'setDate(java.util.Date)')
[ERROR] Scanned 2 class file(s) for forbidden API invocations (in 0.03s), 3 error(s).

The suppression annotation is declared as:

@Retention(RetentionPolicy.CLASS)
@Target({ TYPE, FIELD, METHOD, PARAMETER, CONSTRUCTOR, LOCAL_VARIABLE })
public @interface SuppressForbidden {}

Tests

The following annotations do not suppress any of the three violations:

public class FooBar {
    @SuppressForbidden
    private final Set<String> foos = Collections.singleton("A");

    public Set<String> getBars() {
        @SuppressForbidden
        Set<String> bars = Collections.singleton("Z");
        return bars;
    }

    public void setDate(@SuppressForbidden Date date) { }
}

The following annotations do suppress the second and third violations:

public class FooBar {
    private final Set<String> foos = Collections.singleton("A");

    @SuppressForbidden
    public Set<String> getBars() {
        Set<String> bars = Collections.singleton("Z");
        return bars;
    }

    @SuppressForbidden
    public void setDate(Date date) { }
}

And then, naturally, a single annotation at the class level suppresses all three violations:

@SuppressForbidden
public class FooBar {
    private final Set<String> foos = Collections.singleton("A");

    public Set<String> getBars() {
        Set<String> bars = Collections.singleton("Z");
        return bars;
    }

    public void setDate(Date date) { }
}

Conclusion

I suppose the ultimate question is, are suppression annotations supposed to work on anything other than classes and methods? The maven documentation states "...Those annotations must have at least RetentionPolicy.CLASS. They can be applied to classes, their methods, or fields..." Based on the way that's worded, I can understand if the annotations aren't meant to work on local variables (bars) or method parameters (date) but it seems like it should work for the field (foos).

Can you provide some insight into this functionality? As mentioned at the top, we'd really like to be able to annotate a single field with this so we aren't accidentally suppressing the rest of the class that is not generated code but as it stands, we have to add the annotation to the class declaration for it to work.

Please let me know if you would like any clarification or need more information about our scenario.

uschindler commented 1 year ago

Hi, This unfortunately cannot be fixed.

The retention policy set to at least class or runtime is correct. This ensures that the annotation can be seen after compilation in class files. But this is correct in you case and hasn't anything to do with the problem at all.

Field initialization here is the main problem. Actually in class files there is no way to see the right side of field declarations in field declarations. The suppressions are only be visible for the declaration of the field. The right side is of field (initializer) is hidden by the java compiler behind a invisible method:

for static fields it is called static void clinit(). There is no way to add a suppression to this field. Workaround is to wrap the call into a method.
for instance fields you can add suppression to constructor of class.

In generat the workaround for field initialization is to add a static method that can get the suppression.

Sorry this is a limitation of class files format.

uschindler commented 1 year ago

Actually there might be a way to identify those calls from byte code analysis by heuristics: any putfield opcode in constructor that has a line number different than the others might be a way to detect it. But this is very hard and depends on the internals of compiler and may break.

mttjj commented 1 year ago

Unfortunate but totally understandable. Thankfully, the scenario I described is rare in our codebase and the annotation at the class level won't cause too many problems for us for these particular classes. Thanks for looking into this!

policeman-tools / forbidden-apis