From 937c627b162fe122a07a610d11837d0b4ccbcb8e Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Tue, 21 Jan 2014 14:21:49 -0600 Subject: [PATCH 01/11] Move NotNull and Nullable annotations to their own artifact --- build.xml | 5 +- pom.xml | 1 + runtime/Java/pom.xml | 7 +++ runtime/JavaAnnotations/nb-configuration.xml | 39 ++++++++++++++ runtime/JavaAnnotations/pom.xml | 54 +++++++++++++++++++ .../org/antlr/v4/runtime/misc/NotNull.java | 0 .../org/antlr/v4/runtime/misc/Nullable.java | 0 7 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 runtime/JavaAnnotations/nb-configuration.xml create mode 100644 runtime/JavaAnnotations/pom.xml rename runtime/{Java => JavaAnnotations}/src/org/antlr/v4/runtime/misc/NotNull.java (100%) rename runtime/{Java => JavaAnnotations}/src/org/antlr/v4/runtime/misc/Nullable.java (100%) diff --git a/build.xml b/build.xml index bf73b941d..adbdff51b 100644 --- a/build.xml +++ b/build.xml @@ -209,7 +209,7 @@ - + @@ -246,6 +246,9 @@ + + + diff --git a/pom.xml b/pom.xml index 81f72be25..c257c0534 100644 --- a/pom.xml +++ b/pom.xml @@ -61,6 +61,7 @@ runtime/Java + runtime/JavaAnnotations tool antlr4-maven-plugin diff --git a/runtime/Java/pom.xml b/runtime/Java/pom.xml index e23e2b9fd..a2cebaf5a 100644 --- a/runtime/Java/pom.xml +++ b/runtime/Java/pom.xml @@ -27,6 +27,13 @@ 1.0.1 compile + + + org.antlr + antlr4-annotations + ${project.version} + compile + diff --git a/runtime/JavaAnnotations/nb-configuration.xml b/runtime/JavaAnnotations/nb-configuration.xml new file mode 100644 index 000000000..8ecf24501 --- /dev/null +++ b/runtime/JavaAnnotations/nb-configuration.xml @@ -0,0 +1,39 @@ + + + + + rule's + validator + + + + project + 4 + 4 + 4 + true + 80 + none + false + 4 + 4 + 4 + false + 80 + none + 4 + *;javax;java + test + false + + diff --git a/runtime/JavaAnnotations/pom.xml b/runtime/JavaAnnotations/pom.xml new file mode 100644 index 000000000..f440f7172 --- /dev/null +++ b/runtime/JavaAnnotations/pom.xml @@ -0,0 +1,54 @@ + + + 4.0.0 + + + org.antlr + antlr4-master + 4.2-SNAPSHOT + ../.. + + + antlr4-annotations + + ANTLR 4 Runtime Annotations + A set of annotations used within the ANTLR 4 Runtime + + + + sonatype-oss-release + + + + us.bryon + graphviz-maven-plugin + 1.0 + + + + dot + + + ${dot.path} + ${project.build.directory}/apidocs + svg + + + + + + + + + + + src + + + resources + + + + + diff --git a/runtime/Java/src/org/antlr/v4/runtime/misc/NotNull.java b/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NotNull.java similarity index 100% rename from runtime/Java/src/org/antlr/v4/runtime/misc/NotNull.java rename to runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NotNull.java diff --git a/runtime/Java/src/org/antlr/v4/runtime/misc/Nullable.java b/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/Nullable.java similarity index 100% rename from runtime/Java/src/org/antlr/v4/runtime/misc/Nullable.java rename to runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/Nullable.java From f8dbe1b68f583e9d33de9ced1271f49440434546 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Tue, 21 Jan 2014 14:32:11 -0600 Subject: [PATCH 02/11] Initial annotation processor for NotNull and Nullable annotations --- build.xml | 3 + runtime/JavaAnnotations/pom.xml | 12 +++ .../javax.annotation.processing.Processor | 1 + .../v4/runtime/misc/NullUsageProcessor.java | 91 +++++++++++++++++++ 4 files changed, 107 insertions(+) create mode 100644 runtime/JavaAnnotations/resources/META-INF/services/javax.annotation.processing.Processor create mode 100644 runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java diff --git a/build.xml b/build.xml index adbdff51b..4ea533888 100644 --- a/build.xml +++ b/build.xml @@ -246,6 +246,9 @@ + + + diff --git a/runtime/JavaAnnotations/pom.xml b/runtime/JavaAnnotations/pom.xml index f440f7172..26ef7fa78 100644 --- a/runtime/JavaAnnotations/pom.xml +++ b/runtime/JavaAnnotations/pom.xml @@ -49,6 +49,18 @@ resources + + + + org.apache.maven.plugins + maven-compiler-plugin + + + -proc:none + + + + diff --git a/runtime/JavaAnnotations/resources/META-INF/services/javax.annotation.processing.Processor b/runtime/JavaAnnotations/resources/META-INF/services/javax.annotation.processing.Processor new file mode 100644 index 000000000..1a8dc1b9a --- /dev/null +++ b/runtime/JavaAnnotations/resources/META-INF/services/javax.annotation.processing.Processor @@ -0,0 +1 @@ +org.antlr.v4.runtime.misc.NullUsageProcessor \ No newline at end of file diff --git a/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java b/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java new file mode 100644 index 000000000..00057a5fa --- /dev/null +++ b/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java @@ -0,0 +1,91 @@ +/* + * [The "BSD license"] + * Copyright (c) 2013 Terence Parr + * Copyright (c) 2013 Sam Harwell + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * 3. The name of the author may not be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package org.antlr.v4.runtime.misc; + +import javax.annotation.processing.AbstractProcessor; +import javax.annotation.processing.RoundEnvironment; +import javax.annotation.processing.SupportedAnnotationTypes; +import javax.annotation.processing.SupportedSourceVersion; +import javax.lang.model.SourceVersion; +import javax.lang.model.element.TypeElement; +import javax.tools.Diagnostic; + +import java.util.Set; + +/** + * A compile-time validator for correct usage of the {@link NotNull} and + * {@link Nullable} annotations. + * + *

