Improve field handling (#212)

* Consider enclosing ignore annotations for fields and improve log output
* Support IgnoreJRERequirement annotation for fields
* Set SignatureChecker.ANNOTATION_FQN to constant string again

animal-sniffer-annotations/src/main/java/org/codehaus/mojo/animal_sniffer/IgnoreJRERequirement.java

@@ -36,7 +36,7 @@
  */
 @Retention(CLASS)
 @Documented
-@Target({ElementType.METHOD, ElementType.CONSTRUCTOR, ElementType.TYPE})
+@Target({ElementType.METHOD, ElementType.CONSTRUCTOR, ElementType.TYPE, ElementType.FIELD})
 public @interface IgnoreJRERequirement
 {
 }

animal-sniffer-maven-plugin/pom.xml

@@ -135,6 +135,9 @@
           <filterProperties>
             <java.home>${env.JAVA_HOME}</java.home>
           </filterProperties>
+          <extraArtifacts>
+            <extraArtifact>org.codehaus.mojo:animal-sniffer-annotations:${project.version}</extraArtifact>
+          </extraArtifacts>
         </configuration>
       </plugin>
     </plugins>

animal-sniffer-maven-plugin/src/it/fields/invoker.properties

@@ -0,0 +1 @@
+invoker.buildResult=failure

animal-sniffer-maven-plugin/src/it/fields/pom.xml

@@ -0,0 +1,105 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  The MIT License
+
+  Copyright (c) 2009 codehaus.org.
+
+  Permission is hereby granted, free of charge, to any person obtaining a copy
+  of this software and associated documentation files (the "Software"), to deal
+  in the Software without restriction, including without limitation the rights
+  to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+  copies of the Software, and to permit persons to whom the Software is
+  furnished to do so, subject to the following conditions:
+
+  The above copyright notice and this permission notice shall be included in
+  all copies or substantial portions of the Software.
+
+  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+  IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+  FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+  AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+  LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+  OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+  THE SOFTWARE.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+
+  <groupId>localdomain.localhost</groupId>
+  <artifactId>real-test</artifactId>
+  <version>1.0-SNAPSHOT</version>
+  <packaging>jar</packaging>
+
+  <name>fields</name>
+
+  <dependencies>
+    <dependency>
+      <groupId>org.codehaus.mojo</groupId>
+      <artifactId>animal-sniffer-annotations</artifactId>
+      <version>${annotationsDependencyVersion}</version>
+    </dependency>
+  </dependencies>
+
+  <build>
+    <pluginManagement>
+      <plugins>
+        <plugin>
+          <artifactId>maven-clean-plugin</artifactId>
+          <version>2.2</version>
+        </plugin>
+        <plugin>
+          <artifactId>maven-compiler-plugin</artifactId>
+          <version>3.5.1</version>
+          <configuration>
+            <source>@mojo.java.target@</source>
+            <target>@mojo.java.target@</target>
+          </configuration>
+        </plugin>
+        <plugin>
+          <artifactId>maven-surefire-plugin</artifactId>
+          <version>2.4.2</version>
+        </plugin>
+      </plugins>
+    </pluginManagement>
+    <plugins>
+      <plugin>
+        <artifactId>maven-compiler-plugin</artifactId>
+        <configuration>
+          <source>1.8</source>
+          <target>1.8</target>
+        </configuration>
+      </plugin>
+      <plugin>
+        <groupId>${pluginGroupId}</groupId>
+        <artifactId>${pluginArtifactId}</artifactId>
+        <version>${pluginVersion}</version>
+        <executions>
+          <execution>
+            <phase>test</phase>
+            <goals>
+              <goal>check</goal>
+            </goals>
+            <configuration>
+              <signature>
+                <groupId>org.codehaus.mojo.signature</groupId>
+                <artifactId>java15</artifactId>
+                <version>1.0</version>
+              </signature>
+            </configuration>
+          </execution>
+        </executions>
+      </plugin>
+    </plugins>
+  </build>
+  <properties>
+    <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+    <project.build.outputEncoding>UTF-8</project.build.outputEncoding>
+    <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
+    <pluginGroupId>@project.groupId@</pluginGroupId>
+    <pluginArtifactId>@project.artifactId@</pluginArtifactId>
+    <pluginVersion>@project.version@</pluginVersion>
+    <annotationsDependencyVersion>@project.version@</annotationsDependencyVersion>
+  </properties>
+
+</project>

animal-sniffer-maven-plugin/src/it/fields/src/main/java/localhost/Test.java

@@ -0,0 +1,13 @@
+package localhost;
+
+import java.util.Optional;
+
+public class Test {
+    Optional<String> f;
+
+    // Also include method to make sure field does not influence method detection
+    // in any way
+    Optional<String> test() {
+        return null;
+    }
+}

animal-sniffer-maven-plugin/src/it/fields/src/main/java/localhost/TestSuppressedClass.java

@@ -0,0 +1,13 @@
+package localhost;
+
+import java.util.Optional;
+import org.codehaus.mojo.animal_sniffer.IgnoreJRERequirement;
+
+@IgnoreJRERequirement
+public class TestSuppressedClass {
+    Optional<String> f;
+
+    Optional<String> test() {
+        return null;
+    }
+}

animal-sniffer-maven-plugin/src/it/fields/src/main/java/localhost/TestSuppressedField.java

@@ -0,0 +1,17 @@
+package localhost;
+
+import java.util.Optional;
+import org.codehaus.mojo.animal_sniffer.IgnoreJRERequirement;
+
+public class TestSuppressedField {
+    Optional<String> f1;
+    @IgnoreJRERequirement
+    Optional<String> f2;
+    Optional<String> f3;
+
+    // Also include method to make sure field does not influence method detection
+    // in any way
+    Optional<String> test() {
+        return null;
+    }
+}

animal-sniffer-maven-plugin/src/it/fields/src/main/java/localhost/TestWithNested.java

@@ -0,0 +1,19 @@
+package localhost;
+
+import java.util.Optional;
+
+public class TestWithNested {
+    Optional<String> f;
+
+    Optional<String> test() {
+        return null;
+    }
+
+    public class Nested {
+        Optional<String> f;
+
+        Optional<String> test() {
+            return null;
+        }
+    }
+}

animal-sniffer-maven-plugin/src/it/fields/src/main/java/localhost/TestWithNestedSuppressedClass.java

@@ -0,0 +1,15 @@
+package localhost;
+
+import java.util.Optional;
+import org.codehaus.mojo.animal_sniffer.IgnoreJRERequirement;
+
+public class TestWithNestedSuppressedClass {
+    @IgnoreJRERequirement
+    public class Nested {
+        Optional<String> f;
+
+        Optional<String> test() {
+            return null;
+        }
+    }
+}

animal-sniffer-maven-plugin/src/it/fields/verify.groovy

@@ -0,0 +1,21 @@
+def logFile = new File( basedir, 'build.log' )
+assert logFile.exists()
+
+def buildLog = logFile.getText('UTF-8')
+
+assert buildLog.contains('Test.java: Field f: Undefined reference: java.util.Optional')
+assert buildLog.contains('Test.java:11: Undefined reference: java.util.Optional')
+
+assert !(buildLog =~ /TestSuppressedClass.*Undefined reference/)
+
+assert buildLog.contains('TestSuppressedField.java: Field f1: Undefined reference: java.util.Optional')
+assert !buildLog.contains('Field f2')
+assert buildLog.contains('TestSuppressedField.java: Field f3: Undefined reference: java.util.Optional')
+assert buildLog.contains('TestSuppressedField.java:15: Undefined reference: java.util.Optional')
+
+assert buildLog.contains('TestWithNested.java: Field f: Undefined reference: java.util.Optional')
+assert buildLog.contains('TestWithNested.java:9: Undefined reference: java.util.Optional')
+assert buildLog.contains('TestWithNested.java: Field TestWithNested$Nested.f: Undefined reference: java.util.Optional')
+assert buildLog.contains('TestWithNested.java:16: Undefined reference: java.util.Optional')
+
+assert !(buildLog =~ /TestWithNestedSuppressedClass.*Undefined reference/)

animal-sniffer-maven-plugin/src/it/github-74-1/verify.groovy

@@ -1,10 +1,10 @@
 File log = new File(basedir, 'build.log')
 
 assert log.text.contains( 'IllegalTypeReturn.java:11: Undefined reference: java.nio.file.Path' )
-assert log.text.contains( 'IllegalFieldWithManipulationSample.java: Undefined reference: java.nio.file.Path' )
+assert log.text.contains( 'IllegalFieldWithManipulationSample.java: Field pathField: Undefined reference: java.nio.file.Path' )
 assert log.text.contains( 'IllegalFieldWithManipulationSample.java:14: Undefined reference: java.nio.file.Path' )
 assert log.text.contains( 'IllegalFieldWithManipulationSample.java:18: Undefined reference: java.nio.file.Path' )
 assert log.text.contains( 'IllegalFieldWithManipulationSample.java:18: Undefined reference: java.nio.file.Path java.nio.file.Path.resolve(String)' )
-assert log.text.contains( 'IllegalFieldWithAccessorsSample.java: Undefined reference: java.nio.file.Path' )
+assert log.text.contains( 'IllegalFieldWithAccessorsSample.java: Field pathField: Undefined reference: java.nio.file.Path' )
 assert log.text.contains( 'IllegalFieldWithAccessorsSample.java:22: Undefined reference: java.nio.file.Path' )
-assert log.text.contains( 'IllegalFieldSample.java: Undefined reference: java.nio.file.Paths' )
+assert log.text.contains( 'IllegalFieldSample.java: Field pathsField: Undefined reference: java.nio.file.Paths' )

animal-sniffer/pom.xml

@@ -43,6 +43,11 @@
       <groupId>org.ow2.asm</groupId>
       <artifactId>asm</artifactId>
     </dependency>
+    <dependency>
+      <groupId>org.codehaus.mojo</groupId>
+      <artifactId>animal-sniffer-annotations</artifactId>
+      <version>${project.version}</version>
+    </dependency>
     <dependency>
       <groupId>junit</groupId>
       <artifactId>junit</artifactId>

animal-sniffer/src/main/java/org/codehaus/mojo/animal_sniffer/SignatureChecker.java

@@ -65,6 +65,8 @@ public class SignatureChecker
      * The fully qualified name of the annotation to use to annotate methods/fields/classes that are
      * to be ignored by animal sniffer.
      */
+    // Cannot use IgnoreJRERequirement.class.getName() because value needs to be constant to be
+    // included in Mojo documentation
     public static final String ANNOTATION_FQN = "org.codehaus.mojo.animal_sniffer.IgnoreJRERequirement";
 
     /**
@@ -183,6 +185,7 @@ public void setAnnotationTypes( Collection<String> annotationTypes )
         }
     }
 
+    @Override
     protected void process( final String name, InputStream image )
         throws IOException
     {
@@ -216,6 +219,7 @@ public PrefixMatchRule( String prefix )
             this.prefix = prefix;
         }
 
+        @Override
         public boolean matches( String text )
         {
             return text.startsWith( prefix );
@@ -232,6 +236,7 @@ public ExactMatchRule( String match )
             this.match = match;
         }
 
+        @Override
         public boolean matches( String text )
         {
             return match.equals( text );
@@ -248,6 +253,7 @@ public RegexMatchRule( Pattern regex )
             this.regex = regex;
         }
 
+        @Override
         public boolean matches( String text )
         {
             return regex.matcher( text ).matches();
@@ -280,6 +286,7 @@ private class CheckingVisitor
 
         private String packagePrefix;
         private int line;
+        private String currentFieldName = null;
         private String name;
         private String internalName;
 
@@ -349,9 +356,26 @@ public AnnotationVisitor visitAnnotation(String desc, boolean visible)
         public FieldVisitor visitField(int access, String name, final String descriptor, String signature, Object value) {
             return new FieldVisitor(Opcodes.ASM7) {
 
+                boolean ignoreError = ignoreClass;
+
+                @Override
+                public AnnotationVisitor visitAnnotation( String annoDesc, boolean visible )
+                {
+                    if ( isIgnoreAnnotation(annoDesc) )
+                    {
+                        ignoreError = true;
+                    }
+                    return super.visitAnnotation( annoDesc, visible );
+                }
+
                 @Override
                 public void visitEnd() {
-                    checkType(Type.getType(descriptor), false);
+                    // When field is declared in nested class, include declaring class name (including enclosing
+                    // class name) to make error message more helpful
+                    String fieldNamePrefix = internalName.contains("$") ? internalName.substring(internalName.lastIndexOf('/') + 1) + '.' : "";
+                    currentFieldName = fieldNamePrefix + name;
+                    checkType(Type.getType(descriptor), ignoreError);
+                    currentFieldName = null;
                 }
 
             };
@@ -590,7 +614,14 @@ private boolean find( Clazz c , String sig )
         private void error( String type, String sig )
         {
             hadError = true;
-            logger.error(name + (line > 0 ? ":" + line : "") + ": Undefined reference: " + toSourceForm( type, sig ) );
+
+            String location = "";
+            if (currentFieldName != null) {
+                location = ": Field " + currentFieldName;
+            } else if (line > 0) {
+                location = ":" + line;
+            }
+            logger.error(name + location + ": Undefined reference: " + toSourceForm( type, sig ) );
         }
     }
 

animal-sniffer/src/test/java/org/codehaus/mojo/animal_sniffer/SignatureCheckerTest.java

@@ -34,6 +34,11 @@ public SignatureCheckerTest( String testName )
         super( testName );
     }
 
+    public void testAnnotationFqn()
+    {
+        assertEquals( IgnoreJRERequirement.class.getName(), SignatureChecker.ANNOTATION_FQN );
+    }
+
     public void testToSourceForm()
     {
         assertSourceForm( "java.util.HashMap", "java/util/HashMap", null );