Skip to content

Commit

Permalink
[WIP] Introduce InternalProblemSpec.additionalData(Class, Action)
Browse files Browse the repository at this point in the history
  • Loading branch information
donat committed Apr 25, 2024
1 parent ab7d95f commit 88e9456
Show file tree
Hide file tree
Showing 28 changed files with 763 additions and 90 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@
import java.util.Optional;
import java.util.function.Supplier;

import static org.gradle.internal.reflect.validation.DefaultTypeAwareProblemBuilder.PLUGIN_ID;

abstract public class ProblemRecordingTypeValidationContext implements TypeValidationContext {
private final Class<?> rootType;
private final Supplier<Optional<PluginId>> pluginId;
Expand Down Expand Up @@ -67,7 +65,7 @@ public void visitPropertyProblem(Action<? super TypeAwareProblemBuilder> problem
problemBuilder.withAnnotationType(rootType);
pluginId()
.map(PluginId::getId)
.ifPresent(id -> problemBuilder.additionalData(PLUGIN_ID, id));
.ifPresent(id -> problemBuilder.additionalData(DefaultTypeAwareProblemBuilder.AdditionalData.class, o -> ((DefaultTypeAwareProblemBuilder.AdditionalData) o).setPluginId(id)));
recordProblem(problemBuilder.build());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
package org.gradle.internal.reflect.validation;

import org.gradle.api.NonNullApi;
import org.gradle.api.problems.internal.AdditionalData;
import org.gradle.api.problems.internal.InternalProblemBuilder;
import org.gradle.api.problems.internal.Problem;

import javax.annotation.Nullable;
import java.util.Map;
import java.util.Optional;

import static java.lang.Boolean.TRUE;
Expand All @@ -31,11 +31,59 @@
@NonNullApi
public class DefaultTypeAwareProblemBuilder extends DelegatingProblemBuilder implements TypeAwareProblemBuilder {

public static final String TYPE_NAME = "typeName";
public static final String PLUGIN_ID = "pluginId";
public static final String PARENT_PROPERTY_NAME = "parentPropertyName";
public static final String PROPERTY_NAME = "propertyName";
public static final String TYPE_IS_IRRELEVANT_IN_ERROR_MESSAGE = "typeIsIrrelevantInErrorMessage";
@NonNullApi
public static class AdditionalData implements org.gradle.api.problems.internal.AdditionalData { // TODO (donat) this should be extracted to a separate file
private String pluginId;
private String isIrrelevantInErrorMessage;
private String propertyName;
private String parentPropertyName;
private String typeName;

@Nullable
public String getPluginId() {
return pluginId;
}

@Nullable
public String getIsIrrelevantInErrorMessage() {
return isIrrelevantInErrorMessage;
}

@Nullable
public String getPropertyName() {
return propertyName;
}

@Nullable
public String getParentPropertyName() {
return parentPropertyName;
}

@Nullable
public String getTypeName() {
return typeName;
}

public void setPluginId(@Nullable String pluginId) {
this.pluginId = pluginId;
}

public void setIsIrrelevantInErrorMessage(@Nullable String isIrrelevantInErrorMessage) {
this.isIrrelevantInErrorMessage = isIrrelevantInErrorMessage;
}

public void setPropertyName(@Nullable String propertyName) {
this.propertyName = propertyName;
}

public void setParentPropertyName(@Nullable String parentPropertyName) {
this.parentPropertyName = parentPropertyName;
}

public void setTypeName(@Nullable String typeName) {
this.typeName = typeName;
}
}

public DefaultTypeAwareProblemBuilder(InternalProblemBuilder problemBuilder) {
super(problemBuilder);
Expand All @@ -44,20 +92,20 @@ public DefaultTypeAwareProblemBuilder(InternalProblemBuilder problemBuilder) {
@Override
public TypeAwareProblemBuilder withAnnotationType(@Nullable Class<?> classWithAnnotationAttached) {
if (classWithAnnotationAttached != null) {
additionalData(TYPE_NAME, classWithAnnotationAttached.getName().replaceAll("\\$", "."));
additionalData(AdditionalData.class, o -> ((AdditionalData) o).setTypeName(classWithAnnotationAttached.getName().replaceAll("\\$", ".")));
}
return this;
}

@Override
public TypeAwareProblemBuilder typeIsIrrelevantInErrorMessage() {
additionalData(TYPE_IS_IRRELEVANT_IN_ERROR_MESSAGE, TRUE.toString());
additionalData(AdditionalData.class, o -> ((AdditionalData) o).setIsIrrelevantInErrorMessage(TRUE.toString()));
return this;
}

@Override
public TypeAwareProblemBuilder forProperty(String propertyName) {
additionalData(PROPERTY_NAME, propertyName);
additionalData(AdditionalData.class, o -> ((AdditionalData) o).setPropertyName(propertyName));
return this;
}

Expand All @@ -67,29 +115,32 @@ public TypeAwareProblemBuilder parentProperty(@Nullable String parentProperty) {
return this;
}
String pp = getParentProperty(parentProperty);
additionalData(PARENT_PROPERTY_NAME, pp);
additionalData(AdditionalData.class, o -> ((AdditionalData) o).setParentPropertyName(pp));
parentPropertyAdditionalData = pp;
return this;
}

@Override
public Problem build() {
// TODO (donat) TypeAwareProblemBuilder should have a know additionalData type to avoid unchecked casts
Problem problem = super.build();
String prefix = introductionFor(problem.getAdditionalData());
@SuppressWarnings("unchecked")
AdditionalData additionalData = (AdditionalData) Optional.ofNullable(problem.getAdditionalData()).orElseGet(AdditionalData::new);
String prefix = introductionFor(additionalData);
String text = Optional.ofNullable(problem.getContextualLabel()).orElseGet(() -> problem.getDefinition().getId().getDisplayName());
return problem.toBuilder().contextualLabel(prefix + text).build();
}

public static String introductionFor(Map<String, Object> additionalMetadata) {
Optional<String> rootType = ofNullable(additionalMetadata.get(TYPE_NAME))
public static String introductionFor(AdditionalData additionalMetadata) {
Optional<String> rootType = ofNullable(additionalMetadata.getTypeName())
.map(Object::toString)
.filter(DefaultTypeAwareProblemBuilder::shouldRenderType);
Optional<DefaultPluginId> pluginId = ofNullable(additionalMetadata.get(PLUGIN_ID))
Optional<DefaultPluginId> pluginId = ofNullable(additionalMetadata.getPluginId())
.map(Object::toString)
.map(DefaultPluginId::new);

StringBuilder builder = new StringBuilder();
boolean typeRelevant = rootType.isPresent() && !parseBoolean(additionalMetadata.getOrDefault(TYPE_IS_IRRELEVANT_IN_ERROR_MESSAGE, "").toString());
boolean typeRelevant = rootType.isPresent() && !parseBoolean(Optional.ofNullable(additionalMetadata.isIrrelevantInErrorMessage).orElse("").toString());
if (typeRelevant) {
if (pluginId.isPresent()) {
builder.append("In plugin '")
Expand All @@ -101,7 +152,7 @@ public static String introductionFor(Map<String, Object> additionalMetadata) {
builder.append(rootType.get()).append("' ");
}

Object property = additionalMetadata.get(PROPERTY_NAME);
Object property = additionalMetadata.getPropertyName();
if (property != null) {
if (typeRelevant) {
builder.append("property '");
Expand All @@ -114,7 +165,7 @@ public static String introductionFor(Map<String, Object> additionalMetadata) {
builder.append("Property '");
}
}
ofNullable(additionalMetadata.get(PARENT_PROPERTY_NAME)).ifPresent(parentProperty -> {
ofNullable(additionalMetadata.getParentPropertyName()).ifPresent(parentProperty -> {
builder.append(parentProperty);
builder.append('.');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@

package org.gradle.internal.reflect.validation;

import org.gradle.api.Action;
import org.gradle.api.NonNullApi;
import org.gradle.api.problems.ProblemGroup;
import org.gradle.api.problems.Severity;
import org.gradle.api.problems.internal.AdditionalData;
import org.gradle.api.problems.internal.DocLink;
import org.gradle.api.problems.internal.InternalProblemBuilder;
import org.gradle.api.problems.internal.Problem;
Expand Down Expand Up @@ -115,8 +117,8 @@ public InternalProblemBuilder taskPathLocation(String buildTreePath) {
}

@Override
public InternalProblemBuilder additionalData(String key, Object value) {
return validateDelegate(delegate.additionalData(key, value));
public InternalProblemBuilder additionalData(Class<? extends AdditionalData> type, Action<Object> config) {
return validateDelegate(delegate.additionalData(type, config));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,30 +16,29 @@

package org.gradle.internal.reflect.validation

import com.google.common.collect.ImmutableMap
import spock.lang.Specification

import static DefaultTypeAwareProblemBuilder.PROPERTY_NAME
import static DefaultTypeAwareProblemBuilder.TYPE_IS_IRRELEVANT_IN_ERROR_MESSAGE
import static DefaultTypeAwareProblemBuilder.TYPE_NAME
import static java.lang.Boolean.TRUE
import spock.lang.Specification

class DefaultTypeAwareProblemBuilderTest extends Specification {

def "render introduction without type"() {
given:
def result = DefaultTypeAwareProblemBuilder.introductionFor ImmutableMap.of(TYPE_IS_IRRELEVANT_IN_ERROR_MESSAGE, TRUE.toString(),
TYPE_NAME, "foo", PROPERTY_NAME, "bar")
DefaultTypeAwareProblemBuilder.AdditionalData additionalData = new DefaultTypeAwareProblemBuilder.AdditionalData()
additionalData.setIsIrrelevantInErrorMessage("true")
additionalData.setTypeName("foo")
additionalData.setPropertyName("bar")

expect:
result == "Property 'bar' "
DefaultTypeAwareProblemBuilder.introductionFor(additionalData) == "Property 'bar' "
}

def "render introduction with type"() {
given:
def result = DefaultTypeAwareProblemBuilder.introductionFor ImmutableMap.of(TYPE_NAME, "foo", PROPERTY_NAME, "bar")
DefaultTypeAwareProblemBuilder.AdditionalData additionalData = new DefaultTypeAwareProblemBuilder.AdditionalData()
additionalData.setTypeName("foo")
additionalData.setPropertyName("bar")

expect:
result == "Type 'foo' property 'bar' "
DefaultTypeAwareProblemBuilder.introductionFor(additionalData) == "Type 'foo' property 'bar' "
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* Copyright 2024 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.internal.deprecation;

import org.gradle.api.problems.internal.AdditionalData;

public class DeprecationProblemsApiAdditionalData implements AdditionalData {

private DeprecatedFeatureUsage.Type type;

public DeprecatedFeatureUsage.Type getType() {
return type;
}

public void setType(DeprecatedFeatureUsage.Type type) {
this.type = type;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.gradle.api.problems.internal.Problem;
import org.gradle.internal.SystemProperties;
import org.gradle.internal.deprecation.DeprecatedFeatureUsage;
import org.gradle.internal.deprecation.DeprecationProblemsApiAdditionalData;
import org.gradle.internal.logging.LoggingConfigurationBuildOptions;
import org.gradle.internal.operations.BuildOperationProgressEventEmitter;
import org.gradle.internal.problems.NoOpProblemDiagnosticsFactory;
Expand Down Expand Up @@ -103,7 +104,12 @@ public void execute(InternalProblemSpec builder) {
.contextualLabel(usage.getSummary())
.details(usage.getRemovalDetails())
.documentedAt(usage.getDocumentationUrl())
.additionalData("type", usage.getType().name()) // TODO we should use a custom type here
.additionalData(DeprecationProblemsApiAdditionalData.class, new Action<Object>() {
@Override
public void execute(Object o) {
((DeprecationProblemsApiAdditionalData) o).setType(usage.getType());
}
})
.severity(WARNING);

addPossibleLocation(diagnostics, problemSpec);
Expand Down

0 comments on commit 88e9456

Please sign in to comment.