Fix some ErrorProne issues in :model-core

platforms/core-configuration/model-core/build.gradle.kts

@@ -7,23 +7,12 @@ description = "Implementation of configuration model types and annotation metada
 
 errorprone {
     disabledChecks.addAll(
-        "AnnotateFormatMethod", // 1 occurrences
-        "EmptyBlockTag", // 3 occurrences
-        "FormatString", // 1 occurrences
+        "AnnotateFormatMethod", // 1 occurrence, needs errorprone annotations
         "GetClassOnEnum", // 4 occurrences
-        "HidingField", // 1 occurrences
-        "IdentityHashMapUsage", // 1 occurrences
         "ImmutableEnumChecker", // 1 occurrences
-        "InvalidParam", // 1 occurrences
-        "MixedMutabilityReturnType", // 4 occurrences
-        "MutablePublicArray", // 1 occurrences
-        "OperatorPrecedence", // 5 occurrences
         "ReferenceEquality", // 3 occurrences
-        "StringCaseLocaleUsage", // 13 occurrences
-        "TypeParameterShadowing", // 2 occurrences
         "UndefinedEquals", // 2 occurrences
         "UnusedMethod", // 8 occurrences
-        "UnusedTypeParameter", // 1 occurrences
         "UnusedVariable", // 20 occurrences
     )
 }

platforms/core-configuration/model-core/src/main/java/org/gradle/api/internal/model/NamedObjectInstantiator.java

