From 1c67a063cf918368462711207a92a18f5a40eaf3 Mon Sep 17 00:00:00 2001 From: Simon Gamma Date: Sat, 29 Jun 2024 20:26:44 +0200 Subject: [PATCH 1/6] feat: raw implementation of roundtrippable eslintstep --- .../spotless/npm/EslintFormatterStep.java | 11 +- .../spotless/npm/EslintFormatterStep2.java | 125 ++++++++++++ .../spotless/npm/NodeServerLayout.java | 18 +- .../npm/NpmFormatterStepStateBase.java | 47 +---- .../npm/NpmServerBasedFormatterStep.java | 184 ++++++++++++++++++ .../spotless/npm/NpmServerProcessInfo.java | 59 ++++++ .../spotless/npm/PackageJsonUtil.java | 52 +++++ .../spotless/npm/PrettierFormatterStep.java | 4 +- .../spotless/npm/ServerStartException.java | 24 +++ .../spotless/npm/TsFmtFormatterStep.java | 4 +- 10 files changed, 473 insertions(+), 55 deletions(-) create mode 100644 lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep2.java create mode 100644 lib/src/main/java/com/diffplug/spotless/npm/NpmServerBasedFormatterStep.java create mode 100644 lib/src/main/java/com/diffplug/spotless/npm/NpmServerProcessInfo.java create mode 100644 lib/src/main/java/com/diffplug/spotless/npm/PackageJsonUtil.java create mode 100644 lib/src/main/java/com/diffplug/spotless/npm/ServerStartException.java diff --git a/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java b/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java index 70beaf91ca..29aa444afd 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java @@ -73,9 +73,10 @@ public static FormatterStep create(Map devDependencies, Provisio requireNonNull(provisioner); requireNonNull(projectDir); requireNonNull(buildDir); - return FormatterStep.createLazy(NAME, - () -> new State(NAME, devDependencies, projectDir, buildDir, cacheDir, npmPathResolver, eslintConfig), - State::createFormatterFunc); + return new EslintFormatterStep2(devDependencies, projectDir, buildDir, cacheDir, npmPathResolver, eslintConfig); + // return FormatterStep.createLazy(NAME, + // () -> new State(NAME, devDependencies, projectDir, buildDir, cacheDir, npmPathResolver, eslintConfig), + // State::createFormatterFunc); } private static class State extends NpmFormatterStepStateBase implements Serializable { @@ -122,7 +123,7 @@ public FormatterFunc createFormatterFunc() { try { logger.info("Creating formatter function (starting server)"); Runtime runtime = toRuntime(); - ServerProcessInfo eslintRestServer = runtime.npmRunServer(); + NpmServerProcessInfo 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) { @@ -130,7 +131,7 @@ public FormatterFunc createFormatterFunc() { } } - private void endServer(BaseNpmRestService restService, ServerProcessInfo restServer) throws Exception { + private void endServer(BaseNpmRestService restService, NpmServerProcessInfo restServer) throws Exception { logger.info("Closing formatting function (ending server)."); try { restService.shutdown(); diff --git a/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep2.java b/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep2.java new file mode 100644 index 0000000000..df3c01fa55 --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep2.java @@ -0,0 +1,125 @@ +/* + * 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 static com.diffplug.spotless.npm.PackageJsonUtil.replaceDevDependencies; +import static java.util.Objects.requireNonNull; + +import java.io.File; +import java.io.IOException; +import java.util.Collections; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.TreeMap; + +import javax.annotation.Nonnull; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import com.diffplug.spotless.npm.EslintRestService.FormatOption; + +public class EslintFormatterStep2 extends NpmServerBasedFormatterStep { + + private static final Logger logger = LoggerFactory.getLogger(EslintFormatterStep2.class); + + public static final String NAME = "eslint-format"; + + public static final String DEFAULT_ESLINT_VERSION = "^8.45.0"; + private final EslintConfig origEslintConfig; + private EslintConfig eslintConfigInUse; + + public static Map defaultDevDependenciesForTypescript() { + return defaultDevDependenciesTypescriptWithEslint(DEFAULT_ESLINT_VERSION); + } + + public static Map defaultDevDependenciesTypescriptWithEslint(String eslintVersion) { + Map dependencies = new LinkedHashMap<>(); + 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", requireNonNull(eslintVersion)); + return dependencies; + } + + public static Map defaultDevDependencies() { + return defaultDevDependenciesWithEslint(DEFAULT_ESLINT_VERSION); + } + + public static Map defaultDevDependenciesWithEslint(String version) { + return Collections.singletonMap("eslint", version); + } + + public EslintFormatterStep2(@Nonnull Map devDependencies, @Nonnull File projectDir, @Nonnull File buildDir, @Nonnull File cacheDir, @Nonnull NpmPathResolver npmPathResolver, @Nonnull EslintConfig eslintConfig) { + super(NAME, + 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; + } + + @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()); + this.eslintConfigInUse = this.origEslintConfig.withEslintConfigPath(configFileCopy).verify(); + } + } + + @Override + protected String formatWithServer(NpmServerProcessInfo serverProcessInfo, String rawUnix, File file) { + EslintRestService restService = new EslintRestService(serverProcessInfo.getBaseUrl()); + Map eslintCallOptions = new HashMap<>(); + setConfigToCallOptions(eslintCallOptions); + setFilePathToCallOptions(eslintCallOptions, file); + return restService.format(rawUnix, eslintCallOptions); + } + + private void setFilePathToCallOptions(Map eslintCallOptions, File fileToBeFormatted) { + eslintCallOptions.put(FormatOption.FILE_PATH, fileToBeFormatted.getAbsolutePath()); + } + + private void setConfigToCallOptions(Map eslintCallOptions) { + if (this.eslintConfigInUse.getEslintConfigPath() != null) { + eslintCallOptions.put(FormatOption.ESLINT_OVERRIDE_CONFIG_FILE, this.eslintConfigInUse.getEslintConfigPath().getAbsolutePath()); + } + if (this.eslintConfigInUse.getEslintConfigJs() != null) { + eslintCallOptions.put(FormatOption.ESLINT_OVERRIDE_CONFIG, this.eslintConfigInUse.getEslintConfigJs()); + } + if (this.eslintConfigInUse instanceof EslintTypescriptConfig) { + // if we are a ts config, see if we need to use specific paths or use default projectDir + File tsConfigFilePath = ((EslintTypescriptConfig) this.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()); + } + } +} diff --git a/lib/src/main/java/com/diffplug/spotless/npm/NodeServerLayout.java b/lib/src/main/java/com/diffplug/spotless/npm/NodeServerLayout.java index 850ea4eb6b..d0d5d0d696 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/NodeServerLayout.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/NodeServerLayout.java @@ -1,5 +1,5 @@ /* - * Copyright 2020-2023 DiffPlug + * Copyright 2020-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -18,6 +18,7 @@ import java.io.File; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Objects; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Stream; @@ -110,4 +111,19 @@ public String toString() { this.serveJsFile, this.npmrcFile); } + + @Override + public boolean equals(Object o) { + if (this == o) + return true; + if (o == null || getClass() != o.getClass()) + return false; + NodeServerLayout that = (NodeServerLayout) o; + return Objects.equals(nodeModulesDir, that.nodeModulesDir) && Objects.equals(packageJsonFile, that.packageJsonFile) && Objects.equals(packageLockJsonFile, that.packageLockJsonFile) && Objects.equals(serveJsFile, that.serveJsFile) && Objects.equals(npmrcFile, that.npmrcFile); + } + + @Override + public int hashCode() { + return Objects.hash(nodeModulesDir, packageJsonFile, packageLockJsonFile, serveJsFile, npmrcFile); + } } diff --git a/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java b/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java index 3ec06e0434..a1daf8e15d 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java @@ -25,7 +25,6 @@ import java.util.Iterator; import java.util.Map; import java.util.Map.Entry; -import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import org.slf4j.Logger; @@ -105,7 +104,7 @@ protected boolean needsPrepareNodeServerLayout() { return nodeServeApp.needsPrepareNodeAppLayout(); } - protected ServerProcessInfo npmRunServer() throws ServerStartException, IOException { + protected NpmServerProcessInfo npmRunServer() throws ServerStartException, IOException { assertNodeServerDirReady(); LongRunningProcess server = null; try { @@ -133,7 +132,7 @@ protected ServerProcessInfo npmRunServer() throws ServerStartException, IOExcept } // read the server.port file for resulting port and remember the port for later formatting calls String serverPort = NpmResourceHelper.readUtf8StringFromFile(serverPortFile).trim(); - return new ServerProcessInfo(server, serverPort, serverPortFile); + return new NpmServerProcessInfo(server, serverPort, serverPortFile); } catch (IOException | TimeoutException e) { throw new ServerStartException("Starting server failed." + (server != null ? "\n\nProcess result:\n" + ThrowingEx.get(server::result) : ""), e); } @@ -167,46 +166,4 @@ private static String replacePlaceholders(String template, Map r public abstract FormatterFunc createFormatterFunc(); - protected static class ServerProcessInfo implements AutoCloseable { - private final Process server; - private final String serverPort; - private final File serverPortFile; - - public ServerProcessInfo(Process server, String serverPort, File serverPortFile) { - this.server = server; - this.serverPort = serverPort; - this.serverPortFile = serverPortFile; - } - - public String getBaseUrl() { - return "http://127.0.0.1:" + this.serverPort; - } - - @Override - public void close() throws Exception { - try { - logger.trace("Closing npm server in directory <{}> and port <{}>", - serverPortFile.getParent(), serverPort); - - if (server.isAlive()) { - boolean ended = server.waitFor(5, TimeUnit.SECONDS); - if (!ended) { - logger.info("Force-Closing npm server in directory <{}> and port <{}>", serverPortFile.getParent(), serverPort); - server.destroyForcibly().waitFor(); - logger.trace("Force-Closing npm server in directory <{}> and port <{}> -- Finished", serverPortFile.getParent(), serverPort); - } - } - } finally { - NpmResourceHelper.deleteFileIfExists(serverPortFile); - } - } - } - - protected static class ServerStartException extends RuntimeException { - private static final long serialVersionUID = -8803977379866483002L; - - public ServerStartException(String message, Throwable cause) { - super(message, cause); - } - } } diff --git a/lib/src/main/java/com/diffplug/spotless/npm/NpmServerBasedFormatterStep.java b/lib/src/main/java/com/diffplug/spotless/npm/NpmServerBasedFormatterStep.java new file mode 100644 index 0000000000..d6315c3367 --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/npm/NpmServerBasedFormatterStep.java @@ -0,0 +1,184 @@ +/* + * 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.io.File; +import java.io.IOException; +import java.io.ObjectInputStream; +import java.io.ObjectOutputStream; +import java.io.ObjectStreamException; +import java.time.Duration; +import java.util.Objects; +import java.util.concurrent.TimeoutException; + +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +import com.diffplug.spotless.FormatterStep; +import com.diffplug.spotless.ProcessRunner.LongRunningProcess; +import com.diffplug.spotless.ThrowingEx; + +abstract class NpmServerBasedFormatterStep implements FormatterStep { + + private final String name; + protected final NpmFormatterStepLocations locations; + + protected final NodeServerLayout nodeServerLayout; + private final NodeServeApp nodeServeApp; + + private NpmServerProcessInfo serverProcessInfo; + + public NpmServerBasedFormatterStep(@Nonnull String name, + @Nonnull NpmConfig npmConfig, @Nonnull NpmFormatterStepLocations locations) { + this.name = Objects.requireNonNull(name); + this.locations = Objects.requireNonNull(locations); + this.nodeServerLayout = new NodeServerLayout(Objects.requireNonNull(locations).buildDir(), Objects.requireNonNull(npmConfig).getPackageJsonContent()); + this.nodeServeApp = new NodeServeApp(nodeServerLayout, npmConfig, locations); + } + + // FormatterStep + + @Override + public String getName() { + return name; + } + + @Nullable + @Override + public String format(String rawUnix, File file) throws Exception { + if (this.serverProcessInfo == null) { + assertNodeServerDirReady(); + this.serverProcessInfo = npmRunServer(); + } + return formatWithServer(serverProcessInfo, rawUnix, file); + } + + @Override + public void close() throws Exception { + if (this.serverProcessInfo != null) { + this.serverProcessInfo.close(); + } + this.serverProcessInfo = null; + } + + // Equals and HashCode + + @Override + public boolean equals(Object o) { + if (this == o) + return true; + if (o == null || getClass() != o.getClass()) + return false; + NpmServerBasedFormatterStep that = (NpmServerBasedFormatterStep) o; + return Objects.equals(name, that.name) && Objects.equals(nodeServerLayout, that.nodeServerLayout) && Objects.equals(nodeServeApp, that.nodeServeApp) && Objects.equals(serverProcessInfo, that.serverProcessInfo); + } + + @Override + public int hashCode() { + return Objects.hash(name, nodeServerLayout, nodeServeApp, serverProcessInfo); + } + + // Serializable contract + + // override serialize output + private void writeObject(ObjectOutputStream out) throws IOException { + // TODO (simschla, 27.06.2024): Implement serialization + System.out.println("TODO: Implement serialization - writeObject " + this); + // out.writeObject(state()); + } + + // override serialize input + @SuppressWarnings("unchecked") + private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException { + // TODO (simschla, 27.06.2024): Implement serialization + System.out.println("TODO: Implement serialization - readObject " + this); + // state = (T) Objects.requireNonNull(in.readObject()); + } + + // override serialize input + @SuppressWarnings("unused") + private void readObjectNoData() throws ObjectStreamException { + throw new UnsupportedOperationException(); + } + + // NpmServerBasedFormatterStep + + protected void assertNodeServerDirReady() throws IOException { + if (needsPrepareNodeServerLayout()) { + // reinstall if missing + prepareNodeServerLayout(); + } + if (needsPrepareNodeServer()) { + // run npm install if node_modules is missing + prepareNodeServer(); + } + } + + protected boolean needsPrepareNodeServerLayout() { + return nodeServeApp.needsPrepareNodeAppLayout(); + } + + protected void prepareNodeServerLayout() throws IOException { + nodeServeApp.prepareNodeAppLayout(); + doPrepareNodeServerLayout(nodeServerLayout); + } + + abstract protected void doPrepareNodeServerLayout(NodeServerLayout layout) throws IOException; + + protected boolean needsPrepareNodeServer() { + return nodeServeApp.needsNpmInstall(); + } + + protected void prepareNodeServer() throws IOException { + nodeServeApp.npmInstall(); + } + + protected NpmServerProcessInfo npmRunServer() throws ServerStartException, IOException { + assertNodeServerDirReady(); + LongRunningProcess server = null; + try { + // The npm process will output the randomly selected port of the http server process to 'server.port' file + // so in order to be safe, remove such a file if it exists before starting. + final File serverPortFile = new File(this.nodeServerLayout.nodeModulesDir(), "server.port"); + NpmResourceHelper.deleteFileIfExists(serverPortFile); + // start the http server in node + server = nodeServeApp.startNpmServeProcess(); + + // await the readiness of the http server - wait for at most 60 seconds + try { + NpmResourceHelper.awaitReadableFile(serverPortFile, Duration.ofSeconds(60)); + } catch (TimeoutException timeoutException) { + // forcibly end the server process + try { + if (server.isAlive()) { + server.destroyForcibly(); + server.waitFor(); + } + } catch (Throwable t) { + // ignore + } + throw timeoutException; + } + // read the server.port file for resulting port and remember the port for later formatting calls + String serverPort = NpmResourceHelper.readUtf8StringFromFile(serverPortFile).trim(); + return new NpmServerProcessInfo(server, serverPort, serverPortFile); + } catch (IOException | TimeoutException e) { + throw new ServerStartException("Starting server failed." + (server != null ? "\n\nProcess result:\n" + ThrowingEx.get(server::result) : ""), e); + } + } + + protected abstract String formatWithServer(NpmServerProcessInfo serverProcessInfo, String rawUnix, File file); +} diff --git a/lib/src/main/java/com/diffplug/spotless/npm/NpmServerProcessInfo.java b/lib/src/main/java/com/diffplug/spotless/npm/NpmServerProcessInfo.java new file mode 100644 index 0000000000..b953a532df --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/npm/NpmServerProcessInfo.java @@ -0,0 +1,59 @@ +/* + * 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.io.File; +import java.util.concurrent.TimeUnit; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +class NpmServerProcessInfo implements AutoCloseable { + private static final Logger logger = LoggerFactory.getLogger(NpmServerProcessInfo.class); + + private final Process server; + private final String serverPort; + private final File serverPortFile; + + public NpmServerProcessInfo(Process server, String serverPort, File serverPortFile) { + this.server = server; + this.serverPort = serverPort; + this.serverPortFile = serverPortFile; + } + + public String getBaseUrl() { + return "http://127.0.0.1:" + this.serverPort; + } + + @Override + public void close() throws Exception { + try { + logger.trace("Closing npm server in directory <{}> and port <{}>", + serverPortFile.getParent(), serverPort); + + if (server.isAlive()) { + boolean ended = server.waitFor(5, TimeUnit.SECONDS); + if (!ended) { + logger.info("Force-Closing npm server in directory <{}> and port <{}>", serverPortFile.getParent(), serverPort); + server.destroyForcibly().waitFor(); + logger.trace("Force-Closing npm server in directory <{}> and port <{}> -- Finished", serverPortFile.getParent(), serverPort); + } + } + } finally { + NpmResourceHelper.deleteFileIfExists(serverPortFile); + } + } +} diff --git a/lib/src/main/java/com/diffplug/spotless/npm/PackageJsonUtil.java b/lib/src/main/java/com/diffplug/spotless/npm/PackageJsonUtil.java new file mode 100644 index 0000000000..2afb2a5f47 --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/npm/PackageJsonUtil.java @@ -0,0 +1,52 @@ +/* + * 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.util.Collections; +import java.util.Iterator; +import java.util.Map; + +final class PackageJsonUtil { + + private PackageJsonUtil() { + // prevent instantiation + } + + static String replaceDevDependencies(String template, Map devDependencies) { + StringBuilder builder = new StringBuilder(); + Iterator> entryIter = devDependencies.entrySet().iterator(); + while (entryIter.hasNext()) { + Map.Entry entry = entryIter.next(); + builder.append("\t\t\""); + builder.append(entry.getKey()); + builder.append("\": \""); + builder.append(entry.getValue()); + builder.append("\""); + if (entryIter.hasNext()) { + builder.append(",\n"); + } + } + return replacePlaceholders(template, Collections.singletonMap("devDependencies", builder.toString())); + } + + private static String replacePlaceholders(String template, Map replacements) { + String result = template; + for (Map.Entry entry : replacements.entrySet()) { + result = result.replaceAll("\\Q${" + entry.getKey() + "}\\E", entry.getValue()); + } + return result; + } +} diff --git a/lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java b/lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java index 27a1002df5..e1aab4a274 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java @@ -88,7 +88,7 @@ private static class State extends NpmFormatterStepStateBase implements Serializ public FormatterFunc createFormatterFunc() { try { logger.info("creating formatter function (starting server)"); - ServerProcessInfo prettierRestServer = toRuntime().npmRunServer(); + NpmServerProcessInfo prettierRestServer = toRuntime().npmRunServer(); PrettierRestService restService = new PrettierRestService(prettierRestServer.getBaseUrl()); String prettierConfigOptions = restService.resolveConfig(this.prettierConfig.getPrettierConfigPath(), this.prettierConfig.getOptions()); return Closeable.ofDangerous(() -> endServer(restService, prettierRestServer), new PrettierFilePathPassingFormatterFunc(prettierConfigOptions, restService)); @@ -97,7 +97,7 @@ public FormatterFunc createFormatterFunc() { } } - private void endServer(PrettierRestService restService, ServerProcessInfo restServer) throws Exception { + private void endServer(PrettierRestService restService, NpmServerProcessInfo restServer) throws Exception { logger.info("Closing formatting function (ending server)."); try { restService.shutdown(); diff --git a/lib/src/main/java/com/diffplug/spotless/npm/ServerStartException.java b/lib/src/main/java/com/diffplug/spotless/npm/ServerStartException.java new file mode 100644 index 0000000000..295617d35b --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/npm/ServerStartException.java @@ -0,0 +1,24 @@ +/* + * 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; + +class ServerStartException extends RuntimeException { + private static final long serialVersionUID = -8803977379866483002L; + + public ServerStartException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/lib/src/main/java/com/diffplug/spotless/npm/TsFmtFormatterStep.java b/lib/src/main/java/com/diffplug/spotless/npm/TsFmtFormatterStep.java index b171b0ca7f..38e67a6f92 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/TsFmtFormatterStep.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/TsFmtFormatterStep.java @@ -94,7 +94,7 @@ public State(String stepName, Map versions, File projectDir, Fil public FormatterFunc createFormatterFunc() { try { Map tsFmtOptions = unifyOptions(); - ServerProcessInfo tsfmtRestServer = toRuntime().npmRunServer(); + NpmServerProcessInfo tsfmtRestServer = toRuntime().npmRunServer(); TsFmtRestService restService = new TsFmtRestService(tsfmtRestServer.getBaseUrl()); return Closeable.ofDangerous(() -> endServer(restService, tsfmtRestServer), input -> restService.format(input, tsFmtOptions)); } catch (IOException e) { @@ -116,7 +116,7 @@ private Map unifyOptions() { return unified; } - private void endServer(TsFmtRestService restService, ServerProcessInfo restServer) throws Exception { + private void endServer(TsFmtRestService restService, NpmServerProcessInfo restServer) throws Exception { try { restService.shutdown(); } catch (Throwable t) { From 4b81976d83554e04fb6f76d5e3e1939fd66caf08 Mon Sep 17 00:00:00 2001 From: Simon Gamma Date: Mon, 1 Jul 2024 20:44:39 +0200 Subject: [PATCH 2/6] feat: add FileSignature equivalent for direct FormatterFunc impls --- .../spotless/npm/RoundtrippableFile.java | 158 ++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 lib/src/main/java/com/diffplug/spotless/npm/RoundtrippableFile.java diff --git a/lib/src/main/java/com/diffplug/spotless/npm/RoundtrippableFile.java b/lib/src/main/java/com/diffplug/spotless/npm/RoundtrippableFile.java new file mode 100644 index 0000000000..cf26ae2de8 --- /dev/null +++ b/lib/src/main/java/com/diffplug/spotless/npm/RoundtrippableFile.java @@ -0,0 +1,158 @@ +/* + * 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 static java.util.Objects.requireNonNull; + +import java.io.File; +import java.io.FileInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.Serializable; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; + +import javax.annotation.Nonnull; + +import com.diffplug.spotless.ThrowingEx; + +/** + * Wrapper for a file that conforms to the following idea: + * 1. Equals and hashcode should not include absolute paths and such - optimize for buildcache keys + * 2. Serialized/deserialized state can include absolute paths and such and should recreate a valid/runnable state + */ +class RoundtrippableFile implements Serializable { + + private static final long serialVersionUID = 8667124674782397566L; + + private final File file; + + public RoundtrippableFile(@Nonnull File file) { + this.file = requireNonNull(file); + } + + public File file() { + return file; + } + + public boolean isFile() { + return file.isFile(); + } + + @Override + public boolean equals(Object o) { + if (o == null) { + return false; + } else if (getClass() != o.getClass()) { + return false; + } else { + return sig().equals(((RoundtrippableFile) o).sig()); + } + } + + @Override + public int hashCode() { + return Objects.hash(sig()); + } + + @Override + public String toString() { + return "RoundtrippableFile[" + file.toString() + "]"; + } + + private FileSig sig() { + return FileSigCache.INSTANCE.sign(file); + } + + /** + * A file signature which includes the file name, size, hash and last modified. + * Equals and Hashcode are based on these three values only. + */ + private static class FileSig { + final String name; + final long size; + final byte[] hash; + final long lastModified; + + FileSig(@Nonnull File file) { + Objects.requireNonNull(file); + this.name = file.getName(); + this.size = safeSize(file); + this.hash = ThrowingEx.get(() -> hash(file)); + this.lastModified = safeLastModified(file); + } + + @Override + public boolean equals(Object o) { + if (this == o) + return true; + if (!(o instanceof FileSig)) + return false; + FileSig fileSig = (FileSig) o; + return size == fileSig.size && Objects.equals(name, fileSig.name) && Objects.deepEquals(hash, fileSig.hash) && lastModified == fileSig.lastModified; + } + + @Override + public int hashCode() { + return Objects.hash(name, size, Arrays.hashCode(hash), lastModified); + } + + private static byte[] hash(@Nonnull File file) throws NoSuchAlgorithmException, IOException { + if (!file.isFile()) { + return new byte[0]; + } + MessageDigest digest = MessageDigest.getInstance("SHA-256"); + byte[] buf = new byte[1024]; + try (InputStream input = new FileInputStream(file)) { + int numRead; + while ((numRead = input.read(buf)) != -1) { + digest.update(buf, 0, numRead); + } + } + return digest.digest(); + } + } + + private static long safeLastModified(File file) { + return file.isFile() ? file.lastModified() : -1; + } + + private static long safeSize(File file) { + return file.isFile() ? file.length() : 0; + } + + // heavily inspired by FileSignature.Cache + private enum FileSigCache { + INSTANCE; + + private final Map cache = new HashMap<>(); + + synchronized FileSig sign(File file) { + String canonicalPath = ThrowingEx.get(file::getCanonicalPath); + FileSig cached = cache.computeIfAbsent(canonicalPath, ThrowingEx.wrap(p -> new FileSig(file))); + if (cached.lastModified != safeLastModified(file)) { + cache.remove(canonicalPath); // might have been changed, so re-calculate + return sign(file); + } else { + return cached; + } + } + } +} From 435217caafce0e910978ccbcc6645ed54216665a Mon Sep 17 00:00:00 2001 From: Simon Gamma Date: Mon, 1 Jul 2024 20:45:02 +0200 Subject: [PATCH 3/6] feat: add marker annotation for irrelevant/derived fields --- .../spotless/npm/IrrelevantForEquality.java | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 lib/src/main/java/com/diffplug/spotless/npm/IrrelevantForEquality.java diff --git a/lib/src/main/java/com/diffplug/spotless/npm/IrrelevantForEquality.java b/lib/src/main/java/com/diffplug/spotless/npm/IrrelevantForEquality.java new file mode 100644 index 0000000000..894bcccea2 --- /dev/null +++ b/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 ""; +} From a14229bd083e8f4edcd895e8af4216d5ef90609e Mon Sep 17 00:00:00 2001 From: Simon Gamma Date: Wed, 3 Jul 2024 05:55:39 +0200 Subject: [PATCH 4/6] chore: extract config values into specific map --- .../diffplug/spotless/npm/EslintConfig.java | 29 +++-- .../spotless/npm/EslintFormatterStep2.java | 79 +++++++----- .../spotless/npm/EslintTypescriptConfig.java | 29 ++++- .../npm/NpmServerBasedFormatterStep.java | 114 +++++++++++++++--- 4 files changed, 194 insertions(+), 57 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/npm/EslintConfig.java b/lib/src/main/java/com/diffplug/spotless/npm/EslintConfig.java index 8db9cf07af..e919282961 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/EslintConfig.java +++ b/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); + } } diff --git a/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep2.java b/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep2.java index df3c01fa55..124c760a85 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep2.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep2.java @@ -33,6 +33,18 @@ import com.diffplug.spotless.npm.EslintRestService.FormatOption; +/** + * 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 EslintFormatterStep2 extends NpmServerBasedFormatterStep { private static final Logger logger = LoggerFactory.getLogger(EslintFormatterStep2.class); @@ -40,8 +52,6 @@ public class EslintFormatterStep2 extends NpmServerBasedFormatterStep { public static final String NAME = "eslint-format"; public static final String DEFAULT_ESLINT_VERSION = "^8.45.0"; - private final EslintConfig origEslintConfig; - private EslintConfig eslintConfigInUse; public static Map defaultDevDependenciesForTypescript() { return defaultDevDependenciesTypescriptWithEslint(DEFAULT_ESLINT_VERSION); @@ -66,32 +76,36 @@ public static Map defaultDevDependenciesWithEslint(String versio public EslintFormatterStep2(@Nonnull Map devDependencies, @Nonnull File projectDir, @Nonnull File buildDir, @Nonnull File cacheDir, @Nonnull NpmPathResolver npmPathResolver, @Nonnull EslintConfig eslintConfig) { super(NAME, - 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; + 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(), + Map.of(EslintConfigElement.ESLINT_CONFIG_ORIGINAL_ELEMENT, eslintConfig), + new NpmFormatterStepLocations(projectDir, buildDir, cacheDir, npmPathResolver)); + } + + protected EslintConfig origEslintConfig() { + return configElement(EslintConfigElement.ESLINT_CONFIG_ORIGINAL_ELEMENT); + } + + protected EslintConfig eslintConfigInUse() { + return configElement(EslintConfigElement.ESLINT_CONFIG_IN_USE_ELEMENT); } @Override protected void doPrepareNodeServerLayout(NodeServerLayout nodeServerLayout) throws IOException { - if (origEslintConfig.getEslintConfigPath() != null) { + 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(); + 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()); } } @@ -109,17 +123,26 @@ private void setFilePathToCallOptions(Map eslintCallOption } private void setConfigToCallOptions(Map eslintCallOptions) { - if (this.eslintConfigInUse.getEslintConfigPath() != null) { - eslintCallOptions.put(FormatOption.ESLINT_OVERRIDE_CONFIG_FILE, this.eslintConfigInUse.getEslintConfigPath().getAbsolutePath()); + if (eslintConfigInUse().getEslintConfigPath() != null) { + eslintCallOptions.put(FormatOption.ESLINT_OVERRIDE_CONFIG_FILE, eslintConfigInUse().getEslintConfigPath().getAbsolutePath()); } - if (this.eslintConfigInUse.getEslintConfigJs() != null) { - eslintCallOptions.put(FormatOption.ESLINT_OVERRIDE_CONFIG, this.eslintConfigInUse.getEslintConfigJs()); + if (eslintConfigInUse().getEslintConfigJs() != null) { + eslintCallOptions.put(FormatOption.ESLINT_OVERRIDE_CONFIG, eslintConfigInUse().getEslintConfigJs()); } - if (this.eslintConfigInUse instanceof EslintTypescriptConfig) { + 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) this.eslintConfigInUse).getTypescriptConfigPath(); + 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()); + eslintCallOptions.put(FormatOption.TS_CONFIG_ROOT_DIR, this.nodeServerLayout().nodeModulesDir().getAbsoluteFile().toPath().relativize(tsConfigRootDir.getAbsoluteFile().toPath()).toString()); } } + + private enum EslintConfigElement implements NpmConfigElement { + ESLINT_CONFIG_ORIGINAL_ELEMENT, ESLINT_CONFIG_IN_USE_ELEMENT { + @Override + public boolean equalsHashcodeRelevant() { + return false; + } + }; + } } diff --git a/lib/src/main/java/com/diffplug/spotless/npm/EslintTypescriptConfig.java b/lib/src/main/java/com/diffplug/spotless/npm/EslintTypescriptConfig.java index 42dec6837a..5a7d6ab09f 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/EslintTypescriptConfig.java +++ b/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); } } diff --git a/lib/src/main/java/com/diffplug/spotless/npm/NpmServerBasedFormatterStep.java b/lib/src/main/java/com/diffplug/spotless/npm/NpmServerBasedFormatterStep.java index d6315c3367..68d124aa31 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/NpmServerBasedFormatterStep.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/NpmServerBasedFormatterStep.java @@ -21,6 +21,8 @@ import java.io.ObjectOutputStream; import java.io.ObjectStreamException; import java.time.Duration; +import java.util.HashMap; +import java.util.Map; import java.util.Objects; import java.util.concurrent.TimeoutException; @@ -33,22 +35,44 @@ abstract class NpmServerBasedFormatterStep implements FormatterStep { + // dynamic equals/hascode impl + // => 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 + + private final Map configElements = new HashMap<>(); + private final String name; + protected final NpmFormatterStepLocations locations; - protected final NodeServerLayout nodeServerLayout; - private final NodeServeApp nodeServeApp; + // prev private NpmServerProcessInfo serverProcessInfo; - public NpmServerBasedFormatterStep(@Nonnull String name, - @Nonnull NpmConfig npmConfig, @Nonnull NpmFormatterStepLocations locations) { + protected NpmServerBasedFormatterStep(@Nonnull String name, + @Nonnull String packageJsonContent, + @Nonnull String serveScriptContent, + @Nullable String npmrcContent, + @Nullable Map additionalConfigElements, + @Nonnull NpmFormatterStepLocations locations) { this.name = Objects.requireNonNull(name); + this.configElements.put(GenericNpmConfigElement.PACKAGE_JSON_CONTENT, Objects.requireNonNull(packageJsonContent)); + this.configElements.put(GenericNpmConfigElement.SERVE_SCRIPT_CONTENT, Objects.requireNonNull(serveScriptContent)); + this.configElements.put(GenericNpmConfigElement.NPMRC_CONTENT, npmrcContent); + if (additionalConfigElements != null) { + this.configElements.putAll(additionalConfigElements); + } this.locations = Objects.requireNonNull(locations); - this.nodeServerLayout = new NodeServerLayout(Objects.requireNonNull(locations).buildDir(), Objects.requireNonNull(npmConfig).getPackageJsonContent()); - this.nodeServeApp = new NodeServeApp(nodeServerLayout, npmConfig, locations); } + // public NpmServerBasedFormatterStep(@Nonnull String name, + // @Nonnull NpmConfig npmConfig, @Nonnull NpmFormatterStepLocations locations) { + // this.name = Objects.requireNonNull(name); + // this.locations = Objects.requireNonNull(locations); + // this.nodeServerLayout = new NodeServerLayout(Objects.requireNonNull(locations).buildDir(), Objects.requireNonNull(npmConfig).getPackageJsonContent()); + // this.nodeServeApp = new NodeServeApp(nodeServerLayout, npmConfig, locations); + // } + // FormatterStep @Override @@ -80,15 +104,28 @@ public void close() throws Exception { public boolean equals(Object o) { if (this == o) return true; - if (o == null || getClass() != o.getClass()) + if (!(o instanceof NpmServerBasedFormatterStep)) return false; NpmServerBasedFormatterStep that = (NpmServerBasedFormatterStep) o; - return Objects.equals(name, that.name) && Objects.equals(nodeServerLayout, that.nodeServerLayout) && Objects.equals(nodeServeApp, that.nodeServeApp) && Objects.equals(serverProcessInfo, that.serverProcessInfo); + Map thisConfig = equalsRelevantConfigElements(this.configElements); + Map thatConfig = equalsRelevantConfigElements(that.configElements); + return Objects.equals(thisConfig, thatConfig) && Objects.equals(name, that.name); + } + + private static Map equalsRelevantConfigElements(Map configElements) { + Map result = new HashMap<>(); + for (Map.Entry entry : configElements.entrySet()) { + if (entry.getKey().equalsHashcodeRelevant()) { + result.put(entry.getKey(), entry.getValue()); + } + } + return result; } @Override public int hashCode() { - return Objects.hash(name, nodeServerLayout, nodeServeApp, serverProcessInfo); + Map thisConfig = equalsRelevantConfigElements(this.configElements); + return Objects.hash(thisConfig, name); } // Serializable contract @@ -116,6 +153,32 @@ private void readObjectNoData() throws ObjectStreamException { // NpmServerBasedFormatterStep + @SuppressWarnings("unchecked") + protected T configElement(@Nonnull NpmConfigElement element) { + return (T) configElements.get(element); + } + + @SuppressWarnings("unchecked") + protected T configElement(@Nonnull NpmConfigElement element, T newValue) { + return (T) configElements.put(element, newValue); + } + + protected NodeServerLayout nodeServerLayout() { + return new NodeServerLayout(locations.buildDir(), configElement(GenericNpmConfigElement.PACKAGE_JSON_CONTENT)); + } + + protected NodeServeApp nodeServeApp() { + // TODO (simschla, 01.07.2024): maybe memoize this + return new NodeServeApp(nodeServerLayout(), npmConfig(), locations); + } + + private NpmConfig npmConfig() { + return new NpmConfig( + configElement(GenericNpmConfigElement.PACKAGE_JSON_CONTENT), + configElement(GenericNpmConfigElement.SERVE_SCRIPT_CONTENT), + configElement(GenericNpmConfigElement.NPMRC_CONTENT)); + } + protected void assertNodeServerDirReady() throws IOException { if (needsPrepareNodeServerLayout()) { // reinstall if missing @@ -128,22 +191,22 @@ protected void assertNodeServerDirReady() throws IOException { } protected boolean needsPrepareNodeServerLayout() { - return nodeServeApp.needsPrepareNodeAppLayout(); + return nodeServeApp().needsPrepareNodeAppLayout(); } protected void prepareNodeServerLayout() throws IOException { - nodeServeApp.prepareNodeAppLayout(); - doPrepareNodeServerLayout(nodeServerLayout); + nodeServeApp().prepareNodeAppLayout(); + doPrepareNodeServerLayout(nodeServerLayout()); } abstract protected void doPrepareNodeServerLayout(NodeServerLayout layout) throws IOException; protected boolean needsPrepareNodeServer() { - return nodeServeApp.needsNpmInstall(); + return nodeServeApp().needsNpmInstall(); } protected void prepareNodeServer() throws IOException { - nodeServeApp.npmInstall(); + nodeServeApp().npmInstall(); } protected NpmServerProcessInfo npmRunServer() throws ServerStartException, IOException { @@ -152,10 +215,10 @@ protected NpmServerProcessInfo npmRunServer() throws ServerStartException, IOExc try { // The npm process will output the randomly selected port of the http server process to 'server.port' file // so in order to be safe, remove such a file if it exists before starting. - final File serverPortFile = new File(this.nodeServerLayout.nodeModulesDir(), "server.port"); + final File serverPortFile = new File(nodeServerLayout().nodeModulesDir(), "server.port"); NpmResourceHelper.deleteFileIfExists(serverPortFile); // start the http server in node - server = nodeServeApp.startNpmServeProcess(); + server = nodeServeApp().startNpmServeProcess(); // await the readiness of the http server - wait for at most 60 seconds try { @@ -181,4 +244,23 @@ protected NpmServerProcessInfo npmRunServer() throws ServerStartException, IOExc } protected abstract String formatWithServer(NpmServerProcessInfo serverProcessInfo, String rawUnix, File file); + + interface NpmConfigElement { + String name(); + + default boolean equalsHashcodeRelevant() { + return true; + }; + } + + enum GenericNpmConfigElement implements NpmConfigElement { + PACKAGE_JSON_CONTENT, SERVE_SCRIPT_CONTENT { + @Override + public boolean equalsHashcodeRelevant() { + return false; + } + }, + NPMRC_CONTENT; + + } } From 9310c150b320c3fb3e7bb4a97f3855c1e04ab8c5 Mon Sep 17 00:00:00 2001 From: Simon Gamma Date: Wed, 3 Jul 2024 06:15:53 +0200 Subject: [PATCH 5/6] feat: restrict type of config values to be serializable --- .../npm/NpmServerBasedFormatterStep.java | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/npm/NpmServerBasedFormatterStep.java b/lib/src/main/java/com/diffplug/spotless/npm/NpmServerBasedFormatterStep.java index 68d124aa31..00e84d5d43 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/NpmServerBasedFormatterStep.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/NpmServerBasedFormatterStep.java @@ -20,6 +20,7 @@ import java.io.ObjectInputStream; import java.io.ObjectOutputStream; import java.io.ObjectStreamException; +import java.io.Serializable; import java.time.Duration; import java.util.HashMap; import java.util.Map; @@ -39,7 +40,7 @@ abstract class NpmServerBasedFormatterStep implements FormatterStep { // => 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 - private final Map configElements = new HashMap<>(); + private final Map configElements = new HashMap<>(); private final String name; @@ -53,7 +54,7 @@ protected NpmServerBasedFormatterStep(@Nonnull String name, @Nonnull String packageJsonContent, @Nonnull String serveScriptContent, @Nullable String npmrcContent, - @Nullable Map additionalConfigElements, + @Nullable Map additionalConfigElements, @Nonnull NpmFormatterStepLocations locations) { this.name = Objects.requireNonNull(name); this.configElements.put(GenericNpmConfigElement.PACKAGE_JSON_CONTENT, Objects.requireNonNull(packageJsonContent)); @@ -107,14 +108,14 @@ public boolean equals(Object o) { if (!(o instanceof NpmServerBasedFormatterStep)) return false; NpmServerBasedFormatterStep that = (NpmServerBasedFormatterStep) o; - Map thisConfig = equalsRelevantConfigElements(this.configElements); - Map thatConfig = equalsRelevantConfigElements(that.configElements); + Map thisConfig = equalsRelevantConfigElements(this.configElements); + Map thatConfig = equalsRelevantConfigElements(that.configElements); return Objects.equals(thisConfig, thatConfig) && Objects.equals(name, that.name); } - private static Map equalsRelevantConfigElements(Map configElements) { - Map result = new HashMap<>(); - for (Map.Entry entry : configElements.entrySet()) { + private static Map equalsRelevantConfigElements(Map configElements) { + Map result = new HashMap<>(); + for (Map.Entry entry : configElements.entrySet()) { if (entry.getKey().equalsHashcodeRelevant()) { result.put(entry.getKey(), entry.getValue()); } @@ -124,7 +125,7 @@ private static Map equalsRelevantConfigElements(Map thisConfig = equalsRelevantConfigElements(this.configElements); + Map thisConfig = equalsRelevantConfigElements(this.configElements); return Objects.hash(thisConfig, name); } @@ -134,6 +135,7 @@ public int hashCode() { private void writeObject(ObjectOutputStream out) throws IOException { // TODO (simschla, 27.06.2024): Implement serialization System.out.println("TODO: Implement serialization - writeObject " + this); + out.defaultWriteObject(); // out.writeObject(state()); } @@ -141,6 +143,7 @@ private void writeObject(ObjectOutputStream out) throws IOException { @SuppressWarnings("unchecked") private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException { // TODO (simschla, 27.06.2024): Implement serialization + in.defaultReadObject(); System.out.println("TODO: Implement serialization - readObject " + this); // state = (T) Objects.requireNonNull(in.readObject()); } @@ -154,12 +157,12 @@ private void readObjectNoData() throws ObjectStreamException { // NpmServerBasedFormatterStep @SuppressWarnings("unchecked") - protected T configElement(@Nonnull NpmConfigElement element) { + protected T configElement(@Nonnull NpmConfigElement element) { return (T) configElements.get(element); } @SuppressWarnings("unchecked") - protected T configElement(@Nonnull NpmConfigElement element, T newValue) { + protected T configElement(@Nonnull NpmConfigElement element, T newValue) { return (T) configElements.put(element, newValue); } From 5db1aee6363f854f0084db62b9ed8a0efc07ffaf Mon Sep 17 00:00:00 2001 From: Simon Gamma Date: Wed, 3 Jul 2024 07:38:50 +0200 Subject: [PATCH 6/6] chore: replace previous eslint impl completely --- .../spotless/npm/EslintFormatterStep.java | 196 ++++++++---------- .../spotless/npm/EslintFormatterStep2.java | 148 ------------- .../npm/NpmServerBasedFormatterStep.java | 20 +- .../gradle/spotless/JavascriptExtension.java | 2 +- .../gradle/spotless/TypescriptExtension.java | 2 +- .../maven/javascript/AbstractEslint.java | 4 +- .../spotless/npm/EslintFormatterStepTest.java | 4 - 7 files changed, 96 insertions(+), 280 deletions(-) delete mode 100644 lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep2.java diff --git a/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java b/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java index 29aa444afd..4b3b2e845a 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java +++ b/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 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,119 +78,87 @@ public static Map defaultDevDependenciesWithEslint(String versio return Collections.singletonMap("eslint", version); } - public static FormatterStep create(Map devDependencies, Provisioner provisioner, File projectDir, File buildDir, File cacheDir, NpmPathResolver npmPathResolver, EslintConfig eslintConfig) { - requireNonNull(devDependencies); - requireNonNull(provisioner); - requireNonNull(projectDir); - requireNonNull(buildDir); - return new EslintFormatterStep2(devDependencies, projectDir, buildDir, cacheDir, npmPathResolver, eslintConfig); - // return FormatterStep.createLazy(NAME, - // () -> new State(NAME, devDependencies, projectDir, buildDir, cacheDir, npmPathResolver, eslintConfig), - // State::createFormatterFunc); + public static FormatterStep create(@Nonnull Map 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 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 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(); - NpmServerProcessInfo 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 devDependencies) { + return replaceDevDependencies( + NpmResourceHelper.readUtf8StringFromClasspath(EslintFormatterStep.class, "/com/diffplug/spotless/npm/eslint-package.json"), + new TreeMap<>(devDependencies)); + } - private void endServer(BaseNpmRestService restService, NpmServerProcessInfo 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 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 eslintCallOptions = new HashMap<>(); + setConfigToCallOptions(eslintCallOptions); + setFilePathToCallOptions(eslintCallOptions, file); + return restService.format(rawUnix, eslintCallOptions); + } + + private void setFilePathToCallOptions(Map eslintCallOptions, File fileToBeFormatted) { + eslintCallOptions.put(FormatOption.FILE_PATH, fileToBeFormatted.getAbsolutePath()); + } - private void setFilePathToCallOptions(Map eslintCallOptions, File fileToBeFormatted) { - eslintCallOptions.put(FormatOption.FILE_PATH, fileToBeFormatted.getAbsolutePath()); + private void setConfigToCallOptions(Map 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 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()); - } - } + }; } } diff --git a/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep2.java b/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep2.java deleted file mode 100644 index 124c760a85..0000000000 --- a/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep2.java +++ /dev/null @@ -1,148 +0,0 @@ -/* - * 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 static com.diffplug.spotless.npm.PackageJsonUtil.replaceDevDependencies; -import static java.util.Objects.requireNonNull; - -import java.io.File; -import java.io.IOException; -import java.util.Collections; -import java.util.HashMap; -import java.util.LinkedHashMap; -import java.util.Map; -import java.util.TreeMap; - -import javax.annotation.Nonnull; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import com.diffplug.spotless.npm.EslintRestService.FormatOption; - -/** - * 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 EslintFormatterStep2 extends NpmServerBasedFormatterStep { - - private static final Logger logger = LoggerFactory.getLogger(EslintFormatterStep2.class); - - public static final String NAME = "eslint-format"; - - public static final String DEFAULT_ESLINT_VERSION = "^8.45.0"; - - public static Map defaultDevDependenciesForTypescript() { - return defaultDevDependenciesTypescriptWithEslint(DEFAULT_ESLINT_VERSION); - } - - public static Map defaultDevDependenciesTypescriptWithEslint(String eslintVersion) { - Map dependencies = new LinkedHashMap<>(); - 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", requireNonNull(eslintVersion)); - return dependencies; - } - - public static Map defaultDevDependencies() { - return defaultDevDependenciesWithEslint(DEFAULT_ESLINT_VERSION); - } - - public static Map defaultDevDependenciesWithEslint(String version) { - return Collections.singletonMap("eslint", version); - } - - public EslintFormatterStep2(@Nonnull Map devDependencies, @Nonnull File projectDir, @Nonnull File buildDir, @Nonnull File cacheDir, @Nonnull NpmPathResolver npmPathResolver, @Nonnull EslintConfig eslintConfig) { - super(NAME, - 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(), - Map.of(EslintConfigElement.ESLINT_CONFIG_ORIGINAL_ELEMENT, eslintConfig), - new NpmFormatterStepLocations(projectDir, buildDir, cacheDir, npmPathResolver)); - } - - protected EslintConfig origEslintConfig() { - return configElement(EslintConfigElement.ESLINT_CONFIG_ORIGINAL_ELEMENT); - } - - protected EslintConfig eslintConfigInUse() { - return configElement(EslintConfigElement.ESLINT_CONFIG_IN_USE_ELEMENT); - } - - @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 - protected String formatWithServer(NpmServerProcessInfo serverProcessInfo, String rawUnix, File file) { - EslintRestService restService = new EslintRestService(serverProcessInfo.getBaseUrl()); - Map eslintCallOptions = new HashMap<>(); - setConfigToCallOptions(eslintCallOptions); - setFilePathToCallOptions(eslintCallOptions, file); - return restService.format(rawUnix, eslintCallOptions); - } - - private void setFilePathToCallOptions(Map eslintCallOptions, File fileToBeFormatted) { - eslintCallOptions.put(FormatOption.FILE_PATH, fileToBeFormatted.getAbsolutePath()); - } - - private void setConfigToCallOptions(Map 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 enum EslintConfigElement implements NpmConfigElement { - ESLINT_CONFIG_ORIGINAL_ELEMENT, ESLINT_CONFIG_IN_USE_ELEMENT { - @Override - public boolean equalsHashcodeRelevant() { - return false; - } - }; - } -} diff --git a/lib/src/main/java/com/diffplug/spotless/npm/NpmServerBasedFormatterStep.java b/lib/src/main/java/com/diffplug/spotless/npm/NpmServerBasedFormatterStep.java index 00e84d5d43..79d3b03b0e 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/NpmServerBasedFormatterStep.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/NpmServerBasedFormatterStep.java @@ -66,14 +66,6 @@ protected NpmServerBasedFormatterStep(@Nonnull String name, this.locations = Objects.requireNonNull(locations); } - // public NpmServerBasedFormatterStep(@Nonnull String name, - // @Nonnull NpmConfig npmConfig, @Nonnull NpmFormatterStepLocations locations) { - // this.name = Objects.requireNonNull(name); - // this.locations = Objects.requireNonNull(locations); - // this.nodeServerLayout = new NodeServerLayout(Objects.requireNonNull(locations).buildDir(), Objects.requireNonNull(npmConfig).getPackageJsonContent()); - // this.nodeServeApp = new NodeServeApp(nodeServerLayout, npmConfig, locations); - // } - // FormatterStep @Override @@ -133,19 +125,16 @@ public int hashCode() { // override serialize output private void writeObject(ObjectOutputStream out) throws IOException { - // TODO (simschla, 27.06.2024): Implement serialization - System.out.println("TODO: Implement serialization - writeObject " + this); + // TODO (simschla, 27.06.2024): Implement custom serialization if needed + // TODO (simschla, 03.07.2024): Should we stop the server here if it is running? out.defaultWriteObject(); - // out.writeObject(state()); } // override serialize input @SuppressWarnings("unchecked") private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException { - // TODO (simschla, 27.06.2024): Implement serialization + // TODO (simschla, 27.06.2024): Implement custom serialization if needed in.defaultReadObject(); - System.out.println("TODO: Implement serialization - readObject " + this); - // state = (T) Objects.requireNonNull(in.readObject()); } // override serialize input @@ -171,7 +160,7 @@ protected NodeServerLayout nodeServerLayout() { } protected NodeServeApp nodeServeApp() { - // TODO (simschla, 01.07.2024): maybe memoize this + // TODO (simschla, 01.07.2024): maybe memoize this if it turns out to be expensive return new NodeServeApp(nodeServerLayout(), npmConfig(), locations); } @@ -246,6 +235,7 @@ protected NpmServerProcessInfo npmRunServer() throws ServerStartException, IOExc } } + // main contract method for specific subclasses protected abstract String formatWithServer(NpmServerProcessInfo serverProcessInfo, String rawUnix, File file); interface NpmConfigElement { diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavascriptExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavascriptExtension.java index 5532045bc2..315b0d78a4 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavascriptExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/JavascriptExtension.java @@ -106,7 +106,7 @@ public JavascriptEslintConfig(Map devDependencies) { public FormatterStep createStep() { final Project project = getProject(); - return EslintFormatterStep.create(devDependencies, provisioner(), project.getProjectDir(), + return EslintFormatterStep.create(devDependencies, project.getProjectDir(), project.getLayout().getBuildDirectory().getAsFile().get(), npmModulesCacheOrNull(), new NpmPathResolver(npmFileOrNull(), nodeFileOrNull(), npmrcFileOrNull(), Arrays.asList(project.getProjectDir(), project.getRootDir())), diff --git a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/TypescriptExtension.java b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/TypescriptExtension.java index 39e050d51e..d3ae3ed900 100644 --- a/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/TypescriptExtension.java +++ b/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/TypescriptExtension.java @@ -214,7 +214,7 @@ public TypescriptEslintConfig tsconfigFile(Object path) { public FormatterStep createStep() { final Project project = getProject(); - return EslintFormatterStep.create(devDependencies, provisioner(), project.getProjectDir(), + return EslintFormatterStep.create(devDependencies, project.getProjectDir(), project.getLayout().getBuildDirectory().getAsFile().get(), npmModulesCacheOrNull(), new NpmPathResolver(npmFileOrNull(), nodeFileOrNull(), npmrcFileOrNull(), Arrays.asList(project.getProjectDir(), project.getRootDir())), diff --git a/plugin-maven/src/main/java/com/diffplug/spotless/maven/javascript/AbstractEslint.java b/plugin-maven/src/main/java/com/diffplug/spotless/maven/javascript/AbstractEslint.java index ad113de833..dc74384498 100644 --- a/plugin-maven/src/main/java/com/diffplug/spotless/maven/javascript/AbstractEslint.java +++ b/plugin-maven/src/main/java/com/diffplug/spotless/maven/javascript/AbstractEslint.java @@ -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. @@ -69,7 +69,7 @@ public FormatterStep newFormatterStep(FormatterStepConfig stepConfig) { File baseDir = baseDir(stepConfig); File cacheDir = cacheDir(stepConfig); NpmPathResolver npmPathResolver = npmPathResolver(stepConfig); - return EslintFormatterStep.create(devDependencies, stepConfig.getProvisioner(), baseDir, buildDir, cacheDir, npmPathResolver, eslintConfig(stepConfig)); + return EslintFormatterStep.create(devDependencies, baseDir, buildDir, cacheDir, npmPathResolver, eslintConfig(stepConfig)); } private static IllegalArgumentException onlyOneConfig() { diff --git a/testlib/src/test/java/com/diffplug/spotless/npm/EslintFormatterStepTest.java b/testlib/src/test/java/com/diffplug/spotless/npm/EslintFormatterStepTest.java index ab54b11bee..8b8ff5334f 100644 --- a/testlib/src/test/java/com/diffplug/spotless/npm/EslintFormatterStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/npm/EslintFormatterStepTest.java @@ -27,7 +27,6 @@ import com.diffplug.spotless.FormatterStep; import com.diffplug.spotless.ResourceHarness; import com.diffplug.spotless.StepHarnessWithFile; -import com.diffplug.spotless.TestProvisioner; import com.diffplug.spotless.tag.NpmTest; @NpmTest @@ -60,7 +59,6 @@ void formattingUsingRulesetsFile(String ruleSetName) throws Exception { final FormatterStep formatterStep = EslintFormatterStep.create( devDependenciesForRuleset.get(ruleSetName), - TestProvisioner.mavenCentral(), projectDir(), buildDir(), null, @@ -103,7 +101,6 @@ void formattingUsingRulesetsFile(String ruleSetName) throws Exception { final FormatterStep formatterStep = EslintFormatterStep.create( devDependenciesForRuleset.get(ruleSetName), - TestProvisioner.mavenCentral(), projectDir(), buildDir(), null, @@ -160,7 +157,6 @@ void formattingUsingInlineXoConfig() throws Exception { final FormatterStep formatterStep = EslintFormatterStep.create( EslintStyleGuide.TS_XO_TYPESCRIPT.mergedWith(EslintFormatterStep.defaultDevDependenciesForTypescript()), - TestProvisioner.mavenCentral(), projectDir(), buildDir(), null,