The validation process checks the following items.

+ * + * @author Sam Harwell + */ +@SupportedAnnotationTypes({NullUsageProcessor.NotNullClassName, NullUsageProcessor.NullableClassName}) +@SupportedSourceVersion(SourceVersion.RELEASE_6) +public class NullUsageProcessor extends AbstractProcessor { + public static final String NotNullClassName = "org.antlr.v4.runtime.misc.NotNull"; + public static final String NullableClassName = "org.antlr.v4.runtime.misc.Nullable"; + + public NullUsageProcessor() { + } + + @Override + public boolean process(Set annotations, RoundEnvironment roundEnv) { + if (!checkClassNameConstants()) { + return true; + } + + return true; + } + + private boolean checkClassNameConstants() { + boolean success = checkClassNameConstant(NotNullClassName, NotNull.class); + success &= checkClassNameConstant(NullableClassName, Nullable.class); + return success; + } + + private boolean checkClassNameConstant(String className, Class clazz) { + if (className == null) { + throw new NullPointerException("className"); + } + + if (clazz == null) { + throw new NullPointerException("clazz"); + } + + if (!className.equals(clazz.getCanonicalName())) { + processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, String.format("Unable to process null usage annotations due to class name mismatch: %s != %s", className, clazz.getCanonicalName())); + return false; + } + + return true; + } +} From e472cc4e2318e868e33d670cd94bdbb3a45507b2 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Tue, 21 Jan 2014 14:33:38 -0600 Subject: [PATCH 03/11] Error if an element is annotated with both NotNull and Nullable --- .../v4/runtime/misc/NullUsageProcessor.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java b/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java index 00057a5fa..4c06aeff2 100644 --- a/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java +++ b/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java @@ -35,9 +35,11 @@ import javax.annotation.processing.RoundEnvironment; import javax.annotation.processing.SupportedAnnotationTypes; import javax.annotation.processing.SupportedSourceVersion; import javax.lang.model.SourceVersion; +import javax.lang.model.element.Element; import javax.lang.model.element.TypeElement; import javax.tools.Diagnostic; +import java.util.HashSet; import java.util.Set; /** @@ -46,6 +48,10 @@ import java.util.Set; * *

The validation process checks the following items.

* + *
    + *
  • Error: an element is annotated with both {@link NotNull} and {@link Nullable}.
    • + *
+ * * @author Sam Harwell */ @SupportedAnnotationTypes({NullUsageProcessor.NotNullClassName, NullUsageProcessor.NullableClassName}) @@ -63,6 +69,16 @@ public class NullUsageProcessor extends AbstractProcessor { return true; } + Set notNullElements = roundEnv.getElementsAnnotatedWith(processingEnv.getElementUtils().getTypeElement(NotNullClassName)); + Set nullableElements = roundEnv.getElementsAnnotatedWith(processingEnv.getElementUtils().getTypeElement(NullableClassName)); + + Set intersection = new HashSet(notNullElements); + intersection.retainAll(nullableElements); + for (Element element : intersection) { + String error = String.format("%s cannot be annotated with both NotNull and Nullable", element.getKind().toString().toLowerCase()); + processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, error, element); + } + return true; } From 2647856226c61f378e8b760ec2510434c05d44f7 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Tue, 21 Jan 2014 14:34:38 -0600 Subject: [PATCH 04/11] Error if a void method is annotated with NotNull or Nullable --- .../v4/runtime/misc/NullUsageProcessor.java | 31 +++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java b/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java index 4c06aeff2..f5b9f1716 100644 --- a/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java +++ b/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java @@ -36,7 +36,12 @@ import javax.annotation.processing.SupportedAnnotationTypes; import javax.annotation.processing.SupportedSourceVersion; import javax.lang.model.SourceVersion; import javax.lang.model.element.Element; +import javax.lang.model.element.ElementKind; +import javax.lang.model.element.ExecutableElement; import javax.lang.model.element.TypeElement; +import javax.lang.model.type.NoType; +import javax.lang.model.type.TypeKind; +import javax.lang.model.type.TypeMirror; import javax.tools.Diagnostic; import java.util.HashSet; @@ -50,6 +55,7 @@ import java.util.Set; * *
    *
  • Error: an element is annotated with both {@link NotNull} and {@link Nullable}.
    • + *
  • Error: an method which returns {@code void} is annotated with {@link NotNull} or {@link Nullable}.
    • *
