Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
* Bump default `shfmt` version to latest `3.7.0` -> `3.8.0`. ([#2050](https://github.com/diffplug/spotless/pull/2050))
### Added
* Respect `.editorconfig` settings for formatting shell via `shfmt` ([#2031](https://github.com/diffplug/spotless/pull/2031))
* Skip execution in M2E (incremental) builds by default ([#1814](https://github.com/diffplug/spotless/issues/1814), [#2037](https://github.com/diffplug/spotless/issues/2037))

## [2.43.0] - 2024-01-23
### Added
Expand Down
22 changes: 22 additions & 0 deletions plugin-maven/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1696,6 +1696,28 @@ cmd> mvn spotless:apply -DspotlessFiles=my/file/pattern.java,more/generic/.*-pat

The patterns are matched using `String#matches(String)` against the absolute file path.

## Does Spotless support incremental builds in Eclipse?

Spotless comes with [m2e](https://eclipse.dev/m2e/) support. However, by default its execution is skipped in incremental builds as most developers want to fix all issues in one go via explicit `mvn spotless:apply` prior to raising a PR and don't want to be bothered with Spotless issues during working on the source code in the IDE.
To enable it use the following parameter

```
<configuration>
<m2eEnableForIncrementalBuild>true</m2eEnableForIncrementalBuild><!-- this is false by default -->
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
<m2eEnableForIncrementalBuild>true</m2eEnableForIncrementalBuild><!-- this is false by default -->
<m2eIncrementalBuild>WARNING</m2eIncrementalBuild><!-- this is NONE by default, can also be ERROR -->

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

or maybe m2eIncrementalBuildMessages

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

m2eIncrementalBuild is too fuzzy for me, m2eIncrementalBuildMessages sounds better.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That works for me :)

Copy link
Copy Markdown
Contributor Author

@kwin kwin Apr 4, 2024

Choose a reason for hiding this comment

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

After having a second thought, I think having only one parameter is over-simplifying things, as we talk about two different goals:

  1. apply
  2. check

The message severity is only ever useful for the latter, while m2eEnableForIncrementalBuild applies to both.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If m2eEnableForIncrementalBuild is false, then m2eIncrementalBuildMessages has no effect, correct? And if m2eEnableForIncrementalBuild is true, would it ever make sense for m2eIncrementalBuildMessageSeverity to be NONE or SILENT?

I might be wrong, but this seems like having a separate switch for the fuel pump on a car.

Copy link
Copy Markdown
Contributor Author

@kwin kwin Apr 4, 2024

Choose a reason for hiding this comment

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

If m2eEnableForIncrementalBuild is false, then m2eIncrementalBuildMessages has no effect, correct?

That is correct.

And if m2eEnableForIncrementalBuild is true, would it ever make sense for m2eIncrementalBuildMessageSeverity to be NONE or SILENT?

No, but the point is that noone expects that m2eIncrementalBuildMessageSeverity or m2eIncrementalBuildMessages would toggle off the incremental build at all.

but this seems like having a separate switch for the fuel pump on a car.

For me having one multivalue switch feels more like adjusting your AC will implicitly also start your car ;-)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That's not such a bad idea if the car can't run without A/C. But whatever, I usually defer to the people who are gonna use the thing.

Only thing missing now is a changelog entry.

Copy link
Copy Markdown
Contributor Author

@kwin kwin Apr 4, 2024

Choose a reason for hiding this comment

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

Done

P.S. I would consider m2eIncrementalBuildMessageSeverity advanced setting (rarely adjusted) while m2eEnableForIncrementalBuild is a basic setting (easy to understand for everyone, more often adjusted). Hope this is enough justification for keeping both separate, but this is definitely my last argument to this discussion :-)

</configuration>
```

In addition Eclipse problem markers are being emitted for goal `check`. By default they have the severity `WARNING`.
You can adjust this with

```
<configuration>
<m2eIncrementalBuildMessageSeverity>ERROR</m2eIncrementalBuildMessageSeverity><!-- WARNING or ERROR -->
</configuration>
```

Note that for Incremental build support the goals have to be bound to a phase prior to `test`.

<a name="examples"></a>

## Example configurations (from real-world projects)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,13 @@ public abstract class AbstractSpotlessMojo extends AbstractMojo {
@Parameter
private UpToDateChecking upToDateChecking = UpToDateChecking.enabled();

/**
* If set to {@code true} will also run on incremental builds (i.e. within Eclipse with m2e).
* Otherwise this goal is skipped in incremental builds and only runs on full builds.
*/
@Parameter(defaultValue = "false")
protected boolean m2eEnableForIncrementalBuild;

protected abstract void process(Iterable<File> files, Formatter formatter, UpToDateChecker upToDateChecker) throws MojoExecutionException;

private static final int MINIMUM_JRE = 11;
Expand Down Expand Up @@ -245,6 +252,10 @@ private boolean shouldSkip() {
if (skip) {
return true;
}
if (buildContext.isIncremental() && !m2eEnableForIncrementalBuild) {
getLog().debug("Skipping for incremental builds as parameter 'enableForIncrementalBuilds' is set to 'false'");
return true;
}

switch (goal) {
case GOAL_CHECK:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2016-2023 DiffPlug
* Copyright 2016-2024 DiffPlug
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -24,6 +24,7 @@
import org.apache.maven.plugin.MojoExecutionException;
import org.apache.maven.plugins.annotations.LifecyclePhase;
import org.apache.maven.plugins.annotations.Mojo;
import org.apache.maven.plugins.annotations.Parameter;
import org.sonatype.plexus.build.incremental.BuildContext;

import com.diffplug.spotless.Formatter;
Expand All @@ -38,6 +39,30 @@
@Mojo(name = AbstractSpotlessMojo.GOAL_CHECK, defaultPhase = LifecyclePhase.VERIFY, threadSafe = true)
public class SpotlessCheckMojo extends AbstractSpotlessMojo {

private static final String INCREMENTAL_MESSAGE_PREFIX = "Spotless Violation: ";

public enum MessageSeverity {
WARNING(BuildContext.SEVERITY_WARNING), ERROR(BuildContext.SEVERITY_ERROR);
Comment thread
nedtwigg marked this conversation as resolved.

private final int severity;

MessageSeverity(int severity) {
this.severity = severity;
}

public int getSeverity() {
return severity;
}
}

/**
* The severity used to emit messages during incremental builds.
* Either {@code WARNING} or {@code ERROR}.
* @see AbstractSpotlessMojo#m2eEnableForIncrementalBuild
*/
@Parameter(defaultValue = "WARNING")
private MessageSeverity m2eIncrementalBuildMessageSeverity;

@Override
protected void process(Iterable<File> files, Formatter formatter, UpToDateChecker upToDateChecker) throws MojoExecutionException {
ImpactedFilesTracker counter = new ImpactedFilesTracker();
Expand All @@ -51,14 +76,14 @@ protected void process(Iterable<File> files, Formatter formatter, UpToDateChecke
}
continue;
}

buildContext.removeMessages(file);
try {
PaddedCell.DirtyState dirtyState = PaddedCell.calculateDirtyState(formatter, file);
if (!dirtyState.isClean() && !dirtyState.didNotConverge()) {
problemFiles.add(file);
if (buildContext.isIncremental()) {
Map.Entry<Integer, String> diffEntry = DiffMessageFormatter.diff(formatter, file);
buildContext.addMessage(file, diffEntry.getKey() + 1, 0, diffEntry.getValue(), BuildContext.SEVERITY_ERROR, null);
buildContext.addMessage(file, diffEntry.getKey() + 1, 0, INCREMENTAL_MESSAGE_PREFIX + diffEntry.getValue(), m2eIncrementalBuildMessageSeverity.getSeverity(), null);
}
counter.cleaned();
} else {
Expand Down