Using annotation to ensure that value returned by method is not discarded

Viewed 11799

String in Java is immutable. The following snippet is, broadly speaking, "wrong".

String s = "hello world!";

s.toUpperCase(); // "wrong"!!

System.out.println(s); // still "hello world!"!!!

Despite this being "wrong", the code compiles and runs, perhaps to the confusion of many beginners, who must either be told what the mistake is, or to find out for themselves by consulting the documentation.

Reading the documentation is an essential part of understanding an API, but I'm wondering if this can be supplemented by additional compile-time checks. In particular, I'm wondering if perhaps Java's annotation framework can be used to enforce that the value returned by certain methods are not ignored. API designers/library authors would then use this annotation in their methods to document which return values should not be ignored.

Once the API is supplemented with this annotation (or perhaps another mechanism), then whenever a user writes a code such as above, it would not compile (or do so with a stern warning).

So can this be done, and how would you go about doing something like this?


Appendix: The Motivation

It seems clear that in the general case, Java should allow return values of methods to be ignored. The returned values of methods like List.add (always true), System.setProperty (previous value), can probably be safely ignored most of the times.

However, there are also many methods whose return values should NOT be ignored. Doing so is almost always a programmer error, or otherwise not a proper usage of the API. These includes things like:

  • Methods on immutable types (e.g. String, BigInteger, etc) that return the result of operations instead of mutating the instance it is invoked on.
  • Methods whose return value is a critical part of its behavior and should not be ignored, but people sometimes do anyway (e.g. InputStream.read(byte[]) returns the number of bytes read, which should NOT be assumed to be the entire length of the array)

Currently we can write codes that ignores these return values and have them compile and run without warning. Static analysis checkers/bug finders/style enforcers/etc can almost certainly flag these as possible code smells, but it would seem to be appropriate/ideal if this can be enforced by the API itself, perhaps through annotations.

It is almost impossible for a class to ensure that it is always used "properly", but there are things it can do to help guide clients to proper usage (see: Effective Java 2nd Edition, Item 58: Use checked exceptions for recoverable conditions and runtime exceptions for programming errors and Item 62: Document all exceptions thrown by each method). Having an annotation that would enforce clients to not ignore return values of certain methods, and having it enforced by the compiler at compile-time either in the form of errors or warnings, would seem to be in line with this idea.


Appendix 2: Snippet

The following is a preliminary attempt that succinctly illustrates what I want to achieve:

@interface Undiscardable { }
//attachable to methods to indicate that its
//return value must not be discarded

public class UndiscardableTest {
     public static @Undiscardable int f() {
             return 42;
     }

     public static void main(String[] args) {
             f(); // what do I have to do so this generates
                  // compilation warning/error?

             System.out.println(f()); // this one would be fine!
     }
}

The above code compiles and runs fine (as seen on ideone.com). How can I make it not so? How can I assign the semantics I want to @Undiscardable?

7 Answers

You could also check out jsr305. It defines a @CheckReturnValue annotation:

import java.lang.annotation.Documented;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

import javax.annotation.meta.When;

@Documented
@Target( { ElementType.METHOD, ElementType.CONSTRUCTOR, ElementType.TYPE,
        ElementType.PACKAGE })
@Retention(RetentionPolicy.RUNTIME)
public @interface CheckReturnValue {
    When when() default When.ALWAYS;
}

It's compatible with findbugs and generates a warning when someone forgets to handle the return value.

Guavas Splitter uses it: http://code.google.com/p/guava-libraries/source/browse/guava/src/com/google/common/base/Splitter.java

I must say that I love annotations that can guide static code analysis.

On Android you can use @CheckResult to show a warning if return value isn't used.

public class ImmutableObject {

    public final int value;

    public ImmutableObject(int value) {
        this.value = value;
    }

    @CheckResult
    public ImmutableObject addOne() {
        return new ImmutableObject(value + 1);
    }
}

This will issue a warning:

ImmutableObject obj = new ImmutableObj();
obj.addOne();  // Warning here
ImmutableObject obj2 = obj.addOne();  // No warning

If using RxJava, you can also use @CheckReturnValue.

You have a problem and the problem is that people may mistakenly forget to use the returns of methods. By using annotations you are telling the library writer that they must be responsible for reminding their callers to not throw away the results of certain methods.

While it seems like a good idea, I don't think it is. Do we want to clutter up code with notices to users about their poor practice? There are plenty of products that look at code and tell you when you are doing something wrong (or undesirable) like Lint, Sonar and even JavaDocs to a lesser extent.

What if you disagree with what the library writer has said, are we now expected to use @SuppressWarnings("return-discarded").

While this might be helpful as a learning aid, my point is more to do with the separation of concerns than helping novice programmers. The code (and annotations) in the class should be related to the function of the class and not setting out the policy of when and how to use it's methods.

Related