Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve field handling #212

Merged
merged 4 commits into from
Sep 3, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
invoker.buildResult=failure
109 changes: 109 additions & 0 deletions animal-sniffer-maven-plugin/src/it/fields/pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
<?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>
<!-- This is a temporary workaround until the first version with FIELD as target has been released;
then this can instead use any of the released versions -->
<scope>system</scope>
<systemPath>${project.basedir}/../../../../animal-sniffer-annotations/target/animal-sniffer-annotations-${annotationsDependencyVersion}.jar</systemPath>
olamy marked this conversation as resolved.
Show resolved Hide resolved
</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>
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
21 changes: 21 additions & 0 deletions animal-sniffer-maven-plugin/src/it/fields/verify.groovy
Original file line number Diff line number Diff line change
@@ -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/)
6 changes: 3 additions & 3 deletions animal-sniffer-maven-plugin/src/it/github-74-1/verify.groovy
Original file line number Diff line number Diff line change
@@ -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' )
5 changes: 5 additions & 0 deletions animal-sniffer/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Comment on lines +46 to +50
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why animal-sniffer-annotations was previously not a dependency? It allows directly referring to the annotation type IgnoreJRERequirement in SignatureChecker.

Though if you want I can also revert this again.

<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";

/**
Expand Down Expand Up @@ -183,6 +185,7 @@ public void setAnnotationTypes( Collection<String> annotationTypes )
}
}

@Override
protected void process( final String name, InputStream image )
throws IOException
{
Expand Down Expand Up @@ -216,6 +219,7 @@ public PrefixMatchRule( String prefix )
this.prefix = prefix;
}

@Override
public boolean matches( String text )
{
return text.startsWith( prefix );
Expand All @@ -232,6 +236,7 @@ public ExactMatchRule( String match )
this.match = match;
}

@Override
public boolean matches( String text )
{
return match.equals( text );
Expand All @@ -248,6 +253,7 @@ public RegexMatchRule( Pattern regex )
this.regex = regex;
}

@Override
public boolean matches( String text )
{
return regex.matcher( text ).matches();
Expand Down Expand Up @@ -280,6 +286,7 @@ private class CheckingVisitor

private String packagePrefix;
private int line;
private String currentFieldName = null;
private String name;
private String internalName;

Expand Down Expand Up @@ -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;
}

};
Expand Down Expand Up @@ -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 ) );
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand Down