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

Fixes #1987, no empty files with biome when formatting ignored files #1989

Merged
merged 5 commits into from
Jan 15, 2024
Merged
Changes from 1 commit
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
Next Next commit
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.
awa-xima committed Jan 9, 2024
commit d73d05e5e0b428939880fcca16a9e267d8c70fbc
2 changes: 2 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -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))

20 changes: 20 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -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
18 changes: 16 additions & 2 deletions lib/src/main/java/com/diffplug/spotless/rome/RomeStep.java
Original file line number Diff line number Diff line change
@@ -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);
}

/**
23 changes: 18 additions & 5 deletions plugin-gradle/README.md
Original file line number Diff line number Diff line change
@@ -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,
Original file line number Diff line number Diff line change
@@ -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");
}
}
20 changes: 16 additions & 4 deletions plugin-maven/README.md
Original file line number Diff line number Diff line change
@@ -1300,10 +1300,6 @@ usually you will be creating a generic format.
</configuration>
```

**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:
</configuration>
```

**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,
Original file line number Diff line number Diff line change
@@ -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", "<biome><version>1.5.0</version></biome>");
setFile("package.json").toResource("biome/json/packageBefore.json");
mavenRunner().withArguments("spotless:apply").runNoError();
assertFile("package.json").sameAsResource("biome/json/packageAfter.json");
}
}
3 changes: 3 additions & 0 deletions testlib/src/main/resources/biome/json/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"name": "test-package","version": "0.0.1"
}
3 changes: 3 additions & 0 deletions testlib/src/main/resources/biome/json/packageAfter.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"name": "test-package","version": "0.0.1"
}
3 changes: 3 additions & 0 deletions testlib/src/main/resources/biome/json/packageBefore.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"name": "test-package","version": "0.0.1"
}
13 changes: 13 additions & 0 deletions testlib/src/test/java/com/diffplug/spotless/rome/RomeStepTest.java
Original file line number Diff line number Diff line change
@@ -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