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

Add ReplacedBy attribute to identify properties that are replaced by … #8581

Merged
merged 3 commits into from Feb 26, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
Expand Up @@ -48,5 +48,13 @@ public void renderTo(DslElementDoc elementDoc, String type, Element parent) {
link.appendChild(document.createTextNode("incubating"));
para.appendChild(document.createTextNode(" and may change in a future version of Gradle."));
}
if (elementDoc.isReplaced()) {
Document document = parent.getOwnerDocument();
Element caution = document.createElement("caution");
parent.appendChild(caution);
Element para = document.createElement("para");
caution.appendChild(para);
para.appendChild(document.createTextNode(String.format("Note: This %s has been replaced by %s.", type, elementDoc.getReplacement())));
}
}
}
Expand Up @@ -46,10 +46,13 @@ public void renderTo(PropertyDoc propertyDoc, Element parent) {
Element literal = document.createElement("literal");
title.appendChild(literal);
literal.appendChild(document.createTextNode(propertyDoc.getName()));
if (!propertyDoc.getMetaData().isWriteable()) {
title.appendChild(document.createTextNode(" (read-only)"));
} else if (!propertyDoc.getMetaData().isReadable()) {
title.appendChild(document.createTextNode(" (write-only)"));

if (!propertyDoc.getMetaData().isProviderApi()) {
if (!propertyDoc.getMetaData().isWriteable()) {
title.appendChild(document.createTextNode(" (read-only)"));
} else if (!propertyDoc.getMetaData().isReadable()) {
title.appendChild(document.createTextNode(" (write-only)"));
}
}

warningsRenderer.renderTo(propertyDoc, "property", section);
Expand Down
Expand Up @@ -70,6 +70,11 @@ public void renderTo(Iterable<PropertyDoc> properties, Element parent) {
td.appendChild(caution);
caution.appendChild(document.createTextNode("Incubating"));
}
if (propDoc.isReplaced()) {
Element caution = document.createElement("caution");
td.appendChild(caution);
caution.appendChild(document.createTextNode("Replaced"));
}
td.appendChild(document.importNode(propDoc.getDescription(), true));
}
}
Expand Down
Expand Up @@ -67,6 +67,15 @@ class BlockDoc implements DslElementDoc {
return blockProperty.incubating || blockMethod.incubating
}

boolean isReplaced() {
return blockProperty.replaced
}

@Override
String getReplacement() {
return blockProperty.replacement
}

PropertyDoc getBlockProperty() {
return blockProperty
}
Expand Down
Expand Up @@ -73,6 +73,15 @@ class ClassDoc implements DslElementDoc {
return classMetaData.incubating
}

boolean isReplaced() {
return classMetaData.replaced
}

@Override
String getReplacement() {
return classMetaData.replacement
}

Collection<PropertyDoc> getClassProperties() { return classProperties }

