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.

Author: mark-vieira

buildSrc/build.gradle

@@ -121,6 +121,8 @@ dependencies {
   compile 'com.github.jengelman.gradle.plugins:shadow:5.1.0'
   compile 'de.thetaphi:forbiddenapis:2.7'
   compile 'com.avast.gradle:gradle-docker-compose-plugin:0.8.12'
+  compileOnly "com.puppycrawl.tools:checkstyle:${props.getProperty('checkstyle')}"
+  testCompile "com.puppycrawl.tools:checkstyle:${props.getProperty('checkstyle')}"
   testCompile "junit:junit:${props.getProperty('junit')}"
   testCompile "com.carrotsearch.randomizedtesting:randomizedtesting-runner:${props.getProperty('randomizedrunner')}"
   testCompile 'com.github.tomakehurst:wiremock-jre8-standalone:2.23.2'

buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy

@@ -24,6 +24,7 @@ import de.thetaphi.forbiddenapis.gradle.ForbiddenApisPlugin
 import org.elasticsearch.gradle.ExportElasticsearchBuildResourcesTask
 import org.elasticsearch.gradle.VersionProperties
 import org.elasticsearch.gradle.info.BuildParams
+import org.elasticsearch.gradle.util.Util
 import org.gradle.api.JavaVersion
 import org.gradle.api.Project
 import org.gradle.api.artifacts.Configuration
@@ -38,8 +39,6 @@ class PrecommitTasks {
 
     /** Adds a precommit task, which depends on non-test verification tasks. */
 
-    public static final String CHECKSTYLE_VERSION = '8.20'
-
     public static TaskProvider create(Project project, boolean includeDependencyLicenses) {
         project.configurations.create("forbiddenApisCliJar")
         project.dependencies {
@@ -232,7 +231,10 @@ class PrecommitTasks {
         project.pluginManager.apply('checkstyle')
         project.checkstyle {
             configDir = checkstyleDir
-            toolVersion = CHECKSTYLE_VERSION
+        }
+        project.dependencies {
+            checkstyle "com.puppycrawl.tools:checkstyle:${VersionProperties.versions.checkstyle}"
+            checkstyle project.files(Util.buildSrcCodeSource)
         }
 
         project.tasks.withType(Checkstyle).configureEach { task ->

buildSrc/src/main/java/org/elasticsearch/gradle/checkstyle/SnippetLengthCheck.java

@@ -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();
+        }
+    }
+}

buildSrc/src/main/java/org/elasticsearch/gradle/util/Util.java

@@ -26,6 +26,8 @@
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.io.UncheckedIOException;
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.util.Locale;
 
 public class Util {
@@ -65,4 +67,12 @@ public static String getResourceContents(String resourcePath) {
     public static String capitalize(String s) {
         return s.substring(0, 1).toUpperCase(Locale.ROOT) + s.substring(1);
     }
+
+    public static URI getBuildSrcCodeSource() {
+        try {
+            return Util.class.getProtectionDomain().getCodeSource().getLocation().toURI();
+        } catch (URISyntaxException e) {
+            throw new GradleException("Error determining build tools JAR location", e);
+        }
+    }
 }

buildSrc/src/main/resources/checkstyle.xml

@@ -24,11 +24,9 @@
     characters then it'll need to scroll. This fails the build if it sees
     such snippets.
   -->
-  <module name="RegexpMultiline">
+  <module name="org.elasticsearch.gradle.checkstyle.SnippetLengthCheck">
     <property name="id" value="SnippetLength"/>
-    <property name="format" value="^( *?)//\s*?tag(.+?)\s*?\n(.*?\n)*?\1.{77,}?\n(.*?\n)*?\1//\s*?end\2\s*$"/>
-    <property name="fileExtensions" value="java"/>
-    <property name="message" value="Code snippets longer than 76 characters get cut off when rendered in the docs"/>
+    <property name="max" value="76"/>
   </module>
 
   <module name="TreeWalker">

buildSrc/src/test/java/org/elasticsearch/gradle/checkstyle/SnipptLengthCheckTests.java

@@ -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);
+    }
+}

buildSrc/version.properties

@@ -4,6 +4,8 @@ lucene            = 8.5.0
 bundled_jdk_vendor = adoptopenjdk
 bundled_jdk = 14+36
 
+checkstyle = 8.20
+
 # optional dependencies
 spatial4j         = 0.7
 jts               = 1.15.0