views:

104

answers:

3

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?

+2  A: 

In a nut: you'd like to have a @Deprecated like annotation which would assist the compiler/IDE to warn/error when the method is been called without assigning its result? You can't achieve this without modifying the Java source code and the compiler. The particular method has to be annotated and the compiler has to be aware of them. Without modifying the source and/or compiler, you can at highest create kind of an IDE plugin/setting which recognizes the cases and generates an error/warning accordingly.


Update: you could write a framework/plugin around it which checks the called method and errors accordingly. You would only like to have the annotation available during runtime. You can do this by annotating the annotation using @Retention (RetentionPolicy.RUNTIME). Then, you can use Method#getAnnotation() to determine if this annotation is available. Here's a kickoff example how such a framework could do this job:

package com.example;

import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;

public class Test {

    public static void main(String[] args) throws Exception {
        if (Test.class.getMethod("f", new Class[0]).getAnnotation(Undiscardable.class) != null) {
            System.err.println("You should not discard the return value of f()!");
        } else {
            f();
        }

        System.out.println(f());
    }

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

@Retention(RetentionPolicy.RUNTIME)
@interface Undiscardable {}

Still then, to get the compiler do the job instead, you have to do a bit more work.

BalusC
The source code of the API class would have to be modified, yes, to include these annotations. If this is indeed possible, then I think API designers/library authors would probably gladly do this anyway, since it helps guide the users to proper usage. I have no idea if it is, though. I'm looking at the source code for `java.lang.Override` and I have no idea how this works.
polygenelubricants
The `java.lang` annotations are syntactic sugar. The compiler checks for those annotations. See also [JLS 9.6.1 - Predefined Annotation Types](http://java.sun.com/docs/books/jls/third_edition/html/interfaces.html#9.6.1).
BalusC
@BalusC: Maybe a more "in a nut" question would be: is there a tutorial for writing your own annotation with enforceable compile-time semantics? I think `@Nullable/NotNull` are close cousins of what I want to do, so I probably should look in that direction.
polygenelubricants
You can't do that without writing your own compiler. The `@Nullable` and so on are just "pure" metadata annotations. They are scanned by the framework/API during runtime (thus not during compiletime!). The `java.lang` ones are more than just metadata. They have a special meaning by the compiler.
BalusC
@BalusC: according to this http://www.jetbrains.com/idea/documentation/howto.html "IntelliJ IDEA warns you if these [`@NotNull/Nullable`] contracts are violated." - this seems to indicate that in fact it is processed during compile time, am I wrong?
polygenelubricants
@polygenelubricants annotations may be processed at compile time (javac -processor {procesor.qualifiedName}), run time (via reflection), or both.
emory
@poly: IntelliJ and Eclipse don't use javac. @emory: That's a nice once, I saw the links in Pascal's answer as well.
BalusC
+1  A: 

You do not need to define an annotation. You could define a rule when a method is invoked:

  1. the method has a void return type;
  2. the result of the method is used as the argument for another method invocation; or
  3. the result of the method is assigned to a variable.

You could implement a Processor that enforces this rule or implement a Checkstyle that enforces this rule.

emory
You wouldn't want to do this with _ALL_ methods, though. Probably only select some. Hence the annotation. And what do you mean by Processor? The Annotation Processor Tool? (I guess not since you said no need for annotation?).
polygenelubricants
@polygenelubricants I meant http://download.oracle.com/javase/6/docs/api/javax/annotation/processing/Processor.html. Isn't APT deprecated? I wouldn't want to do this with all methods - only those methods that would trip me up (e.g. String.toUpperCase). But sadly, I have no control of the implementation of these methods and can not apply an annotation. So for me, it is all or nothing.
emory
@emory: Yes, if this functionality is possible through annotation, the responsibility of applying it rests on API publishers/library authors, not the users. I think this is a good enough idea that they would probably do so voluntarily, but I may be wrong.
polygenelubricants
@emory: I have absolutely no idea if APT is deprecated or not. This is an entirely new and exciting adventure for me.
polygenelubricants
@polygenelubricants The [Annotation Processing Tool (`apt`)](http://download.oracle.com/javase/1.5.0/docs/guide/apt/GettingStarted.html) from Java 5 is deprecated. Annotation Processing is definitely not (you just don't need `apt`, `javac` can run them).
Pascal Thivent
@polygenelubricants I was wrong about APT being deprecated. I know that the Ant APT task - http://ant.apache.org/manual/Tasks/apt.html - has some troublesome language "This task requires Java 1.5. It may work on later versions, but this cannot be confirmed until those versions ship. Be advised that the Apt tool does appear to be an unstable part of the JDK framework, so may change radically in future versions. In particular it is likely to be obsolete in JDK 6, which can run annotation processors as part of javac." whether or not apt is deprecated, javac supports annotation processing.
emory
+3  A: 

I'm not sure of the feasibility - especially in a portable way - but have a look at Roman Numerals, in our Java from Adrian Kuhn. He used annotation processing AND Sun's javac private API to adds Roman numeral literals to Java by visiting the source code to do some replacement.

Maybe you could use a similar approach to:

  • find calls to your annotated method in the source code
  • check if the result is assigned (won't be easy IMO)
    • generate a compiler warning if not

And don't miss the following resources from Adrian's post:

You may also like

Reference

Related questions

Pascal Thivent
"check if the result is assigned (won't be easy IMO) " - I was thinking that this can be done by simply checking if the method with `@Undiscardable` return value is grammatically a _ExpressionStatement_ or not (http://java.sun.com/docs/books/jls/third_edition/html/statements.html#14.8). If it is, then raise the warning.
polygenelubricants
@polygenelubricants Don't you actually need to check for [assignment statement](http://java.sun.com/docs/books/jls/third_edition/html/expressions.html#5281)? And what about `foo(f())`?
Pascal Thivent
1. Extend http://download.oracle.com/javase/6/docs/jdk/api/javac/tree/com/sun/source/util/TreePathScanner.html2. Override visitAssignment, visitMethodInvocation, and maybe some others
emory
@emory Ahhh, nice, must dig that.
Pascal Thivent
@emory, @Pascal: I was thinking `@Override visitExpressionStatement`, check if it's a method invocation to an `@Undiscardable`. If so, raise warning.
polygenelubricants
@polygenelubricants I see. That should work. Perhaps you could put something in visitMethodInvocation to guard against the remote possibility that one of them is not in an ExpressionStatement.
emory