honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Cleanup for Checkstyle (#1370) 

Signed-off-by: Owais Kazi <owaiskazi19@gmail.com>
This commit is contained in:
Owais Kazi 2021-11-02 10:59:33 -07:00 committed by GitHub
parent 681e5548c1
commit a439371bc9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 2 additions and 522 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
buildSrc/build.gradle
View File

@ -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()

23
buildSrc/src/integTest/java/org/opensearch/gradle/ExportOpenSearchBuildResourcesTaskIT.java
View File

@ -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(),

1
buildSrc/src/main/groovy/org/opensearch/gradle/precommit/PrecommitTasks.groovy
View File

@ -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)

116
buildSrc/src/main/java/org/opensearch/gradle/checkstyle/SnippetLengthCheck.java
View File

@ -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<Integer, String> 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<Integer, String> 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<String> {
private final Iterator<String> delegate;
private int lastLineNumber;
LineItr(Iterator<String> delegate) {
this.delegate = delegate;
}
@Override
public boolean hasNext() {
return delegate.hasNext();
}
@Override
public String next() {
lastLineNumber++;
return delegate.next();
}
}
}

120
buildSrc/src/main/java/org/opensearch/gradle/precommit/CheckstylePrecommitPlugin.java
View File

@ -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<? extends Task> 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<Task> 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<Task>() {
@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<Task> 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;
}
}

105
buildSrc/src/main/resources/checkstyle.xml
View File

@ -1,105 +0,0 @@
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
"-//Puppy Crawl//DTD Check Configuration 1.3//EN"
"http://www.puppycrawl.com/dtds/configuration_1_3.dtd">
<module name="Checker">
<property name="charset" value="UTF-8" />
<module name="SuppressionFilter">
<property name="file" value="${config_loc}/checkstyle_suppressions.xml" />
</module>
<!-- Checks Java files and forbids empty Javadoc comments -->
<module name="RegexpMultiline">
<property name="id" value="EmptyJavadoc"/>
<property name="format" value="\/\*[\s\*]*\*\/"/>
<property name="fileExtensions" value="java"/>
<property name="message" value="Empty javadoc comments are forbidden"/>
</module>
<!--
We include snippets that are wrapped in `// tag` and `// end` into the
docs, stripping the leading spaces. If the context is wider than 76
characters then it'll need to scroll. This fails the build if it sees
such snippets.
-->
<module name="org.opensearch.gradle.checkstyle.SnippetLengthCheck">
<property name="id" value="SnippetLength"/>
<property name="max" value="76"/>
</module>
<!-- Its our official line length! See checkstyle_suppressions.xml for the files that don't pass this. For now we
suppress the check there but enforce it everywhere else. This prevents the list from getting longer even if it is
unfair. -->
<module name="LineLength">
<property name="max" value="140"/>
<property name="ignorePattern" value="^ *\* *https?://[^ ]+$"/>
</module>
<module name="TreeWalker">
<module name="AvoidStarImport" />
<!-- Unused imports are forbidden -->
<module name="UnusedImports" />
<!-- Non-inner classes must be in files that match their names. -->
<module name="OuterTypeFilename" />
<!-- No line wraps inside of import and package statements. -->
<module name="NoLineWrap" />
<!-- only one statement per line should be allowed -->
<module name="OneStatementPerLine"/>
<!-- Each java file has only one outer class -->
<module name="OneTopLevelClass" />
<!-- The suffix L is preferred, because the letter l (ell) is often
hard to distinguish from the digit 1 (one). -->
<module name="UpperEll"/>
<module name="EqualsHashCode" />
<!-- Checks that the order of modifiers conforms to the suggestions in the
Java Language specification, sections 8.1.1, 8.3.1 and 8.4.3. It is not that
the standard is perfect, but having a consistent order makes the code more
readable and no other order is compellingly better than the standard.
The correct order is:
public
protected
private
abstract
static
final
transient
volatile
synchronized
native
strictfp
-->
<module name="ModifierOrder" />
<!-- Checks that we don't include modifier where they are implied. For
example, this does not allow interface methods to be declared public
because they are *always* public. -->
<module name="RedundantModifier" />
<!-- Checks that all java files have a package declaration and that it
lines up with the directory structure. -->
<module name="PackageDeclaration"/>
<!-- We don't use Java's builtin serialization and we suppress all warning
about it. The flip side of that coin is that we shouldn't _try_ to use
it. We can't outright ban it with ForbiddenApis because it complain about
every we reference a class that implements Serializable like String or
Exception.
-->
<module name="RegexpSinglelineJava">
<property name="format" value="serialVersionUID" />
<property name="message" value="Do not declare serialVersionUID." />
<property name="ignoreComments" value="true" />
</module>
<module name="RegexpSinglelineJava">
<property name="format" value="java\.io\.Serializable" />
<property name="message" value="References java.io.Serializable." />
<property name="ignoreComments" value="true" />
</module>
<!-- end Orwellian suppression of Serializable -->
</module>
</module>

