Forbid wildcard imports (opensearch-project#1180)

* Forbid wildcard imports Needs to be implemented as an throwing custom step as Spotless doesn't have built in support to remove/deny wildcards: diffplug/spotless#649 Signed-off-by: Thomas Farr <[email protected]> * Make conventions plugin Signed-off-by: Thomas Farr <[email protected]> * Fix wildcard imports Signed-off-by: Thomas Farr <[email protected]> * Allow overriding eclipse formatter config file Signed-off-by: Thomas Farr <[email protected]> * Fix message formatting Signed-off-by: Thomas Farr <[email protected]> --------- Signed-off-by: Thomas Farr <[email protected]>
Xtansia · Sep 4, 2024 · e856ecb · e856ecb
1 parent 17c94e0
commit e856ecb
Show file tree

Hide file tree

Showing 12 changed files with 114 additions and 69 deletions.
diff --git a/build.gradle.kts b/build.gradle.kts
@@ -40,7 +40,7 @@ allprojects {
         mavenLocal()
         maven(url = "https://aws.oss.sonatype.org/content/repositories/snapshots")
         mavenCentral()
-        maven(url = "https://plugins.gradle.org/m2/")
+        gradlePluginPortal()
     }
 }
 

diff --git a/buildSrc/build.gradle.kts b/buildSrc/build.gradle.kts
@@ -30,13 +30,19 @@
  * GitHub history for details.
  */
 
