Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support to allow multiple prettier instances for different target source sets #1565

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
## [Unreleased]
### Added
* CleanThat Java Refactorer. ([#1560](https://github.com/diffplug/spotless/pull/1560))
* Introduce `LazyArgLogger` to allow for lazy evaluation of log messages in slf4j logging. ([#1565](https://github.com/diffplug/spotless/pull/1565))
### Fixed
* Allow multiple instances of the same npm-based formatter to be used by separating their `node_modules` directories. ([#1565](https://github.com/diffplug/spotless/pull/1565))
* `ktfmt` default style uses correct continuation indent. ([#1562](https://github.com/diffplug/spotless/pull/1562))

## [2.34.1] - 2023-02-05
Expand Down
40 changes: 40 additions & 0 deletions lib/src/main/java/com/diffplug/spotless/LazyArgLogger.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright 2023 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;

import java.util.function.Supplier;

/**
* This is a utility class to allow for lazy evaluation of arguments to be passed to a logger
* and thus avoid unnecessary computation of the arguments if the log level is not enabled.
*/
public final class LazyArgLogger {

private final Supplier<Object> argSupplier;

private LazyArgLogger(Supplier<Object> argSupplier) {
this.argSupplier = argSupplier;
}

public static LazyArgLogger lazy(Supplier<Object> argSupplier) {
return new LazyArgLogger(argSupplier);
}

@Override
public String toString() {
return String.valueOf(argSupplier.get());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package com.diffplug.spotless.npm;

import static com.diffplug.spotless.LazyArgLogger.lazy;
import static java.util.Objects.requireNonNull;

import java.io.File;
Expand Down Expand Up @@ -115,7 +116,7 @@ protected void prepareNodeServerLayout() throws IOException {
// 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());
logger.info("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();
}
Expand All @@ -125,7 +126,7 @@ protected void prepareNodeServerLayout() throws IOException {
@Nonnull
public FormatterFunc createFormatterFunc() {
try {
FormattedPrinter.SYSOUT.print("creating formatter function (starting server)");
logger.info("Creating formatter function (starting server)");
ServerProcessInfo eslintRestServer = npmRunServer();
EslintRestService restService = new EslintRestService(eslintRestServer.getBaseUrl());
return Closeable.ofDangerous(() -> endServer(restService, eslintRestServer), new EslintFilePathPassingFormatterFunc(locations.projectDir(), nodeServerLayout.nodeModulesDir(), eslintConfigInUse, restService));
Expand All @@ -135,7 +136,7 @@ public FormatterFunc createFormatterFunc() {
}

private void endServer(BaseNpmRestService restService, ServerProcessInfo restServer) throws Exception {
FormattedPrinter.SYSOUT.print("Closing formatting function (ending server).");
logger.info("Closing formatting function (ending server).");
try {
restService.shutdown();
} catch (Throwable t) {
Expand All @@ -161,7 +162,7 @@ public EslintFilePathPassingFormatterFunc(File projectDir, File nodeModulesDir,

@Override
public String applyWithFile(String unix, File file) throws Exception {
FormattedPrinter.SYSOUT.print("formatting String '" + unix.substring(0, Math.min(50, unix.length())) + "[...]' in file '" + file + "'");
logger.info("formatting String '{}[...]' in file '{}'", lazy(() -> unix.substring(0, Math.min(50, unix.length()))), file);

Map<FormatOption, Object> eslintCallOptions = new HashMap<>();
setConfigToCallOptions(eslintCallOptions);
Expand Down
42 changes: 0 additions & 42 deletions lib/src/main/java/com/diffplug/spotless/npm/FormattedPrinter.java

This file was deleted.

40 changes: 34 additions & 6 deletions lib/src/main/java/com/diffplug/spotless/npm/NodeServerLayout.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,25 +18,44 @@
import java.io.File;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Stream;

import com.diffplug.spotless.ThrowingEx;

class NodeServerLayout {

private static final Pattern PACKAGE_JSON_NAME_PATTERN = Pattern.compile("\"name\"\\s*:\\s*\"([^\"]+)\"");

private final File nodeModulesDir;
private final File packageJsonFile;

private final File packageLockJsonFile;

private final File serveJsFile;
private final File npmrcFile;

NodeServerLayout(File buildDir, String stepName) {
this.nodeModulesDir = new File(buildDir, "spotless-node-modules-" + stepName);
NodeServerLayout(File buildDir, String packageJsonContent) {
this.nodeModulesDir = new File(buildDir, nodeModulesDirName(packageJsonContent));
this.packageJsonFile = new File(nodeModulesDir, "package.json");
this.packageLockJsonFile = new File(nodeModulesDir, "package-lock.json");
this.serveJsFile = new File(nodeModulesDir, "serve.js");
this.npmrcFile = new File(nodeModulesDir, ".npmrc");
}

private static String nodeModulesDirName(String packageJsonContent) {
String md5Hash = NpmResourceHelper.md5(packageJsonContent);
Matcher matcher = PACKAGE_JSON_NAME_PATTERN.matcher(packageJsonContent);
if (!matcher.find()) {
throw new IllegalArgumentException("package.json must contain a name property");
}
String packageName = matcher.group(1);
return String.format("%s-node-modules-%s", packageName, md5Hash);
}

File nodeModulesDir() {

return nodeModulesDir;
}

Expand All @@ -52,17 +71,16 @@ public File npmrcFile() {
return npmrcFile;
}

static File getBuildDirFromNodeModulesDir(File nodeModulesDir) {
return nodeModulesDir.getParentFile();
}

public boolean isLayoutPrepared() {
if (!nodeModulesDir().isDirectory()) {
return false;
}
if (!packageJsonFile().isFile()) {
return false;
}
if (!packageLockJsonFile.isFile()) {
return false;
}
if (!serveJsFile().isFile()) {
return false;
}
Expand All @@ -82,4 +100,14 @@ public boolean isNodeModulesPrepared() {
}
});
}

@Override
public String toString() {
return String.format(
"NodeServerLayout[nodeModulesDir=%s, packageJsonFile=%s, serveJsFile=%s, npmrcFile=%s]",
this.nodeModulesDir,
this.packageJsonFile,
this.serveJsFile,
this.npmrcFile);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,13 @@ protected NpmFormatterStepStateBase(String stepName, NpmConfig npmConfig, NpmFor
this.stepName = requireNonNull(stepName);
this.npmConfig = requireNonNull(npmConfig);
this.locations = locations;
this.nodeServerLayout = new NodeServerLayout(locations.buildDir(), stepName);
this.nodeServerLayout = new NodeServerLayout(locations.buildDir(), npmConfig.getPackageJsonContent());
}

protected void prepareNodeServerLayout() throws IOException {
final long started = System.currentTimeMillis();
// maybe introduce trace logger?
logger.info("Preparing {} for npm step {}.", this.nodeServerLayout, getClass().getName());
NpmResourceHelper.assertDirectoryExists(nodeServerLayout.nodeModulesDir());
NpmResourceHelper.writeUtf8StringToFile(nodeServerLayout.packageJsonFile(),
this.npmConfig.getPackageJsonContent());
Expand All @@ -70,12 +73,14 @@ protected void prepareNodeServerLayout() throws IOException {
} else {
NpmResourceHelper.deleteFileIfExists(nodeServerLayout.npmrcFile());
}
logger.info("Prepared {} for npm step {} in {} ms.", this.nodeServerLayout, getClass().getName(), System.currentTimeMillis() - started);
}

protected void prepareNodeServer() throws IOException {
FormattedPrinter.SYSOUT.print("running npm install");
final long started = System.currentTimeMillis();
logger.info("running npm install in {} for npm step {}", this.nodeServerLayout.nodeModulesDir(), getClass().getName());
runNpmInstall(nodeServerLayout.nodeModulesDir());
FormattedPrinter.SYSOUT.print("npm install finished");
logger.info("npm install finished in {} ms in {} for npm step {}", System.currentTimeMillis() - started, this.nodeServerLayout.nodeModulesDir(), getClass().getName());
}

private void runNpmInstall(File npmProjectDir) throws IOException {
Expand Down
17 changes: 17 additions & 0 deletions lib/src/main/java/com/diffplug/spotless/npm/NpmResourceHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.StandardCopyOption;
import java.security.MessageDigest;
import java.time.Duration;
import java.util.Arrays;
import java.util.Objects;
Expand Down Expand Up @@ -122,4 +123,20 @@ static File copyFileToDirAtSubpath(File file, File targetDir, String relativePat
throw ThrowingEx.asRuntime(e);
}
}

static String md5(File file) {
return md5(readUtf8StringFromFile(file));
}

static String md5(String fileContent) {
MessageDigest md = ThrowingEx.get(() -> MessageDigest.getInstance("MD5"));
nedtwigg marked this conversation as resolved.
Show resolved Hide resolved
md.update(fileContent.getBytes(StandardCharsets.UTF_8));
byte[] digest = md.digest();
// convert byte array digest to hex string
StringBuilder sb = new StringBuilder();
for (byte b : digest) {
sb.append(String.format("%02x", b & 0xff));
}
return sb.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package com.diffplug.spotless.npm;

import static com.diffplug.spotless.LazyArgLogger.lazy;
import static java.util.Objects.requireNonNull;

import java.io.File;
Expand Down Expand Up @@ -86,7 +87,7 @@ private static class State extends NpmFormatterStepStateBase implements Serializ
@Nonnull
public FormatterFunc createFormatterFunc() {
try {
FormattedPrinter.SYSOUT.print("creating formatter function (starting server)");
logger.info("creating formatter function (starting server)");
ServerProcessInfo prettierRestServer = npmRunServer();
PrettierRestService restService = new PrettierRestService(prettierRestServer.getBaseUrl());
String prettierConfigOptions = restService.resolveConfig(this.prettierConfig.getPrettierConfigPath(), this.prettierConfig.getOptions());
Expand All @@ -97,7 +98,7 @@ public FormatterFunc createFormatterFunc() {
}

private void endServer(PrettierRestService restService, ServerProcessInfo restServer) throws Exception {
FormattedPrinter.SYSOUT.print("Closing formatting function (ending server).");
logger.info("Closing formatting function (ending server).");
try {
restService.shutdown();
} catch (Throwable t) {
Expand All @@ -119,7 +120,7 @@ public PrettierFilePathPassingFormatterFunc(String prettierConfigOptions, Pretti

@Override
public String applyWithFile(String unix, File file) throws Exception {
FormattedPrinter.SYSOUT.print("formatting String '" + unix.substring(0, Math.min(50, unix.length())) + "[...]' in file '" + file + "'");
logger.info("formatting String '{}[...]' in file '{}'", lazy(() -> unix.substring(0, Math.min(50, unix.length()))), file);

final String prettierConfigOptionsWithFilepath = assertFilepathInConfigOptions(file);
try {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"name": "spotless-eslint-formatter-step",
"name": "spotless-eslint",
"version": "2.0.0",
"description": "Spotless formatter step for running eslint as a rest service.",
"repository": "https://github.com/diffplug/spotless",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"name": "spotless-prettier-formatter-step",
"name": "spotless-prettier",
"version": "2.0.0",
"description": "Spotless formatter step for running prettier as a rest service.",
"repository": "https://github.com/diffplug/spotless",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"name": "spotless-tsfmt-formatter-step",
"name": "spotless-tsfmt",
"version": "2.0.0",
"description": "Spotless formatter step for running tsfmt as a rest service.",
"repository": "https://github.com/diffplug/spotless",
Expand Down
2 changes: 2 additions & 0 deletions plugin-gradle/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
### Added
* CleanThat Java Refactorer. ([#1560](https://github.com/diffplug/spotless/pull/1560))
### Fixed
* Allow multiple instances of the same npm-based formatter to be used simultaneously. E.g. use prettier for typescript
*and* Java (using the community prettier-plugin-java) without messing up their respective `node_module` dependencies. ([#1565](https://github.com/diffplug/spotless/pull/1565))
* `ktfmt` default style uses correct continuation indent. ([#1562](https://github.com/diffplug/spotless/pull/1562))

## [6.14.1] - 2023-02-05
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,50 @@ void usePhpCommunityPlugin() throws IOException {
assertFile("php-example.php").sameAsResource("npm/prettier/plugins/php.clean");
}

/**
* This test is to ensure that we can have multiple prettier instances in one spotless config.
*
* @see <a href="https://github.com/diffplug/spotless/issues/1162">Issue #1162 on github</a>
*/
@Test
void usePhpAndJavaCommunityPlugin() throws IOException {
setFile("build.gradle").toLines(
"plugins {",
" id 'com.diffplug.spotless'",
"}",
"repositories { mavenCentral() }",
"def prettierConfigPhp = [:]",
"prettierConfigPhp['tabWidth'] = 3",
"prettierConfigPhp['parser'] = 'php'",
"def prettierPackagesPhp = [:]",
"prettierPackagesPhp['prettier'] = '2.0.5'",
"prettierPackagesPhp['@prettier/plugin-php'] = '0.14.2'",
"def prettierConfigJava = [:]",
"prettierConfigJava['tabWidth'] = 4",
"prettierConfigJava['parser'] = 'java'",
"def prettierPackagesJava = [:]",
"prettierPackagesJava['prettier'] = '2.0.5'",
"prettierPackagesJava['prettier-plugin-java'] = '0.8.0'",
"spotless {",
" format 'php', {",
" target 'php-example.php'",
" prettier(prettierPackagesPhp).config(prettierConfigPhp)",
" }",
" java {",
" target 'JavaTest.java'",
" prettier(prettierPackagesJava).config(prettierConfigJava)",
" }",
"}");
setFile("php-example.php").toResource("npm/prettier/plugins/php.dirty");
setFile("JavaTest.java").toResource("npm/prettier/plugins/java-test.dirty");
final BuildResult spotlessApply = gradleRunner().forwardOutput().withArguments("--stacktrace", "--info", "spotlessApply").build();
Assertions.assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL");
final BuildResult spotlessApply2 = gradleRunner().forwardOutput().withArguments("--stacktrace", "--info", "spotlessApply").build();
Assertions.assertThat(spotlessApply2.getOutput()).contains("BUILD SUCCESSFUL");
assertFile("php-example.php").sameAsResource("npm/prettier/plugins/php.clean");
assertFile("JavaTest.java").sameAsResource("npm/prettier/plugins/java-test.clean");
}

@Test
void autodetectNpmrcFileConfig() throws IOException {
setFile(".npmrc").toLines(
Expand Down
Loading