46
buildSrc/src/main/resources/checkstyle_suppressions.xml
View File

@ -1,46 +0,0 @@
<?xml version="1.0"?>
<!DOCTYPE suppressions PUBLIC
"-//Puppy Crawl//DTD Suppressions 1.1//EN"
"http://www.puppycrawl.com/dtds/suppressions_1_1.dtd">
<suppressions>
<!-- On Windows, Checkstyle matches files using \ path separator -->
<!-- These files are generated by ANTLR so its silly to hold them to our rules. -->
<suppress files="modules[/\\]lang-painless[/\\]src[/\\]main[/\\]java[/\\]org[/\\]opensearch[/\\]painless[/\\]antlr[/\\]PainlessLexer\.java" checks="." />
<suppress files="modules[/\\]lang-painless[/\\]src[/\\]main[/\\]java[/\\]org[/\\]opensearch[/\\]painless[/\\]antlr[/\\]PainlessParser(|BaseVisitor|Visitor)\.java" checks="." />
<!-- Intentionally doesn't have a package declaration to test logging
configuration of classes that aren't in packages. -->
<suppress files="test[/\\]framework[/\\]src[/\\]test[/\\]java[/\\]Dummy.java" checks="PackageDeclaration" />
<!-- Intentionally has long example curl commands to coincide with sibling Painless tests. -->
<suppress files="modules[/\\]lang-painless[/\\]src[/\\]test[/\\]java[/\\]org[/\\]opensearch[/\\]painless[/\\]ContextExampleTests.java" checks="LineLength" />
<!-- Excludes checkstyle run on server module -->
<suppress files="server" checks="." />
<!-- Excludes checkstyle run on client module -->
<suppress files="client" checks="." />
<!-- Excludes checkstyle run on plugins/examples -->
<suppress files="plugins[/\\]examples" checks="." />
<!-- Excludes checkstyle run on libs module -->
<suppress files="libs" checks="." />
<!-- Excludes checkstyle run on modules module -->
<suppress files="modules" checks="." />
<!-- Excludes checkstyle run on plugins module -->
<suppress files="plugins" checks="." />
<!-- Excludes checkstyle run on below qa module -->
<suppress files="qa[/\\]die-with-dignity" checks="." />
<!-- Excludes checkstyle run on test module -->
<suppress files="test" checks="." />
<!-- Excludes checkstyle run on rest-api-spec module -->
<suppress files="rest-api-spec" checks="." />
<!--
Truly temporary suppressions suppression of snippets included in
documentation that are so wide that they scroll.
-->
<suppress files="modules[/\\]reindex[/\\]src[/\\]test[/\\]java[/\\]org[/\\]opensearch[/\\]client[/\\]documentation[/\\]ReindexDocumentationIT.java" id="SnippetLength" />
<!-- Gradle requires inputs to be seriablizable -->
<suppress files="buildSrc[/\\]src[/\\]main[/\\]java[/\\]org[/\\]opensearch[/\\]gradle[/\\]precommit[/\\]TestingConventionRule.java" checks="RegexpSinglelineJava" />
</suppressions>

100
buildSrc/src/test/java/org/opensearch/gradle/checkstyle/SnipptLengthCheckTests.java
View File

@ -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<String> 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<String> 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<String> 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<Integer, String> failOnError() {
return (line, message) -> fail("checkstyle error on line [" + line + "] with message [" + message + "]");
}
private BiConsumer<Integer, String> collect(List<String> collection) {
return (line, message) -> collection.add(line + ": " + message);
}
}

2
buildSrc/src/testKit/opensearch-build-resources/build.gradle
View File

@ -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) {

2
buildSrc/version.properties
View File

@ -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

7
gradle/formatting.gradle
View File

@ -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 {
@ -83,4 +79,3 @@ subprojects {
precommit.dependsOn 'spotlessJavaCheck'
}
}
}