Respect .editorconfig settings for formatting shell via shfmt (#2031

)

.github/workflows/ci.yml

@@ -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
@@ -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

CHANGES.md

@@ -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))

lib/src/main/java/com/diffplug/spotless/shell/ShfmtStep.java

@@ -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;
 
@@ -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}')}" +
@@ -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;
 
@@ -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() {

plugin-gradle/CHANGES.md

@@ -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))

plugin-gradle/README.md

@@ -992,7 +992,7 @@ spotless {
 ```gradle
 spotless {
   shell {
-    target 'scripts/**/*.sh' // default: '*.sh'
+    target 'scripts/**/*.sh' // default: '**/*.sh'
 
     shfmt()  // has its own section below
   }
@@ -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

plugin-gradle/src/main/java/com/diffplug/gradle/spotless/ShellExtension.java

@@ -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";
 

plugin-gradle/src/test/java/com/diffplug/gradle/spotless/ShellExtensionTest.java

@@ -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");
 	}
 }

plugin-maven/CHANGES.md

@@ -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))

plugin-maven/README.md

@@ -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 -->

plugin-maven/src/main/java/com/diffplug/spotless/maven/shell/Shell.java

@@ -15,7 +15,6 @@
  */
 package com.diffplug.spotless.maven.shell;
 
-import java.util.Collections;
 import java.util.Set;
 
 import org.apache.maven.project.MavenProject;
@@ -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

plugin-maven/src/main/java/com/diffplug/spotless/maven/shell/Shfmt.java

@@ -23,7 +23,6 @@
 import com.diffplug.spotless.shell.ShfmtStep;
 
 public class Shfmt implements FormatterStepFactory {
-
 	@Parameter
 	private String version;
 

plugin-maven/src/test/java/com/diffplug/spotless/maven/shell/ShellTest.java

@@ -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");
 	}
 }

testlib/src/main/resources/shell/shfmt/with-config/.editorconfig

@@ -0,0 +1,10 @@
+root = true
+
+[*]
+charset = utf-8
+
+[*.sh]
+indent_style = space
+indent_size = 2
+space_redirects = true
+switch_case_indent = true

testlib/src/main/resources/shell/shfmt/with-config/other.clean

@@ -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

testlib/src/main/resources/shell/shfmt/with-config/other.sh

@@ -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

testlib/src/main/resources/shell/shfmt/without-config/other.clean

@@ -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