Convert diktat integration to use a compile-only sourceset and fix fu…

…llpath issue (#1190 fixes #1189 and helps #524)
diffplug · May 3, 2022 · a643ae7 · a643ae7
2 parents 4d45472 + 335d495
commit a643ae7
Show file tree

Hide file tree

Showing 7 changed files with 122 additions and 92 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -10,6 +10,10 @@ This document is intended for Spotless developers.
 We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `1.27.0`).
 
 ## [Unreleased]
+### Changes
+* Bump default `diktat` version to latest `1.0.1` -> `1.1.0`. ([#1190](https://github.com/diffplug/spotless/pull/1190))
+  * Converted `diktat` integration to use a compile-only source set. (fixes [#524](https://github.com/diffplug/spotless/issues/524))
+  * Use the full path to a file in `diktat` integration. (fixes [#1189](https://github.com/diffplug/spotless/issues/1189))
 
 ## [2.25.1] - 2022-04-27
 ### Changes

diff --git a/lib/build.gradle b/lib/build.gradle
@@ -11,7 +11,8 @@ def NEEDS_GLUE = [
 	'palantirJavaFormat',
 	'ktfmt',
 	'ktlint',
-	'flexmark'
+	'flexmark',
+	'diktat'
 ]
 for (glue in NEEDS_GLUE) {
 	sourceSets.register(glue) {
@@ -50,6 +51,9 @@ dependencies {
 	ktlintCompileOnly "com.pinterest.ktlint:ktlint-ruleset-experimental:$VER_KTLINT"
 	ktlintCompileOnly "com.pinterest.ktlint:ktlint-ruleset-standard:$VER_KTLINT"
 
+	String VER_DIKTAT = "1.1.0"
+	diktatCompileOnly "org.cqfn.diktat:diktat-rules:$VER_DIKTAT"
+
 	// used for markdown formatting
 	flexmarkCompileOnly 'com.vladsch.flexmark:flexmark-all:0.62.2'
 }

diff --git a/lib/src/diktat/java/com/diffplug/spotless/glue/diktat/DiktatFormatterFunc.java b/lib/src/diktat/java/com/diffplug/spotless/glue/diktat/DiktatFormatterFunc.java
@@ -0,0 +1,97 @@
+/*
+ * Copyright 2021-2022 DiffPlug
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.diffplug.spotless.glue.diktat;
+
+import java.io.File;
+import java.util.*;
+
+import org.cqfn.diktat.ruleset.rules.DiktatRuleSetProvider;
+
+import com.pinterest.ktlint.core.KtLint;
+import com.pinterest.ktlint.core.KtLint.Params;
+import com.pinterest.ktlint.core.LintError;
+import com.pinterest.ktlint.core.RuleSet;
+
+import com.diffplug.spotless.FormatterFunc;
+
+import kotlin.Unit;
+import kotlin.jvm.functions.Function2;
+
+public class DiktatFormatterFunc implements FormatterFunc.NeedsFile {
+
+	private final List<RuleSet> rulesets;
+	private final Map<String, String> userData;
+	private final Function2<? super LintError, ? super Boolean, Unit> formatterCallback;
+	private final boolean isScript;
+
+	private final ArrayList<LintError> errors = new ArrayList<>();
+
+	public DiktatFormatterFunc(boolean isScript, Map<String, String> userData) {
+		rulesets = Collections.singletonList(new DiktatRuleSetProvider().get());
+		this.userData = userData;
+		this.formatterCallback = new FormatterCallback(errors);
+		this.isScript = isScript;
+	}
+
+	static class FormatterCallback implements Function2<LintError, Boolean, Unit> {
+		private final ArrayList<LintError> errors;
+
+		FormatterCallback(ArrayList<LintError> errors) {
+			this.errors = errors;
+		}
+
+		@Override
+		public Unit invoke(LintError lintError, Boolean corrected) {
+			if (!corrected) {
+				errors.add(lintError);
+			}
+			return null;
+		}
+	}
+
+	@Override
+	public String applyWithFile(String unix, File file) throws Exception {
+		errors.clear();
+		userData.put("file_path", file.getAbsolutePath());
+		String result = KtLint.INSTANCE.format(new Params(
+				// Unlike Ktlint, Diktat requires full path to the file.
+				// See https://github.com/diffplug/spotless/issues/1189, https://github.com/analysis-dev/diktat/issues/1202
+				file.getAbsolutePath(),
+				unix,
+				rulesets,
+				userData,
+				formatterCallback,
+				isScript,
+				null,
+				false));
+
+		if (!errors.isEmpty()) {
+			StringBuilder error = new StringBuilder();
+			error.append("There are ").append(errors.size()).append(" unfixed errors:");
+			for (LintError er : errors) {
+				error.append(System.lineSeparator())
+						.append("Error on line: ").append(er.getLine())
+						.append(", column: ").append(er.getCol())
+						.append(" cannot be fixed automatically")
+						.append(System.lineSeparator())
+						.append(er.getDetail());
+			}
+			throw new AssertionError(error);
+		}
+
+		return result;
+	}
+}
diff --git a/lib/src/main/java/com/diffplug/spotless/kotlin/DiktatStep.java b/lib/src/main/java/com/diffplug/spotless/kotlin/DiktatStep.java
@@ -18,9 +18,6 @@
 import java.io.IOException;
 import java.io.Serializable;
 import java.lang.reflect.Constructor;
-import java.lang.reflect.InvocationTargetException;
-import java.lang.reflect.Method;
-import java.lang.reflect.Proxy;
 import java.util.*;
 
 import javax.annotation.Nullable;
@@ -33,10 +30,9 @@ public class DiktatStep {
 	// prevent direct instantiation
 	private DiktatStep() {}
 
-	private static final String DEFAULT_VERSION = "1.0.1";
+	private static final String DEFAULT_VERSION = "1.1.0";
 	static final String NAME = "diktat";
 	static final String PACKAGE_DIKTAT = "org.cqfn.diktat";
-	static final String PACKAGE_KTLINT = "com.pinterest.ktlint";
 	static final String MAVEN_COORDINATE = PACKAGE_DIKTAT + ":diktat-rules:";
 
 	public static String defaultVersionDiktat() {
@@ -82,8 +78,6 @@ static final class State implements Serializable {
 		/** Are the files being linted Kotlin script files. */
 		private final boolean isScript;
 		private final @Nullable FileSignature config;
-		private final String pkg;
-		private final String pkgKtlint;
 		final JarState jar;
 		private final TreeMap<String, String> userData;
 
@@ -93,96 +87,19 @@ static final class State implements Serializable {
 			pkgSet.add(MAVEN_COORDINATE + versionDiktat);
 
 			this.userData = new TreeMap<>(userData);
-			this.pkg = PACKAGE_DIKTAT;
-			this.pkgKtlint = PACKAGE_KTLINT;
 			this.jar = JarState.from(pkgSet, provisioner);
 			this.isScript = isScript;
 			this.config = config;
 		}
 
 		FormatterFunc createFormat() throws Exception {
-
-			ClassLoader classLoader = jar.getClassLoader();
-
-			// first, we get the diktat rules
 			if (config != null) {
 				System.setProperty("diktat.config.path", config.getOnlyFile().getAbsolutePath());
 			}
 
-			Class<?> ruleSetProviderClass = classLoader.loadClass(pkg + ".ruleset.rules.DiktatRuleSetProvider");
-			Object diktatRuleSet = ruleSetProviderClass.getMethod("get").invoke(ruleSetProviderClass.newInstance());
-			Iterable<?> ruleSets = Collections.singletonList(diktatRuleSet);
-
-			// next, we create an error callback which throws an assertion error when the format is bad
-			Class<?> function2Interface = classLoader.loadClass("kotlin.jvm.functions.Function2");
-			Class<?> lintErrorClass = classLoader.loadClass(pkgKtlint + ".core.LintError");
-			Method detailGetter = lintErrorClass.getMethod("getDetail");
-			Method lineGetter = lintErrorClass.getMethod("getLine");
-			Method colGetter = lintErrorClass.getMethod("getCol");
-
-			// grab the KtLint singleton
-			Class<?> ktlintClass = classLoader.loadClass(pkgKtlint + ".core.KtLint");
-			Object ktlint = ktlintClass.getDeclaredField("INSTANCE").get(null);
-
-			Class<?> paramsClass = classLoader.loadClass(pkgKtlint + ".core.KtLint$Params");
-			// and its constructor
-			Constructor<?> constructor = paramsClass.getConstructor(
-					/* fileName, nullable */ String.class,
-					/* text */ String.class,
-					/* ruleSets */ Iterable.class,
-					/* userData */ Map.class,
-					/* callback */ function2Interface,
-					/* script */ boolean.class,
-					/* editorConfigPath, nullable */ String.class,
-					/* debug */ boolean.class);
-			Method formatterMethod = ktlintClass.getMethod("format", paramsClass);
-			FormatterFunc.NeedsFile formatterFunc = (input, file) -> {
-				ArrayList<Object> errors = new ArrayList<>();
-
-				Object formatterCallback = Proxy.newProxyInstance(classLoader, new Class[]{function2Interface},
-						(proxy, method, args) -> {
-							Object lintError = args[0]; //ktlint.core.LintError
-							boolean corrected = (Boolean) args[1];
-							if (!corrected) {
-								errors.add(lintError);
-							}
-							return null;
-						});
-
-				userData.put("file_path", file.getAbsolutePath());
-				try {
-					Object params = constructor.newInstance(
-							/* fileName, nullable */ file.getName(),
-							/* text */ input,
-							/* ruleSets */ ruleSets,
-							/* userData */ userData,
-							/* callback */ formatterCallback,
-							/* script */ isScript,
-							/* editorConfigPath, nullable */ null,
-							/* debug */ false);
-					String result = (String) formatterMethod.invoke(ktlint, params);
-					if (!errors.isEmpty()) {
-						StringBuilder error = new StringBuilder("");
-						error.append("There are ").append(errors.size()).append(" unfixed errors:");
-						for (Object er : errors) {
-							String detail = (String) detailGetter.invoke(er);
-							int line = (Integer) lineGetter.invoke(er);
-							int col = (Integer) colGetter.invoke(er);
-
-							error.append(System.lineSeparator()).append("Error on line: ").append(line).append(", column: ").append(col).append(" cannot be fixed automatically")
-									.append(System.lineSeparator()).append(detail);
-						}
-						throw new AssertionError(error);
-					}
-					return result;
-				} catch (InvocationTargetException e) {
-					throw ThrowingEx.unwrapCause(e);
-				}
-			};
-
-			return formatterFunc;
+			Class<?> formatterFunc = jar.getClassLoader().loadClass("com.diffplug.spotless.glue.diktat.DiktatFormatterFunc");
+			Constructor<?> constructor = formatterFunc.getConstructor(boolean.class, Map.class);
+			return (FormatterFunc.NeedsFile) constructor.newInstance(isScript, userData);
 		}
-
 	}
-
 }
diff --git a/plugin-gradle/CHANGES.md b/plugin-gradle/CHANGES.md
@@ -3,6 +3,10 @@
 We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `3.27.0`).
 
 ## [Unreleased]
+### Changes
+* Bump default `diktat` version to latest `1.0.1` -> `1.1.0`. ([#1190](https://github.com/diffplug/spotless/pull/1190))
+  * Converted `diktat` integration to use a compile-only source set. (fixes [#524](https://github.com/diffplug/spotless/issues/524))
+  * Use the full path to a file in `diktat` integration. (fixes [#1189](https://github.com/diffplug/spotless/issues/1189))
 
 ## [6.5.1] - 2022-04-27
 ### Changes

diff --git a/plugin-maven/CHANGES.md b/plugin-maven/CHANGES.md
@@ -3,6 +3,10 @@
 We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `1.27.0`).
 
 ## [Unreleased]
+### Changes
+* Bump default `diktat` version to latest `1.0.1` -> `1.1.0`. ([#1190](https://github.com/diffplug/spotless/pull/1190))
+  * Converted `diktat` integration to use a compile-only source set. (fixes [#524](https://github.com/diffplug/spotless/issues/524))
+  * Use the full path to a file in `diktat` integration. (fixes [#1189](https://github.com/diffplug/spotless/issues/1189))
 
 ## [2.22.3] - 2022-04-27
 ### Changes

diff --git a/testlib/src/test/java/com/diffplug/spotless/kotlin/DiktatStepTest.java b/testlib/src/test/java/com/diffplug/spotless/kotlin/DiktatStepTest.java
@@ -38,9 +38,9 @@ void behavior() throws Exception {
 					assertion.isInstanceOf(AssertionError.class);
 					assertion.hasMessage("There are 2 unfixed errors:" +
 							System.lineSeparator() + "Error on line: 1, column: 1 cannot be fixed automatically" +
-							System.lineSeparator() + "[FILE_NAME_INCORRECT] file name is incorrect - it should end with .kt extension and be in PascalCase: " +
+							System.lineSeparator() + "[FILE_NAME_INCORRECT] file name is incorrect - it should end with .kt extension and be in PascalCase: testlib" +
 							System.lineSeparator() + "Error on line: 1, column: 1 cannot be fixed automatically" +
-							System.lineSeparator() + "[FILE_NAME_MATCH_CLASS] file name is incorrect - it should match with the class described in it if there is the only one class declared:  vs Unsolvable");
+							System.lineSeparator() + "[FILE_NAME_MATCH_CLASS] file name is incorrect - it should match with the class described in it if there is the only one class declared: testlib vs Unsolvable");
 				});
 	}
 
@@ -57,9 +57,9 @@ void behaviorConf() throws Exception {
 					assertion.isInstanceOf(AssertionError.class);
 					assertion.hasMessage("There are 2 unfixed errors:" +
 							System.lineSeparator() + "Error on line: 1, column: 1 cannot be fixed automatically" +
-							System.lineSeparator() + "[FILE_NAME_INCORRECT] file name is incorrect - it should end with .kt extension and be in PascalCase: " +
+							System.lineSeparator() + "[FILE_NAME_INCORRECT] file name is incorrect - it should end with .kt extension and be in PascalCase: testlib" +
 							System.lineSeparator() + "Error on line: 1, column: 1 cannot be fixed automatically" +
-							System.lineSeparator() + "[FILE_NAME_MATCH_CLASS] file name is incorrect - it should match with the class described in it if there is the only one class declared:  vs Unsolvable");
+							System.lineSeparator() + "[FILE_NAME_MATCH_CLASS] file name is incorrect - it should match with the class described in it if there is the only one class declared: testlib vs Unsolvable");
 				});
 	}