Skip to content

Commit

Permalink
Lint suppression API for Gradle (#2307)
Browse files Browse the repository at this point in the history
  • Loading branch information
nedtwigg authored Oct 24, 2024
2 parents 13d24ae + 611a45d commit 7feb013
Show file tree
Hide file tree
Showing 24 changed files with 567 additions and 298 deletions.
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

0 comments on commit 7feb013

Please sign in to comment.