roterdam / checker-framework

Automatically exported from code.google.com/p/checker-framework
Other
0 stars 0 forks source link

More JDK Nullable annotations #385

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
-Xlint helped me find Nullable annotations I had previously missed on return 
types:

package java.lang;

public class ClassLoader {
    public @Nullable InputStream getResourceAsStream(String name);
}

package java.lang.management;

public interface ThreadMXBean {
    public @Nullable ThreadInfo getThreadInfo(long id, int maxDepth);
}

package java.lang.reflect;

public class Method {
    public @Nullable Object invoke(@Nullable Object obj, @Nullable Object... args);
}

public class InvocationTargetException {
    public @Nullable Throwable getCause();
}

package java.nio.channels;

public class FileChannel {
    public @Nullable FileLock tryLock();
}

package java.util.concurrent;

public class ConcurrentMap<K, V> {
    @Nullable V putIfAbsent(K key, V value);
}

package java.util.jar;

public class Attributes {
    public @Nullable String getValue(String name);
}

public class JarInputStream {
    public @Nullable Manifest getManifest();
    public @Nullable JarEntry getNextJarEntry();
}

package javax.management.openmbean;

public class CompositeType {
    public @Nullable OpenType<?> getType(String itemName);
}

package java.lang.instrument;

public interface Instrumentation {
    Class[] getInitiatedClasses(@Nullable ClassLoader loader);
}

Original issue reported on code.google.com by trask.st...@gmail.com on 24 Dec 2014 at 3:43

GoogleCodeExporter commented 9 years ago
Thanks for the report about missing library annotations.

> package java.lang;
> 
> public class ClassLoader {
>     public @Nullable InputStream getResourceAsStream(String name);
> }

This annotation should already be present in the annotated JDK -- see line 1309 
of file checker-framework/checker/jdk/nullness/src/java/lang/ClassLoader.java . 
 Is it not present during type-checking?  Can you provide a test case?

> package java.lang.reflect;
> 
> public class Method {
>     public @Nullable Object invoke(@Nullable Object obj, @Nullable
>     Object... args);
> }

This seems similar to the previous case: the annotation is already present.  
Please see line 110 of 
checker-framework/checker/jdk/nullness/src/java/lang/reflect/Method.java

> package java.lang.reflect;
>
> public class InvocationTargetException {
>     public @Nullable Throwable getCause();
> }

I added this annotation and 4 others that also belong in the 
InvocationTargetException class.

For all of the remaining annotations (listed below), the class is not present 
in the annotated JDK and needs to be added.  Our policy is to only add 
fully-annotated classes to the JDK, to avoid confusion that could result from 
some methods in a class being correctly annotated and other methods not being 
annotated.

If you can provide a fully-annotated class (this requires examining all the 
methods in the class), that would be great.  Otherwise, I will work on this 
when I get a chance.

> package java.nio.channels;
> 
> public class FileChannel {
>     public @Nullable FileLock tryLock();
> }
>
> package java.lang.management;
> 
> public interface ThreadMXBean {
>     public @Nullable ThreadInfo getThreadInfo(long id, int maxDepth);
> }
>
> 
> package java.util.concurrent;
> 
> public class ConcurrentMap<K, V> {
>     @Nullable V putIfAbsent(K key, V value);
> }
> 
> package java.util.jar;
> 
> public class Attributes {
>     public @Nullable String getValue(String name);
> }
> 
> public class JarInputStream {
>     public @Nullable Manifest getManifest();
>     public @Nullable JarEntry getNextJarEntry();
> }
> 
> package javax.management.openmbean;
> 
> public class CompositeType {
>     public @Nullable OpenType<?> getType(String itemName);
> }
> 
> package java.lang.instrument;
> 
> public interface Instrumentation {
>     Class[] getInitiatedClasses(@Nullable ClassLoader loader);
> }

Original comment by michael.ernst@gmail.com on 24 Dec 2014 at 7:00

GoogleCodeExporter commented 9 years ago
I don't see the @Nullable annotation on the return type of Method.invoke():

    https://code.google.com/p/checker-framework/source/browse/checker/jdk/nullness/src/java/lang/reflect/Method.java#110

It looks like the issue I had with ClassLoader.getResourceAsStream() is that I 
was using URLClassLoader, e.g. this passes when it should fail:

    void test(URLClassLoader loader) {
        loader.getResourceAsStream("test").toString();
    }

but this seems a different issue from library annotations?  I was mistaken that 
the astub made my Alint warning go away in this case.

The policy of all-or-nothing in a given class file makes sense.  Thanks.

Original comment by trask.st...@gmail.com on 24 Dec 2014 at 9:20

GoogleCodeExporter commented 9 years ago
You are correct about Method.invoke.  I have corrected its annotations.  Thanks 
(and sorry for my mistake).

URLClassLoader does not currently appear in the annotated JDK.  Would adding a 
library annotation on URLClassLoader.getResourceAsStream() correct the problem?

Original comment by michael.ernst@gmail.com on 24 Dec 2014 at 4:50

GoogleCodeExporter commented 9 years ago
No problem.

I'm unsure how to correct the URLClassLoader issue.  I tried adding a stub file:

    package java.net;

    public class URLClassLoader {
        @Nullable InputStream getResourceAsStream(String name);
    }

But that didn't help.  I'm not clear if/how stub file is different from 
annotating URLClassLoader in the annotated jdk.

Repro here:  https://github.com/trask/checker-issue-385

Thanks.

Original comment by trask.st...@gmail.com on 24 Dec 2014 at 11:06

GoogleCodeExporter commented 9 years ago
Trask, It looks like you are missing and import in the stub file. After adding 

import org.checkerframework.checker.nullness.qual.*;

the build failed as expected.

Original comment by mcart...@cs.washington.edu on 6 Jan 2015 at 10:10