Skip to content

Commit

Permalink
Respect .editorconfig settings for formatting shell via shfmt (#2031
Browse files Browse the repository at this point in the history
)
  • Loading branch information
nedtwigg committed Feb 12, 2024
2 parents 47d2c0f + 6e52d6e commit f5bfb34
Show file tree
Hide file tree
Showing 22 changed files with 277 additions and 24 deletions.
12 changes: 12 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ jobs:
- kind: npm
jre: 11
os: ubuntu-latest
- kind: shfmt
jre: 11
os: ubuntu-latest
shfmt-version: 3.7.0
runs-on: ${{ matrix.os }}
steps:
- name: Checkout
Expand All @@ -79,6 +83,14 @@ jobs:
- name: test npm
if: matrix.kind == 'npm'
run: ./gradlew testNpm
- name: Install shfmt
if: matrix.kind == 'shfmt'
run: |
curl -sSfL "https://github.com/mvdan/sh/releases/download/v${{ matrix.shfmt-version }}/shfmt_v${{ matrix.shfmt-version }}_linux_amd64" -o /usr/local/bin/shfmt
chmod +x /usr/local/bin/shfmt
- name: Test shfmt
if: matrix.kind == 'shfmt'
run: ./gradlew testShfmt
- name: junit result
uses: mikepenz/action-junit-report@v4
if: always() # always run even if the previous step fails
Expand Down
2 changes: 2 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
## [Unreleased]
### Added
* `FileSignature.Promised` and `JarState.Promised` to facilitate round-trip serialization for the Gradle configuration cache. ([#1945](https://github.com/diffplug/spotless/pull/1945))
* Respect `.editorconfig` settings for formatting shell via `shfmt` ([#2031](https://github.com/diffplug/spotless/pull/2031))

### Removed
* **BREAKING** Remove `JarState.getMavenCoordinate(String prefix)`. ([#1945](https://github.com/diffplug/spotless/pull/1945))
* **BREAKING** Replace `PipeStepPair` with `FenceStep`. ([#1954](https://github.com/diffplug/spotless/pull/1954))
Expand Down
16 changes: 13 additions & 3 deletions lib/src/main/java/com/diffplug/spotless/shell/ShfmtStep.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import java.util.List;
import java.util.Objects;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import javax.annotation.Nullable;

Expand Down Expand Up @@ -72,7 +74,7 @@ private State createState() throws IOException, InterruptedException {
"\n github issue to handle this better: https://github.com/diffplug/spotless/issues/673";
final ForeignExe exe = ForeignExe.nameAndVersion("shfmt", version)
.pathToExe(pathToExe)
.versionRegex(Pattern.compile("(\\S*)"))
.versionRegex(Pattern.compile("([\\d.]+)"))
.fixCantFind(howToInstall)
.fixWrongVersion(
"You can tell Spotless to use the version you already have with {@code shfmt('{versionFound}')}" +
Expand All @@ -83,9 +85,11 @@ private State createState() throws IOException, InterruptedException {
@SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED")
static class State implements Serializable {
private static final long serialVersionUID = -1825662356883926318L;

// used for up-to-date checks and caching
final String version;
final transient ForeignExe exe;

// used for executing
private transient @Nullable List<String> args;

Expand All @@ -96,10 +100,16 @@ static class State implements Serializable {

String format(ProcessRunner runner, String input, File file) throws IOException, InterruptedException {
if (args == null) {
args = List.of(exe.confirmVersionAndGetAbsolutePath(), "-i", "2", "-ci");
// args will be reused during a single Spotless task execution,
// so this "prefix" is being "cached" for each Spotless format with shfmt.
args = List.of(exe.confirmVersionAndGetAbsolutePath(), "--filename");
}

return runner.exec(input.getBytes(StandardCharsets.UTF_8), args).assertExitZero(StandardCharsets.UTF_8);
// This will ensure that the next file name is retrieved on every format
final List<String> finalArgs = Stream.concat(args.stream(), Stream.of(file.getAbsolutePath()))
.collect(Collectors.toList());

return runner.exec(input.getBytes(StandardCharsets.UTF_8), finalArgs).assertExitZero(StandardCharsets.UTF_8);
}

FormatterFunc.Closeable toFunc() {
Expand Down
3 changes: 3 additions & 0 deletions plugin-gradle/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
### Fixed
* Ignore system git config when running tests ([#1990](https://github.com/diffplug/spotless/issues/1990))

### Added
* Respect `.editorconfig` settings for formatting shell via `shfmt` ([#2031](https://github.com/diffplug/spotless/pull/2031))

## [6.25.0] - 2024-01-23
### Added
* Maven / Gradle - Support for formatting Java Docs for the Palantir formatter ([#2009](https://github.com/diffplug/spotless/pull/2009))
Expand Down
6 changes: 5 additions & 1 deletion plugin-gradle/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -992,7 +992,7 @@ spotless {
```gradle
spotless {
shell {
target 'scripts/**/*.sh' // default: '*.sh'
target 'scripts/**/*.sh' // default: '**/*.sh'
shfmt() // has its own section below
}
Expand All @@ -1003,11 +1003,15 @@ spotless {
[homepage](https://github.com/mvdan/sh). [changelog](https://github.com/mvdan/sh/blob/master/CHANGELOG.md).
When formatting shell scripts via `shfmt`, configure `shfmt` settings via `.editorconfig`.
Refer to the `shfmt` [man page](https://github.com/mvdan/sh/blob/master/cmd/shfmt/shfmt.1.scd) for `.editorconfig` settings.
```gradle
shfmt('3.7.0') // version is optional
// if shfmt is not on your path, you must specify its location manually
shfmt().pathToExe('/opt/homebrew/bin/shfmt')
// Spotless always checks the version of the shfmt it is using
// and will fail with an error if it does not match the expected version
// (whether manually specified or default). If there is a problem, Spotless
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import com.diffplug.spotless.shell.ShfmtStep;

public class ShellExtension extends FormatExtension {
private static final String SHELL_FILE_EXTENSION = "*.sh";
private static final String SHELL_FILE_EXTENSION = "**/*.sh";

static final String NAME = "shell";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,52 @@

@ShfmtTest
public class ShellExtensionTest extends GradleIntegrationHarness {

@Test
void shfmt() throws IOException {
setFile("build.gradle").toLines(
void shfmtWithEditorconfig() throws IOException {
String fileDir = "shell/shfmt/with-config/";

setFile("build.gradle.kts").toLines(
"plugins {",
" id 'com.diffplug.spotless'",
" id(\"com.diffplug.spotless\")",
"}",
"spotless {",
" shell {",
" shfmt()",
" }",
"}");
setFile("shfmt.sh").toResource("shell/shfmt/shfmt.sh");

setFile(".editorconfig").toResource(fileDir + ".editorconfig");

setFile("shfmt.sh").toResource(fileDir + "shfmt.sh");
setFile("scripts/other.sh").toResource(fileDir + "other.sh");

gradleRunner().withArguments("spotlessApply").build();
assertFile("shfmt.sh").sameAsResource("shell/shfmt/shfmt.clean");

assertFile("shfmt.sh").sameAsResource(fileDir + "shfmt.clean");
assertFile("scripts/other.sh").sameAsResource(fileDir + "other.clean");
}

@Test
void shfmtWithoutEditorconfig() throws IOException {
String fileDir = "shell/shfmt/without-config/";

setFile("build.gradle.kts").toLines(
"plugins {",
" id(\"com.diffplug.spotless\")",
"}",
"spotless {",
" shell {",
" shfmt()",
" }",
"}");

setFile("shfmt.sh").toResource(fileDir + "shfmt.sh");
setFile("scripts/other.sh").toResource(fileDir + "other.sh");

gradleRunner().withArguments("spotlessApply").build();

assertFile("shfmt.sh").sameAsResource(fileDir + "shfmt.clean");
assertFile("scripts/other.sh").sameAsResource(fileDir + "other.clean");
}
}
3 changes: 3 additions & 0 deletions plugin-maven/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (
### Fixed
* Ignore system git config when running tests ([#1990](https://github.com/diffplug/spotless/issues/1990))

### Added
* Respect `.editorconfig` settings for formatting shell via `shfmt` ([#2031](https://github.com/diffplug/spotless/pull/2031))

## [2.43.0] - 2024-01-23
### Added
* Support for formatting shell scripts via [shfmt](https://github.com/mvdan/sh). ([#1998](https://github.com/diffplug/spotless/issues/1998))
Expand Down
31 changes: 30 additions & 1 deletion plugin-maven/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1021,11 +1021,40 @@ Uses Jackson and YAMLFactory to pretty print objects:
</jackson>
```

## Shell

- `com.diffplug.spotless.maven.FormatterFactory.addStepFactory(FormatterStepFactory)` [code](./src/main/java/com/diffplug/spotless/maven/shell/Shell.java)

```xml
<configuration>
<shell>
<includes> <!-- Not required. Defaults to **/*.sh -->
<include>scripts/**/*.sh</include>
</includes>

<shfmt /> <!-- has its own section below -->
</shell>
</configuration>
```

### shfmt

[homepage](https://github.com/mvdan/sh). [changelog](https://github.com/mvdan/sh/blob/master/CHANGELOG.md).

When formatting shell scripts via `shfmt`, configure `shfmt` settings via `.editorconfig`.

```xml
<shfmt>
<version>3.7.0</version> <!-- optional: Custom version of 'mvdan/sh' -->
<pathToExe>/opt/homebrew/bin/shfmt</pathToExe> <!-- optional: if shfmt is not on your path, you must specify its location manually -->
</shfmt>
```

## Gherkin

- `com.diffplug.spotless.maven.FormatterFactory.addStepFactory(FormatterStepFactory)` [code](https://github.com/diffplug/spotless/blob/main/plugin-maven/src/main/java/com/diffplug/spotless/maven/gherkin/Gherkin.java)

```gradle
```xml
<configuration>
<gherkin>
<includes> <!-- You have to set the target manually -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/
package com.diffplug.spotless.maven.shell;

import java.util.Collections;
import java.util.Set;

import org.apache.maven.project.MavenProject;
Expand All @@ -30,9 +29,11 @@
* and shell-specific (e.g. {@link Shfmt}) steps.
*/
public class Shell extends FormatterFactory {
private static final Set<String> DEFAULT_INCLUDES = Set.of("**/*.sh");

@Override
public Set<String> defaultIncludes(MavenProject project) {
return Collections.emptySet();
return DEFAULT_INCLUDES;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import com.diffplug.spotless.shell.ShfmtStep;

public class Shfmt implements FormatterStepFactory {

@Parameter
private String version;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,36 @@
package com.diffplug.spotless.maven.shell;

import org.junit.jupiter.api.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.diffplug.spotless.maven.MavenIntegrationHarness;
import com.diffplug.spotless.tag.ShfmtTest;

@ShfmtTest
public class ShellTest extends MavenIntegrationHarness {
private static final Logger LOGGER = LoggerFactory.getLogger(ShellTest.class);
@Test
public void testFormatShellWithEditorconfig() throws Exception {
String fileDir = "shell/shfmt/with-config/";
setFile("shfmt.sh").toResource(fileDir + "shfmt.sh");
setFile("scripts/other.sh").toResource(fileDir + "other.sh");
setFile(".editorconfig").toResource(fileDir + ".editorconfig");

writePomWithShellSteps("<shfmt/>");
mavenRunner().withArguments("spotless:apply").runNoError();

assertFile("shfmt.sh").sameAsResource(fileDir + "shfmt.clean");
assertFile("scripts/other.sh").sameAsResource(fileDir + "other.clean");
}

@Test
public void testFormatShell() throws Exception {
public void testFormatShellWithoutEditorconfig() throws Exception {
String fileDir = "shell/shfmt/without-config/";
setFile("shfmt.sh").toResource(fileDir + "shfmt.sh");
setFile("scripts/other.sh").toResource(fileDir + "other.sh");

writePomWithShellSteps("<shfmt/>");
setFile("shfmt.sh").toResource("shell/shfmt/shfmt.sh");
mavenRunner().withArguments("spotless:apply").runNoError();
assertFile("shfmt.sh").sameAsResource("shell/shfmt/shfmt.clean");

assertFile("shfmt.sh").sameAsResource(fileDir + "shfmt.clean");
assertFile("scripts/other.sh").sameAsResource(fileDir + "other.clean");
}
}
10 changes: 10 additions & 0 deletions testlib/src/main/resources/shell/shfmt/with-config/.editorconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
root = true

[*]
charset = utf-8

[*.sh]
indent_style = space
indent_size = 2
space_redirects = true
switch_case_indent = true
20 changes: 20 additions & 0 deletions testlib/src/main/resources/shell/shfmt/with-config/other.clean
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#!/usr/bin/env bash

fruit="apple"

case $fruit in
"apple")
echo "This is a red fruit."
;;
"banana")
echo "This is a yellow fruit."
;;
"orange")
echo "This is an orange fruit."
;;
*)
echo "Unknown fruit."
;;
esac

echo "This is some text." > output.txt
20 changes: 20 additions & 0 deletions testlib/src/main/resources/shell/shfmt/with-config/other.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#!/usr/bin/env bash

fruit="apple"

case $fruit in
"apple")
echo "This is a red fruit."
;;
"banana")
echo "This is a yellow fruit."
;;
"orange")
echo "This is an orange fruit."
;;
*)
echo "Unknown fruit."
;;
esac

echo "This is some text." > output.txt
20 changes: 20 additions & 0 deletions testlib/src/main/resources/shell/shfmt/without-config/other.clean
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#!/usr/bin/env bash

fruit="apple"

case $fruit in
"apple")
echo "This is a red fruit."
;;
"banana")
echo "This is a yellow fruit."
;;
"orange")
echo "This is an orange fruit."
;;
*)
echo "Unknown fruit."
;;
esac

echo "This is some text." >output.txt

0 comments on commit f5bfb34

Please sign in to comment.