Browse Source

Generate an IDE-compatible checkstyle configuration (#66109)

With a few changes, our checkstyle config can be used by e.g. IntelliJ's
Checkstyle plugin. Add a task to generate this file automatically, and
create the necessary IntelliJ config to use it.

Also add a line-length setting to .editorconfig.
Rory Hunter 4 years ago
parent
commit
a67a5f9216
3 changed files with 102 additions and 4 deletions
  1. 2 0
      .editorconfig
  2. 44 0
      CONTRIBUTING.md
  3. 56 4
      gradle/ide.gradle

+ 2 - 0
.editorconfig

@@ -13,9 +13,11 @@ indent_size = 2
 
 [*.groovy]
 indent_size = 4
+max_line_length = 140
 
 [*.java]
 indent_size = 4
+max_line_length = 140
 
 [*.json]
 indent_size = 2

+ 44 - 0
CONTRIBUTING.md

@@ -164,6 +164,47 @@ You can import the Elasticsearch project into IntelliJ IDEA via:
  - In the subsequent dialog navigate to the root `build.gradle` file
  - In the subsequent dialog select **Open as Project**
 
+#### Checkstyle
+
+If you have the [Checkstyle] plugin installed, you can configure IntelliJ to
+check the Elasticsearch code. However, the Checkstyle configuration file does
+not work by default with the IntelliJ plugin, so instead an IDE-specific config
+file is generated automatically after IntelliJ finishes syncing.
+
+   1. Open **Preferences > Tools > Checkstyle**
+   2. Change the "Scan Scope" to "Only Java sources (including tests)"
+   3. Check the "+" under "Configuration file"
+   4. Set "Description" to "Elasticsearch" (or whatever you want)
+   5. Select "Use a local Checkstyle file"
+   6. For the "File", navigate to `build/checkstyle_ide.xml`
+   7. Tick "Store relative to project location"
+   8. Click "Next"
+   9. The Checkstyle config file contains the variable `config_loc`, and
+      IntelliJ will ask for a value. Fill in `buildSrc/src/main/resources`.
+      This allows the config file to reference the exclusions file in that directory.
+   10. Click "Next", then "Finish".
+   11. Click the box next to the new configuration to make it "Active". Without doing this,
+       you'll have to explicitly choose the "Elasticsearch" configuration in the Checkstyle
+       tool window and run the check manually. You can still do this with an active config.
+   12. Click "OK" to apply the new preferences
+
+#### Formatting
+
+We are in the process of migrating towards automatic formatting Java file
+using [spotless], backed by the Eclipse formatter. If you have the [Eclipse
+Code Formatter] installed, you can apply formatting directly in IntelliJ.
+
+   1. Open **Preferences > Other Settings > Eclipse Code Formatter**
+   2. Click "Use the Eclipse Code Formatter"
+   3. Under "Eclipse formatter config", select "Eclipse workspace/project
+      folder or config file"
+   4. Click "Browse", and navigate to the file `buildSrc/formatterConfig.xml`
+   5. Click "OK"
+
+Note that only some sub-projects in the Elasticsearch project are currently
+fully-formatted. You can see a list of project that **are not**
+automatically formatted in [gradle/formatting.gradle](gradle/formatting.gradle).
+
 ### Importing the project into Eclipse
 
 Elasticsearch builds using Gradle and Java 14. When importing into Eclipse you
@@ -699,3 +740,6 @@ non-documentation contribution. This is mentioned above, but it is worth
 repeating in this section because it has come up in this context.
 
 [intellij]: https://blog.jetbrains.com/idea/2017/07/intellij-idea-2017-2-is-here-smart-sleek-and-snappy/
+[Checkstyle]: https://plugins.jetbrains.com/plugin/1065-checkstyle-idea
+[spotless]: https://github.com/diffplug/spotless
+[Eclipse Code Formatter]: https://plugins.jetbrains.com/plugin/6546-eclipse-code-formatter

+ 56 - 4
gradle/ide.gradle

@@ -1,6 +1,9 @@
 import org.elasticsearch.gradle.info.BuildParams
 import org.jetbrains.gradle.ext.Remote
 import org.jetbrains.gradle.ext.JUnit
+import java.nio.file.Files
+import java.nio.file.StandardCopyOption
+import java.nio.file.Paths
 
 buildscript {
   repositories {
@@ -21,6 +24,43 @@ allprojects {
   }
 }
 
+tasks.register('configureIdeCheckstyle') {
+  group = 'ide'
+  description = 'Generated a suitable checkstyle config for IDEs'
+
+  String checkstyleConfig = 'buildSrc/src/main/resources/checkstyle.xml'
+  String checkstyleIdeConfig = "${buildDir}/checkstyle_ide.xml"
+
+  inputs.files(file(checkstyleConfig))
+  outputs.files(file(checkstyleIdeConfig))
+
+  doLast {
+    // Create an IDE-specific checkstyle config by first copying the standard config
+    Files.copy(
+      Paths.get(file(checkstyleConfig).getPath()),
+      Paths.get(file(checkstyleIdeConfig).getPath()),
+      StandardCopyOption.REPLACE_EXISTING
+    )
+    // Edit the copy so that IntelliJ can copy with it
+    modifyXml(checkstyleIdeConfig, { xml ->
+      // This module since it is implemented with custom code
+      xml.module.findAll { it.'@name' == 'org.elasticsearch.gradle.checkstyle.SnippetLengthCheck' }.each { it.parent().remove(it) }
+
+      // Move the line length check because the IDE thinks it can't belong under a TreeWalker. Moving the
+      // configuration in the main file causes the command-line Checkstyle task to fail ¯\_(ツ)_/¯
+      Node treeWalker = xml.module.find { it.'@name' == 'TreeWalker' }
+      Node lineLength = treeWalker.module.find { it.'@name' == 'LineLength' }
+      treeWalker.remove(lineLength)
+      xml.append(lineLength)
+    },
+      "<!DOCTYPE module PUBLIC\n" +
+      "  \"-//Puppy Crawl//DTD Check Configuration 1.3//EN\"\n" +
+      "  \"http://www.puppycrawl.com/dtds/configuration_1_3.dtd\">\n" +
+      "<!-- Generated automatically from ./checkstyle.xml - do not edit this file directly. -->\n"
+    )
+  }
+}
+
 // Applying this stuff, particularly the idea-ext plugin, has a cost so avoid it unless we're running in the IDE
 if (System.getProperty('idea.active') == 'true') {
   apply plugin: org.jetbrains.gradle.ext.IdeaExtPlugin
@@ -55,7 +95,7 @@ if (System.getProperty('idea.active') == 'true') {
           testRunner = 'choose_per_test'
         }
         taskTriggers {
-          afterSync tasks.named('configureIdeaGradleJvm'), tasks.named('buildDependencyArtifacts')
+          afterSync tasks.named('configureIdeCheckstyle'), tasks.named('configureIdeaGradleJvm'), tasks.named('buildDependencyArtifacts')
         }
         codeStyle {
           java {
@@ -122,13 +162,25 @@ if (System.getProperty('idea.active') == 'true') {
  *
  * @param path Path to existing XML file
  * @param action Action to perform on parsed XML document
+ * @param preface optional front matter to add after the XML declaration
+ * but before the XML document, e.g. a doctype or comment
  */
-void modifyXml(Object path, Action<? super Node> action) {
+void modifyXml(Object path, Action<? super Node> action, String preface = null) {
   File xmlFile = project.file(path)
-  Node xml = new XmlParser().parse(xmlFile)
+  XmlParser xmlParser = new XmlParser(false, true, true)
+  xmlParser.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false)
+  Node xml = xmlParser.parse(xmlFile)
   action.execute(xml)
 
   xmlFile.withPrintWriter { writer ->
-    new XmlNodePrinter(writer).print(xml)
+    def printer = new XmlNodePrinter(writer)
+    printer.namespaceAware = true
+    printer.preserveWhitespace = true
+    writer.write("<?xml version=\"1.0\"?>\n")
+
+    if (preface != null) {
+      writer.write(preface)
+    }
+    printer.print(xml)
   }
 }