@@ -275,7 +275,7 @@ private void visitFields(Class<?> type, ValidationProblemCollector collector) {
         // Disallow instance fields. This doesn't guarantee that the object is immutable, just makes it less likely
         // We might tighten this constraint to also disallow any _code_ on immutable types that reaches out to static state
         for (Field field : type.getDeclaredFields()) {
-            if (Modifier.isStatic(field.getModifiers()) || GroovyObject.class.isAssignableFrom(type) && field.getName().equals("metaClass")) {
+            if (Modifier.isStatic(field.getModifiers()) || (GroovyObject.class.isAssignableFrom(type) && field.getName().equals("metaClass"))) {
                 continue;
             }
             collector.add(field, "A Named implementation class must not define any instance fields.");

platforms/core-configuration/model-core/src/main/java/org/gradle/api/internal/provider/AbstractCollectionProperty.java

@@ -30,6 +30,7 @@
 import org.gradle.api.provider.Provider;
 import org.gradle.api.provider.SupportsConvention;
 import org.gradle.internal.Cast;
+import org.gradle.util.internal.TextUtil;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
@@ -104,7 +105,7 @@ private CollectionSupplier<T, C> emptySupplier() {
     }
 
     private CollectionSupplier<T, C> noValueSupplier() {
-        return Cast.uncheckedCast(new NoValueSupplier<>(Value.missing()));
+        return new NoValueSupplier(Value.missing());
     }
 
     /**
@@ -127,7 +128,9 @@ protected boolean isDefaultConvention() {
     }
 
     private boolean isNoValueSupplier(CollectionSupplier<T, C> valueSupplier) {
-        return valueSupplier instanceof NoValueSupplier;
+        // Cannot use plain NoValueSupplier because of Java restrictions:
+        // a generic type [AbstractCollectionProperty<T, C>.]NoValueSupplier cannot be used in instanceof.
+        return valueSupplier instanceof AbstractCollectionProperty<?, ?>.NoValueSupplier;
     }
 
     @Override
@@ -192,7 +195,7 @@ public int size() {
     /**
      * Adds the given supplier as the new root supplier for this collection.
      *
-     * @param collector
+     * @param collector the collector to add
      * @param ignoreAbsent whether elements that are missing values should be ignored
      */
     private void addExplicitCollector(Collector<T> collector, boolean ignoreAbsent) {
@@ -309,7 +312,7 @@ protected CollectionSupplier<T, C> finalValue(EvaluationContext.ScopeContext con
         } else if (result.getPathToOrigin().isEmpty()) {
             return noValueSupplier();
         } else {
-            return new NoValueSupplier<>(result);
+            return new NoValueSupplier(result);
         }
     }
 
@@ -336,10 +339,11 @@ public HasMultipleValues<T> convention(Provider<? extends Iterable<? extends T>>
 
     @Override
     protected String describeContents() {
-        return String.format("%s(%s, %s)", collectionType.getSimpleName().toLowerCase(), elementType, describeValue());
+        String typeDisplayName = TextUtil.toLowerCaseLocaleSafe(collectionType.getSimpleName());
+        return String.format("%s(%s, %s)", typeDisplayName, elementType, describeValue());
     }
 
-    class NoValueSupplier<T, C extends Collection<? extends T>> implements CollectionSupplier<T, C> {
+    class NoValueSupplier implements CollectionSupplier<T, C> {
         private final Value<? extends C> value;
 
         public NoValueSupplier(Value<? extends C> value) {

platforms/core-configuration/model-core/src/main/java/org/gradle/api/internal/provider/Collectors.java

@@ -277,7 +277,7 @@ public Value<Void> collectEntries(ValueConsumer consumer, ValueCollector<T> coll
             return collectEntriesFromValue(collector, collection, value);
         }
 
-        private <T> ValueSupplier.Value<Void> collectEntriesFromValue(ValueCollector<T> collector, ImmutableCollection.Builder<T> collection, ValueSupplier.Value<? extends Iterable<? extends T>> value) {
+        private ValueSupplier.Value<Void> collectEntriesFromValue(ValueCollector<T> collector, ImmutableCollection.Builder<T> collection, ValueSupplier.Value<? extends Iterable<? extends T>> value) {
             if (value.isMissing()) {
                 return ignoreAbsent ? Value.present() : value.asType();
             }

platforms/core-configuration/model-core/src/main/java/org/gradle/api/internal/provider/DefaultMapProperty.java

@@ -324,7 +324,7 @@ public void replace(Transformer<? extends @org.jetbrains.annotations.Nullable Pr
 
     @Override
     protected String describeContents() {
-        return String.format("Map(%s->%s, %s)", keyType.getSimpleName().toLowerCase(), valueType.getSimpleName(), describeValue());
+        return String.format("Map(%s->%s, %s)", keyType.getSimpleName(), valueType.getSimpleName(), describeValue());
     }
 
     @Override

platforms/core-configuration/model-core/src/main/java/org/gradle/api/internal/provider/DefaultProperty.java

@@ -28,7 +28,7 @@
  * The implementation for general-purpose (atomic, non-composite) properties, where
  * the value is supplied by some provider.
  *
- * @param <T>
+ * @param <T> the type of the property value
  */
 public class DefaultProperty<T> extends AbstractProperty<T, ProviderInternal<? extends T>> implements Property<T> {
     private final Class<T> type;

platforms/core-configuration/model-core/src/main/java/org/gradle/api/internal/provider/ValueState.java

@@ -79,7 +79,7 @@ public static <S> ValueState<S> newState(PropertyHost host, Function<S, S> copie
     /**
      * Marks this value state as being explicitly assigned. Does not remember the given value in any way.
      *
-     * @param value
+     * @param value the new explicitly assigned value
      * @return the very <code>value</code> given
      */
     //TODO-RC rename this or the overload as they have significantly different semantics
@@ -92,8 +92,8 @@ public static <S> ValueState<S> newState(PropertyHost host, Function<S, S> copie
      * A default value is a fallback value that is sensible to the caller, in the absence of the explicit value.
      * The default value is not related in any way to the convention value.
      *
-     * @param value
-     * @param defaultValue
+     * @param value the current explicit value
+     * @param defaultValue the default value
      * @return the given value, if this value state is not explicit, or given default value
      */
     //TODO-RC rename this or the overload as they have significantly different semantics
@@ -106,8 +106,8 @@ public static <S> ValueState<S> newState(PropertyHost host, Function<S, S> copie
      *
      * This is similar to calling {@link #setConvention(Object)} followed by {@link #explicitValue(Object, Object)}.
      *
-     * @param value
-     * @param convention
+     * @param value the current explicit value
+     * @param convention the new convention
      * @return the given value, if this value state is not explicit, otherwise the new convention value
      */
     public abstract S applyConvention(S value, S convention);

platforms/core-configuration/model-core/src/main/java/org/gradle/internal/extensibility/ConventionAwareHelper.java

@@ -83,7 +83,8 @@ private MappedProperty map(String propertyName, MappedPropertyImpl mapping) {
                     throw new IllegalStateException(String.format("Could not access property %s.%s", sourceType.getSimpleName(), propertyName), e);
                 }
                 if (!mapConventionOn(target, mapping)) {
-                    throw new IllegalStateException(String.format("Unexpected convention-supporting type used in property %s.%s", sourceType.getSimpleName(), propertyName, getter.getReturnType().getName()));
+                    throw new IllegalStateException(String.format(
+                        "Unexpected convention-supporting type %s used in property %s.%s", getter.getReturnType().getName(), sourceType.getSimpleName(), propertyName));
                 }
             } else {
                 throw DocumentedFailure.builder()

platforms/core-configuration/model-core/src/main/java/org/gradle/internal/extensibility/DefaultConvention.java

@@ -54,7 +54,7 @@ public class DefaultConvention implements org.gradle.api.plugins.Convention, Ext
     private final InstanceGenerator instanceGenerator;
 
     private Map<String, Object> plugins;
-    private Map<Object, BeanDynamicObject> dynamicObjects;
+    private IdentityHashMap<Object, BeanDynamicObject> dynamicObjects;
 
     public DefaultConvention(InstanceGenerator instanceGenerator) {
         this.instanceGenerator = instanceGenerator;

platforms/core-configuration/model-core/src/main/java/org/gradle/internal/instantiation/generator/AbstractClassGenerator.java

@@ -1198,8 +1198,8 @@ void visitProperty(PropertyMetadata property) {
             // For ConfigurableFileCollection we generate setters just for readonly properties,
             // since we want to support += for mutable FileCollection properties, but we don't support += for ConfigurableFileCollection (yet).
             // And if we generate setter override for ConfigurableFileCollection, it's difficult to distinguish between these two cases in setFromAnyValue method.
-            if (property.isReadable() && hasPropertyType(property) ||
-                property.isReadOnly() && isConfigurableFileCollectionType(property.getType())) {
+            if ((property.isReadable() && hasPropertyType(property)) ||
+                (property.isReadOnly() && isConfigurableFileCollectionType(property.getType()))) {
                 lazyGroovySupportTyped.add(property);
             }
         }

platforms/core-configuration/model-core/src/main/java/org/gradle/internal/isolated/IsolationScheme.java

@@ -167,8 +167,8 @@ private Type unwrapTypeVariable(Type type) {
     }
 
     private static boolean isAssignableFromType(Class<?> clazz, Type type) {
-        return type instanceof Class && clazz.isAssignableFrom((Class<?>) type)
-            || type instanceof ParameterizedType && clazz.isAssignableFrom((Class<?>) ((ParameterizedType) type).getRawType());
+        return (type instanceof Class && clazz.isAssignableFrom((Class<?>) type))
+            || (type instanceof ParameterizedType && clazz.isAssignableFrom((Class<?>) ((ParameterizedType) type).getRawType()));
     }
 
     /**

platforms/core-configuration/model-core/src/main/java/org/gradle/internal/metaobject/BeanDynamicObject.java

@@ -471,6 +471,8 @@ protected DynamicInvokeResult setOpaqueProperty(MetaClass metaClass, String name
             return DynamicInvokeResult.notFound();
         }
 
+        @SuppressWarnings("MixedMutabilityReturnType")
+        // This might be too invasive to fix properly because it is in the dynamic code.
         public Map<String, ?> getProperties() {
             if (!includeProperties) {
                 return Collections.emptyMap();

platforms/core-configuration/model-core/src/main/java/org/gradle/internal/properties/annotations/DefaultTypeMetadataStore.java

@@ -120,7 +120,7 @@ private <T> TypeMetadata createTypeMetadata(Class<T> type) {
                         .forProperty(propertyAnnotationMetadata.getPropertyName())
                         .id(TextUtil.screamingSnakeToKebabCase(ANNOTATION_INVALID_IN_CONTEXT), "Invalid annotation in context", GradleCoreProblemGroup.validation().property())
                         .contextualLabel(String.format("is annotated with invalid property type @%s", propertyType.getSimpleName()))
-                        .documentedAt(userManual("validation_problems", ANNOTATION_INVALID_IN_CONTEXT.toLowerCase()))
+                        .documentedAt(userManual("validation_problems", TextUtil.toLowerCaseLocaleSafe(ANNOTATION_INVALID_IN_CONTEXT)))
                         .severity(ERROR)
                         .details("The '@" + propertyType.getSimpleName() + "' annotation cannot be used in this context")
                         .solution("Remove the property")
@@ -142,7 +142,7 @@ private <T> TypeMetadata createTypeMetadata(Class<T> type) {
                             .forProperty(propertyAnnotationMetadata.getPropertyName())
                             .id(TextUtil.screamingSnakeToKebabCase(INCOMPATIBLE_ANNOTATIONS), "Incompatible annotations", GradleCoreProblemGroup.validation().property())
                             .contextualLabel("is annotated with @" + annotationType.getSimpleName() + " but that is not allowed for '" + propertyType.getSimpleName() + "' properties")
-                            .documentedAt(userManual("validation_problems", INCOMPATIBLE_ANNOTATIONS.toLowerCase()))
+                            .documentedAt(userManual("validation_problems", TextUtil.toLowerCaseLocaleSafe(INCOMPATIBLE_ANNOTATIONS)))
                             .severity(ERROR)
                             .details("This modifier is used in conjunction with a property of type '" + propertyType.getSimpleName() + "' but this doesn't have semantics")
                             .solution("Remove the '@" + annotationType.getSimpleName() + "' annotation"));
@@ -152,7 +152,7 @@ private <T> TypeMetadata createTypeMetadata(Class<T> type) {
                             .forProperty(propertyAnnotationMetadata.getPropertyName())
                             .id(TextUtil.screamingSnakeToKebabCase(ANNOTATION_INVALID_IN_CONTEXT), "Invalid annotation in context", GradleCoreProblemGroup.validation().property())
                             .contextualLabel(String.format("is annotated with invalid modifier @%s", annotationType.getSimpleName()))
-                            .documentedAt(userManual("validation_problems", ANNOTATION_INVALID_IN_CONTEXT.toLowerCase()))
+                            .documentedAt(userManual("validation_problems", TextUtil.toLowerCaseLocaleSafe(ANNOTATION_INVALID_IN_CONTEXT)))
                             .severity(ERROR)
                             .details("The '@" + annotationType.getSimpleName() + "' annotation cannot be used in this context")
                             .solution("Use a different annotation, e.g one of " + toListOfAnnotations(allowedPropertyModifiers))

platforms/core-configuration/model-core/src/main/java/org/gradle/internal/properties/annotations/MissingPropertyAnnotationHandler.java

@@ -39,7 +39,7 @@ public interface MissingPropertyAnnotationHandler {
             .forProperty(annotationMetadata.getPropertyName())
             .id(TextUtil.screamingSnakeToKebabCase(missingAnnotation), "Missing annotation", GradleCoreProblemGroup.validation().property())
             .contextualLabel("is missing " + displayName)
-            .documentedAt(userManual("validation_problems", missingAnnotation.toLowerCase()))
+            .documentedAt(userManual("validation_problems", TextUtil.toLowerCaseLocaleSafe(missingAnnotation)))
             .severity(ERROR)
             .details("A property without annotation isn't considered during up-to-date checking")
             .solution("Add " + displayName)