void addClassProperty(PropertyDoc propertyDoc) {
Expand Down
Expand Up @@ -30,4 +30,8 @@ public interface DslElementDoc {
boolean isDeprecated();

boolean isIncubating();

boolean isReplaced();

String getReplacement();
}
Expand Up @@ -64,6 +64,15 @@ class MethodDoc implements DslElementDoc {
return metaData.incubating || metaData.ownerClass.incubating
}

boolean isReplaced() {
return metaData.replaced
}

@Override
String getReplacement() {
return metaData.replacement
}

Element getDescription() {
return comment.find { it.nodeName == 'para' }
}
Expand Down
Expand Up @@ -75,6 +75,15 @@ class PropertyDoc implements DslElementDoc {
return metaData.incubating || metaData.ownerClass.incubating
}

boolean isReplaced() {
return metaData.replaced
}

@Override
String getReplacement() {
return metaData.replacement
}

Element getDescription() {
return comment.find { it.nodeName == 'para' }
}
Expand Down
Expand Up @@ -28,18 +28,26 @@
import com.github.javaparser.ast.expr.AnnotationExpr;
import com.github.javaparser.ast.expr.Expression;
import com.github.javaparser.ast.expr.LiteralStringValueExpr;
import com.github.javaparser.ast.expr.SingleMemberAnnotationExpr;
import com.github.javaparser.ast.nodeTypes.NodeWithAnnotations;
import com.github.javaparser.ast.nodeTypes.NodeWithExtends;
import com.github.javaparser.ast.nodeTypes.NodeWithImplements;
import com.github.javaparser.ast.nodeTypes.NodeWithJavadoc;
import com.github.javaparser.ast.type.ClassOrInterfaceType;
import com.github.javaparser.ast.type.Type;
import com.github.javaparser.ast.visitor.VoidVisitorAdapter;
import org.gradle.build.docs.dsl.source.model.*;
import org.gradle.build.docs.dsl.source.model.AbstractLanguageElement;
import org.gradle.build.docs.dsl.source.model.ClassMetaData;
import org.gradle.build.docs.dsl.source.model.ClassMetaData.MetaType;
import org.gradle.build.docs.dsl.source.model.MethodMetaData;
import org.gradle.build.docs.dsl.source.model.PropertyMetaData;
import org.gradle.build.docs.dsl.source.model.TypeMetaData;
import org.gradle.build.docs.model.ClassMetaDataRepository;

import java.util.*;
import java.util.ArrayList;
import java.util.Deque;
import java.util.LinkedList;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

Expand Down Expand Up @@ -149,6 +157,7 @@ public void visit(MethodDeclaration methodDeclaration, ClassMetaDataRepository<C
String propName = name.substring(startName, startName + 1).toLowerCase() + name.substring(startName + 1);
PropertyMetaData property = getCurrentClass().addReadableProperty(propName, returnType, rawCommentText, methodMetaData);
methodMetaData.getAnnotationTypeNames().forEach(property::addAnnotationTypeName);
property.setReplacement(methodMetaData.getReplacement());
return;
}

Expand Down Expand Up @@ -218,6 +227,9 @@ private void extractElementTypeName(Type elementType, TypeMetaData typeMetaData)

private void findAnnotations(NodeWithAnnotations<?> node, AbstractLanguageElement currentElement) {
for (AnnotationExpr child : node.getAnnotations()) {
if (child instanceof SingleMemberAnnotationExpr && child.getNameAsString().endsWith("ReplacedBy")) {
currentElement.setReplacement(((SingleMemberAnnotationExpr) child).getMemberValue().asLiteralStringValueExpr().getValue());
}
currentElement.addAnnotationTypeName(child.getNameAsString());
}
}
Expand Down
Expand Up @@ -26,6 +26,7 @@
public abstract class AbstractLanguageElement implements LanguageElement, Serializable {
private String rawCommentText;
private final List<String> annotationNames = new ArrayList<String>();
private String replacement;

protected AbstractLanguageElement() {
}
Expand Down Expand Up @@ -58,6 +59,18 @@ public boolean isIncubating() {
return annotationNames.contains("org.gradle.api.Incubating");
}

public boolean isReplaced() {
return annotationNames.contains("org.gradle.api.ReplacedBy");
}

public String getReplacement() {
return replacement;
}

public void setReplacement(String replacement) {
this.replacement = replacement;
}

public void resolveTypes(Transformer<String, String> transformer) {
for (int i = 0; i < annotationNames.size(); i++) {
annotationNames.set(i, transformer.transform(annotationNames.get(i)));
Expand Down
Expand Up @@ -59,6 +59,11 @@ public boolean isReadable() {
return getter != null;
}

public boolean isProviderApi() {
// TODO: Crude approximation
return setter == null && (getType().getName().contains("Provider") || getType().getName().contains("Property"));
}

public ClassMetaData getOwnerClass() {
return ownerClass;
}
Expand Down
@@ -0,0 +1,48 @@
/*
* Copyright 2019 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.gradle.api;

import org.gradle.api.tasks.Internal;

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;

/**
* <p>Attached to a task property to indicate that the property has been replaced by another. Like {@link Internal}, the property is ignored during up-to-date checks.</p>
*
* <p>This annotation should be attached to the getter method in Java or the property field in Groovy. You should also consider adding {@link Deprecated} to any replaced property.</p>
*
* <p>This will cause the task <em>not</em> to be considered out-of-date when the property has changed.</p>
*
* @since 5.3
*/
@Documented
@Retention(RetentionPolicy.RUNTIME)
@Target({ElementType.METHOD, ElementType.FIELD})
@Incubating
public @interface ReplacedBy {
/**
* The Java Bean-style name of the replacement property.
* <p>
* If the property has been replaced with a method named {@code getFooBar()}, then this should be {@code fooBar}.
* </p>
*/
String value();
}
Expand Up @@ -57,7 +57,7 @@
public class DefaultTypeMetadataStore implements TypeMetadataStore {
// Avoid reflecting on classes we know we don't need to look at
private static final ImmutableSet<Class<?>> IGNORED_SUPER_CLASSES = ImmutableSet.of(
ConventionTask.class, DefaultTask.class, AbstractTask.class, Task.class, Object.class, GroovyObject.class, IConventionAware.class, ExtensionAware.class, HasConvention.class, ScriptOrigin.class, DynamicObjectAware.class
ConventionTask.class, DefaultTask.class, AbstractTask.class, Task.class, Object.class, GroovyObject.class, IConventionAware.class, ExtensionAware.class, HasConvention.class, ScriptOrigin.class, DynamicObjectAware.class
);

private static final ImmutableSet<Class<?>> IGNORED_METHODS = ImmutableSet.of(Object.class, GroovyObject.class, ScriptOrigin.class);
Expand Down Expand Up @@ -114,11 +114,11 @@ private static Multimap<Class<? extends Annotation>, Class<? extends Annotation>

private static Set<Class<? extends Annotation>> collectRelevantAnnotationTypes(Set<Class<? extends Annotation>> propertyTypeAnnotations) {
return ImmutableSet.<Class<? extends Annotation>>builder()
.addAll(propertyTypeAnnotations)
.add(Optional.class)
.add(SkipWhenEmpty.class)
.add(PathSensitive.class)
.build();
.addAll(propertyTypeAnnotations)
.add(Optional.class)
.add(SkipWhenEmpty.class)
.add(PathSensitive.class)
.build();
}

@Override
Expand Down
Expand Up @@ -29,6 +29,7 @@
import org.gradle.api.tasks.Input;
import org.gradle.api.tasks.Internal;
import org.gradle.api.tasks.OutputFile;
import org.gradle.api.ReplacedBy;
import org.gradle.internal.nativeplatform.filesystem.FileSystem;
import org.gradle.internal.reflect.Instantiator;
import org.gradle.util.GUtil;
Expand Down Expand Up @@ -107,7 +108,7 @@ private static String maybe(@Nullable String prefix, @Nullable String value) {
* @deprecated Use {@link #getArchiveFileName()}
*/
@Deprecated
@Internal("Represented as part of archiveFile")
@ReplacedBy("archiveFileName")
public String getArchiveName() {
return archiveName.get();
}
Expand Down Expand Up @@ -142,7 +143,7 @@ public Property<String> getArchiveFileName() {
* @deprecated Use {@link #getArchiveFile()}
*/
@Deprecated
@Internal("Represented as a part of the archiveFile")
@ReplacedBy("archiveFile")
public File getArchivePath() {
return getArchiveFile().get().getAsFile();
}
Expand Down Expand Up @@ -175,8 +176,8 @@ public Provider<RegularFile> getArchiveFile() {
* @return the directory
* @deprecated Use {@link #getDestinationDirectory()}
*/
@Internal("Represented as part of archiveFile")
@Deprecated
@ReplacedBy("destinationDirectory")
public File getDestinationDir() {
return archiveDestinationDirectory.getAsFile().get();
}
Expand Down Expand Up @@ -208,8 +209,8 @@ public DirectoryProperty getDestinationDirectory() {
* @deprecated Use {@link #getArchiveBaseName()}
*/
@Nullable
@Internal("Represented as part of archiveFile")
@Deprecated
@ReplacedBy("archiveBaseName")
public String getBaseName() {
return archiveBaseName.getOrNull();
}
Expand Down Expand Up @@ -242,8 +243,8 @@ public Property<String> getArchiveBaseName() {
* @deprecated Use {@link #getArchiveAppendix()}
*/
@Nullable
@Internal("Represented as part of archiveFile")
@Deprecated
@ReplacedBy("archiveAppendix")
public String getAppendix() {
return archiveAppendix.getOrNull();
}
Expand Down Expand Up @@ -276,8 +277,8 @@ public Property<String> getArchiveAppendix() {
* @deprecated Use {@link #getArchiveVersion()}
*/
@Nullable
@Internal("Represented as part of archiveFile")
@Deprecated
@ReplacedBy("archiveVersion")
public String getVersion() {
return archiveVersion.getOrNull();
}
Expand Down Expand Up @@ -308,8 +309,8 @@ public Property<String> getArchiveVersion() {
* @deprecated Use {@link #getArchiveExtension()}
*/
@Nullable
@Internal("Represented as part of archiveFile")
@Deprecated
@ReplacedBy("archiveExtension")
public String getExtension() {
return archiveExtension.getOrNull();
}
Expand Down Expand Up @@ -340,8 +341,8 @@ public Property<String> getArchiveExtension() {
* @deprecated Use {@link #getArchiveClassifier()}
*/
@Nullable
@Internal("Represented as part of archiveFile")
@Deprecated
@ReplacedBy("archiveClassifier")
public String getClassifier() {
return archiveClassifier.getOrNull();
}
Expand Down
Expand Up @@ -16,6 +16,7 @@
package org.gradle.internal.service.scopes;

import com.google.common.collect.ImmutableSet;
import org.gradle.api.ReplacedBy;
import org.gradle.api.internal.project.taskfactory.DefaultTaskClassInfoStore;
import org.gradle.api.internal.project.taskfactory.TaskClassInfoStore;
import org.gradle.api.internal.tasks.properties.InspectionScheme;
Expand Down Expand Up @@ -65,7 +66,7 @@ InspectionSchemeFactory createInspectionSchemeFactory(List<PropertyAnnotationHan

TaskScheme createTaskScheme(InspectionSchemeFactory inspectionSchemeFactory, InstantiatorFactory instantiatorFactory) {
InstantiationScheme instantiationScheme = instantiatorFactory.decorateScheme();
InspectionScheme inspectionScheme = inspectionSchemeFactory.inspectionScheme(ImmutableSet.of(Input.class, InputFile.class, InputFiles.class, InputDirectory.class, OutputFile.class, OutputFiles.class, OutputDirectory.class, OutputDirectories.class, Classpath.class, CompileClasspath.class, Destroys.class, LocalState.class, Nested.class, Inject.class, Console.class, Internal.class, OptionValues.class));
InspectionScheme inspectionScheme = inspectionSchemeFactory.inspectionScheme(ImmutableSet.of(Input.class, InputFile.class, InputFiles.class, InputDirectory.class, OutputFile.class, OutputFiles.class, OutputDirectory.class, OutputDirectories.class, Classpath.class, CompileClasspath.class, Destroys.class, LocalState.class, Nested.class, Inject.class, Console.class, ReplacedBy.class, Internal.class, OptionValues.class));
return new TaskScheme(instantiationScheme, inspectionScheme);
}

Expand All @@ -89,6 +90,10 @@ PropertyAnnotationHandler createInternalAnnotationHandler() {
return new NoOpPropertyAnnotationHandler(Internal.class);
}

PropertyAnnotationHandler createReplacedByAnnotationHandler() {
Copy link
Member

Choose a reason for hiding this comment

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

Can you also update 'ValidateTaskPropertiesIntegrationTest` to verify we don't complain about a property with this annotation.

Also, we should probably complain or fail when another input or output annotation is attached to a @ReplacedBy property, as the annotations should move to the new property.

return new NoOpPropertyAnnotationHandler(ReplacedBy.class);
}

PropertyAnnotationHandler createOptionValuesAnnotationHandler() {
return new NoOpPropertyAnnotationHandler(OptionValues.class);
}
Expand Down