-dependencies {
-    implementation("org.ajoberstar.grgit:grgit-gradle:5.2.2")
+plugins {
+    `kotlin-dsl`
 }
 
 repositories {
     mavenLocal()
     maven(url = "https://aws.oss.sonatype.org/content/repositories/snapshots")
     mavenCentral()
-    maven(url = "https://plugins.gradle.org/m2/")
+    gradlePluginPortal()
 }
+
+dependencies {
+    implementation("org.ajoberstar.grgit:grgit-gradle:5.2.2")
+    implementation("com.diffplug.spotless", "spotless-plugin-gradle", "6.25.0")
+}
+
diff --git a/buildSrc/src/main/kotlin/opensearch-java.spotless-conventions.gradle.kts b/buildSrc/src/main/kotlin/opensearch-java.spotless-conventions.gradle.kts
@@ -0,0 +1,58 @@
+plugins {
+    id("com.diffplug.spotless")
+}
+
+interface SpotlessConventionsPluginExtension {
+    val eclipseFormatterConfigFile: RegularFileProperty
+}
+
+val extension = project.extensions.create<SpotlessConventionsPluginExtension>("spotlessConventions")
+
+extension.eclipseFormatterConfigFile.convention(rootProject.layout.projectDirectory.file("buildSrc/formatterConfig.xml"))
+
+spotless {
+    java {
+        target("**/*.java")
+
+        licenseHeaderFile(rootProject.file("LICENSE_HEADER.txt"))
+            .named("PrimaryLicenseHeader")
+            .onlyIfContentMatches("^((?!Licensed to Elasticsearch)[\\s\\S])*$")
+            .delimiter("(package |//-----)")
+
+        licenseHeaderFile(rootProject.file("LICENSE_HEADER_FORKED.txt"))
+            .named("ForkedLicenseHeader")
+            .onlyIfContentMatches("Licensed to Elasticsearch")
+            .delimiter("(package |//-----)")
+
+        // Use the default importOrder configuration
+        importOrder()
+        removeUnusedImports()
+
+        eclipse().configFile(extension.eclipseFormatterConfigFile)
+
+        trimTrailingWhitespace()
+        endWithNewline()
+
+        // NOTE: Any time a custom step below is modified, bump this number.
+        // Allows up-to-date checks to work correctly with custom steps.
+        bumpThisNumberIfACustomStepChanges(1)
+
+        val wildcardImportRegex = Regex("""^import\s+(?:static\s+)?[^*\s]+\.\*;$""", RegexOption.MULTILINE)
+        custom("Refuse wildcard imports") { contents ->
+            // Wildcard imports can't be resolved by spotless itself.
+            // This will require the developer themselves to adhere to best practices.
+            val wildcardImports = wildcardImportRegex.findAll(contents)
+            if (wildcardImports.any()) {
+                var msg = """
+                    Please replace the following wildcard imports with explicit imports ('spotlessApply' cannot resolve this issue):
+                """.trimIndent()
+                wildcardImports.forEach {
+                    msg += "\n\t- ${it.value}"
+                }
+                msg += "\n"
+                throw AssertionError(msg)
+            }
+            contents
+        }
+    }
+}
diff --git a/java-client/build.gradle.kts b/java-client/build.gradle.kts
@@ -40,7 +40,7 @@ buildscript {
         mavenLocal()
         maven(url = "https://aws.oss.sonatype.org/content/repositories/snapshots")
         mavenCentral()
-        maven(url = "https://plugins.gradle.org/m2/")
+        gradlePluginPortal()
     }
     dependencies {
         "classpath"(group = "org.opensearch.gradle", name = "build-tools", version = "3.0.0-SNAPSHOT")
@@ -53,7 +53,8 @@ plugins {
     `maven-publish`
     id("com.github.jk1.dependency-license-report") version "2.9"
     id("org.owasp.dependencycheck") version "10.0.4"
-    id("com.diffplug.spotless") version "6.25.0"
+
+    id("opensearch-java.spotless-conventions")
 }
 apply(plugin = "opensearch.repositories")
 apply(plugin = "org.owasp.dependencycheck")
@@ -299,31 +300,6 @@ tasks.withType<Jar> {
     }
 }
 
-spotless {
-    java {
-        target("**/*.java")
-
-        licenseHeaderFile("../LICENSE_HEADER.txt")
-            .named("PrimaryLicenseHeader")
-            .onlyIfContentMatches("^((?!Licensed to Elasticsearch)[\\s\\S])*$")
-            .delimiter("(package |//-----)")
-
-        licenseHeaderFile("../LICENSE_HEADER_FORKED.txt")
-            .named("ForkedLicenseHeader")
-            .onlyIfContentMatches("Licensed to Elasticsearch")
-            .delimiter("(package |//-----)")
-
-        // Use the default importOrder configuration
-        importOrder()
-        removeUnusedImports()
-
-        eclipse().configFile("../buildSrc/formatterConfig.xml")
-
-        trimTrailingWhitespace()
-        endWithNewline()
-    }
-}
-
 publishing {
     repositories{
         if (version.toString().endsWith("SNAPSHOT")) {

diff --git a/...ain/java/org/opensearch/client/opensearch/_types/mapping/IcuCollationKeywordProperty.java b/...ain/java/org/opensearch/client/opensearch/_types/mapping/IcuCollationKeywordProperty.java
@@ -11,7 +11,11 @@
 import jakarta.json.stream.JsonGenerator;
 import java.util.function.Function;
 import javax.annotation.Nullable;
-import org.opensearch.client.json.*;
+import org.opensearch.client.json.JsonpDeserializable;
+import org.opensearch.client.json.JsonpDeserializer;
+import org.opensearch.client.json.JsonpMapper;
+import org.opensearch.client.json.ObjectBuilderDeserializer;
+import org.opensearch.client.json.ObjectDeserializer;
 import org.opensearch.client.opensearch._types.analysis.IcuCollationAlternate;
 import org.opensearch.client.opensearch._types.analysis.IcuCollationCaseFirst;
 import org.opensearch.client.opensearch._types.analysis.IcuCollationDecomposition;

diff --git a/...ent/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2GetRequestIT.java b/...ent/src/test/java/org/opensearch/client/opensearch/integTest/aws/AwsSdk2GetRequestIT.java
@@ -13,7 +13,8 @@
 import org.junit.Test;
 import org.opensearch.client.opensearch.OpenSearchAsyncClient;
 import org.opensearch.client.opensearch.OpenSearchClient;
-import org.opensearch.client.opensearch.core.*;
+import org.opensearch.client.opensearch.core.GetRequest;
+import org.opensearch.client.opensearch.core.GetResponse;
 
 public class AwsSdk2GetRequestIT extends AwsSdk2TransportTestCase {
     @Test

diff --git a/java-codegen/build.gradle.kts b/java-codegen/build.gradle.kts
@@ -21,7 +21,7 @@ buildscript {
         mavenLocal()
         maven(url = "https://aws.oss.sonatype.org/content/repositories/snapshots")
         mavenCentral()
-        maven(url = "https://plugins.gradle.org/m2/")
+        gradlePluginPortal()
     }
     dependencies {
         "classpath"(group = "org.opensearch.gradle", name = "build-tools", version = "3.0.0-SNAPSHOT")
@@ -32,8 +32,9 @@ plugins {
     application
     id("com.github.jk1.dependency-license-report") version "2.9"
     id("org.owasp.dependencycheck") version "10.0.2"
-    id("com.diffplug.spotless") version "6.25.0"
     id("de.undercouch.download") version "5.6.0"
+
+    id("opensearch-java.spotless-conventions")
 }
 apply(plugin = "opensearch.repositories")
 apply(plugin = "org.owasp.dependencycheck")
@@ -259,19 +260,6 @@ tasks.withType<Jar> {
     }
 }
 
-spotless {
-    java {
-        target("**/*.java")
-
-        licenseHeaderFile("../LICENSE_HEADER.txt")
-
-        // Use the default importOrder configuration
-        importOrder()
-        removeUnusedImports()
-
-        eclipse().configFile("../buildSrc/formatterConfig-generated.xml")
-
-        trimTrailingWhitespace()
-        endWithNewline()
-    }
+spotlessConventions {
+    eclipseFormatterConfigFile = rootProject.file("buildSrc/formatterConfig-generated.xml")
 }
diff --git a/java-codegen/src/main/java/org/opensearch/client/codegen/CodeGenerator.java b/java-codegen/src/main/java/org/opensearch/client/codegen/CodeGenerator.java
@@ -8,7 +8,11 @@
 
 package org.opensearch.client.codegen;
 
-import static org.opensearch.client.codegen.model.OperationGroupMatcher.*;
+import static org.opensearch.client.codegen.model.OperationGroupMatcher.and;
+import static org.opensearch.client.codegen.model.OperationGroupMatcher.named;
+import static org.opensearch.client.codegen.model.OperationGroupMatcher.namespace;
+import static org.opensearch.client.codegen.model.OperationGroupMatcher.not;
+import static org.opensearch.client.codegen.model.OperationGroupMatcher.or;
 
 import java.io.File;
 import java.io.IOException;

diff --git a/samples/build.gradle.kts b/samples/build.gradle.kts
@@ -9,7 +9,8 @@
 plugins {
     java
     application
-    id("com.diffplug.spotless") version "6.25.0"
+
+    id("opensearch-java.spotless-conventions")
 }
 
 java {
@@ -26,22 +27,6 @@ dependencies {
     implementation("com.fasterxml.jackson.core", "jackson-databind", "2.15.2")
 }
 
-spotless {
-  java {
-
-    target("**/*.java")
-
-    // Use the default importOrder configuration
-    importOrder()
-    removeUnusedImports()
-
-    eclipse().configFile("../buildSrc/formatterConfig.xml")
-
-    trimTrailingWhitespace()
-    endWithNewline()
-  }
-}
-
 application {
     mainClass.set("org.opensearch.client.samples.Main")
 }

diff --git a/samples/src/main/java/org/opensearch/client/samples/FlatObjectBasics.java b/samples/src/main/java/org/opensearch/client/samples/FlatObjectBasics.java
@@ -1,3 +1,11 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
 package org.opensearch.client.samples;
 
 import java.util.List;

diff --git a/samples/src/main/java/org/opensearch/client/samples/IndexTemplates.java b/samples/src/main/java/org/opensearch/client/samples/IndexTemplates.java
@@ -13,7 +13,14 @@
 import org.apache.logging.log4j.Logger;
 import org.opensearch.client.opensearch._types.Time;
 import org.opensearch.client.opensearch.cluster.PutComponentTemplateRequest;
-import org.opensearch.client.opensearch.indices.*;
+import org.opensearch.client.opensearch.indices.CreateIndexRequest;
+import org.opensearch.client.opensearch.indices.DeleteIndexRequest;
+import org.opensearch.client.opensearch.indices.DeleteIndexTemplateRequest;
+import org.opensearch.client.opensearch.indices.GetIndicesSettingsRequest;
+import org.opensearch.client.opensearch.indices.GetIndicesSettingsResponse;
+import org.opensearch.client.opensearch.indices.GetMappingRequest;
+import org.opensearch.client.opensearch.indices.GetMappingResponse;
+import org.opensearch.client.opensearch.indices.PutIndexTemplateRequest;
 
 /**
  * Run with: <c>./gradlew :samples:run -Dsamples.mainClass=IndexTemplates</c>

diff --git a/samples/src/main/java/org/opensearch/client/samples/util/IssueDocument.java b/samples/src/main/java/org/opensearch/client/samples/util/IssueDocument.java
@@ -1,3 +1,11 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
 package org.opensearch.client.samples.util;
 
 import java.util.List;