-
Notifications
You must be signed in to change notification settings - Fork 24.9k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Improve checkstyle performance and readability (#54308)
Drop a nasty regex in our checkstyle config that I wrote a long time ago in favor of a checkstyle extension. This is better because: * It is faster. It saves a little more than a minute across the entire build. * It is easier to read. Who knew 100 lines of Java would be easier to read than a regex, but it is. * It has tests.
- Loading branch information
1 parent
e23de0e
commit f06ebf0
Showing
7 changed files
with
211 additions
and
7 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
103 changes: 103 additions & 0 deletions
103
buildSrc/src/main/java/org/elasticsearch/gradle/checkstyle/SnippetLengthCheck.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| /* | ||
| * 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. | ||
| */ | ||
|
|
||
| package org.elasticsearch.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(); | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
87 changes: 87 additions & 0 deletions
87
buildSrc/src/test/java/org/elasticsearch/gradle/checkstyle/SnipptLengthCheckTests.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| /* | ||
| * 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. | ||
| */ | ||
|
|
||
| package org.elasticsearch.gradle.checkstyle; | ||
|
|
||
| import org.elasticsearch.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); | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters