I'm interested in this feature, so I'm planning to implement it myself. 🚀

Wonderful!!!

the feature was blocked due to internal changes, but looking at the PR, it seems that it has since been merged.

Correct, the blockers to this have been solved. Our core abstractions are now very stable, I anticipate no changes to our core abstractions in the foreseeable future.

Big picture, the goal of Spotless is to be a formatter, not a linter. But inevitably, there are some "formatting issues" which can't be automatically fixed. Syntax errors, for example. And there are linters which can sometimes automatically provide fixes, which makes them more like a formatter. So the line is blurrier than it first appears.

Spotless handles this like so:

class DirtyState {
  boolean isClean()
  byte[] canonicalBytes() // only legal to call if !isClean()
}
class Lint {
  int lineStart, lineEnd; //1-indexed, inclusive
  String shortCode, detail;
}
class LintState {
  DirtyState dirtyState;
  List<List<Lint>> lintsPerStep;
}
// key entrypoint
public static LintState of(Formatter formatter, File file) { ... }

So inside a spotlessCheck, you have the LintState of each file, which tells you two separate things:

the LintState.dirtyState gives you the clean state of the file. You'll have to diff it yourself to create the hunks that can automatically move the code towards its formatted state

spotless/lib-extra/src/main/java/com/diffplug/spotless/extra/integration/DiffMessageFormatter.java

Lines 278 to 281 in d97b95d

    
           RawText a = new RawText(dirty.getBytes(StandardCharsets.UTF_8)); 
        
           RawText b = new RawText(clean.getBytes(StandardCharsets.UTF_8)); 
        
           EditList edits = new EditList(); 
        
           edits.addAll(MyersDiff.INSTANCE.diff(RawTextComparator.DEFAULT, a, b));

the LintState.lintsPerStep give you the things which could not be fixed automatically. Syntax errors, maybe a rule that says "don't import lib.internal.*".

key issue these lints are relative to the clean version
Spotless always does idempotency fixing. It's possible that in the original version, a lint exists which is resolved by the formatting. And it's possible that the final formatted version creates a lint from the perspective of one of the steps.
So we actually can't meaningfully show the lints until the formatting has happened

spotless/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/SpotlessCheck.java

Lines 69 to 85 in d97b95d

    
           if (!unformattedFiles.isEmpty()) { 
        
           	// if any files are unformatted, we show those 
        
           	throw new GradleException(DiffMessageFormatter.builder() 
        
           			.runToFix(getRunToFixMessage().get()) 
        
           			.formatterFolder( 
        
           					getProjectDir().get().getAsFile().toPath(), 
        
           					getSpotlessCleanDirectory().get().toPath(), 
        
           					getEncoding().get()) 
        
           			.problemFiles(unformattedFiles) 
        
           			.getMessage()); 
        
           } else { 
        
           	// We only show lints if there are no unformatted files. 
        
           	// This is because lint line numbers are relative to the 
        
           	// formatted content, and formatting often fixes lints. 
        
           	boolean detailed = false; 
        
           	throw new GradleException(super.allLintsErrorMsgDetailed(lintsFiles, detailed)); 
        
           }

It seems more complicated than it needs to be. But until we separated out this lint concept, it was common for silly little quibbles to prevent a file from being formatted. Now the file can get formatted first, and then we fail the build based on the quibble.

If you run spotlessApply, you only get one failure - the formatting is fixed automatically and then it yells at you about the lints. But if you're using spotlessCheck to generate PR comments, there is some jankiness in that you will first get yelled at because of formatting, and then you'll come back and get yelled at about lints. I think it's okay, because there aren't many lints - the point of Spotless is to just fix stuff! But the "pure formatter" abstraction is a bit leaky, and this is the fallout.

to ensure a smooth design process, I'd like to confirm the latest state of the issue

I have no strong opinion that it needs to use or avoid ReviewDog vs some other middleware vs go straight through hub4j/github-api. Implementor's choice. If one gets built I think it will be relatively easy for people to integrate with it in other ways later.

This isn't required, but I think snapshot testing will be quite helpful. We are already using Selfie for inline snapshots like so:

spotless/testlib/src/test/java/com/diffplug/spotless/java/GoogleJavaFormatStepTest.java

Lines 69 to 70 in d97b95d

    
           .expectLintsOfResource("java/googlejavaformat/JavaCodeWithLicenseUnformatted.test") 
        
           .toBe("LINE_UNDEFINED google-java-format(jvm-version) You are running Spotless on JVM 21. This requires google-java-format of at least 1.17.0 (you are using 1.2). (...)");

	RawText a = new RawText(dirty.getBytes(StandardCharsets.UTF_8));
	RawText b = new RawText(clean.getBytes(StandardCharsets.UTF_8));
	EditList edits = new EditList();
	edits.addAll(MyersDiff.INSTANCE.diff(RawTextComparator.DEFAULT, a, b));

	if (!unformattedFiles.isEmpty()) {
	// if any files are unformatted, we show those
	throw new GradleException(DiffMessageFormatter.builder()
	.runToFix(getRunToFixMessage().get())
	.formatterFolder(
	getProjectDir().get().getAsFile().toPath(),
	getSpotlessCleanDirectory().get().toPath(),
	getEncoding().get())
	.problemFiles(unformattedFiles)
	.getMessage());
	} else {
	// We only show lints if there are no unformatted files.
	// This is because lint line numbers are relative to the
	// formatted content, and formatting often fixes lints.
	boolean detailed = false;
	throw new GradleException(super.allLintsErrorMsgDetailed(lintsFiles, detailed));
	}

	.expectLintsOfResource("java/googlejavaformat/JavaCodeWithLicenseUnformatted.test")
	.toBe("LINE_UNDEFINED google-java-format(jvm-version) You are running Spotless on JVM 21. This requires google-java-format of at least 1.17.0 (you are using 1.2). (...)");

spotlessCheck could add comments and suggested fix to GitHub PR #655

Description

Activity

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

`spotlessCheck` could add comments and suggested fix to GitHub PR #655