From d73d05e5e0b428939880fcca16a9e267d8c70fbc Mon Sep 17 00:00:00 2001 From: Andre Wachsmuth Date: Tue, 9 Jan 2024 21:52:42 +0100 Subject: [PATCH 1/4] Fixes #1987, no empty files with biome when formatting ignored files The issue happens only with the latest release 1.5.0. * When the biome output is empty, the contents of the input file to format are used unchanged instead. * When biome outputs warnings / errors to stderr, these are now logged. This includes warnings such as when biome ignores files. Otherwise people would be left wondering why certain files are not formatted. I also added a test for this scenarion. On another note, I mostly use Maven, so I always need to look up how to run tests via Gradle. I added a short section to CONTRIBUTING.md that gives some example commands how to run tests for this project, especially how to run only a single test. --- CHANGES.md | 2 ++ CONTRIBUTING.md | 20 +++++++++++++ .../com/diffplug/spotless/rome/RomeStep.java | 18 ++++++++++-- plugin-gradle/README.md | 23 +++++++++++---- .../gradle/spotless/BiomeIntegrationTest.java | 28 +++++++++++++++++++ plugin-maven/README.md | 20 ++++++++++--- .../spotless/maven/biome/BiomeMavenTest.java | 16 +++++++++++ .../main/resources/biome/json/package.json | 3 ++ .../resources/biome/json/packageAfter.json | 3 ++ .../resources/biome/json/packageBefore.json | 3 ++ .../diffplug/spotless/rome/RomeStepTest.java | 13 +++++++++ 11 files changed, 138 insertions(+), 11 deletions(-) create mode 100644 testlib/src/main/resources/biome/json/package.json create mode 100644 testlib/src/main/resources/biome/json/packageAfter.json create mode 100644 testlib/src/main/resources/biome/json/packageBefore.json diff --git a/CHANGES.md b/CHANGES.md index dc040c9aa2..23f58ea9d1 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -12,6 +12,8 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( ## [Unreleased] ### Added * New static method to `DiffMessageFormatter` which allows to retrieve diffs with their line numbers ([#1960](https://github.com/diffplug/spotless/issues/1960)) +### Fixed +* Fix empty files with biome >= 1.5.0 when formatting files that are in the ignore list of the biome configuration file. (fixes [#1987](https://github.com/diffplug/spotless/issues/1987)) ### Changes * Use palantir-java-format 2.39.0 on Java 21. ([#1948](https://github.com/diffplug/spotless/pull/1948)) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 287d835bca..6fb993f419 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -151,6 +151,26 @@ The gist of it is that you will have to: If you get something running, we'd love to host your plugin within this repo as a peer to `plugin-gradle` and `plugin-maven`. +## Run tests + +To run all tests, simply do + +> gradlew test + +Since that takes some time, you might only want to run the tests +concerning what you are working on: + +```shell +# Run only from test from the "lib" project +gradlew :testlib:test --tests com.diffplug.spotless.generic.IndentStepTest + +# Run only one test from the "plugin-maven" project +gradlew :plugin-maven:test --tests com.diffplug.spotless.maven.pom.SortPomMavenTest + +# Run only one test from the "plugin-gradle" project +gradlew :plugin-gradle:test --tests com.diffplug.gradle.spotless.FreshMarkExtensionTest +``` + ## Integration testing ### Gradle - locally diff --git a/lib/src/main/java/com/diffplug/spotless/rome/RomeStep.java b/lib/src/main/java/com/diffplug/spotless/rome/RomeStep.java index 5ba33d593d..92a0fdc689 100644 --- a/lib/src/main/java/com/diffplug/spotless/rome/RomeStep.java +++ b/lib/src/main/java/com/diffplug/spotless/rome/RomeStep.java @@ -427,9 +427,23 @@ private String format(ProcessRunner runner, String input, File file) throws IOEx var stdin = input.getBytes(StandardCharsets.UTF_8); var args = buildBiomeCommand(file); if (logger.isDebugEnabled()) { - logger.debug("Running Biome comand to format code: '{}'", String.join(", ", args)); + logger.debug("Running Biome command to format code: '{}'", String.join(", ", args)); + } + var runnerResult = runner.exec(stdin, args); + var stdErr = runnerResult.stdErrUtf8(); + if (stdErr != null && !stdErr.isEmpty()) { + logger.warn("Biome stderr ouptut for file '{}'\n{}", file, stdErr.trim()); + } + var formatted = runnerResult.assertExitZero(StandardCharsets.UTF_8); + // When biome encounters an ignored file, it does not output any formatted code + // Ignored files come from (a) the biome.json configuration file and (b) from + // a list of hard-coded file names, such as package.json or tsconfig.json. + if (formatted == null || formatted.isEmpty()) { + return input; + } + else { + return formatted; } - return runner.exec(stdin, args).assertExitZero(StandardCharsets.UTF_8); } /** diff --git a/plugin-gradle/README.md b/plugin-gradle/README.md index 7ad010567a..98416cdd05 100644 --- a/plugin-gradle/README.md +++ b/plugin-gradle/README.md @@ -1218,11 +1218,6 @@ spotless { } ``` -**Limitations:** - -- The auto-discovery of config files (up the file tree) will not work when using - Biome within spotless. - To apply Biome to more kinds of files with a different configuration, just add more formats: @@ -1243,6 +1238,24 @@ spotless { } ``` +**Limitations:** + +- The auto-discovery of config files (up the file tree) will not work when using + Biome within spotless. +- The `ignore` option of the `biome.json` configuration file will not be applied. + Include and exclude patterns are configured in the spotless configuration in the + Gradle settings file instead. + +Note: Due to a limitation of biome, if the name of a file matches a pattern in +the `ignore` option of the specified `biome.json` configuration file, it will not be +formatted, even if included in the biome configuration section of the Gradle settings +file. +You could specify a different `biome.json` configuration file without an `ignore` +pattern to circumvent this. + +Note 2: Biome is hard-coded to ignore certain special files, such as `package.json` +or `tsconfig.json`. These files will never be formatted. + ### Biome binary To format with Biome, spotless needs to find the Biome binary. By default, diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BiomeIntegrationTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BiomeIntegrationTest.java index a2665a3bf1..139d477bf0 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BiomeIntegrationTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BiomeIntegrationTest.java @@ -340,4 +340,32 @@ void failureWhenNotParseable() throws Exception { assertThat(spotlessApply.getOutput()).contains("Format with errors is disabled."); assertThat(spotlessApply.getOutput()).contains("Step 'biome' found problem in 'biome_test.js'"); } + + /** + * Biome is hard-coded to ignore certain files, such as package.json. Since version 1.5.0, + * the biome CLI does not output any formatted code anymore, whereas previously it printed + * the input as-is. This tests checks that when the biome formatter outputs an empty string, + * the contents of the file to format are used instead. + * + * @throws Exception When a test failure occurs. + */ + @Test + void preservesIgnoredFiles() throws Exception { + setFile("build.gradle").toLines( + "plugins {", + " id 'com.diffplug.spotless'", + "}", + "repositories { mavenCentral() }", + "spotless {", + " json {", + " target '**/*.json'", + " biome('1.5.0')", + " }", + "}"); + setFile("package.json").toResource("biome/json/packageBefore.json"); + + var spotlessApply = gradleRunner().withArguments("--stacktrace", "spotlessApply").build(); + assertThat(spotlessApply.getOutput()).contains("BUILD SUCCESSFUL"); + assertFile("package.json").sameAsResource("biome/json/packageAfter.json"); + } } diff --git a/plugin-maven/README.md b/plugin-maven/README.md index f632d78ea7..593368c7ac 100644 --- a/plugin-maven/README.md +++ b/plugin-maven/README.md @@ -1300,10 +1300,6 @@ usually you will be creating a generic format. ``` -**Limitations:** -- The auto-discovery of config files (up the file tree) will not work when using - Biome within spotless. - To apply Biome to more kinds of files with a different configuration, just add more formats: @@ -1315,6 +1311,22 @@ more formats: ``` +**Limitations:** +- The auto-discovery of config files (up the file tree) will not work when using + Biome within spotless. +- The `ignore` option of the `biome.json` configuration file will not be applied. + Include and exclude patterns are configured in the spotless configuration in the + Maven pom instead. + +Note: Due to a limitation of biome, if the name of a file matches a pattern in +the `ignore` option of the specified `biome.json` configuration file, it will not be +formatted, even if included in the biome configuration section of the Maven pom. +You could specify a different `biome.json` configuration file without an `ignore` +pattern to circumvent this. + +Note 2: Biome is hard-coded to ignore certain special files, such as `package.json` +or `tsconfig.json`. These files will never be formatted. + ### Biome binary To format with Biome, spotless needs to find the Biome binary. By default, diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/biome/BiomeMavenTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/biome/BiomeMavenTest.java index e015b934f5..ffbd73484f 100644 --- a/plugin-maven/src/test/java/com/diffplug/spotless/maven/biome/BiomeMavenTest.java +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/biome/BiomeMavenTest.java @@ -199,4 +199,20 @@ void failureWhenNotParseable() throws Exception { assertThat(result.stdOutUtf8()).contains("Unable to format file"); assertThat(result.stdOutUtf8()).contains("Step 'biome' found problem in 'biome_test.js'"); } + + /** + * Biome is hard-coded to ignore certain files, such as package.json. Since version 1.5.0, + * the biome CLI does not output any formatted code anymore, whereas previously it printed + * the input as-is. This tests checks that when the biome formatter outputs an empty string, + * the contents of the file to format are used instead. + * + * @throws Exception When a test failure occurs. + */ + @Test + void preservesIgnoredFiles() throws Exception { + writePomWithJsonSteps("**/*.json", "1.5.0"); + setFile("package.json").toResource("biome/json/packageBefore.json"); + mavenRunner().withArguments("spotless:apply").runNoError(); + assertFile("package.json").sameAsResource("biome/json/packageAfter.json"); + } } diff --git a/testlib/src/main/resources/biome/json/package.json b/testlib/src/main/resources/biome/json/package.json new file mode 100644 index 0000000000..94e8f88de5 --- /dev/null +++ b/testlib/src/main/resources/biome/json/package.json @@ -0,0 +1,3 @@ +{ +"name": "test-package","version": "0.0.1" + } \ No newline at end of file diff --git a/testlib/src/main/resources/biome/json/packageAfter.json b/testlib/src/main/resources/biome/json/packageAfter.json new file mode 100644 index 0000000000..94e8f88de5 --- /dev/null +++ b/testlib/src/main/resources/biome/json/packageAfter.json @@ -0,0 +1,3 @@ +{ +"name": "test-package","version": "0.0.1" + } \ No newline at end of file diff --git a/testlib/src/main/resources/biome/json/packageBefore.json b/testlib/src/main/resources/biome/json/packageBefore.json new file mode 100644 index 0000000000..94e8f88de5 --- /dev/null +++ b/testlib/src/main/resources/biome/json/packageBefore.json @@ -0,0 +1,3 @@ +{ +"name": "test-package","version": "0.0.1" + } \ No newline at end of file diff --git a/testlib/src/test/java/com/diffplug/spotless/rome/RomeStepTest.java b/testlib/src/test/java/com/diffplug/spotless/rome/RomeStepTest.java index f723a561c2..7ab4112bef 100644 --- a/testlib/src/test/java/com/diffplug/spotless/rome/RomeStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/rome/RomeStepTest.java @@ -392,6 +392,19 @@ void testAutoDetectTsx() { var stepHarness = StepHarnessWithFile.forStep(RomeStepTest.this, step); stepHarness.testResource("biome/ts/fileBefore.tsx", "biome/ts/fileAfter.tsx"); } + + /** + * Biome is hard-coded to ignore certain files, such as package.json. Since version 1.5.0, + * the biome CLI does not output any formatted code anymore, whereas previously it printed + * the input as-is. This tests checks that when the biome formatter outputs an empty string, + * the contents of the file to format are used instead. + */ + @Test + void preservesIgnoredFiles() { + var step = RomeStep.withExeDownload(BiomeFlavor.BIOME, "1.5.0", downloadDir.toString()).create(); + var stepHarness = StepHarnessWithFile.forStep(RomeStepTest.this, step); + stepHarness.testResource("biome/json/package.json", "biome/json/packageAfter.json"); + } } @Nested From b82e75bbba14371bed58efd59e79107c7e900a0d Mon Sep 17 00:00:00 2001 From: Andre Wachsmuth Date: Tue, 9 Jan 2024 22:01:31 +0100 Subject: [PATCH 2/4] Update changelog --- CHANGES.md | 2 +- plugin-gradle/CHANGES.md | 2 ++ plugin-maven/CHANGES.md | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 23f58ea9d1..cd641ef75f 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -13,7 +13,7 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( ### Added * New static method to `DiffMessageFormatter` which allows to retrieve diffs with their line numbers ([#1960](https://github.com/diffplug/spotless/issues/1960)) ### Fixed -* Fix empty files with biome >= 1.5.0 when formatting files that are in the ignore list of the biome configuration file. (fixes [#1987](https://github.com/diffplug/spotless/issues/1987)) +* Fix empty files with biome >= 1.5.0 when formatting files that are in the ignore list of the biome configuration file. ([#1989](https://github.com/diffplug/spotless/pull/1989) fixes [#1987](https://github.com/diffplug/spotless/issues/1987)) ### Changes * Use palantir-java-format 2.39.0 on Java 21. ([#1948](https://github.com/diffplug/spotless/pull/1948)) diff --git a/plugin-gradle/CHANGES.md b/plugin-gradle/CHANGES.md index d888f4863f..20c9a6029c 100644 --- a/plugin-gradle/CHANGES.md +++ b/plugin-gradle/CHANGES.md @@ -3,6 +3,8 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `3.27.0`). ## [Unreleased] +### Fixed +* Fix empty files with biome >= 1.5.0 when formatting files that are in the ignore list of the biome configuration file. ([#1989](https://github.com/diffplug/spotless/pull/1989) fixes [#1987](https://github.com/diffplug/spotless/issues/1987)) ### Changes * Use palantir-java-format 2.39.0 on Java 21. ([#1948](https://github.com/diffplug/spotless/pull/1948)) diff --git a/plugin-maven/CHANGES.md b/plugin-maven/CHANGES.md index 55f451ad4e..cea32ce89e 100644 --- a/plugin-maven/CHANGES.md +++ b/plugin-maven/CHANGES.md @@ -5,6 +5,8 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format ( ## [Unreleased] ### Added * M2E support: Emit file specific errors during incremental build. ([#1960](https://github.com/diffplug/spotless/issues/1960)) +### Fixed +* Fix empty files with biome >= 1.5.0 when formatting files that are in the ignore list of the biome configuration file. ([#1989](https://github.com/diffplug/spotless/pull/1989) fixes [#1987](https://github.com/diffplug/spotless/issues/1987)) ### Changes * Use palantir-java-format 2.39.0 on Java 21. ([#1948](https://github.com/diffplug/spotless/pull/1948)) From c431a705ab0745459d1be9f64e24722335c0f99a Mon Sep 17 00:00:00 2001 From: Andre Wachsmuth Date: Tue, 9 Jan 2024 22:04:04 +0100 Subject: [PATCH 3/4] Format code --- .../com/diffplug/spotless/rome/RomeStep.java | 5 ++--- .../gradle/spotless/BiomeIntegrationTest.java | 22 +++++++++---------- .../spotless/maven/biome/BiomeMavenTest.java | 2 +- .../diffplug/spotless/rome/RomeStepTest.java | 2 +- 4 files changed, 15 insertions(+), 16 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/rome/RomeStep.java b/lib/src/main/java/com/diffplug/spotless/rome/RomeStep.java index 92a0fdc689..0d0cca65a0 100644 --- a/lib/src/main/java/com/diffplug/spotless/rome/RomeStep.java +++ b/lib/src/main/java/com/diffplug/spotless/rome/RomeStep.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. @@ -440,8 +440,7 @@ private String format(ProcessRunner runner, String input, File file) throws IOEx // a list of hard-coded file names, such as package.json or tsconfig.json. if (formatted == null || formatted.isEmpty()) { return input; - } - else { + } else { return formatted; } } diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BiomeIntegrationTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BiomeIntegrationTest.java index 139d477bf0..db4ca4b027 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BiomeIntegrationTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/BiomeIntegrationTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2023 DiffPlug + * Copyright 2023-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -352,16 +352,16 @@ void failureWhenNotParseable() throws Exception { @Test void preservesIgnoredFiles() throws Exception { setFile("build.gradle").toLines( - "plugins {", - " id 'com.diffplug.spotless'", - "}", - "repositories { mavenCentral() }", - "spotless {", - " json {", - " target '**/*.json'", - " biome('1.5.0')", - " }", - "}"); + "plugins {", + " id 'com.diffplug.spotless'", + "}", + "repositories { mavenCentral() }", + "spotless {", + " json {", + " target '**/*.json'", + " biome('1.5.0')", + " }", + "}"); setFile("package.json").toResource("biome/json/packageBefore.json"); var spotlessApply = gradleRunner().withArguments("--stacktrace", "spotlessApply").build(); diff --git a/plugin-maven/src/test/java/com/diffplug/spotless/maven/biome/BiomeMavenTest.java b/plugin-maven/src/test/java/com/diffplug/spotless/maven/biome/BiomeMavenTest.java index ffbd73484f..ff763c3cf8 100644 --- a/plugin-maven/src/test/java/com/diffplug/spotless/maven/biome/BiomeMavenTest.java +++ b/plugin-maven/src/test/java/com/diffplug/spotless/maven/biome/BiomeMavenTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2023 DiffPlug + * Copyright 2023-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/testlib/src/test/java/com/diffplug/spotless/rome/RomeStepTest.java b/testlib/src/test/java/com/diffplug/spotless/rome/RomeStepTest.java index 7ab4112bef..8ab92e90e0 100644 --- a/testlib/src/test/java/com/diffplug/spotless/rome/RomeStepTest.java +++ b/testlib/src/test/java/com/diffplug/spotless/rome/RomeStepTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2023 DiffPlug + * Copyright 2023-2024 DiffPlug * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. From ac505ea898e9f7bd9213cbc5464a80745599e273 Mon Sep 17 00:00:00 2001 From: Andre Wachsmuth Date: Tue, 9 Jan 2024 22:14:02 +0100 Subject: [PATCH 4/4] Remove null check --- lib/src/main/java/com/diffplug/spotless/rome/RomeStep.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/rome/RomeStep.java b/lib/src/main/java/com/diffplug/spotless/rome/RomeStep.java index 0d0cca65a0..de5b3e80ee 100644 --- a/lib/src/main/java/com/diffplug/spotless/rome/RomeStep.java +++ b/lib/src/main/java/com/diffplug/spotless/rome/RomeStep.java @@ -431,14 +431,14 @@ private String format(ProcessRunner runner, String input, File file) throws IOEx } var runnerResult = runner.exec(stdin, args); var stdErr = runnerResult.stdErrUtf8(); - if (stdErr != null && !stdErr.isEmpty()) { + if (!stdErr.isEmpty()) { logger.warn("Biome stderr ouptut for file '{}'\n{}", file, stdErr.trim()); } var formatted = runnerResult.assertExitZero(StandardCharsets.UTF_8); // When biome encounters an ignored file, it does not output any formatted code // Ignored files come from (a) the biome.json configuration file and (b) from // a list of hard-coded file names, such as package.json or tsconfig.json. - if (formatted == null || formatted.isEmpty()) { + if (formatted.isEmpty()) { return input; } else { return formatted;