spotless

spotless copied to clipboard

I have a multi-module Maven project that contains a number of XML files, which I'd like to format with the WTP formatter, using a custom configuration for it. The configuration file resides in a top-level directory. Here is a snipped of the Maven pom file:

      <!-- This one sets the top level directory reference -->
      <plugin>
        <groupId>org.commonjava.maven.plugins</groupId>
        <artifactId>directory-maven-plugin</artifactId>
        <version>0.3.1</version>
        <executions>
          <execution>
            <id>directories</id>
            <phase>initialize</phase>
            <goals>
              <goal>highest-basedir</goal>
            </goals>
            <configuration>
              <property>geoserverBaseDir</property>
            </configuration>
          </execution>
        </executions>
      </plugin>

      <plugin>
        <groupId>com.diffplug.spotless</groupId>
        <artifactId>spotless-maven-plugin</artifactId>
        <version>2.20.0</version>
        <executions>
          <execution>
            <phase>process-sources</phase>
            <goals>
              <goal>${spotless.action}</goal>
            </goals>
          </execution>
        </executions>
        <configuration>
          <java>
            <googleJavaFormat>
              <version>1.7</version>
              <style>AOSP</style>
            </googleJavaFormat>
          </java>
          <formats>
            <format>
              <includes>
                <include>src/main/java/application*Context.xml</include>
                <include>src/main/resources/application*Context.xml</include>
                <include>src/test/resources/**/application*Context.xml</include>
              </includes>
              <eclipseWtp>
                <type>XML</type>
                <files>
                  <!-- uses the top level dir to find the configuration file, one for all 100+ modules -->
                  <file>${geoserverBaseDir}/../build/qa/xml-format.properties</file>
                </files>
                <version>4.13.0</version>
              </eclipseWtp>
            </format>
          </formats>

          <upToDateChecking>
            <enabled>true</enabled>
            <indexFile>${project.basedir}/.spotless-index</indexFile>
          </upToDateChecking>
        </configuration>
      </plugin>

Trying to run mvn spotless:apply fails because the geoserverBaseDir is not initialized. I've been told in other reports this is likely happening because the SpotlessApplyMojo is not bound to a default phase, unlike for example the SpotlessCheckMojo.

The process-sources phase seems like it would be a good candidate for a default phase? Here is the reference to all phases.

Reference versions and OS:

• Apache Maven 3.6.3
• Spotless 2.20.0
• Linux Mint 20.3
• Java 1.8.0_312

Hi @aaime,

I investigated this problem using your branch mentioned in the previous issue: https://github.com/aaime/geoserver/tree/spotless-wtp.

I think setting the default phase for SpotlessApplyMojo does not help. Both mvn spotless:apply and mvn spotless:check fail because they can't read ${geoserverBaseDir}/../build/qa/xml-format.properties. Checkstyle plugin also fails with the same error:

$ mvn -Pcheckstyle checkstyle:check

[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  1.002 s
[INFO] Finished at: 2022-03-30T22:58:27+02:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.1.1:check (default-cli) on project geoserver: Failed during checkstyle execution: Unable to find configuration file at location: ${geoserverBaseDir}/../build/qa/checkstyle.xmll: Could not find resource '${geoserverBaseDir}/../build/qa/checkstyle.xmll'. -> [Help 1]

Note the -Pcheckstyle that activates the checkstyle profile and makes the plugin try to load the resource from the <configLocation> XML element. I think without -Pcheckstyle, mvn checkstyle:check simply runs with some default configuration and does not try to load the resource from <configLocation>.

Direct plugin invocations seem to be problematic because they rely on the geoserverBaseDir property that is set by a different plugin. This could work during a maven build that executes multiple phases, but direct plugin invocations execute only individual goals. To check this assumption, we can execute both directory-maven-plugin and spotless in a single command:

$ mvn org.commonjava.maven.plugins:directory-maven-plugin:highest-basedir@directories spotless:check
...

[INFO] ----------------------< org.geoserver:geoserver >-----------------------
[INFO] Building GeoServer 2.20-SNAPSHOT                                  [1/37]
[INFO] --------------------------------[ pom ]---------------------------------
[INFO]
[INFO] --- directory-maven-plugin:0.3.1:highest-basedir (directories) @ geoserver ---
[INFO] Highest basedir set to: /Users/lutovich/Projects/geoserver/src
[INFO]
[INFO] --- spotless-maven-plugin:2.22.0-SNAPSHOT:check (default-cli) @ geoserver ---
[INFO]
[INFO] ---------------------< org.geoserver:gs-platform >----------------------
[INFO] Building Core Platform Module 2.20-SNAPSHOT                       [2/37]
[INFO] --------------------------------[ jar ]---------------------------------
[INFO]
[INFO] --- directory-maven-plugin:0.3.1:highest-basedir (directories) @ gs-platform ---
[INFO] Highest basedir set to: /Users/lutovich/Projects/geoserver/src
[INFO]
[INFO] --- spotless-maven-plugin:2.22.0-SNAPSHOT:check (default-cli) @ gs-platform ---
...
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  4.476 s
[INFO] Finished at: 2022-03-30T23:07:06+02:00
[INFO] ------------------------------------------------------------------------

Note the @directories that tells Maven to run a specific directory-maven-plugin execution with ID directories that initializes geoserverBaseDir.

I think we can’t teach Spotless resolve geoserverBaseDir because this property comes from a different plugin.

We could bind SpotlessApplyMojo to process-sources phase but it wouldn't solve your problem. I think this will only allow users to omit <phase> in apply goal's configuration:

<plugin>
  <groupId>com.diffplug.spotless</groupId>
    <artifactId>spotless-maven-plugin</artifactId>
    <version>${spotless.version}</version>
    <executions>
      <execution>
        <id>apply-id</id>
        <!-- <phase>process-sources</phase> --> <!-- no need for this line -->
        <goals>
          <goal>apply</goal>
        </goals>
      </execution>
    </executions>
 </plugin>

Any thoughts or input much appreciated :)

