UP-TO-DATE checking for npm based step config

lib/src/main/java/com/diffplug/spotless/npm/EslintConfig.java

@@ -17,20 +17,20 @@
 
 import java.io.File;
 import java.io.Serializable;
+import java.util.Objects;
 
 import javax.annotation.Nullable;
 
-import com.diffplug.spotless.FileSignature;
-
 public class EslintConfig implements Serializable {
-	private static final long serialVersionUID = 1L;
+
+	private static final long serialVersionUID = -5436020379478813853L;
 
 	@SuppressWarnings("unused")
-	private final FileSignature.Promised eslintConfigPathSignature;
+	private final RoundtrippableFile eslintConfigPath;
 	private final String eslintConfigJs;
 
 	public EslintConfig(@Nullable File eslintConfigPath, @Nullable String eslintConfigJs) {
-		this.eslintConfigPathSignature = eslintConfigPath == null ? null : FileSignature.promise(eslintConfigPath);
+		this.eslintConfigPath = eslintConfigPath == null ? null : new RoundtrippableFile(eslintConfigPath);
 		this.eslintConfigJs = eslintConfigJs;
 	}
 
@@ -40,7 +40,7 @@ public EslintConfig withEslintConfigPath(@Nullable File eslintConfigPath) {
 
 	@Nullable
 	public File getEslintConfigPath() {
-		return eslintConfigPathSignature == null ? null : eslintConfigPathSignature.get().getOnlyFile();
+		return eslintConfigPath == null ? null : eslintConfigPath.file();
 	}
 
 	@Nullable
@@ -49,9 +49,24 @@ public String getEslintConfigJs() {
 	}
 
 	public EslintConfig verify() {
-		if (eslintConfigPathSignature == null && eslintConfigJs == null) {
+		if (eslintConfigPath == null && eslintConfigJs == null) {
 			throw new IllegalArgumentException("ESLint must be configured using either a configFile or a configJs - but both are null.");
 		}
 		return this;
 	}
+
+	@Override
+	public boolean equals(Object o) {
+		if (this == o)
+			return true;
+		if (!(o instanceof EslintConfig))
+			return false;
+		EslintConfig that = (EslintConfig) o;
+		return Objects.equals(eslintConfigPath, that.eslintConfigPath) && Objects.equals(eslintConfigJs, that.eslintConfigJs);
+	}
+
+	@Override
+	public int hashCode() {
+		return Objects.hash(eslintConfigPath, eslintConfigJs);
+	}
 }

lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2016-2024 DiffPlug
+ * Copyright 2024 DiffPlug
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -15,31 +15,41 @@
  */
 package com.diffplug.spotless.npm;
 
+import static com.diffplug.spotless.npm.PackageJsonUtil.replaceDevDependencies;
 import static java.util.Objects.requireNonNull;
 
 import java.io.File;
 import java.io.IOException;
-import java.io.Serializable;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.Map;
-import java.util.Objects;
 import java.util.TreeMap;
 
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.diffplug.spotless.FormatterFunc;
-import com.diffplug.spotless.FormatterFunc.Closeable;
 import com.diffplug.spotless.FormatterStep;
-import com.diffplug.spotless.Provisioner;
-import com.diffplug.spotless.ThrowingEx;
 import com.diffplug.spotless.npm.EslintRestService.FormatOption;
 
-public class EslintFormatterStep {
+/**
+ * Standard implementation of FormatterStep which cleanly enforces
+ * separation of a lazily computed "state" object whose serialized form
+ * is used as the basis for equality and hashCode, which is separate
+ * from the serialized form of the step itself, which can include absolute paths
+ * and such without interfering with buildcache keys.
+ */
+
+// => equals/hashcode should not include absolute paths and such
+
+// => serialized/deserialized state can include absolute paths and such and should recreate a valid/runnable state
+
+public class EslintFormatterStep extends NpmServerBasedFormatterStep {
+
+	// TODO (simschla, 03.07.2024): add test that asserts all fields of step behave correctly
 
 	private static final Logger logger = LoggerFactory.getLogger(EslintFormatterStep.class);
 
@@ -56,7 +66,7 @@ public static Map<String, String> defaultDevDependenciesTypescriptWithEslint(Str
 		dependencies.put("@typescript-eslint/eslint-plugin", "^6.1.0");
 		dependencies.put("@typescript-eslint/parser", "^6.1.0");
 		dependencies.put("typescript", "^5.1.6");
-		dependencies.put("eslint", Objects.requireNonNull(eslintVersion));
+		dependencies.put("eslint", requireNonNull(eslintVersion));
 		return dependencies;
 	}
 
@@ -68,118 +78,87 @@ public static Map<String, String> defaultDevDependenciesWithEslint(String versio
 		return Collections.singletonMap("eslint", version);
 	}
 
-	public static FormatterStep create(Map<String, String> devDependencies, Provisioner provisioner, File projectDir, File buildDir, File cacheDir, NpmPathResolver npmPathResolver, EslintConfig eslintConfig) {
-		requireNonNull(devDependencies);
-		requireNonNull(provisioner);
-		requireNonNull(projectDir);
-		requireNonNull(buildDir);
-		return FormatterStep.createLazy(NAME,
-				() -> new State(NAME, devDependencies, projectDir, buildDir, cacheDir, npmPathResolver, eslintConfig),
-				State::createFormatterFunc);
+	public static FormatterStep create(@Nonnull Map<String, String> devDependencies, @Nonnull File projectDir, @Nonnull File buildDir, @Nullable File cacheDir, @Nonnull NpmPathResolver npmPathResolver, @Nonnull EslintConfig eslintConfig) {
+		return new EslintFormatterStep(devDependencies, projectDir, buildDir, cacheDir, npmPathResolver, eslintConfig);
 	}
 
-	private static class State extends NpmFormatterStepStateBase implements Serializable {
-		private static final long serialVersionUID = 1L;
-
-		private final EslintConfig origEslintConfig;
-		private EslintConfig eslintConfigInUse;
-
-		State(String stepName, Map<String, String> devDependencies, File projectDir, File buildDir, File cacheDir, NpmPathResolver npmPathResolver, EslintConfig eslintConfig) throws IOException {
-			super(stepName,
-					new NpmConfig(
-							replaceDevDependencies(
-									NpmResourceHelper.readUtf8StringFromClasspath(EslintFormatterStep.class, "/com/diffplug/spotless/npm/eslint-package.json"),
-									new TreeMap<>(devDependencies)),
-							NpmResourceHelper.readUtf8StringFromClasspath(EslintFormatterStep.class,
-									"/com/diffplug/spotless/npm/common-serve.js",
-									"/com/diffplug/spotless/npm/eslint-serve.js"),
-							npmPathResolver.resolveNpmrcContent()),
-					new NpmFormatterStepLocations(
-							projectDir,
-							buildDir,
-							cacheDir,
-							npmPathResolver));
-			this.origEslintConfig = requireNonNull(eslintConfig.verify());
-			this.eslintConfigInUse = eslintConfig;
-		}
+	private EslintFormatterStep(@Nonnull Map<String, String> devDependencies, @Nonnull File projectDir, @Nonnull File buildDir, @Nullable File cacheDir, @Nonnull NpmPathResolver npmPathResolver, @Nonnull EslintConfig eslintConfig) {
+		super(NAME,
+				readPackageJsonContent(devDependencies),
+				readServeScriptContent(),
+				npmPathResolver.resolveNpmrcContent(),
+				Map.of(EslintConfigElement.ESLINT_CONFIG_ORIGINAL_ELEMENT, requireNonNull(eslintConfig)),
+				new NpmFormatterStepLocations(projectDir, buildDir, cacheDir, npmPathResolver));
+	}
 
-		@Override
-		protected void prepareNodeServerLayout(NodeServerLayout nodeServerLayout) throws IOException {
-			super.prepareNodeServerLayout(nodeServerLayout);
-			if (origEslintConfig.getEslintConfigPath() != null) {
-				// If any config files are provided, we need to make sure they are at the same location as the node modules
-				// as eslint will try to resolve plugin/config names relatively to the config file location and some
-				// eslint configs contain relative paths to additional config files (such as tsconfig.json e.g.)
-				logger.debug("Copying config file <{}> to <{}> and using the copy", origEslintConfig.getEslintConfigPath(), nodeServerLayout.nodeModulesDir());
-				File configFileCopy = NpmResourceHelper.copyFileToDir(origEslintConfig.getEslintConfigPath(), nodeServerLayout.nodeModulesDir());
-				this.eslintConfigInUse = this.origEslintConfig.withEslintConfigPath(configFileCopy).verify();
-			}
-		}
+	private static String readServeScriptContent() {
+		return NpmResourceHelper.readUtf8StringFromClasspath(EslintFormatterStep.class,
+				"/com/diffplug/spotless/npm/common-serve.js",
+				"/com/diffplug/spotless/npm/eslint-serve.js");
+	}
 
-		@Override
-		@Nonnull
-		public FormatterFunc createFormatterFunc() {
-			try {
-				logger.info("Creating formatter function (starting server)");
-				Runtime runtime = toRuntime();
-				ServerProcessInfo eslintRestServer = runtime.npmRunServer();
-				EslintRestService restService = new EslintRestService(eslintRestServer.getBaseUrl());
-				return Closeable.ofDangerous(() -> endServer(restService, eslintRestServer), new EslintFilePathPassingFormatterFunc(locations.projectDir(), runtime.nodeServerLayout().nodeModulesDir(), eslintConfigInUse, restService));
-			} catch (IOException e) {
-				throw ThrowingEx.asRuntime(e);
-			}
-		}
+	private static String readPackageJsonContent(Map<String, String> devDependencies) {
+		return replaceDevDependencies(
+				NpmResourceHelper.readUtf8StringFromClasspath(EslintFormatterStep.class, "/com/diffplug/spotless/npm/eslint-package.json"),
+				new TreeMap<>(devDependencies));
+	}
 
-		private void endServer(BaseNpmRestService restService, ServerProcessInfo restServer) throws Exception {
-			logger.info("Closing formatting function (ending server).");
-			try {
-				restService.shutdown();
-			} catch (Throwable t) {
-				logger.info("Failed to request shutdown of rest service via api. Trying via process.", t);
-			}
-			restServer.close();
-		}
+	protected EslintConfig origEslintConfig() {
+		return configElement(EslintConfigElement.ESLINT_CONFIG_ORIGINAL_ELEMENT);
+	}
 
+	protected EslintConfig eslintConfigInUse() {
+		return configElement(EslintConfigElement.ESLINT_CONFIG_IN_USE_ELEMENT);
 	}
 
-	private static class EslintFilePathPassingFormatterFunc implements FormatterFunc.NeedsFile {
-		private final File projectDir;
-		private final File nodeModulesDir;
-		private final EslintConfig eslintConfig;
-		private final EslintRestService restService;
-
-		public EslintFilePathPassingFormatterFunc(File projectDir, File nodeModulesDir, EslintConfig eslintConfig, EslintRestService restService) {
-			this.projectDir = requireNonNull(projectDir);
-			this.nodeModulesDir = requireNonNull(nodeModulesDir);
-			this.eslintConfig = requireNonNull(eslintConfig);
-			this.restService = requireNonNull(restService);
+	@Override
+	protected void doPrepareNodeServerLayout(NodeServerLayout nodeServerLayout) throws IOException {
+		if (origEslintConfig().getEslintConfigPath() != null) {
+			// If any config files are provided, we need to make sure they are at the same location as the node modules
+			// as eslint will try to resolve plugin/config names relatively to the config file location and some
+			// eslint configs contain relative paths to additional config files (such as tsconfig.json e.g.)
+			logger.debug("Copying config file <{}> to <{}> and using the copy", origEslintConfig().getEslintConfigPath(), nodeServerLayout.nodeModulesDir());
+			File configFileCopy = NpmResourceHelper.copyFileToDir(origEslintConfig().getEslintConfigPath(), nodeServerLayout.nodeModulesDir());
+			configElement(EslintConfigElement.ESLINT_CONFIG_IN_USE_ELEMENT, origEslintConfig().withEslintConfigPath(configFileCopy).verify());
+		} else {
+			configElement(EslintConfigElement.ESLINT_CONFIG_IN_USE_ELEMENT, origEslintConfig().verify());
 		}
+	}
 
-		@Override
-		public String applyWithFile(String unix, File file) throws Exception {
-			Map<FormatOption, Object> eslintCallOptions = new HashMap<>();
-			setConfigToCallOptions(eslintCallOptions);
-			setFilePathToCallOptions(eslintCallOptions, file);
-			return restService.format(unix, eslintCallOptions);
-		}
+	@Override
+	protected String formatWithServer(NpmServerProcessInfo serverProcessInfo, String rawUnix, File file) {
+		EslintRestService restService = new EslintRestService(serverProcessInfo.getBaseUrl());
+		Map<FormatOption, Object> eslintCallOptions = new HashMap<>();
+		setConfigToCallOptions(eslintCallOptions);
+		setFilePathToCallOptions(eslintCallOptions, file);
+		return restService.format(rawUnix, eslintCallOptions);
+	}
+
+	private void setFilePathToCallOptions(Map<FormatOption, Object> eslintCallOptions, File fileToBeFormatted) {
+		eslintCallOptions.put(FormatOption.FILE_PATH, fileToBeFormatted.getAbsolutePath());
+	}
 
-		private void setFilePathToCallOptions(Map<FormatOption, Object> eslintCallOptions, File fileToBeFormatted) {
-			eslintCallOptions.put(FormatOption.FILE_PATH, fileToBeFormatted.getAbsolutePath());
+	private void setConfigToCallOptions(Map<FormatOption, Object> eslintCallOptions) {
+		if (eslintConfigInUse().getEslintConfigPath() != null) {
+			eslintCallOptions.put(FormatOption.ESLINT_OVERRIDE_CONFIG_FILE, eslintConfigInUse().getEslintConfigPath().getAbsolutePath());
 		}
+		if (eslintConfigInUse().getEslintConfigJs() != null) {
+			eslintCallOptions.put(FormatOption.ESLINT_OVERRIDE_CONFIG, eslintConfigInUse().getEslintConfigJs());
+		}
+		if (eslintConfigInUse() instanceof EslintTypescriptConfig) {
+			// if we are a ts config, see if we need to use specific paths or use default projectDir
+			File tsConfigFilePath = ((EslintTypescriptConfig) eslintConfigInUse()).getTypescriptConfigPath();
+			File tsConfigRootDir = tsConfigFilePath != null ? tsConfigFilePath.getParentFile() : this.locations.projectDir();
+			eslintCallOptions.put(FormatOption.TS_CONFIG_ROOT_DIR, this.nodeServerLayout().nodeModulesDir().getAbsoluteFile().toPath().relativize(tsConfigRootDir.getAbsoluteFile().toPath()).toString());
+		}
+	}
 
-		private void setConfigToCallOptions(Map<FormatOption, Object> eslintCallOptions) {
-			if (eslintConfig.getEslintConfigPath() != null) {
-				eslintCallOptions.put(FormatOption.ESLINT_OVERRIDE_CONFIG_FILE, eslintConfig.getEslintConfigPath().getAbsolutePath());
+	private enum EslintConfigElement implements NpmConfigElement {
+		ESLINT_CONFIG_ORIGINAL_ELEMENT, ESLINT_CONFIG_IN_USE_ELEMENT {
+			@Override
+			public boolean equalsHashcodeRelevant() {
+				return false;
 			}
-			if (eslintConfig.getEslintConfigJs() != null) {
-				eslintCallOptions.put(FormatOption.ESLINT_OVERRIDE_CONFIG, eslintConfig.getEslintConfigJs());
-			}
-			if (eslintConfig instanceof EslintTypescriptConfig) {
-				// if we are a ts config, see if we need to use specific paths or use default projectDir
-				File tsConfigFilePath = ((EslintTypescriptConfig) eslintConfig).getTypescriptConfigPath();
-				File tsConfigRootDir = tsConfigFilePath != null ? tsConfigFilePath.getParentFile() : projectDir;
-				eslintCallOptions.put(FormatOption.TS_CONFIG_ROOT_DIR, nodeModulesDir.getAbsoluteFile().toPath().relativize(tsConfigRootDir.getAbsoluteFile().toPath()).toString());
-			}
-		}
+		};
 	}
 }

lib/src/main/java/com/diffplug/spotless/npm/EslintTypescriptConfig.java

@@ -16,20 +16,20 @@
 package com.diffplug.spotless.npm;
 
 import java.io.File;
+import java.util.Objects;
 
 import javax.annotation.Nullable;
 
-import com.diffplug.spotless.FileSignature;
-
 public class EslintTypescriptConfig extends EslintConfig {
-	private static final long serialVersionUID = 2L;
+
+	private static final long serialVersionUID = 7047648793633604218L;
 
 	@SuppressWarnings("unused")
-	private final FileSignature.Promised typescriptConfigPathSignature;
+	private final RoundtrippableFile typescriptConfigPath;
 
 	public EslintTypescriptConfig(@Nullable File eslintConfigPath, @Nullable String eslintConfigJs, @Nullable File typescriptConfigPath) {
 		super(eslintConfigPath, eslintConfigJs);
-		this.typescriptConfigPathSignature = typescriptConfigPath != null ? FileSignature.promise(typescriptConfigPath) : null;
+		this.typescriptConfigPath = typescriptConfigPath != null ? new RoundtrippableFile(typescriptConfigPath) : null;
 	}
 
 	@Override
@@ -39,6 +39,23 @@ public EslintConfig withEslintConfigPath(@Nullable File eslintConfigPath) {
 
 	@Nullable
 	public File getTypescriptConfigPath() {
-		return typescriptConfigPathSignature == null ? null : this.typescriptConfigPathSignature.get().getOnlyFile();
+		return typescriptConfigPath == null ? null : this.typescriptConfigPath.file();
+	}
+
+	@Override
+	public boolean equals(Object o) {
+		if (this == o)
+			return true;
+		if (!(o instanceof EslintTypescriptConfig))
+			return false;
+		if (!super.equals(o))
+			return false;
+		EslintTypescriptConfig that = (EslintTypescriptConfig) o;
+		return Objects.equals(typescriptConfigPath, that.typescriptConfigPath);
+	}
+
+	@Override
+	public int hashCode() {
+		return Objects.hash(super.hashCode(), typescriptConfigPath);
 	}
 }

lib/src/main/java/com/diffplug/spotless/npm/IrrelevantForEquality.java

@@ -0,0 +1,29 @@
+/*
+ * Copyright 2024 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.npm;
+
+import java.lang.annotation.Documented;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+@Retention(RetentionPolicy.SOURCE)
+@Documented
+@Target(ElementType.FIELD)
+@interface IrrelevantForEquality {
+	String reason() default "";
+}