Skip to content

feat: add execution ID parameter to failing spotless check log #2483

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat: add execution ID parameter to failing spotless check log #2483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a377b94
feat: add execution ID parameter to SpotlessCheckMojo for improved er…
dpolivaevUnite May 20, 2025
aa0d8ba
chore: update CHANGES.md to include execution ID in error messages fo…
dpolivaevUnite May 20, 2025
a1903b8
fix: refine execution ID handling in error messages for SpotlessCheck…
dpolivaevUnite May 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions plugin-maven/CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
### Changed
* Bump default `eclipse` version to latest `4.34` -> `4.35`. ([#2458](https://github.com/diffplug/spotless/pull/2458))
* Bump default `greclipse` version to latest `4.32` -> `4.35`. ([#2458](https://github.com/diffplug/spotless/pull/2458))
* Include execution ID in error messages when format violations are found. ([#1988](https://github.com/diffplug/spotless/issues/1988))

## [2.44.4] - 2025-04-07
### Changed
Expand Down
16 changes: 15 additions & 1 deletion plugin-maven/src/main/java/com/diffplug/spotless/maven/SpotlessCheckMojo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ public int getSeverity() {
@Parameter(defaultValue = "WARNING")
private MessageSeverity m2eIncrementalBuildMessageSeverity;

/**
* The execution ID from Maven's configuration.
*/
@Parameter(defaultValue = "${mojoExecution.executionId}")
private String executionId;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The execution ids of different goals usually differ as well!

Copy link
Author

@dpolivaev dpolivaev May 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how we use it:

    <profile>
      <id>palantir-java-format-profile</id>
      <activation>
        <jdk>[11,)</jdk>
      </activation>
      <build>
        <plugins>
          <plugin>
            <groupId>com.diffplug.spotless</groupId>
            <artifactId>spotless-maven-plugin</artifactId>
            <executions>
              <execution>
                <!--
                Run "mvn spotless:check@palantir-java-format" to check the
                formatting outside of the "mvn verify" phase.

                Similar: To format your code, run "mvn spotless:apply@palantir-java-format".

                You need to add the execution id with "@palantir-java-format" to your comand,
                so spotless will find its configuration containing the palantirJavaFormat.
                -->
                <id>palantir-java-format</id>
                <phase>verify</phase>
                <goals>
                  <goal>check</goal>
                </goals>
                <configuration>
                  <skip>${spotless.skip.palantirJavaFormat}</skip>
                  <java>
                    <palantirJavaFormat/>
                  </java>
                </configuration>
              </execution>
            </executions>
          </plugin>
        </plugins>
      </build>
    </profile>

Therefore we do not need different configurations and different execution IDs for the different goals (check and apply). We don't repeat ourselves.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They can be the same but don’t have to. With the default assuming the same there is a high chance something is emitted which does not work. I would leave out the default value for this parameter

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we need an optional separate parameter with a dedicated value? How would you name it?

The whole idea is to output correct command line automatically. Currently it outputs just run 'mvn spotless:apply' and it was the reason why I created the pull request.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think just changing the output inncase a dedicated execution id is configured is totally sufficient


@Override
protected void process(String name, Iterable<File> files, Formatter formatter, UpToDateChecker upToDateChecker) throws MojoExecutionException {
ImpactedFilesTracker counter = new ImpactedFilesTracker();
Expand Down Expand Up @@ -104,8 +110,16 @@ protected void process(String name, Iterable<File> files, Formatter formatter, U
}

if (!problemFiles.isEmpty()) {
final String runToFixMessage;
if (executionId != null && !executionId.isEmpty()
&& !executionId.equals("default")
&& !executionId.startsWith("default-")) {
runToFixMessage = "Run 'mvn spotless:apply@" + executionId + "' to fix these violations.";
} else {
runToFixMessage = "Run 'mvn spotless:apply' to fix these violations.";
}
throw new MojoExecutionException(DiffMessageFormatter.builder()
.runToFix("Run 'mvn spotless:apply' to fix these violations.")
.runToFix(runToFixMessage)
.formatter(baseDir.toPath(), formatter)
.problemFiles(problemFiles)
.getMessage());
Expand Down
39 changes: 37 additions & 2 deletions plugin-maven/src/test/java/com/diffplug/spotless/maven/SpotlessCheckMojoTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,18 +65,53 @@ void testSpotlessCheckBindingToVerifyPhase() throws Exception {
null,
null);

testSpotlessCheck(UNFORMATTED_FILE, "verify", true);
testSpotlessCheck(UNFORMATTED_FILE, "verify", "check");
}

@Test
void testSpotlessCheckWithSimplifiedConfiguration() throws Exception {
// Use configuration with default execution ID
writePom(
new String[]{
"<execution>",
" <!-- No ID specified = default ID -->",
" <phase>verify</phase>",
" <goals>",
" <goal>check</goal>",
" </goals>",
"</execution>"},
new String[]{
"<java>",
" <licenseHeader>",
" <file>${basedir}/license.txt</file>",
" </licenseHeader>",
"</java>"},
null,
null);

// Without explicit execution ID, we should get a simple message without @id
testSpotlessCheck(UNFORMATTED_FILE, "verify", true, "");
}

private void testSpotlessCheck(String fileName, String command, boolean expectError) throws Exception {
testSpotlessCheck(fileName, command, expectError, "");
}

private void testSpotlessCheck(String fileName, String command, String expectedFailingExecutionId) throws Exception {
testSpotlessCheck(fileName, command, true, "@" + expectedFailingExecutionId);
}

private void testSpotlessCheck(String fileName, String command, boolean expectError, String expectedExecutionId) throws Exception {
setFile("license.txt").toResource("license/TestLicense");
setFile("src/main/java/com.github.youribonnaffe.gradle.format/Java8Test.java").toResource(fileName);

MavenRunner mavenRunner = mavenRunner().withArguments(command);

if (expectError) {
ProcessRunner.Result result = mavenRunner.runHasError();
assertThat(result.stdOutUtf8()).contains("The following files had format violations");
String stdOutUtf8 = result.stdOutUtf8();
assertThat(stdOutUtf8).contains("The following files had format violations");
assertThat(stdOutUtf8).contains("Run 'mvn spotless:apply" + expectedExecutionId + "' to fix these violations.");
} else {
mavenRunner.runNoError();
}
Expand Down
Loading