From a439371bc90cfd4eee5335efabf63d4b5139a44b Mon Sep 17 00:00:00 2001 From: Owais Kazi Date: Tue, 2 Nov 2021 10:59:33 -0700 Subject: [PATCH] Cleanup for Checkstyle (#1370) Signed-off-by: Owais Kazi --- buildSrc/build.gradle | 2 - .../ExportOpenSearchBuildResourcesTaskIT.java | 23 ---- .../gradle/precommit/PrecommitTasks.groovy | 1 - .../gradle/checkstyle/SnippetLengthCheck.java | 116 ----------------- .../precommit/CheckstylePrecommitPlugin.java | 120 ------------------ buildSrc/src/main/resources/checkstyle.xml | 105 --------------- .../resources/checkstyle_suppressions.xml | 46 ------- .../checkstyle/SnipptLengthCheckTests.java | 100 --------------- .../opensearch-build-resources/build.gradle | 2 - buildSrc/version.properties | 2 +- gradle/formatting.gradle | 7 +- 11 files changed, 2 insertions(+), 522 deletions(-) delete mode 100644 buildSrc/src/main/java/org/opensearch/gradle/checkstyle/SnippetLengthCheck.java delete mode 100644 buildSrc/src/main/java/org/opensearch/gradle/precommit/CheckstylePrecommitPlugin.java delete mode 100644 buildSrc/src/main/resources/checkstyle.xml delete mode 100644 buildSrc/src/main/resources/checkstyle_suppressions.xml delete mode 100644 buildSrc/src/test/java/org/opensearch/gradle/checkstyle/SnipptLengthCheckTests.java diff --git a/buildSrc/build.gradle b/buildSrc/build.gradle index b5e5be949b6..13b63904c21 100644 --- a/buildSrc/build.gradle +++ b/buildSrc/build.gradle @@ -116,8 +116,6 @@ dependencies { api 'org.apache.maven:maven-model:3.6.2' api 'com.networknt:json-schema-validator:1.0.36' - compileOnly "com.puppycrawl.tools:checkstyle:${props.getProperty('checkstyle')}" - testImplementation "com.puppycrawl.tools:checkstyle:${props.getProperty('checkstyle')}" testFixturesApi "junit:junit:${props.getProperty('junit')}" testFixturesApi "com.carrotsearch.randomizedtesting:randomizedtesting-runner:${props.getProperty('randomizedrunner')}" testFixturesApi gradleApi() diff --git a/buildSrc/src/integTest/java/org/opensearch/gradle/ExportOpenSearchBuildResourcesTaskIT.java b/buildSrc/src/integTest/java/org/opensearch/gradle/ExportOpenSearchBuildResourcesTaskIT.java index cccd294f5dd..b05250f8a66 100644 --- a/buildSrc/src/integTest/java/org/opensearch/gradle/ExportOpenSearchBuildResourcesTaskIT.java +++ b/buildSrc/src/integTest/java/org/opensearch/gradle/ExportOpenSearchBuildResourcesTaskIT.java @@ -33,34 +33,11 @@ package org.opensearch.gradle; import org.opensearch.gradle.test.GradleIntegrationTestCase; -import org.gradle.testkit.runner.BuildResult; public class ExportOpenSearchBuildResourcesTaskIT extends GradleIntegrationTestCase { public static final String PROJECT_NAME = "opensearch-build-resources"; - public void testUpToDateWithSourcesConfigured() { - getGradleRunner(PROJECT_NAME).withArguments("clean", "-s").build(); - - BuildResult result = getGradleRunner(PROJECT_NAME).withArguments("buildResources", "-s", "-i").build(); - assertTaskSuccessful(result, ":buildResources"); - assertBuildFileExists(result, PROJECT_NAME, "build-tools-exported/checkstyle.xml"); - assertBuildFileExists(result, PROJECT_NAME, "build-tools-exported/checkstyle_suppressions.xml"); - - result = getGradleRunner(PROJECT_NAME).withArguments("buildResources", "-s", "-i").build(); - assertTaskUpToDate(result, ":buildResources"); - assertBuildFileExists(result, PROJECT_NAME, "build-tools-exported/checkstyle.xml"); - assertBuildFileExists(result, PROJECT_NAME, "build-tools-exported/checkstyle_suppressions.xml"); - } - - public void testOutputAsInput() { - BuildResult result = getGradleRunner(PROJECT_NAME).withArguments("clean", "sampleCopy", "-s", "-i").build(); - - assertTaskSuccessful(result, ":sampleCopy"); - assertBuildFileExists(result, PROJECT_NAME, "sampleCopy/checkstyle.xml"); - assertBuildFileExists(result, PROJECT_NAME, "sampleCopy/checkstyle_suppressions.xml"); - } - public void testIncorrectUsage() { assertOutputContains( getGradleRunner(PROJECT_NAME).withArguments("noConfigAfterExecution", "-s", "-i").buildAndFail().getOutput(), diff --git a/buildSrc/src/main/groovy/org/opensearch/gradle/precommit/PrecommitTasks.groovy b/buildSrc/src/main/groovy/org/opensearch/gradle/precommit/PrecommitTasks.groovy index 7c4b3e6118d..e5cd4cda650 100644 --- a/buildSrc/src/main/groovy/org/opensearch/gradle/precommit/PrecommitTasks.groovy +++ b/buildSrc/src/main/groovy/org/opensearch/gradle/precommit/PrecommitTasks.groovy @@ -40,7 +40,6 @@ class PrecommitTasks { static void create(Project project, boolean includeDependencyLicenses) { - project.pluginManager.apply(CheckstylePrecommitPlugin) project.pluginManager.apply(ForbiddenApisPrecommitPlugin) project.pluginManager.apply(JarHellPrecommitPlugin) project.pluginManager.apply(ForbiddenPatternsPrecommitPlugin) diff --git a/buildSrc/src/main/java/org/opensearch/gradle/checkstyle/SnippetLengthCheck.java b/buildSrc/src/main/java/org/opensearch/gradle/checkstyle/SnippetLengthCheck.java deleted file mode 100644 index 701f44b533e..00000000000 --- a/buildSrc/src/main/java/org/opensearch/gradle/checkstyle/SnippetLengthCheck.java +++ /dev/null @@ -1,116 +0,0 @@ -/* - * 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. - */ - -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you 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. - */ - -/* - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -package org.opensearch.gradle.checkstyle; - -import com.puppycrawl.tools.checkstyle.api.AbstractFileSetCheck; -import com.puppycrawl.tools.checkstyle.api.CheckstyleException; -import com.puppycrawl.tools.checkstyle.api.FileText; - -import java.io.File; -import java.util.Arrays; -import java.util.Iterator; -import java.util.function.BiConsumer; -import java.util.regex.Matcher; -import java.util.regex.Pattern; - -/** - * Checks the snippets included in the docs aren't too wide to fit on - * the page. - */ -public class SnippetLengthCheck extends AbstractFileSetCheck { - private static final Pattern START = Pattern.compile("^( *)//\\s*tag::(.+?)\\s*$", Pattern.MULTILINE); - private int max; - - /** - * The maximum width that a snippet may have. - */ - public void setMax(int max) { - this.max = max; - } - - @Override - protected void processFiltered(File file, FileText fileText) throws CheckstyleException { - checkFile((line, message) -> log(line, message), max, fileText.toLinesArray()); - } - - static void checkFile(BiConsumer log, int max, String... lineArray) { - LineItr lines = new LineItr(Arrays.asList(lineArray).iterator()); - while (lines.hasNext()) { - Matcher m = START.matcher(lines.next()); - if (m.matches()) { - checkSnippet(log, max, lines, m.group(1), m.group(2)); - } - } - } - - private static void checkSnippet(BiConsumer log, int max, LineItr lines, String leadingSpaces, String name) { - Pattern end = Pattern.compile("^ *//\\s*end::" + name + "\\s*$", Pattern.MULTILINE); - while (lines.hasNext()) { - String line = lines.next(); - if (end.matcher(line).matches()) { - return; - } - if (line.isEmpty()) { - continue; - } - if (false == line.startsWith(leadingSpaces)) { - log.accept(lines.lastLineNumber, "snippet line should start with [" + leadingSpaces + "]"); - continue; - } - int width = line.length() - leadingSpaces.length(); - if (width > max) { - log.accept(lines.lastLineNumber, "snippet line should be no more than [" + max + "] characters but was [" + width + "]"); - } - } - } - - private static class LineItr implements Iterator { - private final Iterator delegate; - private int lastLineNumber; - - LineItr(Iterator delegate) { - this.delegate = delegate; - } - - @Override - public boolean hasNext() { - return delegate.hasNext(); - } - - @Override - public String next() { - lastLineNumber++; - return delegate.next(); - } - } -} diff --git a/buildSrc/src/main/java/org/opensearch/gradle/precommit/CheckstylePrecommitPlugin.java b/buildSrc/src/main/java/org/opensearch/gradle/precommit/CheckstylePrecommitPlugin.java deleted file mode 100644 index 30d26eff9f9..00000000000 --- a/buildSrc/src/main/java/org/opensearch/gradle/precommit/CheckstylePrecommitPlugin.java +++ /dev/null @@ -1,120 +0,0 @@ -/* - * 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. - */ - -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you 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. - */ - -/* - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -package org.opensearch.gradle.precommit; - -import org.opensearch.gradle.VersionProperties; -import org.opensearch.gradle.util.Util; -import org.gradle.api.Action; -import org.gradle.api.Project; -import org.gradle.api.Task; -import org.gradle.api.artifacts.dsl.DependencyHandler; -import org.gradle.api.plugins.quality.Checkstyle; -import org.gradle.api.plugins.quality.CheckstyleExtension; -import org.gradle.api.tasks.TaskProvider; - -import java.io.File; -import java.io.IOException; -import java.io.InputStream; -import java.io.UncheckedIOException; -import java.net.JarURLConnection; -import java.net.URL; -import java.nio.file.Files; -import java.nio.file.StandardCopyOption; - -public class CheckstylePrecommitPlugin extends PrecommitPlugin { - @Override - public TaskProvider createTask(Project project) { - // Always copy the checkstyle configuration files to 'buildDir/checkstyle' since the resources could be located in a jar - // file. If the resources are located in a jar, Gradle will fail when it tries to turn the URL into a file - URL checkstyleConfUrl = CheckstylePrecommitPlugin.class.getResource("/checkstyle.xml"); - URL checkstyleSuppressionsUrl = CheckstylePrecommitPlugin.class.getResource("/checkstyle_suppressions.xml"); - File checkstyleDir = new File(project.getBuildDir(), "checkstyle"); - File checkstyleSuppressions = new File(checkstyleDir, "checkstyle_suppressions.xml"); - File checkstyleConf = new File(checkstyleDir, "checkstyle.xml"); - TaskProvider copyCheckstyleConf = project.getTasks().register("copyCheckstyleConf"); - - // configure inputs and outputs so up to date works properly - copyCheckstyleConf.configure(t -> t.getOutputs().files(checkstyleSuppressions, checkstyleConf)); - if ("jar".equals(checkstyleConfUrl.getProtocol())) { - try { - JarURLConnection jarURLConnection = (JarURLConnection) checkstyleConfUrl.openConnection(); - copyCheckstyleConf.configure(t -> t.getInputs().file(jarURLConnection.getJarFileURL())); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - } else if ("file".equals(checkstyleConfUrl.getProtocol())) { - copyCheckstyleConf.configure(t -> t.getInputs().files(checkstyleConfUrl.getFile(), checkstyleSuppressionsUrl.getFile())); - } - - // Explicitly using an Action interface as java lambdas - // are not supported by Gradle up-to-date checks - copyCheckstyleConf.configure(t -> t.doLast(new Action() { - @Override - public void execute(Task task) { - checkstyleDir.mkdirs(); - try (InputStream stream = checkstyleConfUrl.openStream()) { - Files.copy(stream, checkstyleConf.toPath(), StandardCopyOption.REPLACE_EXISTING); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - try (InputStream stream = checkstyleSuppressionsUrl.openStream()) { - Files.copy(stream, checkstyleSuppressions.toPath(), StandardCopyOption.REPLACE_EXISTING); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - } - })); - - TaskProvider checkstyleTask = project.getTasks().register("checkstyle"); - checkstyleTask.configure(t -> t.dependsOn(project.getTasks().withType(Checkstyle.class))); - - // Apply the checkstyle plugin to create `checkstyleMain` and `checkstyleTest`. It only - // creates them if there is main or test code to check and it makes `check` depend - // on them. We also want `precommit` to depend on `checkstyle`. - project.getPluginManager().apply("checkstyle"); - CheckstyleExtension checkstyle = project.getExtensions().getByType(CheckstyleExtension.class); - checkstyle.getConfigDirectory().set(checkstyleDir); - - DependencyHandler dependencies = project.getDependencies(); - String checkstyleVersion = VersionProperties.getVersions().get("checkstyle"); - dependencies.add("checkstyle", "com.puppycrawl.tools:checkstyle:" + checkstyleVersion); - dependencies.add("checkstyle", project.files(Util.getBuildSrcCodeSource())); - - project.getTasks().withType(Checkstyle.class).configureEach(t -> { - t.dependsOn(copyCheckstyleConf); - t.reports(r -> r.getHtml().setEnabled(false)); - }); - - return checkstyleTask; - } -} diff --git a/buildSrc/src/main/resources/checkstyle.xml b/buildSrc/src/main/resources/checkstyle.xml deleted file mode 100644 index e77a3efbfe5..00000000000 --- a/buildSrc/src/main/resources/checkstyle.xml +++ /dev/null @@ -1,105 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/buildSrc/src/main/resources/checkstyle_suppressions.xml b/buildSrc/src/main/resources/checkstyle_suppressions.xml deleted file mode 100644 index 365c28d36b6..00000000000 --- a/buildSrc/src/main/resources/checkstyle_suppressions.xml +++ /dev/null @@ -1,46 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/buildSrc/src/test/java/org/opensearch/gradle/checkstyle/SnipptLengthCheckTests.java b/buildSrc/src/test/java/org/opensearch/gradle/checkstyle/SnipptLengthCheckTests.java deleted file mode 100644 index b25bdd045dd..00000000000 --- a/buildSrc/src/test/java/org/opensearch/gradle/checkstyle/SnipptLengthCheckTests.java +++ /dev/null @@ -1,100 +0,0 @@ -/* - * 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. - */ - -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you 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. - */ - -/* - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -package org.opensearch.gradle.checkstyle; - -import org.opensearch.gradle.test.GradleUnitTestCase; - -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; -import java.util.function.BiConsumer; - -import static java.util.Collections.singletonList; -import static org.hamcrest.CoreMatchers.equalTo; - -public class SnipptLengthCheckTests extends GradleUnitTestCase { - public void testNoSnippets() { - SnippetLengthCheck.checkFile(failOnError(), 10, "There a no snippets"); - } - - public void testEmptySnippet() { - SnippetLengthCheck.checkFile(failOnError(), 10, "// tag::foo", "// end::foo"); - } - - public void testSnippetWithSmallText() { - SnippetLengthCheck.checkFile(failOnError(), 10, "// tag::foo", "some words", "// end::foo"); - } - - public void testSnippetWithLeadingSpaces() { - SnippetLengthCheck.checkFile(failOnError(), 10, " // tag::foo", " some words", " // end::foo"); - } - - public void testSnippetWithEmptyLine() { - SnippetLengthCheck.checkFile(failOnError(), 10, " // tag::foo", "", " some words", " // end::foo"); - } - - public void testSnippetBrokenLeadingSpaces() { - List collection = new ArrayList<>(); - SnippetLengthCheck.checkFile(collect(collection), 10, " // tag::foo", "some words", " // end::foo"); - assertThat(collection, equalTo(singletonList("2: snippet line should start with [ ]"))); - } - - public void testSnippetTooLong() { - List collection = new ArrayList<>(); - SnippetLengthCheck.checkFile(collect(collection), 10, " // tag::foo", " too long words", " // end::foo"); - assertThat(collection, equalTo(singletonList("2: snippet line should be no more than [10] characters but was [14]"))); - } - - public void testLotsOfErrors() { - List collection = new ArrayList<>(); - SnippetLengthCheck.checkFile(collect(collection), 10, " // tag::foo", "asdfadf", " too long words", "asdfadf", " // end::foo"); - assertThat( - collection, - equalTo( - Arrays.asList( - "2: snippet line should start with [ ]", - "3: snippet line should be no more than [10] characters but was [14]", - "4: snippet line should start with [ ]" - ) - ) - ); - } - - private BiConsumer failOnError() { - return (line, message) -> fail("checkstyle error on line [" + line + "] with message [" + message + "]"); - } - - private BiConsumer collect(List collection) { - return (line, message) -> collection.add(line + ": " + message); - } -} diff --git a/buildSrc/src/testKit/opensearch-build-resources/build.gradle b/buildSrc/src/testKit/opensearch-build-resources/build.gradle index b94bbe884b8..3d939ccffc6 100644 --- a/buildSrc/src/testKit/opensearch-build-resources/build.gradle +++ b/buildSrc/src/testKit/opensearch-build-resources/build.gradle @@ -19,8 +19,6 @@ plugins { File buildResourcesDir = new File(project.getBuildDir(), 'build-tools-exported') TaskProvider buildResourcesTask = tasks.register('buildResources', ExportOpenSearchBuildResourcesTask) { outputDir = buildResourcesDir - copy 'checkstyle_suppressions.xml' - copy 'checkstyle.xml' } tasks.register("sampleCopy", Sync) { diff --git a/buildSrc/version.properties b/buildSrc/version.properties index 4c1217d251d..7b172165abe 100644 --- a/buildSrc/version.properties +++ b/buildSrc/version.properties @@ -4,7 +4,7 @@ lucene = 8.10.1 bundled_jdk_vendor = adoptium bundled_jdk = 17.0.1+12 -checkstyle = 8.29 + # optional dependencies spatial4j = 0.7 diff --git a/gradle/formatting.gradle b/gradle/formatting.gradle index ba34dc200a2..19adea33b23 100644 --- a/gradle/formatting.gradle +++ b/gradle/formatting.gradle @@ -55,12 +55,8 @@ import org.opensearch.gradle.BuildPlugin * https://github.com/diffplug/spotless/tree/master/plugin-gradle */ -// Do not add new sub-projects here! -def projectPathsToExclude = [] - -subprojects { +allprojects { plugins.withType(BuildPlugin).whenPluginAdded { - if (projectPathsToExclude.contains(project.path) == false) { project.apply plugin: "com.diffplug.spotless" spotless { @@ -81,6 +77,5 @@ subprojects { } precommit.dependsOn 'spotlessJavaCheck' - } } }