From 70ad96cd566396097e53520f3ebf8b2aa32011b9 Mon Sep 17 00:00:00 2001 From: Simon Gamma Date: Tue, 24 Jan 2023 16:48:14 +0100 Subject: [PATCH 1/8] 1499: delay npmInstall and npmExec resolution to enable interoperability with `node-gradle-plugin` --- .../spotless/npm/EslintFormatterStep.java | 32 +++++++------ .../npm/NpmFormatterStepLocations.java | 11 +++-- .../npm/NpmFormatterStepStateBase.java | 47 ++++++++++--------- .../spotless/npm/PrettierFormatterStep.java | 4 +- .../spotless/npm/TsFmtFormatterStep.java | 4 +- .../NpmTestsWithoutNpmInstallationTest.java | 3 -- 6 files changed, 53 insertions(+), 48 deletions(-) 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 74979d90aa..2a095e9ad7 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java @@ -81,7 +81,9 @@ public static FormatterStep create(Map devDependencies, Provisio private static class State extends NpmFormatterStepStateBase implements Serializable { private static final long serialVersionUID = -539537027004745812L; - private final EslintConfig eslintConfig; + private final EslintConfig origEslintConfig; + + private transient EslintConfig eslintConfigInUse; State(String stepName, Map devDependencies, File projectDir, File buildDir, NpmPathResolver npmPathResolver, EslintConfig eslintConfig) throws IOException { super(stepName, @@ -97,21 +99,23 @@ private static class State extends NpmFormatterStepStateBase implements Serializ new NpmFormatterStepLocations( projectDir, buildDir, - npmPathResolver.resolveNpmExecutable(), - npmPathResolver.resolveNodeExecutable())); - this.eslintConfig = localCopyFiles(requireNonNull(eslintConfig)); + npmPathResolver::resolveNpmExecutable, + npmPathResolver::resolveNodeExecutable)); + this.origEslintConfig = requireNonNull(eslintConfig.verify()); + this.eslintConfigInUse = eslintConfig; } - private EslintConfig localCopyFiles(EslintConfig orig) { - if (orig.getEslintConfigPath() == null) { - return orig.verify(); + @Override + protected void prepareNodeServerLayout() throws IOException { + super.prepareNodeServerLayout(); + 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.) + FormattedPrinter.SYSOUT.print("Copying config file <%s> to <%s> and using the copy", origEslintConfig.getEslintConfigPath(), nodeServerLayout.nodeModulesDir()); + File configFileCopy = NpmResourceHelper.copyFileToDir(origEslintConfig.getEslintConfigPath(), nodeServerLayout.nodeModulesDir()); + this.eslintConfigInUse = this.origEslintConfig.withEslintConfigPath(configFileCopy).verify(); } - // 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.) - FormattedPrinter.SYSOUT.print("Copying config file <%s> to <%s> and using the copy", orig.getEslintConfigPath(), nodeModulesDir); - File configFileCopy = NpmResourceHelper.copyFileToDir(orig.getEslintConfigPath(), nodeModulesDir); - return orig.withEslintConfigPath(configFileCopy).verify(); } @Override @@ -121,7 +125,7 @@ public FormatterFunc createFormatterFunc() { FormattedPrinter.SYSOUT.print("creating formatter function (starting server)"); ServerProcessInfo eslintRestServer = npmRunServer(); EslintRestService restService = new EslintRestService(eslintRestServer.getBaseUrl()); - return Closeable.ofDangerous(() -> endServer(restService, eslintRestServer), new EslintFilePathPassingFormatterFunc(locations.projectDir(), nodeModulesDir, eslintConfig, restService)); + return Closeable.ofDangerous(() -> endServer(restService, eslintRestServer), new EslintFilePathPassingFormatterFunc(locations.projectDir(), nodeServerLayout.nodeModulesDir(), eslintConfigInUse, restService)); } catch (IOException e) { throw ThrowingEx.asRuntime(e); } diff --git a/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepLocations.java b/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepLocations.java index 0c417733e8..0e99e1afab 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepLocations.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepLocations.java @@ -19,6 +19,7 @@ import java.io.File; import java.io.Serializable; +import java.util.function.Supplier; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; @@ -32,12 +33,12 @@ class NpmFormatterStepLocations implements Serializable { private final transient File buildDir; @SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED") - private final transient File npmExecutable; + private final transient Supplier npmExecutable; @SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED") - private final transient File nodeExecutable; + private final transient Supplier nodeExecutable; - public NpmFormatterStepLocations(File projectDir, File buildDir, File npmExecutable, File nodeExecutable) { + public NpmFormatterStepLocations(File projectDir, File buildDir, Supplier npmExecutable, Supplier nodeExecutable) { this.projectDir = requireNonNull(projectDir); this.buildDir = requireNonNull(buildDir); this.npmExecutable = requireNonNull(npmExecutable); @@ -53,10 +54,10 @@ public File buildDir() { } public File npmExecutable() { - return npmExecutable; + return npmExecutable.get(); } public File nodeExecutable() { - return nodeExecutable; + return nodeExecutable.get(); } } 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 4f555a177e..996400ce33 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java @@ -31,7 +31,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.diffplug.spotless.FileSignature; import com.diffplug.spotless.FormatterFunc; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; @@ -42,11 +41,8 @@ abstract class NpmFormatterStepStateBase implements Serializable { private static final long serialVersionUID = 1460749955865959948L; - @SuppressWarnings("unused") - private final FileSignature packageJsonSignature; - @SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED") - public final transient File nodeModulesDir; + protected final transient NodeServerLayout nodeServerLayout; public final NpmFormatterStepLocations locations; @@ -58,45 +54,52 @@ protected NpmFormatterStepStateBase(String stepName, NpmConfig npmConfig, NpmFor this.stepName = requireNonNull(stepName); this.npmConfig = requireNonNull(npmConfig); this.locations = locations; - NodeServerLayout layout = prepareNodeServer(locations.buildDir()); - this.nodeModulesDir = layout.nodeModulesDir(); - this.packageJsonSignature = FileSignature.signAsList(layout.packageJsonFile()); + this.nodeServerLayout = new NodeServerLayout(locations.buildDir(), stepName); } - private NodeServerLayout prepareNodeServer(File buildDir) throws IOException { - NodeServerLayout layout = new NodeServerLayout(buildDir, stepName); - NpmResourceHelper.assertDirectoryExists(layout.nodeModulesDir()); - NpmResourceHelper.writeUtf8StringToFile(layout.packageJsonFile(), + protected void prepareNodeServerLayout() throws IOException { + NpmResourceHelper.assertDirectoryExists(nodeServerLayout.nodeModulesDir()); + NpmResourceHelper.writeUtf8StringToFile(nodeServerLayout.packageJsonFile(), this.npmConfig.getPackageJsonContent()); NpmResourceHelper - .writeUtf8StringToFile(layout.serveJsFile(), this.npmConfig.getServeScriptContent()); + .writeUtf8StringToFile(nodeServerLayout.serveJsFile(), this.npmConfig.getServeScriptContent()); if (this.npmConfig.getNpmrcContent() != null) { - NpmResourceHelper.writeUtf8StringToFile(layout.npmrcFile(), this.npmConfig.getNpmrcContent()); + NpmResourceHelper.writeUtf8StringToFile(nodeServerLayout.npmrcFile(), this.npmConfig.getNpmrcContent()); } else { - NpmResourceHelper.deleteFileIfExists(layout.npmrcFile()); + NpmResourceHelper.deleteFileIfExists(nodeServerLayout.npmrcFile()); } + } + + protected void prepareNodeServer() throws IOException { FormattedPrinter.SYSOUT.print("running npm install"); - runNpmInstall(layout.nodeModulesDir()); + runNpmInstall(nodeServerLayout.nodeModulesDir()); FormattedPrinter.SYSOUT.print("npm install finished"); - return layout; } private void runNpmInstall(File npmProjectDir) throws IOException { new NpmProcess(npmProjectDir, this.locations.npmExecutable(), this.locations.nodeExecutable()).install(); } - protected ServerProcessInfo npmRunServer() throws ServerStartException, IOException { - if (!this.nodeModulesDir.exists()) { - prepareNodeServer(NodeServerLayout.getBuildDirFromNodeModulesDir(this.nodeModulesDir)); + protected void assertNodeServerDirReady() throws IOException { + if (!this.nodeServerLayout.nodeModulesDir().exists() || !this.nodeServerLayout.packageJsonFile().isFile()) { + // reinstall if missing + prepareNodeServerLayout(); } + if (!new File(this.nodeServerLayout.nodeModulesDir(), "node_modules").isDirectory()) { + // run npm install if node_modules is missing + prepareNodeServer(); + } + } + protected ServerProcessInfo npmRunServer() throws ServerStartException, IOException { + assertNodeServerDirReady(); 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.nodeModulesDir, "server.port"); + final File serverPortFile = new File(this.nodeServerLayout.nodeModulesDir(), "server.port"); NpmResourceHelper.deleteFileIfExists(serverPortFile); // start the http server in node - Process server = new NpmProcess(this.nodeModulesDir, this.locations.npmExecutable(), this.locations.nodeExecutable()).start(); + Process server = new NpmProcess(this.nodeServerLayout.nodeModulesDir(), this.locations.npmExecutable(), this.locations.nodeExecutable()).start(); // await the readiness of the http server - wait for at most 60 seconds try { 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 22dd4586ee..22f1a69e7c 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java @@ -77,8 +77,8 @@ private static class State extends NpmFormatterStepStateBase implements Serializ new NpmFormatterStepLocations( projectDir, buildDir, - npmPathResolver.resolveNpmExecutable(), - npmPathResolver.resolveNodeExecutable())); + npmPathResolver::resolveNpmExecutable, + npmPathResolver::resolveNodeExecutable)); this.prettierConfig = requireNonNull(prettierConfig); } 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 14d6b1bbd0..4bd665c764 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/TsFmtFormatterStep.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/TsFmtFormatterStep.java @@ -83,8 +83,8 @@ public State(String stepName, Map versions, File projectDir, Fil new NpmFormatterStepLocations( projectDir, buildDir, - npmPathResolver.resolveNpmExecutable(), - npmPathResolver.resolveNodeExecutable())); + npmPathResolver::resolveNpmExecutable, + npmPathResolver::resolveNodeExecutable)); this.buildDir = requireNonNull(buildDir); this.configFile = configFile; this.inlineTsFmtSettings = inlineTsFmtSettings == null ? new TreeMap<>() : new TreeMap<>(inlineTsFmtSettings); diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/NpmTestsWithoutNpmInstallationTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/NpmTestsWithoutNpmInstallationTest.java index 916d853920..986e2726cb 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/NpmTestsWithoutNpmInstallationTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/NpmTestsWithoutNpmInstallationTest.java @@ -17,7 +17,6 @@ import org.assertj.core.api.Assertions; import org.gradle.testkit.runner.BuildResult; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import com.diffplug.common.base.Predicates; @@ -74,8 +73,6 @@ private void printContents() { } @Test - @Disabled("This test is disabled because we currently don't support using npm/node installed by the" + - "node-gradle-plugin as the installation takes place in the *execution* phase, but spotless needs it in the *configuration* phase.") void useNodeAndNpmFromNodeGradlePluginInOneSweep() throws Exception { try { setFile("build.gradle").toLines( From fac7fd1ece0887111e06a2b24def6b24d1cee77c Mon Sep 17 00:00:00 2001 From: Simon Gamma Date: Tue, 24 Jan 2023 19:51:51 +0100 Subject: [PATCH 2/8] 1499: document changes for delaying npm install --- CHANGES.md | 2 ++ plugin-gradle/CHANGES.md | 3 +++ plugin-maven/CHANGES.md | 1 + 3 files changed, 6 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index e8af5aec03..d41f40aaa6 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -23,6 +23,8 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( * ** POTENTIALLY BREAKING** Removed support for KtLint 0.3x and 0.45.2 ([#1475](https://github.com/diffplug/spotless/pull/1475)) * `KtLint` does not maintain a stable API - before this PR, we supported every breaking change in the API since 2019. * From now on, we will support no more than 2 breaking changes at a time. +* NpmFormatterStepStateBase delays `npm install` call until the formatter is first used. This enables better integration + with `gradle-node-plugin`. ([#1522](https://github.com/diffplug/spotless/pull/1522)) ## [2.32.0] - 2023-01-13 ### Added diff --git a/plugin-gradle/CHANGES.md b/plugin-gradle/CHANGES.md index 37d08f9b75..cbcd3bd953 100644 --- a/plugin-gradle/CHANGES.md +++ b/plugin-gradle/CHANGES.md @@ -15,6 +15,9 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( * **POTENTIALLY BREAKING** Removed support for KtLint 0.3x and 0.45.2 ([#1475](https://github.com/diffplug/spotless/pull/1475)) * `KtLint` does not maintain a stable API - before this PR, we supported every breaking change in the API since 2019. * From now on, we will support no more than 2 breaking changes at a time. +* `npm`-based formatters `ESLint`, `prettier` and `tsfmt` delay their `npm install` call until the formatters are first + used. For gradle this effectively moves the `npm install` call out of the configuration phase and as such enables + better integration with `gradle-node-plugin`. ([#1522](https://github.com/diffplug/spotless/pull/1522)) ## [6.13.0] - 2023-01-14 ### Added diff --git a/plugin-maven/CHANGES.md b/plugin-maven/CHANGES.md index 2a23405d35..93dd005ab6 100644 --- a/plugin-maven/CHANGES.md +++ b/plugin-maven/CHANGES.md @@ -18,6 +18,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( * **POTENTIALLY BREAKING** Removed support for KtLint 0.3x and 0.45.2 ([#1475](https://github.com/diffplug/spotless/pull/1475)) * `KtLint` does not maintain a stable API - before this PR, we supported every breaking change in the API since 2019. * From now on, we will support no more than 2 breaking changes at a time. +* `npm`-based formatters `ESLint`, `prettier` and `tsfmt` delay their `npm install` call until the formatters are first used. ([#1522](https://github.com/diffplug/spotless/pull/1522) ## [2.30.0] - 2023-01-13 ### Added From 7caf4c08c0daea72d15439bd8cd364e77638158f Mon Sep 17 00:00:00 2001 From: Simon Gamma Date: Tue, 24 Jan 2023 20:52:34 +0100 Subject: [PATCH 3/8] 1499: add documentation and examples showcasing the integration --- plugin-gradle/README.md | 7 +++ .../NpmTestsWithoutNpmInstallationTest.java | 47 +++++++------------ ...onTest_gradle_node_plugin_example_1.gradle | 38 +++++++++++++++ ...onTest_gradle_node_plugin_example_2.gradle | 35 ++++++++++++++ 4 files changed, 96 insertions(+), 31 deletions(-) create mode 100644 plugin-gradle/src/test/resources/com/diffplug/gradle/spotless/NpmTestsWithoutNpmInstallationTest_gradle_node_plugin_example_1.gradle create mode 100644 plugin-gradle/src/test/resources/com/diffplug/gradle/spotless/NpmTestsWithoutNpmInstallationTest_gradle_node_plugin_example_2.gradle diff --git a/plugin-gradle/README.md b/plugin-gradle/README.md index b6e95807d9..7e486719e7 100644 --- a/plugin-gradle/README.md +++ b/plugin-gradle/README.md @@ -907,6 +907,13 @@ spotless { If you provide both `npmExecutable` and `nodeExecutable`, spotless will use these paths. If you specify only one of the two, spotless will assume the other one is in the same directory. +If you use the `gradle-node-plugin` ([github](https://github.com/node-gradle/gradle-node-plugin)), it is possible to use the +node- and npm-binaries dynamically installed by this plugin. See +[this](https://github.com/diffplug/spotless/blob/main/plugin-gradle/src/test/resources/com/diffplug/gradle/spotless/NpmTestsWithoutNpmInstallationTest_gradle_node_plugin_example_1.gradle) +or [this](https://github.com/diffplug/spotless/blob/main/plugin-gradle/src/test/resources/com/diffplug/gradle/spotless/NpmTestsWithoutNpmInstallationTest_gradle_node_plugin_example_2.gradle) example. + +```gradle + ### `.npmrc` detection Spotless picks up npm configuration stored in a `.npmrc` file either in the project directory or in your user home. diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/NpmTestsWithoutNpmInstallationTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/NpmTestsWithoutNpmInstallationTest.java index 986e2726cb..03d14292e9 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/NpmTestsWithoutNpmInstallationTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/NpmTestsWithoutNpmInstallationTest.java @@ -73,38 +73,23 @@ private void printContents() { } @Test - void useNodeAndNpmFromNodeGradlePluginInOneSweep() throws Exception { + void useNodeAndNpmFromNodeGradlePlugin_example1() throws Exception { try { - setFile("build.gradle").toLines( - "plugins {", - " id 'com.diffplug.spotless'", - " id 'com.github.node-gradle.node' version '3.5.1'", - "}", - "repositories { mavenCentral() }", - "node {", - " download = true", - " version = '18.13.0'", - " npmVersion = '8.19.2'", - " workDir = file(\"${buildDir}/nodejs\")", - " npmWorkDir = file(\"${buildDir}/npm\")", - "}", - "def prettierConfig = [:]", - "prettierConfig['printWidth'] = 50", - "prettierConfig['parser'] = 'typescript'", - "def npmExec = System.getProperty('os.name').toLowerCase().contains('windows') ? '/npm.cmd' : '/bin/npm'", - "def nodeExec = System.getProperty('os.name').toLowerCase().contains('windows') ? '/node.exe' : '/bin/node'", - "spotless {", - " format 'mytypescript', {", - " target 'test.ts'", - " prettier()", - " .npmExecutable(\"${tasks.named('npmSetup').get().npmDir.get()}${npmExec}\")", - " .nodeExecutable(\"${tasks.named('nodeSetup').get().nodeDir.get()}${nodeExec}\")", - " .config(prettierConfig)", - " }", - "}", - "tasks.named('spotlessMytypescript').configure {", - " it.dependsOn('nodeSetup', 'npmSetup')", - "}"); + setFile("build.gradle").toResource("com/diffplug/gradle/spotless/NpmTestsWithoutNpmInstallationTest_gradle_node_plugin_example_1.gradle"); + setFile("test.ts").toResource("npm/prettier/config/typescript.dirty"); + final BuildResult spotlessApply = gradleRunner().withArguments("--stacktrace", "spotlessApply").build(); + Assertions.assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); + assertFile("test.ts").sameAsResource("npm/prettier/config/typescript.configfile.clean"); + } catch (Exception e) { + printContents(); + throw e; + } + } + + @Test + void useNpmFromNodeGradlePlugin_example2() throws Exception { + try { + setFile("build.gradle").toResource("com/diffplug/gradle/spotless/NpmTestsWithoutNpmInstallationTest_gradle_node_plugin_example_2.gradle"); setFile("test.ts").toResource("npm/prettier/config/typescript.dirty"); final BuildResult spotlessApply = gradleRunner().withArguments("--stacktrace", "spotlessApply").build(); Assertions.assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); diff --git a/plugin-gradle/src/test/resources/com/diffplug/gradle/spotless/NpmTestsWithoutNpmInstallationTest_gradle_node_plugin_example_1.gradle b/plugin-gradle/src/test/resources/com/diffplug/gradle/spotless/NpmTestsWithoutNpmInstallationTest_gradle_node_plugin_example_1.gradle new file mode 100644 index 0000000000..7ec7a2c592 --- /dev/null +++ b/plugin-gradle/src/test/resources/com/diffplug/gradle/spotless/NpmTestsWithoutNpmInstallationTest_gradle_node_plugin_example_1.gradle @@ -0,0 +1,38 @@ +/** + * This example shows how to use the gradle-node-plugin to install node and npm in separate directories + * and use these binaries with the prettier formatter. + */ +plugins { + id 'com.diffplug.spotless' + id 'com.github.node-gradle.node' version '3.5.1' +} +repositories { mavenCentral() } +node { + download = true + version = '18.13.0' + npmVersion = '8.19.2' + // when setting both these directories, npm and node will be in separate directories + workDir = file("${buildDir}/nodejs") + npmWorkDir = file("${buildDir}/npm") +} +def prettierConfig = [:] +prettierConfig['printWidth'] = 50 +prettierConfig['parser'] = 'typescript' + +// the executable names +def npmExec = System.getProperty('os.name').toLowerCase().contains('windows') ? '/npm.cmd' : '/bin/npm' +def nodeExec = System.getProperty('os.name').toLowerCase().contains('windows') ? '/node.exe' : '/bin/node' + +spotless { + format 'mytypescript', { + target 'test.ts' + prettier() + .npmExecutable("${tasks.named('npmSetup').get().npmDir.get()}${npmExec}") // get the npm executable path from gradle-node-plugin + .nodeExecutable("${tasks.named('nodeSetup').get().nodeDir.get()}${nodeExec}") // get the node executable path from gradle-node-plugin + .config(prettierConfig) + } +} + +tasks.named('spotlessMytypescript').configure { + it.dependsOn('nodeSetup', 'npmSetup') +} diff --git a/plugin-gradle/src/test/resources/com/diffplug/gradle/spotless/NpmTestsWithoutNpmInstallationTest_gradle_node_plugin_example_2.gradle b/plugin-gradle/src/test/resources/com/diffplug/gradle/spotless/NpmTestsWithoutNpmInstallationTest_gradle_node_plugin_example_2.gradle new file mode 100644 index 0000000000..eb59b85cf0 --- /dev/null +++ b/plugin-gradle/src/test/resources/com/diffplug/gradle/spotless/NpmTestsWithoutNpmInstallationTest_gradle_node_plugin_example_2.gradle @@ -0,0 +1,35 @@ +/** + * This example shows how to use the gradle-node-plugin to install node and npm together and + * use these binaries with the prettier formatter. + */ +plugins { + id 'com.diffplug.spotless' + id 'com.github.node-gradle.node' version '3.5.1' +} +repositories { mavenCentral() } +node { + download = true + version = '18.13.0' + // when not setting an explicit `npmWorkDir`, the npm binary will be installed next to the node binary + workDir = file("${buildDir}/nodejs") +} +def prettierConfig = [:] +prettierConfig['printWidth'] = 50 +prettierConfig['parser'] = 'typescript' + +// the executable name +def npmExec = System.getProperty('os.name').toLowerCase().contains('windows') ? '/npm.cmd' : '/bin/npm' + +spotless { + typescript { + target 'test.ts' + prettier() + .npmExecutable("${tasks.named('npmSetup').get().npmDir.get()}${npmExec}") // get the npm executable path from gradle-node-plugin + // setting the nodeExecutable is not necessary, since it will be found in the same directory as the npm executable (see above) + .config(prettierConfig) + } +} + +tasks.named('spotlessTypescript').configure { + it.dependsOn('nodeSetup', 'npmSetup') +} From 28ba93b8f3098b5ec104c87ab97d1eecb67b18ff Mon Sep 17 00:00:00 2001 From: Simon Gamma Date: Tue, 24 Jan 2023 20:54:09 +0100 Subject: [PATCH 4/8] 1499: drive-by-bugfix when using singleton map directly, it could occur that we tried to manipulate that map later (combination of using `prettier()` inside `typescript` block while supplying a prettier config via `config([...])` --- .../java/com/diffplug/gradle/spotless/TypescriptExtension.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 2283afccff..a11f1b0c39 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 @@ -166,7 +166,7 @@ protected FormatterStep createStep() { private void fixParserToTypescript() { if (this.prettierConfig == null) { - this.prettierConfig = Collections.singletonMap("parser", "typescript"); + this.prettierConfig = new TreeMap<>(Collections.singletonMap("parser", "typescript")); } else { final Object replaced = this.prettierConfig.put("parser", "typescript"); if (replaced != null) { From 292d7e2bd4fb86f10c05207fc4ff918f3a9dd5df Mon Sep 17 00:00:00 2001 From: Simon Gamma Date: Wed, 25 Jan 2023 11:18:36 +0100 Subject: [PATCH 5/8] 1499: refactor re-install needed detection --- .../spotless/npm/NodeServerLayout.java | 34 ++++++++++++++++++- .../npm/NpmFormatterStepStateBase.java | 12 +++++-- 2 files changed, 43 insertions(+), 3 deletions(-) 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 a175080517..63a6a923da 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-2021 DiffPlug + * Copyright 2020-2023 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,11 @@ package com.diffplug.spotless.npm; import java.io.File; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.stream.Stream; + +import com.diffplug.spotless.ThrowingEx; class NodeServerLayout { @@ -50,4 +55,31 @@ public File npmrcFile() { static File getBuildDirFromNodeModulesDir(File nodeModulesDir) { return nodeModulesDir.getParentFile(); } + + public boolean isLayoutPrepared() { + if (!nodeModulesDir().isDirectory()) { + return false; + } + if (!packageJsonFile().isFile()) { + return false; + } + if (!serveJsFile().isFile()) { + return false; + } + // npmrc is optional, so must not be checked here + return true; + } + + public boolean isNodeModulesPrepared() { + Path nodeModulesInstallDirPath = new File(nodeModulesDir(), "node_modules").toPath(); + if (!Files.isDirectory(nodeModulesInstallDirPath)) { + return false; + } + // check if it is NOT empty + return ThrowingEx.get(() -> { + try (Stream entries = Files.list(nodeModulesInstallDirPath)) { + return entries.findFirst().isPresent(); + } + }); + } } 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 996400ce33..9cff41e604 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java @@ -81,16 +81,24 @@ private void runNpmInstall(File npmProjectDir) throws IOException { } protected void assertNodeServerDirReady() throws IOException { - if (!this.nodeServerLayout.nodeModulesDir().exists() || !this.nodeServerLayout.packageJsonFile().isFile()) { + if (needsPrepareNodeServerLayout()) { // reinstall if missing prepareNodeServerLayout(); } - if (!new File(this.nodeServerLayout.nodeModulesDir(), "node_modules").isDirectory()) { + if (needsPrepareNodeServer()) { // run npm install if node_modules is missing prepareNodeServer(); } } + protected boolean needsPrepareNodeServer() { + return this.nodeServerLayout.isNodeModulesPrepared(); + } + + protected boolean needsPrepareNodeServerLayout() { + return !this.nodeServerLayout.isLayoutPrepared(); + } + protected ServerProcessInfo npmRunServer() throws ServerStartException, IOException { assertNodeServerDirReady(); try { From 7326772bad72c2855fecc62421b4e43031609af6 Mon Sep 17 00:00:00 2001 From: Simon Gamma Date: Wed, 25 Jan 2023 11:27:58 +0100 Subject: [PATCH 6/8] 1499: spotbugs --- .../java/com/diffplug/spotless/npm/EslintFormatterStep.java | 3 +++ 1 file changed, 3 insertions(+) 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 2a095e9ad7..221a745ad7 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java @@ -39,6 +39,8 @@ import com.diffplug.spotless.ThrowingEx; import com.diffplug.spotless.npm.EslintRestService.FormatOption; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + public class EslintFormatterStep { private static final Logger logger = LoggerFactory.getLogger(EslintFormatterStep.class); @@ -83,6 +85,7 @@ private static class State extends NpmFormatterStepStateBase implements Serializ private static final long serialVersionUID = -539537027004745812L; private final EslintConfig origEslintConfig; + @SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED") private transient EslintConfig eslintConfigInUse; State(String stepName, Map devDependencies, File projectDir, File buildDir, NpmPathResolver npmPathResolver, EslintConfig eslintConfig) throws IOException { From 46d370dd5e5e0ebaadcd4ebca2eef1927d2806f3 Mon Sep 17 00:00:00 2001 From: Simon Gamma Date: Wed, 25 Jan 2023 19:20:05 +0100 Subject: [PATCH 7/8] 1499: provide npm output if server starting fails --- .../diffplug/spotless/npm/NpmFormatterStepStateBase.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) 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 9cff41e604..3e5fa1ff18 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java @@ -32,6 +32,8 @@ import org.slf4j.LoggerFactory; import com.diffplug.spotless.FormatterFunc; +import com.diffplug.spotless.ProcessRunner.LongRunningProcess; +import com.diffplug.spotless.ThrowingEx; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; @@ -101,13 +103,14 @@ protected boolean needsPrepareNodeServerLayout() { protected ServerProcessInfo 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 - Process server = new NpmProcess(this.nodeServerLayout.nodeModulesDir(), this.locations.npmExecutable(), this.locations.nodeExecutable()).start(); + server = new NpmProcess(this.nodeServerLayout.nodeModulesDir(), this.locations.npmExecutable(), this.locations.nodeExecutable()).start(); // await the readiness of the http server - wait for at most 60 seconds try { @@ -128,7 +131,7 @@ protected ServerProcessInfo npmRunServer() throws ServerStartException, IOExcept String serverPort = NpmResourceHelper.readUtf8StringFromFile(serverPortFile).trim(); return new ServerProcessInfo(server, serverPort, serverPortFile); } catch (IOException | TimeoutException e) { - throw new ServerStartException(e); + throw new ServerStartException("Starting server failed." + (server != null ? "\n\nProcess result:\n" + ThrowingEx.get(server::result) : ""), e); } } @@ -197,7 +200,7 @@ public void close() throws Exception { protected static class ServerStartException extends RuntimeException { private static final long serialVersionUID = -8803977379866483002L; - public ServerStartException(Throwable cause) { + public ServerStartException(String message, Throwable cause) { super(cause); } } From 012e4ae55cd0c9c3783ed6130e6edf6e10700277 Mon Sep 17 00:00:00 2001 From: Simon Gamma Date: Wed, 25 Jan 2023 19:21:35 +0100 Subject: [PATCH 8/8] 1499: fix condition --- .../com/diffplug/spotless/npm/NpmFormatterStepStateBase.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 3e5fa1ff18..92ac7df6f2 100644 --- a/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java +++ b/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java @@ -94,7 +94,7 @@ protected void assertNodeServerDirReady() throws IOException { } protected boolean needsPrepareNodeServer() { - return this.nodeServerLayout.isNodeModulesPrepared(); + return !this.nodeServerLayout.isNodeModulesPrepared(); } protected boolean needsPrepareNodeServerLayout() {