Let me go back to the question that triggered this report then: is there a way to use the Spotless with WTP in a multi-module maven project, having multiple nesting levels, so pretty much needing an absolute reference to the configuration file? Maybe including the config inline in the pom? But I did not see such a possibility.

The main difference between spotless and checkstyle is that we run spotless in every build, to reformat the files, while checkstyle is only used when specifically invoked (part of the QA profiles) because it would take too much time. In other words, we should not ask developers to enable a profile for something that should always be done.

Hi @aaime,

It is not possible to inline the formatter config into the pom. Spotless maven plugin supports multi-module projects but the project structure requires a bit of additional configuration. You could follow this guide for PMD plugin multi-module configuration or take a look at https://github.com/diffplug/spotless/pull/210 file MultiModuleProjectTest.java for a concrete example. Essentially, you add a new sub-module that contains all build-related resources like formatter XML config files. Then, pom configuration of the plugin itself should depend on the new sub-module. This way you remove the need to reference config files by an absolute path.

Please let us know if this suggestion works for you 🙏

Perhaps we need a new documentation section explaining multi-module setup with concrete examples 🤔

Hi @aaime,

I investigated this problem using your branch mentioned in the previous issue: https://github.com/aaime/geoserver/tree/spotless-wtp.

I think setting the default phase for SpotlessApplyMojo does not help. Both mvn spotless:apply and mvn spotless:check fail because they can't read ${geoserverBaseDir}/../build/qa/xml-format.properties. Checkstyle plugin also fails with the same error:

$ mvn -Pcheckstyle checkstyle:check

[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  1.002 s
[INFO] Finished at: 2022-03-30T22:58:27+02:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.1.1:check (default-cli) on project geoserver: Failed during checkstyle execution: Unable to find configuration file at location: ${geoserverBaseDir}/../build/qa/checkstyle.xmll: Could not find resource '${geoserverBaseDir}/../build/qa/checkstyle.xmll'. -> [Help 1]

Note the -Pcheckstyle that activates the checkstyle profile and makes the plugin try to load the resource from the <configLocation> XML element. I think without -Pcheckstyle, mvn checkstyle:check simply runs with some default configuration and does not try to load the resource from <configLocation>.

Direct plugin invocations seem to be problematic because they rely on the geoserverBaseDir property that is set by a different plugin. This could work during a maven build that executes multiple phases, but direct plugin invocations execute only individual goals. To check this assumption, we can execute both directory-maven-plugin and spotless in a single command:

$ mvn org.commonjava.maven.plugins:directory-maven-plugin:highest-basedir@directories spotless:check
...

[INFO] ----------------------< org.geoserver:geoserver >-----------------------
[INFO] Building GeoServer 2.20-SNAPSHOT                                  [1/37]
[INFO] --------------------------------[ pom ]---------------------------------
[INFO]
[INFO] --- directory-maven-plugin:0.3.1:highest-basedir (directories) @ geoserver ---
[INFO] Highest basedir set to: /Users/lutovich/Projects/geoserver/src
[INFO]
[INFO] --- spotless-maven-plugin:2.22.0-SNAPSHOT:check (default-cli) @ geoserver ---
[INFO]
[INFO] ---------------------< org.geoserver:gs-platform >----------------------
[INFO] Building Core Platform Module 2.20-SNAPSHOT                       [2/37]
[INFO] --------------------------------[ jar ]---------------------------------
[INFO]
[INFO] --- directory-maven-plugin:0.3.1:highest-basedir (directories) @ gs-platform ---
[INFO] Highest basedir set to: /Users/lutovich/Projects/geoserver/src
[INFO]
[INFO] --- spotless-maven-plugin:2.22.0-SNAPSHOT:check (default-cli) @ gs-platform ---
...
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  4.476 s
[INFO] Finished at: 2022-03-30T23:07:06+02:00
[INFO] ------------------------------------------------------------------------

Note the @directories that tells Maven to run a specific directory-maven-plugin execution with ID directories that initializes geoserverBaseDir.

I think we can’t teach Spotless resolve geoserverBaseDir because this property comes from a different plugin.

We could bind SpotlessApplyMojo to process-sources phase but it wouldn't solve your problem. I think this will only allow users to omit <phase> in apply goal's configuration:

<plugin>
  <groupId>com.diffplug.spotless</groupId>
    <artifactId>spotless-maven-plugin</artifactId>
    <version>${spotless.version}</version>
    <executions>
      <execution>
        <id>apply-id</id>
        <!-- <phase>process-sources</phase> --> <!-- no need for this line -->
        <goals>
          <goal>apply</goal>
        </goals>
      </execution>
    </executions>
 </plugin>

Any thoughts or input much appreciated :)

I think binding to the process-sources phase by default is a good choice