Skip to content
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.

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

Lint suppression API for Gradle #2307

Merged
merged 16 commits into from
Oct 24, 2024
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
2 changes: 1 addition & 1 deletion CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (

## [Unreleased]
### Added
* APIs to support linting. (implemented in [#2148](https://github.com/diffplug/spotless/pull/2148) and [#2149](https://github.com/diffplug/spotless/pull/2149))
* APIs to support linting. (implemented in [#2148](https://github.com/diffplug/spotless/pull/2148), [#2149](https://github.com/diffplug/spotless/pull/2149), [#2307](https://github.com/diffplug/spotless/pull/2307))
* Spotless is still primarily a formatter, not a linter. But when formatting fails, it's more flexible to model those failures as lints so that the formatting can continue and ideally we can also capture the line numbers causing the failure.
* `Lint` models a single change. A `FormatterStep` can create a lint by:
* throwing an exception during formatting, ideally `throw Lint.atLine(127, "code", "Well what happened was...")`
Expand Down
2 changes: 1 addition & 1 deletion lib/src/main/java/com/diffplug/spotless/DirtyState.java
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public static DirtyState of(Formatter formatter, File file, byte[] rawBytes) {
public static DirtyState of(Formatter formatter, File file, byte[] rawBytes, String raw) {
var valuePerStep = new ValuePerStep<Throwable>(formatter);
DirtyState state = of(formatter, file, rawBytes, raw, valuePerStep);
LintPolicy.legacyBehavior(formatter, file, valuePerStep);
Formatter.legacyErrorBehavior(formatter, file, valuePerStep);
return state;
}

Expand Down

This file was deleted.

17 changes: 16 additions & 1 deletion lib/src/main/java/com/diffplug/spotless/Formatter.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@
import java.util.List;
import java.util.Objects;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/** Formatter which performs the full formatting. */
public final class Formatter implements Serializable, AutoCloseable {
private static final long serialVersionUID = 1L;
Expand Down Expand Up @@ -130,10 +133,22 @@ public String computeLineEndings(String unix, File file) {
public String compute(String unix, File file) {
ValuePerStep<Throwable> exceptionPerStep = new ValuePerStep<>(this);
String result = computeWithLint(unix, file, exceptionPerStep);
LintPolicy.legacyBehavior(this, file, exceptionPerStep);
legacyErrorBehavior(this, file, exceptionPerStep);
return result;
}

static void legacyErrorBehavior(Formatter formatter, File file, ValuePerStep<Throwable> exceptionPerStep) {
for (int i = 0; i < formatter.getSteps().size(); ++i) {
Throwable exception = exceptionPerStep.get(i);
if (exception != null && exception != LintState.formatStepCausedNoChange()) {
logger.error("Step '{}' found problem in '{}':\n{}", formatter.getSteps().get(i).getName(), file.getName(), exception.getMessage(), exception);
throw ThrowingEx.asRuntimeRethrowError(exception);
}
}
}

private static final Logger logger = LoggerFactory.getLogger(Formatter.class);

/**
* Returns the result of calling all of the FormatterSteps, while also
* tracking any exceptions which are thrown.
Expand Down
52 changes: 38 additions & 14 deletions lib/src/main/java/com/diffplug/spotless/Lint.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,28 +37,28 @@ public static Lint atLine(int line, String ruleId, String detail) {
return new Lint(line, ruleId, detail);
}

public static Lint atLineRange(int lineStart, int lineEnd, String ruleId, String detail) {
return new Lint(lineStart, lineEnd, ruleId, detail);
public static Lint atLineRange(int lineStart, int lineEnd, String shortCode, String detail) {
return new Lint(lineStart, lineEnd, shortCode, detail);
}

private static final long serialVersionUID = 1L;

private int lineStart, lineEnd; // 1-indexed, inclusive
private String ruleId; // e.g. CN_IDIOM https://spotbugs.readthedocs.io/en/stable/bugDescriptions.html#cn-class-implements-cloneable-but-does-not-define-or-use-clone-method-cn-idiom
private String shortCode; // e.g. CN_IDIOM https://spotbugs.readthedocs.io/en/stable/bugDescriptions.html#cn-class-implements-cloneable-but-does-not-define-or-use-clone-method-cn-idiom
private String detail;

private Lint(int lineStart, int lineEnd, String ruleId, String detail) {
private Lint(int lineStart, int lineEnd, String shortCode, String detail) {
if (lineEnd < lineStart) {
throw new IllegalArgumentException("lineEnd must be >= lineStart: lineStart=" + lineStart + " lineEnd=" + lineEnd);
}
this.lineStart = lineStart;
this.lineEnd = lineEnd;
this.ruleId = LineEnding.toUnix(ruleId);
this.shortCode = LineEnding.toUnix(shortCode);
this.detail = LineEnding.toUnix(detail);
}

private Lint(int line, String ruleId, String detail) {
this(line, line, ruleId, detail);
private Lint(int line, String shortCode, String detail) {
this(line, line, shortCode, detail);
}

public int getLineStart() {
Expand All @@ -69,8 +69,8 @@ public int getLineEnd() {
return lineEnd;
}

public String getRuleId() {
return ruleId;
public String getShortCode() {
return shortCode;
}

public String getDetail() {
Expand Down Expand Up @@ -115,12 +115,12 @@ public RuntimeException shortcut() {
public String toString() {
if (lineStart == lineEnd) {
if (lineStart == LINE_UNDEFINED) {
return "LINE_UNDEFINED: (" + ruleId + ") " + detail;
return "LINE_UNDEFINED: (" + shortCode + ") " + detail;
} else {
return "L" + lineStart + ": (" + ruleId + ") " + detail;
return "L" + lineStart + ": (" + shortCode + ") " + detail;
}
} else {
return "L" + lineStart + "-" + lineEnd + ": (" + ruleId + ") " + detail;
return "L" + lineStart + "-" + lineEnd + ": (" + shortCode + ") " + detail;
}
}

Expand All @@ -131,12 +131,12 @@ public boolean equals(Object o) {
if (o == null || getClass() != o.getClass())
return false;
Lint lint = (Lint) o;
return lineStart == lint.lineStart && lineEnd == lint.lineEnd && Objects.equals(ruleId, lint.ruleId) && Objects.equals(detail, lint.detail);
return lineStart == lint.lineStart && lineEnd == lint.lineEnd && Objects.equals(shortCode, lint.shortCode) && Objects.equals(detail, lint.detail);
}

@Override
public int hashCode() {
return Objects.hash(lineStart, lineEnd, ruleId, detail);
return Objects.hash(lineStart, lineEnd, shortCode, detail);
}

/** Attempts to parse a line number from the given exception. */
Expand Down Expand Up @@ -189,4 +189,28 @@ private static String msgFrom(String message) {
}

public static final int LINE_UNDEFINED = -1;

public void addWarningMessageTo(StringBuilder buffer, String stepName, boolean oneLine) {
if (lineStart == Lint.LINE_UNDEFINED) {
buffer.append("LINE_UNDEFINED");
} else {
buffer.append("L");
buffer.append(lineStart);
if (lineEnd != lineStart) {
buffer.append("-").append(lineEnd);
}
}
buffer.append(" ");
buffer.append(stepName).append("(").append(shortCode).append(") ");

int firstNewline = detail.indexOf('\n');
if (firstNewline == -1) {
buffer.append(detail);
} else if (oneLine) {
buffer.append(detail, 0, firstNewline);
buffer.append(" (...)");
} else {
buffer.append(detail);
}
}
}
43 changes: 0 additions & 43 deletions lib/src/main/java/com/diffplug/spotless/LintPolicy.java

This file was deleted.

71 changes: 45 additions & 26 deletions lib/src/main/java/com/diffplug/spotless/LintState.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,18 @@
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;

import javax.annotation.Nullable;

public class LintState {
private final DirtyState dirtyState;
private final @Nullable List<List<Lint>> lintsPerStep;

private LintState(DirtyState dirtyState, @Nullable List<List<Lint>> lintsPerStep) {
LintState(DirtyState dirtyState, @Nullable List<List<Lint>> lintsPerStep) {
this.dirtyState = dirtyState;
this.lintsPerStep = lintsPerStep;
}
Expand All @@ -45,23 +46,61 @@ public boolean isClean() {
return dirtyState.isClean() && !isHasLints();
}

public Map<FormatterStep, List<Lint>> getLints(Formatter formatter) {
public LinkedHashMap<String, List<Lint>> getLintsByStep(Formatter formatter) {
if (lintsPerStep == null) {
throw new IllegalStateException("Check `isHasLints` first!");
}
if (lintsPerStep.size() != formatter.getSteps().size()) {
throw new IllegalStateException("LintState was created with a different formatter!");
}
Map<FormatterStep, List<Lint>> result = new LinkedHashMap<>();
LinkedHashMap<String, List<Lint>> result = new LinkedHashMap<>();
for (int i = 0; i < lintsPerStep.size(); i++) {
List<Lint> lints = lintsPerStep.get(i);
if (lints != null) {
result.put(formatter.getSteps().get(i), lints);
FormatterStep step = formatter.getSteps().get(i);
result.put(step.getName(), lints);
}
}
return result;
}

public LintState withRemovedSuppressions(Formatter formatter, String relativePath, List<LintSuppression> suppressions) {
if (lintsPerStep == null) {
return this;
}
if (formatter.getSteps().size() != lintsPerStep.size()) {
throw new IllegalStateException("LintState was created with a different formatter!");
}
boolean changed = false;
ValuePerStep<List<Lint>> perStepFiltered = new ValuePerStep<>(formatter);
for (int i = 0; i < lintsPerStep.size(); i++) {
FormatterStep step = formatter.getSteps().get(i);
List<Lint> lintsOriginal = lintsPerStep.get(i);
if (lintsOriginal != null) {
List<Lint> lints = new ArrayList<>(lintsOriginal);
Iterator<Lint> iter = lints.iterator();
while (iter.hasNext()) {
Lint lint = iter.next();
for (LintSuppression suppression : suppressions) {
if (suppression.suppresses(relativePath, step, lint)) {
changed = true;
iter.remove();
break;
}
}
}
if (!lints.isEmpty()) {
perStepFiltered.set(i, lints);
}
}
}
if (changed) {
return new LintState(dirtyState, perStepFiltered.indexOfFirstValue() == -1 ? null : perStepFiltered);
} else {
return this;
}
}

public String asStringDetailed(File file, Formatter formatter) {
return asString(file, formatter, false);
}
Expand All @@ -81,27 +120,7 @@ private String asString(File file, Formatter formatter, boolean oneLine) {
FormatterStep step = formatter.getSteps().get(i);
for (Lint lint : lints) {
result.append(file.getName()).append(":");
if (lint.getLineStart() == Lint.LINE_UNDEFINED) {
result.append("LINE_UNDEFINED");
} else {
result.append("L");
result.append(lint.getLineStart());
if (lint.getLineEnd() != lint.getLineStart()) {
result.append("-").append(lint.getLineEnd());
}
}
result.append(" ");
result.append(step.getName()).append("(").append(lint.getRuleId()).append(") ");

int firstNewline = lint.getDetail().indexOf('\n');
if (firstNewline == -1) {
result.append(lint.getDetail());
} else if (oneLine) {
result.append(lint.getDetail(), 0, firstNewline);
result.append(" (...)");
} else {
result.append(lint.getDetail());
}
lint.addWarningMessageTo(result, step.getName(), oneLine);
result.append("\n");
}
}
Expand Down
Loading
Loading