* * @author Sam Harwell @@ -69,8 +75,10 @@ public class NullUsageProcessor extends AbstractProcessor { return true; } - Set notNullElements = roundEnv.getElementsAnnotatedWith(processingEnv.getElementUtils().getTypeElement(NotNullClassName)); - Set nullableElements = roundEnv.getElementsAnnotatedWith(processingEnv.getElementUtils().getTypeElement(NullableClassName)); + TypeElement notNullType = processingEnv.getElementUtils().getTypeElement(NotNullClassName); + TypeElement nullableType = processingEnv.getElementUtils().getTypeElement(NullableClassName); + Set notNullElements = roundEnv.getElementsAnnotatedWith(notNullType); + Set nullableElements = roundEnv.getElementsAnnotatedWith(nullableType); Set intersection = new HashSet(notNullElements); intersection.retainAll(nullableElements); @@ -79,6 +87,9 @@ public class NullUsageProcessor extends AbstractProcessor { processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, error, element); } + checkVoidMethodAnnotations(notNullElements, notNullType); + checkVoidMethodAnnotations(nullableElements, nullableType); + return true; } @@ -104,4 +115,20 @@ public class NullUsageProcessor extends AbstractProcessor { return true; } + + private void checkVoidMethodAnnotations(Set elements, TypeElement annotationType) { + for (Element element : elements) { + if (element.getKind() != ElementKind.METHOD) { + continue; + } + + ExecutableElement executableElement = (ExecutableElement)element; + TypeMirror returnType = executableElement.getReturnType(); + if (returnType instanceof NoType && returnType.getKind() == TypeKind.VOID) { + // TODO: report the error on the annotation usage instead of the method + String error = String.format("void method cannot be annotated with %s", annotationType.getSimpleName()); + processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, error, element); + } + } + } } From e253954bcf5125025fd8c7e17334ca5656e1b2a1 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Tue, 21 Jan 2014 14:35:27 -0600 Subject: [PATCH 05/11] Check for NotNull and Nullable annotations on primitive types --- .../v4/runtime/misc/NullUsageProcessor.java | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java b/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java index f5b9f1716..423c2c525 100644 --- a/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java +++ b/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java @@ -39,7 +39,9 @@ import javax.lang.model.element.Element; import javax.lang.model.element.ElementKind; import javax.lang.model.element.ExecutableElement; import javax.lang.model.element.TypeElement; +import javax.lang.model.element.VariableElement; import javax.lang.model.type.NoType; +import javax.lang.model.type.PrimitiveType; import javax.lang.model.type.TypeKind; import javax.lang.model.type.TypeMirror; import javax.tools.Diagnostic; @@ -56,6 +58,8 @@ import java.util.Set; *
    *
  • Error: an element is annotated with both {@link NotNull} and {@link Nullable}.
    • *
  • Error: an method which returns {@code void} is annotated with {@link NotNull} or {@link Nullable}.
    • + *
  • Error: an element with a primitive type is annotated with {@link Nullable}.
    • + *
  • Warning: an element with a primitive type is annotated with {@link NotNull}.
    • *
* * @author Sam Harwell @@ -90,6 +94,9 @@ public class NullUsageProcessor extends AbstractProcessor { checkVoidMethodAnnotations(notNullElements, notNullType); checkVoidMethodAnnotations(nullableElements, nullableType); + checkPrimitiveTypeAnnotations(nullableElements, Diagnostic.Kind.ERROR, nullableType); + checkPrimitiveTypeAnnotations(notNullElements, Diagnostic.Kind.WARNING, notNullType); + return true; } @@ -131,4 +138,33 @@ public class NullUsageProcessor extends AbstractProcessor { } } } + + private void checkPrimitiveTypeAnnotations(Set elements, Diagnostic.Kind kind, TypeElement annotationType) { + for (Element element : elements) { + TypeMirror typeToCheck; + switch (element.getKind()) { + case FIELD: + case PARAMETER: + case LOCAL_VARIABLE: + // checking variable type + VariableElement variableElement = (VariableElement)element; + typeToCheck = variableElement.asType(); + break; + + case METHOD: + // checking return type + ExecutableElement executableElement = (ExecutableElement)element; + typeToCheck = executableElement.getReturnType(); + break; + + default: + continue; + } + + if (typeToCheck instanceof PrimitiveType && typeToCheck.getKind().isPrimitive()) { + String error = String.format("%s with a primitive type %s be annotated with %s", element.getKind().toString().replace('_', ' ').toLowerCase(), kind == Diagnostic.Kind.ERROR ? "cannot" : "should not", annotationType.getSimpleName()); + processingEnv.getMessager().printMessage(kind, error, element); + } + } + } } From c1125fe474f8be10d92b2a2fd6083fd92a4fc870 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Tue, 21 Jan 2014 14:36:25 -0600 Subject: [PATCH 06/11] Add check for erroneous overriding NotNull and Nullable annotations --- .../v4/runtime/misc/NullUsageProcessor.java | 128 +++++++++++++++++- 1 file changed, 126 insertions(+), 2 deletions(-) diff --git a/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java b/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java index 423c2c525..d024c23f4 100644 --- a/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java +++ b/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java @@ -35,6 +35,7 @@ import javax.annotation.processing.RoundEnvironment; import javax.annotation.processing.SupportedAnnotationTypes; import javax.annotation.processing.SupportedSourceVersion; import javax.lang.model.SourceVersion; +import javax.lang.model.element.AnnotationMirror; import javax.lang.model.element.Element; import javax.lang.model.element.ElementKind; import javax.lang.model.element.ExecutableElement; @@ -46,7 +47,13 @@ import javax.lang.model.type.TypeKind; import javax.lang.model.type.TypeMirror; import javax.tools.Diagnostic; +import java.util.ArrayDeque; +import java.util.ArrayList; +import java.util.Deque; +import java.util.HashMap; import java.util.HashSet; +import java.util.List; +import java.util.Map; import java.util.Set; /** @@ -59,6 +66,8 @@ import java.util.Set; *
  • Error: an element is annotated with both {@link NotNull} and {@link Nullable}.
    • *
  • Error: an method which returns {@code void} is annotated with {@link NotNull} or {@link Nullable}.
    • *
  • Error: an element with a primitive type is annotated with {@link Nullable}.
    • + *
  • Error: a parameter is annotated with {@link NotNull}, but the method overrides or implements a method where the parameter is annotated {@link Nullable}.
    • + *
  • Error: a method is annotated with {@link Nullable}, but the method overrides or implements a method that is annotated with {@link NotNull}.
    • *
  • Warning: an element with a primitive type is annotated with {@link NotNull}.
    • * * @@ -70,6 +79,9 @@ public class NullUsageProcessor extends AbstractProcessor { public static final String NotNullClassName = "org.antlr.v4.runtime.misc.NotNull"; public static final String NullableClassName = "org.antlr.v4.runtime.misc.Nullable"; + private TypeElement notNullType; + private TypeElement nullableType; + public NullUsageProcessor() { } @@ -79,8 +91,8 @@ public class NullUsageProcessor extends AbstractProcessor { return true; } - TypeElement notNullType = processingEnv.getElementUtils().getTypeElement(NotNullClassName); - TypeElement nullableType = processingEnv.getElementUtils().getTypeElement(NullableClassName); + notNullType = processingEnv.getElementUtils().getTypeElement(NotNullClassName); + nullableType = processingEnv.getElementUtils().getTypeElement(NullableClassName); Set notNullElements = roundEnv.getElementsAnnotatedWith(notNullType); Set nullableElements = roundEnv.getElementsAnnotatedWith(nullableType); @@ -97,6 +109,18 @@ public class NullUsageProcessor extends AbstractProcessor { checkPrimitiveTypeAnnotations(nullableElements, Diagnostic.Kind.ERROR, nullableType); checkPrimitiveTypeAnnotations(notNullElements, Diagnostic.Kind.WARNING, notNullType); + // method name -> method -> annotated elements of method + Map>> namedMethodMap = + new HashMap>>(); + addElementsToNamedMethodMap(notNullElements, namedMethodMap); + addElementsToNamedMethodMap(nullableElements, namedMethodMap); + + for (Map.Entry>> entry : namedMethodMap.entrySet()) { + for (Map.Entry> subentry : entry.getValue().entrySet()) { + checkOverriddenMethods(subentry.getKey()); + } + } + return true; } @@ -167,4 +191,104 @@ public class NullUsageProcessor extends AbstractProcessor { } } } + + private void addElementsToNamedMethodMap(Set elements, Map>> namedMethodMap) { + for (Element element : elements) { + ExecutableElement method; + switch (element.getKind()) { + case PARAMETER: + method = (ExecutableElement)element.getEnclosingElement(); + assert method.getKind() == ElementKind.METHOD; + break; + + case METHOD: + method = (ExecutableElement)element; + break; + + default: + continue; + } + + Map> annotatedMethodWithName = + namedMethodMap.get(method.getSimpleName().toString()); + if (annotatedMethodWithName == null) { + annotatedMethodWithName = new HashMap>(); + namedMethodMap.put(method.getSimpleName().toString(), annotatedMethodWithName); + } + + List annotatedElementsOfMethod = annotatedMethodWithName.get(method); + if (annotatedElementsOfMethod == null) { + annotatedElementsOfMethod = new ArrayList(); + annotatedMethodWithName.put(method, annotatedElementsOfMethod); + } + + annotatedElementsOfMethod.add(element); + } + } + + private void checkOverriddenMethods(ExecutableElement method) { + TypeElement declaringType = (TypeElement)method.getEnclosingElement(); + typeLoop: + for (TypeMirror supertypeMirror : getAllSupertypes(processingEnv.getTypeUtils().getDeclaredType(declaringType))) { + for (Element element : ((TypeElement)processingEnv.getTypeUtils().asElement(supertypeMirror)).getEnclosedElements()) { + if (element instanceof ExecutableElement) { + if (processingEnv.getElementUtils().overrides(method, (ExecutableElement)element, declaringType)) { + checkOverriddenMethod(method, (ExecutableElement)element); + continue typeLoop; + } + } + } + } + } + + private List getAllSupertypes(TypeMirror type) { + Set supertypes = new HashSet(); + Deque worklist = new ArrayDeque(); + worklist.add(type); + while (!worklist.isEmpty()) { + List next = processingEnv.getTypeUtils().directSupertypes(worklist.poll()); + if (supertypes.addAll(next)) { + worklist.addAll(next); + } + } + + return new ArrayList(supertypes); + } + + private void checkOverriddenMethod(ExecutableElement overrider, ExecutableElement overridden) { + // check method annotation + if (isNullable(overrider) && isNotNull(overridden)) { + String error = String.format("method annotated with %s cannot override or implement a method annotated with %s", nullableType.getSimpleName(), notNullType.getSimpleName()); + processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, error, overrider); + } + + List overriderParameters = overrider.getParameters(); + List overriddenParameters = overridden.getParameters(); + for (int i = 0; i < overriderParameters.size(); i++) { + if (isNotNull(overriderParameters.get(i)) && isNullable(overriddenParameters.get(i))) { + String error = String.format("parameter annotated with %s cannot override or implement a parameter annotated with %s", notNullType.getSimpleName(), nullableType.getSimpleName()); + processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, error, overrider); + } + } + } + + private boolean isNotNull(Element element) { + for (AnnotationMirror annotationMirror : element.getAnnotationMirrors()) { + if (annotationMirror.getAnnotationType().asElement() == notNullType) { + return true; + } + } + + return false; + } + + private boolean isNullable(Element element) { + for (AnnotationMirror annotationMirror : element.getAnnotationMirrors()) { + if (annotationMirror.getAnnotationType().asElement() == nullableType) { + return true; + } + } + + return false; + } } From 37f83c653738e32d923258fe0ce862370993ef6a Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Tue, 21 Jan 2014 14:37:19 -0600 Subject: [PATCH 07/11] Add warning for potentially incorrect NotNull or Nullable annotations on derived methods and parameters --- .../v4/runtime/misc/NullUsageProcessor.java | 38 ++++++++++++++----- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java b/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java index d024c23f4..331926fee 100644 --- a/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java +++ b/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java @@ -69,6 +69,8 @@ import java.util.Set; *
  • Error: a parameter is annotated with {@link NotNull}, but the method overrides or implements a method where the parameter is annotated {@link Nullable}.
    • *
  • Error: a method is annotated with {@link Nullable}, but the method overrides or implements a method that is annotated with {@link NotNull}.
    • *
  • Warning: an element with a primitive type is annotated with {@link NotNull}.
    • + *
  • Warning: a parameter is annotated with {@link NotNull}, but the method overrides or implements a method where the parameter is not annotated.
    • + *
  • Warning: a method is annotated with {@link Nullable}, but the method overrides or implements a method that is not annotated.
    • * * * @author Sam Harwell @@ -261,6 +263,10 @@ public class NullUsageProcessor extends AbstractProcessor { String error = String.format("method annotated with %s cannot override or implement a method annotated with %s", nullableType.getSimpleName(), notNullType.getSimpleName()); processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, error, overrider); } + else if (isNullable(overrider) && !(isNullable(overridden) || isNotNull(overridden))) { + String error = String.format("method annotated with %s overrides a method that is not annotated", nullableType.getSimpleName()); + processingEnv.getMessager().printMessage(Diagnostic.Kind.WARNING, error, overrider, getNullableAnnotationMirror(overrider)); + } List overriderParameters = overrider.getParameters(); List overriddenParameters = overridden.getParameters(); @@ -269,26 +275,38 @@ public class NullUsageProcessor extends AbstractProcessor { String error = String.format("parameter annotated with %s cannot override or implement a parameter annotated with %s", notNullType.getSimpleName(), nullableType.getSimpleName()); processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, error, overrider); } + else if (isNotNull(overriderParameters.get(i)) && !(isNullable(overriddenParameters.get(i)) || isNotNull(overriddenParameters.get(i)))) { + String error = String.format("parameter %s annotated with %s overrides a parameter that is not annotated", overriderParameters.get(i).getSimpleName(), notNullType.getSimpleName()); + processingEnv.getMessager().printMessage(Diagnostic.Kind.WARNING, error, overriderParameters.get(i), getNotNullAnnotationMirror(overriderParameters.get(i))); + } } } private boolean isNotNull(Element element) { - for (AnnotationMirror annotationMirror : element.getAnnotationMirrors()) { - if (annotationMirror.getAnnotationType().asElement() == notNullType) { - return true; - } - } - - return false; + return getNotNullAnnotationMirror(element) != null; } private boolean isNullable(Element element) { + return getNullableAnnotationMirror(element) != null; + } + + private AnnotationMirror getNotNullAnnotationMirror(Element element) { for (AnnotationMirror annotationMirror : element.getAnnotationMirrors()) { - if (annotationMirror.getAnnotationType().asElement() == nullableType) { - return true; + if (annotationMirror.getAnnotationType().asElement() == notNullType) { + return annotationMirror; } } - return false; + return null; + } + + private AnnotationMirror getNullableAnnotationMirror(Element element) { + for (AnnotationMirror annotationMirror : element.getAnnotationMirrors()) { + if (annotationMirror.getAnnotationType().asElement() == nullableType) { + return annotationMirror; + } + } + + return null; } } From 4bc593b20b2be50565b7f06ca17da79c8be2d0d6 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Tue, 21 Jan 2014 14:37:59 -0600 Subject: [PATCH 08/11] Improved error reporting --- .../v4/runtime/misc/NullUsageProcessor.java | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java b/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java index 331926fee..e81206c1f 100644 --- a/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java +++ b/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java @@ -101,7 +101,7 @@ public class NullUsageProcessor extends AbstractProcessor { Set intersection = new HashSet(notNullElements); intersection.retainAll(nullableElements); for (Element element : intersection) { - String error = String.format("%s cannot be annotated with both NotNull and Nullable", element.getKind().toString().toLowerCase()); + String error = String.format("%s cannot be annotated with both %s and %s", element.getKind().toString().replace('_', ' ').toLowerCase(), notNullType.getSimpleName(), nullableType.getSimpleName()); processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, error, element); } @@ -158,9 +158,8 @@ public class NullUsageProcessor extends AbstractProcessor { ExecutableElement executableElement = (ExecutableElement)element; TypeMirror returnType = executableElement.getReturnType(); if (returnType instanceof NoType && returnType.getKind() == TypeKind.VOID) { - // TODO: report the error on the annotation usage instead of the method String error = String.format("void method cannot be annotated with %s", annotationType.getSimpleName()); - processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, error, element); + processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, error, element, getAnnotationMirror(element, annotationType)); } } } @@ -189,7 +188,7 @@ public class NullUsageProcessor extends AbstractProcessor { if (typeToCheck instanceof PrimitiveType && typeToCheck.getKind().isPrimitive()) { String error = String.format("%s with a primitive type %s be annotated with %s", element.getKind().toString().replace('_', ' ').toLowerCase(), kind == Diagnostic.Kind.ERROR ? "cannot" : "should not", annotationType.getSimpleName()); - processingEnv.getMessager().printMessage(kind, error, element); + processingEnv.getMessager().printMessage(kind, error, element, getAnnotationMirror(element, annotationType)); } } } @@ -261,7 +260,7 @@ public class NullUsageProcessor extends AbstractProcessor { // check method annotation if (isNullable(overrider) && isNotNull(overridden)) { String error = String.format("method annotated with %s cannot override or implement a method annotated with %s", nullableType.getSimpleName(), notNullType.getSimpleName()); - processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, error, overrider); + processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, error, overrider, getNullableAnnotationMirror(overrider)); } else if (isNullable(overrider) && !(isNullable(overridden) || isNotNull(overridden))) { String error = String.format("method annotated with %s overrides a method that is not annotated", nullableType.getSimpleName()); @@ -272,8 +271,8 @@ public class NullUsageProcessor extends AbstractProcessor { List overriddenParameters = overridden.getParameters(); for (int i = 0; i < overriderParameters.size(); i++) { if (isNotNull(overriderParameters.get(i)) && isNullable(overriddenParameters.get(i))) { - String error = String.format("parameter annotated with %s cannot override or implement a parameter annotated with %s", notNullType.getSimpleName(), nullableType.getSimpleName()); - processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, error, overrider); + String error = String.format("parameter %s annotated with %s cannot override or implement a parameter annotated with %s", overriderParameters.get(i).getSimpleName(), notNullType.getSimpleName(), nullableType.getSimpleName()); + processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, error, overriderParameters.get(i), getNotNullAnnotationMirror(overriderParameters.get(i))); } else if (isNotNull(overriderParameters.get(i)) && !(isNullable(overriddenParameters.get(i)) || isNotNull(overriddenParameters.get(i)))) { String error = String.format("parameter %s annotated with %s overrides a parameter that is not annotated", overriderParameters.get(i).getSimpleName(), notNullType.getSimpleName()); @@ -291,18 +290,16 @@ public class NullUsageProcessor extends AbstractProcessor { } private AnnotationMirror getNotNullAnnotationMirror(Element element) { - for (AnnotationMirror annotationMirror : element.getAnnotationMirrors()) { - if (annotationMirror.getAnnotationType().asElement() == notNullType) { - return annotationMirror; - } - } - - return null; + return getAnnotationMirror(element, notNullType); } private AnnotationMirror getNullableAnnotationMirror(Element element) { + return getAnnotationMirror(element, nullableType); + } + + private AnnotationMirror getAnnotationMirror(Element element, TypeElement annotationType) { for (AnnotationMirror annotationMirror : element.getAnnotationMirrors()) { - if (annotationMirror.getAnnotationType().asElement() == nullableType) { + if (annotationMirror.getAnnotationType().asElement() == annotationType) { return annotationMirror; } } From 49f12fd28d20e14307584f0dd5ac89196d693244 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Tue, 21 Jan 2014 14:38:33 -0600 Subject: [PATCH 09/11] Add note about potential future features --- .../src/org/antlr/v4/runtime/misc/NullUsageProcessor.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java b/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java index e81206c1f..03dd1a61a 100644 --- a/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java +++ b/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java @@ -73,6 +73,13 @@ import java.util.Set; *
  • Warning: a method is annotated with {@link Nullable}, but the method overrides or implements a method that is not annotated.
    • * * + *

    In the future, the validation process may be updated to check the following additional items.

    + * + *
      + *
    • Warning: a parameter is not annotated, but the method overrides or implements a method where the parameter is annotated with {@link NotNull} or {@link Nullable}.
      • + *
    • Warning: a method is not annotated, but the method overrides or implements a method that is annotated with with {@link NotNull} or {@link Nullable}.
      • + *
    + * * @author Sam Harwell */ @SupportedAnnotationTypes({NullUsageProcessor.NotNullClassName, NullUsageProcessor.NullableClassName}) From 07708d922397724611c71b4a2e49951293453295 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Tue, 21 Jan 2014 14:40:25 -0600 Subject: [PATCH 10/11] Limit the number of errors reported for a single method or parameter --- .../antlr/v4/runtime/misc/NullUsageProcessor.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java b/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java index 03dd1a61a..1e3e29030 100644 --- a/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java +++ b/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NullUsageProcessor.java @@ -236,12 +236,14 @@ public class NullUsageProcessor extends AbstractProcessor { private void checkOverriddenMethods(ExecutableElement method) { TypeElement declaringType = (TypeElement)method.getEnclosingElement(); + Set errorElements = new HashSet(); + Set warnedElements = new HashSet(); typeLoop: for (TypeMirror supertypeMirror : getAllSupertypes(processingEnv.getTypeUtils().getDeclaredType(declaringType))) { for (Element element : ((TypeElement)processingEnv.getTypeUtils().asElement(supertypeMirror)).getEnclosedElements()) { if (element instanceof ExecutableElement) { if (processingEnv.getElementUtils().overrides(method, (ExecutableElement)element, declaringType)) { - checkOverriddenMethod(method, (ExecutableElement)element); + checkOverriddenMethod(method, (ExecutableElement)element, errorElements, warnedElements); continue typeLoop; } } @@ -263,13 +265,13 @@ public class NullUsageProcessor extends AbstractProcessor { return new ArrayList(supertypes); } - private void checkOverriddenMethod(ExecutableElement overrider, ExecutableElement overridden) { + private void checkOverriddenMethod(ExecutableElement overrider, ExecutableElement overridden, Set errorElements, Set warnedElements) { // check method annotation - if (isNullable(overrider) && isNotNull(overridden)) { + if (isNullable(overrider) && isNotNull(overridden) && errorElements.add(overrider)) { String error = String.format("method annotated with %s cannot override or implement a method annotated with %s", nullableType.getSimpleName(), notNullType.getSimpleName()); processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, error, overrider, getNullableAnnotationMirror(overrider)); } - else if (isNullable(overrider) && !(isNullable(overridden) || isNotNull(overridden))) { + else if (isNullable(overrider) && !(isNullable(overridden) || isNotNull(overridden)) && !errorElements.contains(overrider) && warnedElements.add(overrider)) { String error = String.format("method annotated with %s overrides a method that is not annotated", nullableType.getSimpleName()); processingEnv.getMessager().printMessage(Diagnostic.Kind.WARNING, error, overrider, getNullableAnnotationMirror(overrider)); } @@ -277,11 +279,11 @@ public class NullUsageProcessor extends AbstractProcessor { List overriderParameters = overrider.getParameters(); List overriddenParameters = overridden.getParameters(); for (int i = 0; i < overriderParameters.size(); i++) { - if (isNotNull(overriderParameters.get(i)) && isNullable(overriddenParameters.get(i))) { + if (isNotNull(overriderParameters.get(i)) && isNullable(overriddenParameters.get(i)) && errorElements.add(overriderParameters.get(i))) { String error = String.format("parameter %s annotated with %s cannot override or implement a parameter annotated with %s", overriderParameters.get(i).getSimpleName(), notNullType.getSimpleName(), nullableType.getSimpleName()); processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, error, overriderParameters.get(i), getNotNullAnnotationMirror(overriderParameters.get(i))); } - else if (isNotNull(overriderParameters.get(i)) && !(isNullable(overriddenParameters.get(i)) || isNotNull(overriddenParameters.get(i)))) { + else if (isNotNull(overriderParameters.get(i)) && !(isNullable(overriddenParameters.get(i)) || isNotNull(overriddenParameters.get(i))) /*&& !errorElements.contains(overriderParameters.get(i)) && warnedElements.add(overriderParameters.get(i))*/) { String error = String.format("parameter %s annotated with %s overrides a parameter that is not annotated", overriderParameters.get(i).getSimpleName(), notNullType.getSimpleName()); processingEnv.getMessager().printMessage(Diagnostic.Kind.WARNING, error, overriderParameters.get(i), getNotNullAnnotationMirror(overriderParameters.get(i))); } From 0454916a7057d334dbf4fcb3b0e6dcf817e800a2 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Tue, 21 Jan 2014 15:24:57 -0600 Subject: [PATCH 11/11] Updated documentation for NotNull and Nullable --- .../org/antlr/v4/runtime/misc/NotNull.java | 41 +++++++++++++++++-- .../org/antlr/v4/runtime/misc/Nullable.java | 41 +++++++++++++++++-- 2 files changed, 76 insertions(+), 6 deletions(-) diff --git a/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NotNull.java b/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NotNull.java index 8beacf2ee..55edba7fe 100644 --- a/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NotNull.java +++ b/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/NotNull.java @@ -30,8 +30,43 @@ package org.antlr.v4.runtime.misc; -@java.lang.annotation.Documented -@java.lang.annotation.Retention(java.lang.annotation.RetentionPolicy.CLASS) -@java.lang.annotation.Target({java.lang.annotation.ElementType.FIELD, java.lang.annotation.ElementType.METHOD, java.lang.annotation.ElementType.PARAMETER, java.lang.annotation.ElementType.LOCAL_VARIABLE}) +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; + +/** + * This annotation marks a field, parameter, local variable, or method (return + * value) as never being {@code null}. The specific semantics implied by this + * annotation depend on the kind of element the annotation is applied to. + * + *
      + *
    • Field or Local Variable: Code reading the field or local + * variable may assume that the value is never {@code null}. Code writing to the + * field or local variable should ensure that a {@code null} reference is never + * written.
      • + *
    • Parameter: Code calling the method should never pass + * {@code null} for this parameter. The implementation may assume that the value + * is never {@code null}, and the behavior of the method if the parameter is + * {@code null} is undefined. Overriding methods may optionally use the + * {@link Nullable} annotation instead of this annotation for the parameter, + * indicating that the overriding method provides additional code to handle a + * {@code null} reference passed for the parameter.
      • + *
    • Method (Return Value): Code calling the method may + * assume that the result of the method is never {@code null}. The + * implementation of the method should ensure that a {@code null} reference is + * never returned.
      • + *
    + * + *

    + * The {@link NullUsageProcessor} annotation processor validates certain usage + * scenarios for this annotation, with compile-time errors or warnings reported + * for misuse. For detailed information about the supported analysis, see the + * documentation for {@link NullUsageProcessor}.

    + */ +@Documented +@Retention(RetentionPolicy.CLASS) +@Target({ElementType.FIELD, ElementType.METHOD, ElementType.PARAMETER, ElementType.LOCAL_VARIABLE}) public @interface NotNull { } diff --git a/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/Nullable.java b/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/Nullable.java index ae527a477..7b9836ad2 100644 --- a/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/Nullable.java +++ b/runtime/JavaAnnotations/src/org/antlr/v4/runtime/misc/Nullable.java @@ -30,8 +30,43 @@ package org.antlr.v4.runtime.misc; -@java.lang.annotation.Documented -@java.lang.annotation.Retention(java.lang.annotation.RetentionPolicy.CLASS) -@java.lang.annotation.Target({java.lang.annotation.ElementType.FIELD, java.lang.annotation.ElementType.METHOD, java.lang.annotation.ElementType.PARAMETER, java.lang.annotation.ElementType.LOCAL_VARIABLE}) +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; + +/** + * This annotation marks a field, parameter, local variable, or method (return + * value) as potentially having the value {@code null}. The specific semantics + * implied by this annotation depend on the kind of element the annotation is + * applied to. + * + *
      + *
    • Field or Local Variable: Code reading the field or local + * variable may not assume that the value is never {@code null}.
      • + *
    • Parameter: Code calling the method might pass + * {@code null} for this parameter. The documentation for the method should + * describe the behavior of the method in the event this parameter is + * {@code null}. + *
      • + *
    • Method (Return Value): Code calling the method may not + * assume that the result of the method is never {@code null}. The documentation + * for the method should describe the meaning of a {@code null} reference being + * returned. Overriding methods may optionally use the {@link NotNull} + * annotation instead of this annotation for the method, indicating that the + * overriding method (and any method which overrides it) will never return a + * {@code null} reference.
      • + *
    + * + *

    + * The {@link NullUsageProcessor} annotation processor validates certain usage + * scenarios for this annotation, with compile-time errors or warnings reported + * for misuse. For detailed information about the supported analysis, see the + * documentation for {@link NullUsageProcessor}.

    + */ +@Documented +@Retention(RetentionPolicy.CLASS) +@Target({ElementType.FIELD, ElementType.METHOD, ElementType.PARAMETER, ElementType.LOCAL_VARIABLE}) public @interface Nullable { }