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

Adjust existing steps to be lint-friendly #2305

Merged
merged 9 commits into from
Oct 21, 2024
Merged
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
Loading