Skip to content

Commit

Permalink
Merge pull request gradle#8986 from gradle/wolfs/xforms/better-inject…
Browse files Browse the repository at this point in the history
…ion-error-messages

Validate types for injection annotations
  • Loading branch information
wolfs authored Apr 15, 2019
2 parents 5af372c + 211a779 commit bea59cb
Show file tree
Hide file tree
Showing 7 changed files with 183 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import com.github.javaparser.JavaParser;
import com.github.javaparser.ast.body.AnnotationDeclaration;
import com.github.javaparser.ast.body.AnnotationMemberDeclaration;
import com.github.javaparser.ast.body.BodyDeclaration;
import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration;
import com.github.javaparser.ast.body.ConstructorDeclaration;
Expand Down Expand Up @@ -70,6 +71,13 @@ public Object visit(AnnotationDeclaration annotationDeclaration, Void arg) {
return super.visit(annotationDeclaration, arg);
}
@Override
public Object visit(AnnotationMemberDeclaration annotationMemberDeclaration, Void arg) {
if (matchesNameAndContainsAnnotation(annotationMemberDeclaration.getName().asString(), method.getName(), annotationMemberDeclaration)) {
return new Object();
}
return super.visit(annotationMemberDeclaration, arg);
}
@Override
public Object visit(EnumDeclaration enumDeclaration, Void arg) {
if (matchesNameAndContainsAnnotation(enumDeclaration.getName().asString(), toSimpleName(method.getjApiClass().getFullyQualifiedName()), enumDeclaration)) {
return new Object();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,28 +118,39 @@ class PublicAPIRulesTest extends Specification {
}
"""
: apiElement.startsWith('annotation') ? """
public @interface $TEST_INTERFACE_SIMPLE_NAME { }
"""
: apiElement in ['class', 'constructor'] ? """
public class $TEST_INTERFACE_SIMPLE_NAME {
public ApiTest() { }
public @interface $TEST_INTERFACE_SIMPLE_NAME {
String method();
}
"""
: """
: apiElement == 'interface' ? """
public interface $TEST_INTERFACE_SIMPLE_NAME {
String field = "value";
void method();
}
"""
: """
public class $TEST_INTERFACE_SIMPLE_NAME {
public String field = "value";
public void method() { }
public $TEST_INTERFACE_SIMPLE_NAME() { }
}
"""

then:
rule.maybeViolation(jApiType).humanExplanation =~ 'Is not annotated with @since 11.38'

when:
sourceFile.text = apiElement.startsWith('enum') ? """
sourceFile.text = apiElement == 'enum' ? """
/**
* @since 11.38
*/
public enum $TEST_INTERFACE_SIMPLE_NAME {
field;
void method() { }
}
"""
: apiElement.startsWith('enum') ? """
public enum $TEST_INTERFACE_SIMPLE_NAME {
/**
* @since 11.38
Expand All @@ -152,33 +163,48 @@ class PublicAPIRulesTest extends Specification {
void method() { }
}
"""
: apiElement.startsWith('annotation') ? """
/**
* @since 11.38
*/
public @interface $TEST_INTERFACE_SIMPLE_NAME { }
"""
: apiElement.startsWith('class') ? """
: apiElement == 'annotation' ? """
/**
* @since 11.38
*/
public class $TEST_INTERFACE_SIMPLE_NAME {
public ApiTest() { }
public @interface $TEST_INTERFACE_SIMPLE_NAME {
String method();
}
"""
: apiElement.startsWith('constructor') ? """
public class $TEST_INTERFACE_SIMPLE_NAME {
: apiElement == 'annotation member' ? """
public @interface $TEST_INTERFACE_SIMPLE_NAME {
/**
* @since 11.38
*/
public ApiTest() { }
String method();
}
"""
: """
: apiElement == 'class' ? """
/**
* @since 11.38
*/
public class $TEST_INTERFACE_SIMPLE_NAME {
public String field = "value";
public void method() { }
public $TEST_INTERFACE_SIMPLE_NAME() { }
}
"""
: apiElement == 'interface' ? """
/**
* @since 11.38
*/
public interface $TEST_INTERFACE_SIMPLE_NAME {
String field = "value";
void method();
}
"""
: """
public class $TEST_INTERFACE_SIMPLE_NAME {
/**
* @since 11.38
*/
public $TEST_INTERFACE_SIMPLE_NAME() { }
/**
* @since 11.38
*/
Expand All @@ -195,16 +221,17 @@ class PublicAPIRulesTest extends Specification {
rule.maybeViolation(jApiType) == null

where:
apiElement | jApiTypeName
'interface' | 'jApiClassifier'
'class' | 'jApiClassifier'
'method' | 'jApiMethod'
'field' | 'jApiField'
'constructor' | 'jApiConstructor'
'enum' | 'jApiClassifier'
'enum literal' | 'jApiField'
'enum method' | 'jApiMethod'
'annotation' | 'jApiClassifier'
apiElement | jApiTypeName
'interface' | 'jApiClassifier'
'class' | 'jApiClassifier'
'method' | 'jApiMethod'
'field' | 'jApiField'
'constructor' | 'jApiConstructor'
'enum' | 'jApiClassifier'
'enum literal' | 'jApiField'
'enum method' | 'jApiMethod'
'annotation' | 'jApiClassifier'
'annotation member' | 'jApiMethod'
}

@Unroll
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.gradle.api.artifacts.transform;

import org.gradle.api.Incubating;
import org.gradle.api.file.FileSystemLocation;
import org.gradle.api.provider.Provider;
import org.gradle.api.reflect.InjectionPointQualifier;

Expand All @@ -40,6 +41,6 @@
@Retention(RetentionPolicy.RUNTIME)
@Target({ElementType.METHOD})
@Documented
@InjectionPointQualifier(supportedTypes = { File.class, Provider.class })
@InjectionPointQualifier(supportedTypes = { File.class }, supportedProviderTypes = { FileSystemLocation.class })
public @interface InputArtifact {
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
/**
* The annotated annotation can be used to inject elements of the supported types.
*
* <p>If no {@code supportedTypes} are supplied, all types are supported.</p>
* <p>If both {@link #supportedTypes()} and {@link #supportedProviderTypes()} are empty, all types are supported.</p>
*
* @since 5.3
*/
Expand All @@ -36,5 +36,17 @@
@Target({ElementType.ANNOTATION_TYPE})
@Documented
public @interface InjectionPointQualifier {
/**
* The types which are supported for injection.
*/
Class<?>[] supportedTypes() default {};

/**
* The types of {@link org.gradle.api.provider.Provider}s supported for injection.
* <br>
* If e.g. {@link org.gradle.api.file.FileSystemLocation} is in the list, then this annotation can be used for injecting {@link org.gradle.api.provider.Provider}&lt;{@link org.gradle.api.file.FileSystemLocation}&gt;.
*
* @since 5.5
*/
Class<?>[] supportedProviderTypes() default {};
}
Original file line number Diff line number Diff line change
Expand Up @@ -677,9 +677,10 @@ abstract class MakeGreen extends ArtifactTransform {
@Unroll
def "transform cannot use @InputArtifact to receive #propertyType"() {
settingsFile << """
include 'a', 'b', 'c'
include 'a', 'b'
"""
setupBuildWithColorTransformAction()
def typeName = propertyType instanceof Class ? propertyType.name : propertyType.toString()
buildFile << """

project(':a') {
Expand All @@ -690,7 +691,7 @@ project(':a') {

abstract class MakeGreen implements TransformAction<TransformParameters.None> {
@InputArtifact
abstract ${propertyType instanceof Class ? propertyType.name : propertyType} getInput()
abstract ${typeName} getInput()

void transform(TransformOutputs outputs) {
input
Expand All @@ -703,15 +704,53 @@ abstract class MakeGreen implements TransformAction<TransformParameters.None> {
fails(":a:resolve")
then:
// Documents existing behaviour. Should fail eagerly and with a better error message
failure.assertHasDescription("Execution failed for task ':a:resolve'.")
failure.assertHasCause("Execution failed for MakeGreen: ${file('b/build/b.jar')}.")
failure.assertHasCause("No service of type ${propertyType} available.")
failure.assertHasDescription("A problem occurred evaluating root project")
failure.assertHasCause("Cannot register artifact transform MakeGreen (from {color=blue} to {color=green})")
failure.assertHasCause("Cannot use @InputArtifact annotation on property MakeGreen.getInput() of type ${typeName}. Allowed property types: java.io.File, org.gradle.api.provider.Provider<org.gradle.api.file.FileSystemLocation>.")
where:
propertyType << [FileCollection, new TypeToken<Provider<File>>() {}.getType(), new TypeToken<Provider<String>>() {}.getType()]
}
@Unroll
def "transform cannot use @InputArtifactDependencies to receive #propertyType"() {
settingsFile << """
include 'a', 'b'
"""
setupBuildWithColorTransformAction()
buildFile << """

project(':a') {
dependencies {
implementation project(':b')
}
}

abstract class MakeGreen implements TransformAction<TransformParameters.None> {
@${annotation.name}
abstract ${propertyType.name} getDependencies()

void transform(TransformOutputs outputs) {
dependencies
throw new RuntimeException("broken")
}
}
"""
when:
fails(":a:resolve")
then:
failure.assertHasDescription("A problem occurred evaluating root project")
failure.assertHasCause("Cannot register artifact transform MakeGreen (from {color=blue} to {color=green})")
failure.assertHasCause("Cannot use @InputArtifactDependencies annotation on property MakeGreen.getDependencies() of type ${propertyType.name}. Allowed property types: org.gradle.api.file.FileCollection.")
where:
annotation | propertyType
InputArtifactDependencies | File
InputArtifactDependencies | String
}
def "transform cannot use @Inject to receive input file"() {
settingsFile << """
include 'a', 'b', 'c'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ private <T extends TransformParameters> void register(RecordingRegistration regi
ArtifactTransformRegistration finalizedRegistration = registrationFactory.create(registration.from.asImmutable(), registration.to.asImmutable(), actionType, parameterObject);
transforms.add(finalizedRegistration);
} catch (Exception e) {
throw new VariantTransformConfigurationException(String.format("Cannot register artifact transform %s with parameters %s", ModelType.of(actionType).getDisplayName(), parameterObject), e);
throw new VariantTransformConfigurationException(String.format("Cannot register artifact transform %s (from %s to %s)", ModelType.of(actionType).getDisplayName(), registration.from, registration.to), e);
}
}

Expand Down
Loading

0 comments on commit bea59cb

Please sign in to comment.