Skip to content

Commit

Permalink
Adjust existing steps to be lint-friendly (#2305)
Browse files Browse the repository at this point in the history
  • Loading branch information
nedtwigg authored Oct 21, 2024
2 parents b2cbba2 + 7986002 commit b47e70e
Show file tree
Hide file tree
Showing 21 changed files with 137 additions and 106 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2023 DiffPlug
* Copyright 2023-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 Down Expand Up @@ -77,9 +77,7 @@ public String format(final File file, final String content, final boolean isScri
false,
new EditorConfigOverride(),
false));

DiktatReporting.reportIfRequired(errors, LintError::getLine, LintError::getCol, LintError::getDetail);

DiktatReporting.reportIfRequired(errors, LintError::getLine, LintError::getRuleId, LintError::getDetail);
return result;
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2023 DiffPlug
* Copyright 2023-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 Down Expand Up @@ -49,7 +49,7 @@ public DiktatCompat2Dot0Dot0Adapter(@Nullable File configFile) {
public String format(File file, String content, boolean isScript) {
errors.clear();
String result = processor.fix(content, file.toPath(), formatterCallback);
DiktatReporting.reportIfRequired(errors, DiktatError::getLine, DiktatError::getCol, DiktatError::getDetail);
DiktatReporting.reportIfRequired(errors, DiktatError::getLine, DiktatError::getRuleId, DiktatError::getDetail);
return result;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2023 DiffPlug
* Copyright 2023-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 @@ -15,28 +15,45 @@
*/
package com.diffplug.spotless.glue.diktat.compat;

import java.io.Serializable;
import java.util.ArrayList;
import java.util.List;
import java.util.function.Function;
import java.util.function.ToIntFunction;

interface DiktatReporting {
public interface DiktatReporting {
class Lint implements Serializable {
private static final long serialVersionUID = 1L;
public final int line;
public final String ruleId;
public final String detail;

Lint(int line, String ruleId, String detail) {
this.line = line;
this.ruleId = ruleId;
this.detail = detail;
}
}

class LintException extends RuntimeException {
public final List<Lint> lints;

LintException(List<Lint> lints) {
this.lints = lints;
}
}

static <T> void reportIfRequired(
List<T> errors,
ToIntFunction<T> lineGetter,
ToIntFunction<T> columnGetter,
Function<T, String> codeGetter,
Function<T, String> detailGetter) {
if (!errors.isEmpty()) {
StringBuilder error = new StringBuilder();
error.append("There are ").append(errors.size()).append(" unfixed errors:");
var lints = new ArrayList<Lint>();
for (T er : errors) {
error.append(System.lineSeparator())
.append("Error on line: ").append(lineGetter.applyAsInt(er))
.append(", column: ").append(columnGetter.applyAsInt(er))
.append(" cannot be fixed automatically")
.append(System.lineSeparator())
.append(detailGetter.apply(er));
lints.add(new Lint(lineGetter.applyAsInt(er), codeGetter.apply(er), detailGetter.apply(er)));
}
throw new AssertionError(error);
throw new LintException(lints);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2022 DiffPlug
* Copyright 2022-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 @@ -15,11 +15,25 @@
*/
package com.diffplug.spotless.glue.ktlint.compat;

final class KtLintCompatReporting {
public final class KtLintCompatReporting {

private KtLintCompatReporting() {}

static void report(int line, int column, String ruleId, String detail) {
throw new AssertionError("Error on line: " + line + ", column: " + column + "\nrule: " + ruleId + "\n" + detail);
throw new KtlintSpotlessException(line, ruleId, detail);
}

public static class KtlintSpotlessException extends RuntimeException {
private static final long serialVersionUID = 1L;

public final int line;
public final String ruleId;
public final String detail;

KtlintSpotlessException(int line, String ruleId, String detail) {
this.line = line;
this.ruleId = ruleId;
this.detail = detail;
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2021-2023 DiffPlug
* Copyright 2021-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 @@ -16,11 +16,14 @@
package com.diffplug.spotless.glue.diktat;

import java.io.File;
import java.util.stream.Collectors;

import com.diffplug.spotless.FormatterFunc;
import com.diffplug.spotless.Lint;
import com.diffplug.spotless.glue.diktat.compat.DiktatCompat1Dot2Dot5Adapter;
import com.diffplug.spotless.glue.diktat.compat.DiktatCompat2Dot0Dot0Adapter;
import com.diffplug.spotless.glue.diktat.compat.DiktatCompatAdapter;
import com.diffplug.spotless.glue.diktat.compat.DiktatReporting;

public class DiktatFormatterFunc implements FormatterFunc.NeedsFile {
private final DiktatCompatAdapter adapter;
Expand All @@ -40,6 +43,10 @@ public DiktatFormatterFunc(

@Override
public String applyWithFile(String unix, File file) {
return adapter.format(file, unix, isScript);
try {
return adapter.format(file, unix, isScript);
} catch (DiktatReporting.LintException e) {
throw Lint.shortcut(e.lints.stream().map(lint -> Lint.atLine(lint.line, lint.ruleId, lint.detail)).collect(Collectors.toList()));
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2023 DiffPlug
* Copyright 2023-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 Down Expand Up @@ -32,7 +32,7 @@

public class GsonFormatterFunc implements FormatterFunc {

private static final String FAILED_TO_PARSE_ERROR_MESSAGE = "Unable to format JSON";
private static final String FAILED_TO_PARSE_ERROR_MESSAGE = "Unable to parse JSON";

private final Gson gson;
private final GsonConfig gsonConfig;
Expand All @@ -56,7 +56,7 @@ public String apply(String inputString) {
} else {
JsonElement jsonElement = gson.fromJson(inputString, JsonElement.class);
if (jsonElement == null) {
throw new AssertionError(FAILED_TO_PARSE_ERROR_MESSAGE);
throw new IllegalArgumentException(FAILED_TO_PARSE_ERROR_MESSAGE);
}
if (gsonConfig.isSortByKeys()) {
jsonElement = sortByKeys(jsonElement);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import com.diffplug.spotless.FileSignature;
import com.diffplug.spotless.FormatterFunc;
import com.diffplug.spotless.Lint;
import com.diffplug.spotless.glue.ktlint.compat.*;

public class KtlintFormatterFunc implements FormatterFunc.NeedsFile {
Expand Down Expand Up @@ -65,10 +66,14 @@ public String applyWithFile(String unix, File file) throws NoSuchFieldException,
if (editorConfigPath != null) {
absoluteEditorConfigPath = editorConfigPath.getOnlyFile().toPath();
}
return adapter.format(
unix,
file.toPath(),
absoluteEditorConfigPath,
editorConfigOverrideMap);
try {
return adapter.format(
unix,
file.toPath(),
absoluteEditorConfigPath,
editorConfigOverrideMap);
} catch (KtLintCompatReporting.KtlintSpotlessException e) {
throw Lint.atLine(e.line, e.ruleId, e.detail).shortcut();
}
}
}
6 changes: 4 additions & 2 deletions lib/src/main/java/com/diffplug/spotless/Jvm.java
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 Down Expand Up @@ -61,6 +61,8 @@ public static int version() {
* @param <V> Version type of formatter
*/
public static class Support<V> {
static final String LINT_CODE = "jvm-version";

private final String fmtName;
private final Comparator<? super V> fmtVersionComparator;
private final NavigableMap<Integer, V> jvm2fmtMaxVersion;
Expand Down Expand Up @@ -139,7 +141,7 @@ public void assertFormatterSupported(V formatterVersion) {
Objects.requireNonNull(formatterVersion);
String error = buildUnsupportedFormatterMessage(formatterVersion);
if (!error.isEmpty()) {
throw new IllegalArgumentException(error);
throw Lint.atUndefinedLine(LINT_CODE, error).shortcut();
}
}

Expand Down
21 changes: 19 additions & 2 deletions lib/src/main/java/com/diffplug/spotless/LintState.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,15 @@ public Map<FormatterStep, List<Lint>> getLints(Formatter formatter) {
return result;
}

public String asString(File file, Formatter formatter) {
public String asStringDetailed(File file, Formatter formatter) {
return asString(file, formatter, false);
}

public String asStringOneLine(File file, Formatter formatter) {
return asString(file, formatter, true);
}

private String asString(File file, Formatter formatter, boolean oneLine) {
if (!isHasLints()) {
return "(none)";
} else {
Expand All @@ -84,7 +92,16 @@ public String asString(File file, Formatter formatter) {
}
result.append(" ");
result.append(step.getName()).append("(").append(lint.getRuleId()).append(") ");
result.append(lint.getDetail());

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());
}
result.append("\n");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,9 +210,8 @@ protected String assembleGroups(String unix) {
} else {
pattern = regex.pattern();
}
throw new Lint.ShortcutException(Lint.create("fenceRemoved",
"An intermediate step removed a match of " + pattern,
startLine, endLine));
throw Lint.atLineRange(startLine, endLine, "fenceRemoved",
"An intermediate step removed a match of " + pattern).shortcut();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
package com.diffplug.spotless.kotlin;

import java.io.File;
import java.io.IOException;
import java.io.Serializable;
import java.lang.reflect.Constructor;
import java.util.*;
Expand Down Expand Up @@ -95,7 +94,7 @@ static final class State implements Serializable {
private final boolean isScript;
private final @Nullable FileSignature config;

State(JarState jar, String versionDiktat, boolean isScript, @Nullable FileSignature config) throws IOException {
State(JarState jar, String versionDiktat, boolean isScript, @Nullable FileSignature config) {
this.jar = jar;
this.versionDiktat = versionDiktat;
this.isScript = isScript;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@
*/
package com.diffplug.spotless.maven.kotlin;

import static org.junit.jupiter.api.Assertions.assertTrue;

import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.Test;

import com.diffplug.spotless.ProcessRunner;
Expand Down Expand Up @@ -89,7 +88,7 @@ void testWithCustomRuleSetApply() throws Exception {
"</ktlint>");
setFile("src/main/kotlin/Main.kt").toResource("kotlin/ktlint/listScreen.dirty");
ProcessRunner.Result result = mavenRunner().withArguments("spotless:check").runHasError();
assertTrue(result.toString().contains("Composable functions that return Unit should start with an uppercase letter."));
Assertions.assertThat(result.toString()).contains("Composable functions that return Unit should start with an uppercase letter.");
}

private void checkKtlintOfficialStyle() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public StringSelfie expectLintsOf(String before) {
}

static StringSelfie expectLintsOf(LintState state, Formatter formatter) {
String assertAgainst = state.asString(Formatter.NO_FILE_SENTINEL, formatter);
String assertAgainst = state.asStringOneLine(Formatter.NO_FILE_SENTINEL, formatter);
String cleaned = assertAgainst.replace("NO_FILE_SENTINEL:", "");

int numLines = 1;
Expand Down
10 changes: 5 additions & 5 deletions testlib/src/test/java/com/diffplug/spotless/JvmTest.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2021-2023 DiffPlug
* Copyright 2021-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 Down Expand Up @@ -94,9 +94,9 @@ void supportListsMinimumJvmIfOnlyHigherJvmSupported() {
assertNull(testSupport.getRecommendedFormatterVersion(), "No formatter version is supported");

for (String fmtVersion : Arrays.asList("1.2", "1.2.3", "1.2-SNAPSHOT", "1.2.3-SNAPSHOT")) {
String proposal = assertThrows(Exception.class, () -> {
String proposal = assertThrows(Lint.ShortcutException.class, () -> {
testSupport.assertFormatterSupported(fmtVersion);
}).getMessage();
}).getLints().get(0).getDetail();
assertThat(proposal).contains(String.format("on JVM %d", Jvm.version()));
assertThat(proposal).contains(String.format("%s %s requires JVM %d+", TEST_NAME, fmtVersion, higherJvmVersion));
assertThat(proposal).contains(String.format("try %s alternatives", TEST_NAME));
Expand All @@ -112,9 +112,9 @@ void supportListsMinimumJvmIfOnlyHigherJvmSupported() {
}

for (String fmtVersion : Arrays.asList("1.2.4", "2", "1.2.5-SNAPSHOT")) {
String proposal = assertThrows(Exception.class, () -> {
String proposal = assertThrows(Lint.ShortcutException.class, () -> {
testSupport.assertFormatterSupported(fmtVersion);
}).getMessage();
}).getLints().get(0).getDetail();
assertThat(proposal).contains(String.format("%s %s requires JVM %d+", TEST_NAME, fmtVersion, higherJvmVersion + 1));

proposal = assertThrows(Exception.class, () -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ void broken() {
"spotless:off",
"D E F",
"spotless:on",
"G H I")).toBe("1-6 fence(fenceRemoved) An intermediate step removed a match of spotless:off spotless:on");
"G H I")).toBe("L1-6 fence(fenceRemoved) An intermediate step removed a match of spotless:off spotless:on");
}

@Test
Expand Down
Loading

0 comments on commit b47e70e

Please sign in to comment.