From 39ee6a2efda73fb89ee6019f4ec356e8a3bb6db5 Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Thu, 24 Oct 2024 06:59:26 -0700 Subject: [PATCH 01/14] Run Windows tests before tagging a release The GitHub workflows enabled Windows in the testing matrix. We needed to port the pre-build commands that apply the release commits to Windows to make the Windows checks pass. --- .github/workflows/create-release-commits.sh | 28 ------ .github/workflows/publish_release.yml | 99 +++++++++++++++++---- 2 files changed, 81 insertions(+), 46 deletions(-) delete mode 100755 .github/workflows/create-release-commits.sh diff --git a/.github/workflows/create-release-commits.sh b/.github/workflows/create-release-commits.sh deleted file mode 100755 index 81fa8eba..00000000 --- a/.github/workflows/create-release-commits.sh +++ /dev/null @@ -1,28 +0,0 @@ -#!/usr/bin/env bash - -set -ex - -SWIFT_SYNTAX_TAG="$1" -SWIFT_FORMAT_VERSION="$2" - -if [[ -z "$SWIFT_SYNTAX_TAG" || -z "$SWIFT_FORMAT_VERSION" ]]; then - echo "Update the Package manifest to reference a specific version of swift-syntax and embed the given version in the swift-format --version command" - echo "Usage create-release-commits.sh " - echo " SWIFT_SYNTAX_TAG: The tag of swift-syntax to depend on" - echo " SWIFT_FORMAT_VERSION: The version of swift-format that is about to be released" - exit 1 -fi - -# Without this, we can't perform git operations in GitHub actions. -git config --global --add safe.directory "$(realpath .)" - -git config --local user.name 'swift-ci' -git config --local user.email 'swift-ci@users.noreply.github.com' - -sed -E -i "s#branch: \"(main|release/[0-9]+\.[0-9]+)\"#from: \"$SWIFT_SYNTAX_TAG\"#" Package.swift -git add Package.swift -git commit -m "Change swift-syntax dependency to $SWIFT_SYNTAX_TAG" - -sed -E -i "s#print\(\".*\"\)#print\(\"$SWIFT_FORMAT_VERSION\"\)#" Sources/swift-format/PrintVersion.swift -git add Sources/swift-format/PrintVersion.swift -git commit -m "Change version to $SWIFT_FORMAT_VERSION" diff --git a/.github/workflows/publish_release.yml b/.github/workflows/publish_release.yml index 2528ebf6..2081696e 100644 --- a/.github/workflows/publish_release.yml +++ b/.github/workflows/publish_release.yml @@ -34,12 +34,12 @@ jobs: echo "${{ github.triggering_actor }} is not allowed to create a release" exit 1 fi - define_tags: - name: Determine dependent swift-syntax version and prerelease date + create_release_commits: + name: Create release commits runs-on: ubuntu-latest outputs: - swift_syntax_tag: ${{ steps.swift_syntax_tag.outputs.swift_syntax_tag }} swift_format_version: ${{ steps.swift_format_version.outputs.swift_format_version }} + release_commit_patch: ${{ steps.create_release_commits.outputs.release_commit_patch }} steps: - name: Determine swift-syntax tag to depend on id: swift_syntax_tag @@ -65,37 +65,100 @@ jobs: fi echo "Using swift-format version: $SWIFT_FORMAT_VERSION" echo "swift_format_version=$SWIFT_FORMAT_VERSION" >> "$GITHUB_OUTPUT" + - name: Checkout repository + uses: actions/checkout@v4 + - name: Create release commits + id: create_release_commits + run: | + # Without this, we can't perform git operations in GitHub actions. + git config --global --add safe.directory "$(realpath .)" + git config --local user.name 'swift-ci' + git config --local user.email 'swift-ci@users.noreply.github.com' + + BASE_COMMIT=$(git rev-parse HEAD) + + sed -E -i "s#branch: \"(main|release/[0-9]+\.[0-9]+)\"#from: \"${{ steps.swift_syntax_tag.outputs.swift_syntax_tag }}\"#" Package.swift + git add Package.swift + git commit -m "Change swift-syntax dependency to ${{ steps.swift_syntax_tag.outputs.swift_syntax_tag }}" + + sed -E -i "s#print\(\".*\"\)#print\(\"${{ steps.swift_format_version.outputs.swift_format_version }}\"\)#" Sources/swift-format/PrintVersion.swift + git add Sources/swift-format/PrintVersion.swift + git commit -m "Change version to ${{ steps.swift_format_version.outputs.swift_format_version }}" + + { + echo 'release_commit_patch<> "$GITHUB_OUTPUT" test_debug: name: Test in Debug configuration - uses: swiftlang/github-workflows/.github/workflows/swift_package_test.yml@main - needs: define_tags + uses: swiftlang/github-workflows/.github/workflows/swift_package_test.yml@windows-pre-build + needs: create_release_commits with: - pre_build_command: bash .github/workflows/create-release-commits.sh '${{ needs.define_tags.outputs.swift_syntax_tag }}' '${{ needs.define_tags.outputs.swift_format_version }}' + linux_pre_build_command: | + git config --global --add safe.directory "$(realpath .)" + git config --local user.name 'swift-ci' + git config --local user.email 'swift-ci@users.noreply.github.com' + git am << EOF + ${{ needs.create_release_commits.outputs.release_commit_patch }} + EOF + windows_pre_build_command: | + git config --local user.name "swift-ci" + git config --local user.email "swift-ci@users.noreply.github.com" + echo @" + ${{ needs.create_release_commits.outputs.release_commit_patch }} + "@ > $env:TEMP\patch.diff + # For some reason `git am` fails in Powershell with the following error. Executing it in cmd works... + # fatal: empty ident name (for <>) not allowed + cmd /c "type $env:TEMP\patch.diff | git am || (exit /b 1)" # We require that releases of swift-format build without warnings - build_command: swift test -Xswiftc -warnings-as-errors + linux_build_command: swift test -Xswiftc -warnings-as-errors + windows_build_command: swift test -Xswiftc -warnings-as-errors test_release: name: Test in Release configuration - uses: swiftlang/github-workflows/.github/workflows/swift_package_test.yml@main - needs: define_tags + uses: swiftlang/github-workflows/.github/workflows/swift_package_test.yml@windows-pre-build + needs: create_release_commits with: - pre_build_command: bash .github/workflows/create-release-commits.sh '${{ needs.define_tags.outputs.swift_syntax_tag }}' '${{ needs.define_tags.outputs.swift_format_version }}' + linux_pre_build_command: | + git config --global --add safe.directory "$(realpath .)" + git config --local user.name 'swift-ci' + git config --local user.email 'swift-ci@users.noreply.github.com' + git am << EOF + ${{ needs.create_release_commits.outputs.release_commit_patch }} + EOF + windows_pre_build_command: | + git config --local user.name "swift-ci" + git config --local user.email "swift-ci@users.noreply.github.com" + echo @" + ${{ needs.create_release_commits.outputs.release_commit_patch }} + "@ > $env:TEMP\patch.diff + # For some reason `git am` fails in Powershell with the following error. Executing it in cmd works... + # fatal: empty ident name (for <>) not allowed + cmd /c "type $env:TEMP\patch.diff | git am || (exit /b 1)" # We require that releases of swift-format build without warnings - build_command: swift test -c release -Xswiftc -warnings-as-errors + linux_build_command: swift test -Xswiftc -warnings-as-errors + windows_build_command: swift test -Xswiftc -warnings-as-errors create_tag: name: Create Tag runs-on: ubuntu-latest - needs: [check_triggering_actor, test_debug, test_release, define_tags] + needs: [check_triggering_actor, test_debug, create_release_commits] permissions: contents: write steps: - name: Checkout repository uses: actions/checkout@v4 - - name: Create release commits - run: bash .github/workflows/create-release-commits.sh '${{ needs.define_tags.outputs.swift_syntax_tag }}' '${{ needs.define_tags.outputs.swift_format_version }}' + - name: Apply release commits + run: | + git config --global --add safe.directory "$(realpath .)" + git config --local user.name 'swift-ci' + git config --local user.email 'swift-ci@users.noreply.github.com' + git am << EOF + ${{ needs.create_release_commits.outputs.release_commit_patch }} + EOF - name: Tag release run: | - git tag "${{ needs.define_tags.outputs.swift_format_version }}" - git push origin "${{ needs.define_tags.outputs.swift_format_version }}" + git tag "${{ needs.create_release_commits.outputs.swift_format_version }}" + git push origin "${{ needs.create_release_commits.outputs.swift_format_version }}" - name: Create release env: GH_TOKEN: ${{ github.token }} @@ -104,6 +167,6 @@ jobs: # Only create a release automatically for prereleases. For real releases, release notes should be crafted by hand. exit fi - gh release create "${{ needs.define_tags.outputs.swift_format_version }}" \ - --title "${{ needs.define_tags.outputs.swift_format_version }}" \ + gh release create "${{ needs.create_release_commits.outputs.swift_format_version }}" \ + --title "${{ needs.create_release_commits.outputs.swift_format_version }}" \ --prerelease From ce212cab75d84524c66757d597b165a7d353c36d Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Thu, 24 Oct 2024 08:42:44 -0700 Subject: [PATCH 02/14] Use matrix to run debug and release --- .github/workflows/publish_release.yml | 40 +++++++-------------------- 1 file changed, 10 insertions(+), 30 deletions(-) diff --git a/.github/workflows/publish_release.yml b/.github/workflows/publish_release.yml index 2081696e..03c4f04f 100644 --- a/.github/workflows/publish_release.yml +++ b/.github/workflows/publish_release.yml @@ -90,10 +90,14 @@ jobs: git format-patch "$BASE_COMMIT"..HEAD --stdout echo EOF } >> "$GITHUB_OUTPUT" - test_debug: - name: Test in Debug configuration - uses: swiftlang/github-workflows/.github/workflows/swift_package_test.yml@windows-pre-build + test: + name: Test in ${{ matrix.release && 'Release' || 'Debug' }} configuration + uses: swiftlang/github-workflows/.github/workflows/swift_package_test.yml@main needs: create_release_commits + strategy: + fail-fast: false + matrix: + release: [true, false] with: linux_pre_build_command: | git config --global --add safe.directory "$(realpath .)" @@ -112,36 +116,12 @@ jobs: # fatal: empty ident name (for <>) not allowed cmd /c "type $env:TEMP\patch.diff | git am || (exit /b 1)" # We require that releases of swift-format build without warnings - linux_build_command: swift test -Xswiftc -warnings-as-errors - windows_build_command: swift test -Xswiftc -warnings-as-errors - test_release: - name: Test in Release configuration - uses: swiftlang/github-workflows/.github/workflows/swift_package_test.yml@windows-pre-build - needs: create_release_commits - with: - linux_pre_build_command: | - git config --global --add safe.directory "$(realpath .)" - git config --local user.name 'swift-ci' - git config --local user.email 'swift-ci@users.noreply.github.com' - git am << EOF - ${{ needs.create_release_commits.outputs.release_commit_patch }} - EOF - windows_pre_build_command: | - git config --local user.name "swift-ci" - git config --local user.email "swift-ci@users.noreply.github.com" - echo @" - ${{ needs.create_release_commits.outputs.release_commit_patch }} - "@ > $env:TEMP\patch.diff - # For some reason `git am` fails in Powershell with the following error. Executing it in cmd works... - # fatal: empty ident name (for <>) not allowed - cmd /c "type $env:TEMP\patch.diff | git am || (exit /b 1)" - # We require that releases of swift-format build without warnings - linux_build_command: swift test -Xswiftc -warnings-as-errors - windows_build_command: swift test -Xswiftc -warnings-as-errors + linux_build_command: swift test -Xswiftc -warnings-as-errors ${{ matrix.release && '-c release' }} + windows_build_command: swift test -Xswiftc -warnings-as-errors ${{ matrix.release && '-c release' }} create_tag: name: Create Tag runs-on: ubuntu-latest - needs: [check_triggering_actor, test_debug, create_release_commits] + needs: [check_triggering_actor, test, create_release_commits] permissions: contents: write steps: From c3b0c9f1bf24a0a6ad587999a450d506e2a3405b Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Fri, 25 Oct 2024 12:59:59 -0700 Subject: [PATCH 03/14] Prepare for integer generics with new generic argument node --- .../Rules/UseShorthandTypeNames.swift | 37 ++++++++++++------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/Sources/SwiftFormat/Rules/UseShorthandTypeNames.swift b/Sources/SwiftFormat/Rules/UseShorthandTypeNames.swift index dc101a01..67a8d8aa 100644 --- a/Sources/SwiftFormat/Rules/UseShorthandTypeNames.swift +++ b/Sources/SwiftFormat/Rules/UseShorthandTypeNames.swift @@ -47,24 +47,28 @@ public final class UseShorthandTypeNames: SyntaxFormatRule { switch node.name.text { case "Array": - guard let typeArgument = genericArgumentList.firstAndOnly else { + guard let argument = genericArgumentList.firstAndOnly, + case .type(let typeArgument) = argument else { newNode = nil break } + newNode = shorthandArrayType( - element: typeArgument.argument, + element: typeArgument, leadingTrivia: leadingTrivia, trailingTrivia: trailingTrivia ) case "Dictionary": - guard let typeArguments = exactlyTwoChildren(of: genericArgumentList) else { + guard let arguments = exactlyTwoChildren(of: genericArgumentList), + case .type(let type0Argument) = arguments.0.argument, + caes .type(let type1Argument) = arguments.1.argument else { newNode = nil break } newNode = shorthandDictionaryType( - key: typeArguments.0.argument, - value: typeArguments.1.argument, + key: type0Argument, + value: type1Argument, leadingTrivia: leadingTrivia, trailingTrivia: trailingTrivia ) @@ -74,12 +78,13 @@ public final class UseShorthandTypeNames: SyntaxFormatRule { newNode = nil break } - guard let typeArgument = genericArgumentList.firstAndOnly else { + guard let argument = genericArgumentList.firstAndOnly, + case .type(let typeArgument) = argument else { newNode = nil break } newNode = shorthandOptionalType( - wrapping: typeArgument.argument, + wrapping: typeArgument, leadingTrivia: leadingTrivia, trailingTrivia: trailingTrivia ) @@ -137,25 +142,28 @@ public final class UseShorthandTypeNames: SyntaxFormatRule { switch expression.baseName.text { case "Array": - guard let typeArgument = genericArgumentList.firstAndOnly else { + guard let argument = genericArgumentList.firstAndOnly, + case .type(let typeArgument) = argument else { newNode = nil break } let arrayTypeExpr = makeArrayTypeExpression( - elementType: typeArgument.argument, + elementType: typeArgument, leftSquare: TokenSyntax.leftSquareToken(leadingTrivia: leadingTrivia), rightSquare: TokenSyntax.rightSquareToken(trailingTrivia: trailingTrivia) ) newNode = ExprSyntax(arrayTypeExpr) case "Dictionary": - guard let typeArguments = exactlyTwoChildren(of: genericArgumentList) else { + guard let arguments = exactlyTwoChildren(of: genericArgumentList), + case .type(let type0Argument) = arguments.0.argument, + case .type(let type1Argument) = arguments.1.argument else { newNode = nil break } let dictTypeExpr = makeDictionaryTypeExpression( - keyType: typeArguments.0.argument, - valueType: typeArguments.1.argument, + keyType: type0Argument, + valueType: type1Argument, leftSquare: TokenSyntax.leftSquareToken(leadingTrivia: leadingTrivia), colon: TokenSyntax.colonToken(trailingTrivia: .spaces(1)), rightSquare: TokenSyntax.rightSquareToken(trailingTrivia: trailingTrivia) @@ -163,12 +171,13 @@ public final class UseShorthandTypeNames: SyntaxFormatRule { newNode = ExprSyntax(dictTypeExpr) case "Optional": - guard let typeArgument = genericArgumentList.firstAndOnly else { + guard let argument = genericArgumentList.firstAndOnly, + case .type(let typeArgument) = argument else { newNode = nil break } let optionalTypeExpr = makeOptionalTypeExpression( - wrapping: typeArgument.argument, + wrapping: typeArgument, leadingTrivia: leadingTrivia, questionMark: TokenSyntax.postfixQuestionMarkToken(trailingTrivia: trailingTrivia) ) From dfb366a022a222fbca84591af8486669f460afa4 Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Fri, 25 Oct 2024 14:02:19 -0700 Subject: [PATCH 04/14] Fix formatting --- .../Rules/UseShorthandTypeNames.swift | 23 +++++++------------ 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/Sources/SwiftFormat/Rules/UseShorthandTypeNames.swift b/Sources/SwiftFormat/Rules/UseShorthandTypeNames.swift index 67a8d8aa..d24b32b5 100644 --- a/Sources/SwiftFormat/Rules/UseShorthandTypeNames.swift +++ b/Sources/SwiftFormat/Rules/UseShorthandTypeNames.swift @@ -47,12 +47,10 @@ public final class UseShorthandTypeNames: SyntaxFormatRule { switch node.name.text { case "Array": - guard let argument = genericArgumentList.firstAndOnly, - case .type(let typeArgument) = argument else { + guard case .type(let typeArgument) = genericArgumentList.firstAndOnly else { newNode = nil break } - newNode = shorthandArrayType( element: typeArgument, leadingTrivia: leadingTrivia, @@ -60,9 +58,8 @@ public final class UseShorthandTypeNames: SyntaxFormatRule { ) case "Dictionary": - guard let arguments = exactlyTwoChildren(of: genericArgumentList), - case .type(let type0Argument) = arguments.0.argument, - caes .type(let type1Argument) = arguments.1.argument else { + guard case (.type(let type0Argument), .type(let type1Argument)) = exactlyTwoChildren(of: genericArgumentList) + else { newNode = nil break } @@ -78,8 +75,7 @@ public final class UseShorthandTypeNames: SyntaxFormatRule { newNode = nil break } - guard let argument = genericArgumentList.firstAndOnly, - case .type(let typeArgument) = argument else { + guard case .type(let typeArgument) = genericArgumentList.firstAndOnly else { newNode = nil break } @@ -142,8 +138,7 @@ public final class UseShorthandTypeNames: SyntaxFormatRule { switch expression.baseName.text { case "Array": - guard let argument = genericArgumentList.firstAndOnly, - case .type(let typeArgument) = argument else { + guard case .type(let typeArgument) = genericArgumentList.firstAndOnly else { newNode = nil break } @@ -155,9 +150,8 @@ public final class UseShorthandTypeNames: SyntaxFormatRule { newNode = ExprSyntax(arrayTypeExpr) case "Dictionary": - guard let arguments = exactlyTwoChildren(of: genericArgumentList), - case .type(let type0Argument) = arguments.0.argument, - case .type(let type1Argument) = arguments.1.argument else { + guard case (.type(let type0Argument), .type(let type1Argument)) = exactlyTwoChildren(of: genericArgumentList) + else { newNode = nil break } @@ -171,8 +165,7 @@ public final class UseShorthandTypeNames: SyntaxFormatRule { newNode = ExprSyntax(dictTypeExpr) case "Optional": - guard let argument = genericArgumentList.firstAndOnly, - case .type(let typeArgument) = argument else { + guard case .type(let typeArgument) = genericArgumentList.firstAndOnly else { newNode = nil break } From 9cbc942f5907c569bfd29564376db184a7ef9953 Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Mon, 28 Oct 2024 13:43:39 -0700 Subject: [PATCH 05/14] Update UseShorthandTypeNames.swift --- .../SwiftFormat/Rules/UseShorthandTypeNames.swift | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/Sources/SwiftFormat/Rules/UseShorthandTypeNames.swift b/Sources/SwiftFormat/Rules/UseShorthandTypeNames.swift index d24b32b5..35ff8450 100644 --- a/Sources/SwiftFormat/Rules/UseShorthandTypeNames.swift +++ b/Sources/SwiftFormat/Rules/UseShorthandTypeNames.swift @@ -47,7 +47,7 @@ public final class UseShorthandTypeNames: SyntaxFormatRule { switch node.name.text { case "Array": - guard case .type(let typeArgument) = genericArgumentList.firstAndOnly else { + guard case .type(let typeArgument) = genericArgumentList.firstAndOnly.argument else { newNode = nil break } @@ -58,7 +58,8 @@ public final class UseShorthandTypeNames: SyntaxFormatRule { ) case "Dictionary": - guard case (.type(let type0Argument), .type(let type1Argument)) = exactlyTwoChildren(of: genericArgumentList) + guard let arguments = exactlyTwoChildren(of: genericArgumentList), + case (.type(let type0Argument), .type(let type1Argument)) = (arguments.0.argument, arguments.1.argument) else { newNode = nil break @@ -75,7 +76,7 @@ public final class UseShorthandTypeNames: SyntaxFormatRule { newNode = nil break } - guard case .type(let typeArgument) = genericArgumentList.firstAndOnly else { + guard case .type(let typeArgument) = genericArgumentList.firstAndOnly.argument else { newNode = nil break } @@ -138,7 +139,7 @@ public final class UseShorthandTypeNames: SyntaxFormatRule { switch expression.baseName.text { case "Array": - guard case .type(let typeArgument) = genericArgumentList.firstAndOnly else { + guard case .type(let typeArgument) = genericArgumentList.firstAndOnly.argument else { newNode = nil break } @@ -150,7 +151,8 @@ public final class UseShorthandTypeNames: SyntaxFormatRule { newNode = ExprSyntax(arrayTypeExpr) case "Dictionary": - guard case (.type(let type0Argument), .type(let type1Argument)) = exactlyTwoChildren(of: genericArgumentList) + guard let arguments = exactlyTwoChildren(of: genericArgumentList), + case (.type(let type0Argument), .type(let type1Argument)) = (arguments.0.argument, arguments.1.argument) else { newNode = nil break @@ -165,7 +167,7 @@ public final class UseShorthandTypeNames: SyntaxFormatRule { newNode = ExprSyntax(dictTypeExpr) case "Optional": - guard case .type(let typeArgument) = genericArgumentList.firstAndOnly else { + guard case .type(let typeArgument) = genericArgumentList.firstAndOnly.argument else { newNode = nil break } From e6aa9ec987dc088746fcbcaa3ebd56764c32c2c9 Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Mon, 4 Nov 2024 12:24:07 -0800 Subject: [PATCH 06/14] Update UseShorthandTypeNames.swift --- Sources/SwiftFormat/Rules/UseShorthandTypeNames.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Sources/SwiftFormat/Rules/UseShorthandTypeNames.swift b/Sources/SwiftFormat/Rules/UseShorthandTypeNames.swift index 35ff8450..28d048fa 100644 --- a/Sources/SwiftFormat/Rules/UseShorthandTypeNames.swift +++ b/Sources/SwiftFormat/Rules/UseShorthandTypeNames.swift @@ -47,7 +47,7 @@ public final class UseShorthandTypeNames: SyntaxFormatRule { switch node.name.text { case "Array": - guard case .type(let typeArgument) = genericArgumentList.firstAndOnly.argument else { + guard case .type(let typeArgument) = genericArgumentList.firstAndOnly?.argument else { newNode = nil break } @@ -76,7 +76,7 @@ public final class UseShorthandTypeNames: SyntaxFormatRule { newNode = nil break } - guard case .type(let typeArgument) = genericArgumentList.firstAndOnly.argument else { + guard case .type(let typeArgument) = genericArgumentList.firstAndOnly?.argument else { newNode = nil break } @@ -139,7 +139,7 @@ public final class UseShorthandTypeNames: SyntaxFormatRule { switch expression.baseName.text { case "Array": - guard case .type(let typeArgument) = genericArgumentList.firstAndOnly.argument else { + guard case .type(let typeArgument) = genericArgumentList.firstAndOnly?.argument else { newNode = nil break } @@ -167,7 +167,7 @@ public final class UseShorthandTypeNames: SyntaxFormatRule { newNode = ExprSyntax(dictTypeExpr) case "Optional": - guard case .type(let typeArgument) = genericArgumentList.firstAndOnly.argument else { + guard case .type(let typeArgument) = genericArgumentList.firstAndOnly?.argument else { newNode = nil break } From 211884fde1c0eea428795bd5477aebb00744744b Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Wed, 30 Oct 2024 18:21:41 -0700 Subject: [PATCH 07/14] Fix tests when building swift-format using Swift 6.0.2 When traversing the file URL with Foundation from Swift 6.0.2, you get the following components - `["/", "C:", "test.swift"]` - `["/", "C:"]` - `[]` The component count never reaches 1. Foundation from Swift 6.1 goes - `["/", "C:", "test.swift"]` - `["/", "C:"]` - `["/"]` Cover both cases by checking for `<= 1` instead of `== 1` --- Sources/SwiftFormat/API/Configuration.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/SwiftFormat/API/Configuration.swift b/Sources/SwiftFormat/API/Configuration.swift index 7cc0d385..2586024a 100644 --- a/Sources/SwiftFormat/API/Configuration.swift +++ b/Sources/SwiftFormat/API/Configuration.swift @@ -478,7 +478,7 @@ fileprivate extension URL { #if os(Windows) // FIXME: We should call into Windows' native check to check if this path is a root once https://github.com/swiftlang/swift-foundation/issues/976 is fixed. // https://github.com/swiftlang/swift-format/issues/844 - return self.pathComponents.count == 1 + return self.pathComponents.count <= 1 #else // On Linux, we may end up with an string for the path due to https://github.com/swiftlang/swift-foundation/issues/980 // TODO: Remove the check for "" once https://github.com/swiftlang/swift-foundation/issues/980 is fixed. From 5e4caa81d0dcd1c60a9c44bb3e08b02ef11ee546 Mon Sep 17 00:00:00 2001 From: danthorpe Date: Wed, 10 Apr 2024 14:05:01 +0100 Subject: [PATCH 08/14] feat: add pre-commit hooks --- .pre-commit-hooks.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .pre-commit-hooks.yaml diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml new file mode 100644 index 00000000..f7fd11fa --- /dev/null +++ b/.pre-commit-hooks.yaml @@ -0,0 +1,6 @@ +- id: swift-format + name: swift-format + entry: swift-format format --in-place --recursive --parallel + language: swift + types: [swift] + require_serial: true From fee42c92b1c1502f80e7494ae3af9cf26f708d23 Mon Sep 17 00:00:00 2001 From: Tony Allevato Date: Fri, 8 Nov 2024 09:47:49 -0500 Subject: [PATCH 09/14] Add `--enable-experimental-feature` to enable those features in the parser. Also add a couple small tests for value generics to exercise the capability in tests. Fixes #875. --- .../SwiftFormat/API/SwiftFormatError.swift | 22 ++++++++- Sources/SwiftFormat/API/SwiftFormatter.swift | 6 +++ Sources/SwiftFormat/API/SwiftLinter.swift | 6 +++ Sources/SwiftFormat/Core/Parsing.swift | 24 ++++++++-- Sources/_SwiftFormatTestSupport/Parsing.swift | 37 +++++++++++++++ .../Frontend/FormatFrontend.swift | 13 ++---- .../swift-format/Frontend/LintFrontend.swift | 16 ++----- .../Subcommands/LintFormatOptions.swift | 9 ++++ .../PrettyPrint/PrettyPrintTestCase.swift | 14 +++++- .../PrettyPrint/ValueGenericsTests.swift | 46 +++++++++++++++++++ .../Rules/LintOrFormatRuleTestCase.swift | 14 ++++-- 11 files changed, 176 insertions(+), 31 deletions(-) create mode 100644 Sources/_SwiftFormatTestSupport/Parsing.swift create mode 100644 Tests/SwiftFormatTests/PrettyPrint/ValueGenericsTests.swift diff --git a/Sources/SwiftFormat/API/SwiftFormatError.swift b/Sources/SwiftFormat/API/SwiftFormatError.swift index 2e3a890c..6e418316 100644 --- a/Sources/SwiftFormat/API/SwiftFormatError.swift +++ b/Sources/SwiftFormat/API/SwiftFormatError.swift @@ -10,10 +10,11 @@ // //===----------------------------------------------------------------------===// +import Foundation import SwiftSyntax /// Errors that can be thrown by the `SwiftFormatter` and `SwiftLinter` APIs. -public enum SwiftFormatError: Error { +public enum SwiftFormatError: LocalizedError { /// The requested file was not readable or it did not exist. case fileNotReadable @@ -23,4 +24,23 @@ public enum SwiftFormatError: Error { /// The file contains invalid or unrecognized Swift syntax and cannot be handled safely. case fileContainsInvalidSyntax + + /// The requested experimental feature name was not recognized by the parser. + case unrecognizedExperimentalFeature(String) + + public var errorDescription: String? { + switch self { + case .fileNotReadable: + return "file is not readable or does not exist" + case .isDirectory: + return "requested path is a directory, not a file" + case .fileContainsInvalidSyntax: + return "file contains invalid Swift syntax" + case .unrecognizedExperimentalFeature(let name): + return "experimental feature '\(name)' was not recognized by the Swift parser" + } + } } + +extension SwiftFormatError: Equatable {} +extension SwiftFormatError: Hashable {} diff --git a/Sources/SwiftFormat/API/SwiftFormatter.swift b/Sources/SwiftFormat/API/SwiftFormatter.swift index 0daeb22c..ddd0bbe2 100644 --- a/Sources/SwiftFormat/API/SwiftFormatter.swift +++ b/Sources/SwiftFormat/API/SwiftFormatter.swift @@ -89,6 +89,10 @@ public final class SwiftFormatter { /// which is associated with any diagnostics emitted during formatting. If this is nil, a /// dummy value will be used. /// - selection: The ranges to format + /// - experimentalFeatures: The set of experimental features that should be enabled in the + /// parser. These names must be from the set of parser-recognized experimental language + /// features in `SwiftParser`'s `Parser.ExperimentalFeatures` enum, which match the spelling + /// defined in the compiler's `Features.def` file. /// - outputStream: A value conforming to `TextOutputStream` to which the formatted output will /// be written. /// - parsingDiagnosticHandler: An optional callback that will be notified if there are any @@ -98,6 +102,7 @@ public final class SwiftFormatter { source: String, assumingFileURL url: URL?, selection: Selection, + experimentalFeatures: Set = [], to outputStream: inout Output, parsingDiagnosticHandler: ((Diagnostic, SourceLocation) -> Void)? = nil ) throws { @@ -110,6 +115,7 @@ public final class SwiftFormatter { source: source, operatorTable: .standardOperators, assumingFileURL: url, + experimentalFeatures: experimentalFeatures, parsingDiagnosticHandler: parsingDiagnosticHandler ) try format( diff --git a/Sources/SwiftFormat/API/SwiftLinter.swift b/Sources/SwiftFormat/API/SwiftLinter.swift index 25cfa2af..cfc4639f 100644 --- a/Sources/SwiftFormat/API/SwiftLinter.swift +++ b/Sources/SwiftFormat/API/SwiftLinter.swift @@ -81,12 +81,17 @@ public final class SwiftLinter { /// - Parameters: /// - source: The Swift source code to be linted. /// - url: A file URL denoting the filename/path that should be assumed for this source code. + /// - experimentalFeatures: The set of experimental features that should be enabled in the + /// parser. These names must be from the set of parser-recognized experimental language + /// features in `SwiftParser`'s `Parser.ExperimentalFeatures` enum, which match the spelling + /// defined in the compiler's `Features.def` file. /// - parsingDiagnosticHandler: An optional callback that will be notified if there are any /// errors when parsing the source code. /// - Throws: If an unrecoverable error occurs when formatting the code. public func lint( source: String, assumingFileURL url: URL, + experimentalFeatures: Set = [], parsingDiagnosticHandler: ((Diagnostic, SourceLocation) -> Void)? = nil ) throws { // If the file or input string is completely empty, do nothing. This prevents even a trailing @@ -98,6 +103,7 @@ public final class SwiftLinter { source: source, operatorTable: .standardOperators, assumingFileURL: url, + experimentalFeatures: experimentalFeatures, parsingDiagnosticHandler: parsingDiagnosticHandler ) try lint( diff --git a/Sources/SwiftFormat/Core/Parsing.swift b/Sources/SwiftFormat/Core/Parsing.swift index da57a017..09b20ef0 100644 --- a/Sources/SwiftFormat/Core/Parsing.swift +++ b/Sources/SwiftFormat/Core/Parsing.swift @@ -13,7 +13,7 @@ import Foundation import SwiftDiagnostics import SwiftOperators -import SwiftParser +@_spi(ExperimentalLanguageFeatures) import SwiftParser import SwiftParserDiagnostics import SwiftSyntax @@ -25,10 +25,14 @@ import SwiftSyntax /// /// - Parameters: /// - source: The Swift source code to be formatted. +/// - operatorTable: The operator table to use for sequence folding. /// - url: A file URL denoting the filename/path that should be assumed for this syntax tree, /// which is associated with any diagnostics emitted during formatting. If this is nil, a /// dummy value will be used. -/// - operatorTable: The operator table to use for sequence folding. +/// - experimentalFeatures: The set of experimental features that should be enabled in the parser. +/// These names must be from the set of parser-recognized experimental language features in +/// `SwiftParser`'s `Parser.ExperimentalFeatures` enum, which match the spelling defined in the +/// compiler's `Features.def` file. /// - parsingDiagnosticHandler: An optional callback that will be notified if there are any /// errors when parsing the source code. /// - Throws: If an unrecoverable error occurs when formatting the code. @@ -36,11 +40,21 @@ func parseAndEmitDiagnostics( source: String, operatorTable: OperatorTable, assumingFileURL url: URL?, + experimentalFeatures: Set, parsingDiagnosticHandler: ((Diagnostic, SourceLocation) -> Void)? = nil ) throws -> SourceFileSyntax { - let sourceFile = - operatorTable.foldAll(Parser.parse(source: source)) { _ in }.as(SourceFileSyntax.self)! - + var experimentalFeaturesSet: Parser.ExperimentalFeatures = [] + for featureName in experimentalFeatures { + guard let featureValue = Parser.ExperimentalFeatures(name: featureName) else { + throw SwiftFormatError.unrecognizedExperimentalFeature(featureName) + } + experimentalFeaturesSet.formUnion(featureValue) + } + var source = source + let sourceFile = source.withUTF8 { sourceBytes in + operatorTable.foldAll(Parser.parse(source: sourceBytes, experimentalFeatures: experimentalFeaturesSet)) { _ in } + .as(SourceFileSyntax.self)! + } let diagnostics = ParseDiagnosticsGenerator.diagnostics(for: sourceFile) var hasErrors = false if let parsingDiagnosticHandler = parsingDiagnosticHandler { diff --git a/Sources/_SwiftFormatTestSupport/Parsing.swift b/Sources/_SwiftFormatTestSupport/Parsing.swift new file mode 100644 index 00000000..71f1a64d --- /dev/null +++ b/Sources/_SwiftFormatTestSupport/Parsing.swift @@ -0,0 +1,37 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +@_spi(ExperimentalLanguageFeatures) import SwiftParser +import SwiftSyntax +import XCTest + +extension Parser { + /// Parses the given source string and returns the corresponding `SourceFileSyntax` node. + /// + /// - Parameters: + /// - source: The source text to parse. + /// - experimentalFeatures: The set of experimental features that should be enabled in the + /// parser. + @_spi(Testing) + public static func parse( + source: String, + experimentalFeatures: Parser.ExperimentalFeatures + ) -> SourceFileSyntax { + var source = source + return source.withUTF8 { sourceBytes in + parse( + source: sourceBytes, + experimentalFeatures: experimentalFeatures + ) + } + } +} diff --git a/Sources/swift-format/Frontend/FormatFrontend.swift b/Sources/swift-format/Frontend/FormatFrontend.swift index 850086d6..23d12771 100644 --- a/Sources/swift-format/Frontend/FormatFrontend.swift +++ b/Sources/swift-format/Frontend/FormatFrontend.swift @@ -56,6 +56,7 @@ class FormatFrontend: Frontend { source: source, assumingFileURL: url, selection: fileToProcess.selection, + experimentalFeatures: Set(lintFormatOptions.experimentalFeatures), to: &buffer, parsingDiagnosticHandler: diagnosticHandler ) @@ -69,15 +70,11 @@ class FormatFrontend: Frontend { source: source, assumingFileURL: url, selection: fileToProcess.selection, + experimentalFeatures: Set(lintFormatOptions.experimentalFeatures), to: &stdoutStream, parsingDiagnosticHandler: diagnosticHandler ) } - } catch SwiftFormatError.fileNotReadable { - diagnosticsEngine.emitError( - "Unable to format \(url.relativePath): file is not readable or does not exist." - ) - return } catch SwiftFormatError.fileContainsInvalidSyntax { guard !lintFormatOptions.ignoreUnparsableFiles else { guard !inPlace else { @@ -87,10 +84,10 @@ class FormatFrontend: Frontend { stdoutStream.write(source) return } - // Otherwise, relevant diagnostics about the problematic nodes have been emitted. - return + // Otherwise, relevant diagnostics about the problematic nodes have already been emitted; we + // don't need to print anything else. } catch { - diagnosticsEngine.emitError("Unable to format \(url.relativePath): \(error)") + diagnosticsEngine.emitError("Unable to format \(url.relativePath): \(error.localizedDescription).") } } } diff --git a/Sources/swift-format/Frontend/LintFrontend.swift b/Sources/swift-format/Frontend/LintFrontend.swift index f7c4ee52..c231266a 100644 --- a/Sources/swift-format/Frontend/LintFrontend.swift +++ b/Sources/swift-format/Frontend/LintFrontend.swift @@ -35,7 +35,8 @@ class LintFrontend: Frontend { do { try linter.lint( source: source, - assumingFileURL: url + assumingFileURL: url, + experimentalFeatures: Set(lintFormatOptions.experimentalFeatures) ) { (diagnostic, location) in guard !self.lintFormatOptions.ignoreUnparsableFiles else { // No diagnostics should be emitted in this mode. @@ -43,22 +44,15 @@ class LintFrontend: Frontend { } self.diagnosticsEngine.consumeParserDiagnostic(diagnostic, location) } - - } catch SwiftFormatError.fileNotReadable { - diagnosticsEngine.emitError( - "Unable to lint \(url.relativePath): file is not readable or does not exist." - ) - return } catch SwiftFormatError.fileContainsInvalidSyntax { guard !lintFormatOptions.ignoreUnparsableFiles else { // The caller wants to silently ignore this error. return } - // Otherwise, relevant diagnostics about the problematic nodes have been emitted. - return + // Otherwise, relevant diagnostics about the problematic nodes have already been emitted; we + // don't need to print anything else. } catch { - diagnosticsEngine.emitError("Unable to lint \(url.relativePath): \(error)") - return + diagnosticsEngine.emitError("Unable to lint \(url.relativePath): \(error.localizedDescription).") } } } diff --git a/Sources/swift-format/Subcommands/LintFormatOptions.swift b/Sources/swift-format/Subcommands/LintFormatOptions.swift index bd15dc4e..520eaf97 100644 --- a/Sources/swift-format/Subcommands/LintFormatOptions.swift +++ b/Sources/swift-format/Subcommands/LintFormatOptions.swift @@ -98,6 +98,15 @@ struct LintFormatOptions: ParsableArguments { ) var followSymlinks: Bool = false + @Option( + name: .customLong("enable-experimental-feature"), + help: """ + The name of an experimental swift-syntax parser feature that should be enabled by \ + swift-format. Multiple features can be enabled by specifying this flag multiple times. + """ + ) + var experimentalFeatures: [String] = [] + /// The list of paths to Swift source files that should be formatted or linted. @Argument(help: "Zero or more input filenames.") var paths: [String] = [] diff --git a/Tests/SwiftFormatTests/PrettyPrint/PrettyPrintTestCase.swift b/Tests/SwiftFormatTests/PrettyPrint/PrettyPrintTestCase.swift index abe0400b..375e5fd7 100644 --- a/Tests/SwiftFormatTests/PrettyPrint/PrettyPrintTestCase.swift +++ b/Tests/SwiftFormatTests/PrettyPrint/PrettyPrintTestCase.swift @@ -1,7 +1,7 @@ import SwiftFormat @_spi(Rules) @_spi(Testing) import SwiftFormat import SwiftOperators -import SwiftParser +@_spi(ExperimentalLanguageFeatures) import SwiftParser import SwiftSyntax import XCTest @_spi(Testing) import _SwiftFormatTestSupport @@ -18,6 +18,8 @@ class PrettyPrintTestCase: DiagnosingTestCase { /// changes that insert or remove non-whitespace characters (like trailing commas). /// - findings: A list of `FindingSpec` values that describe the findings that are expected to /// be emitted. These are currently only checked if `whitespaceOnly` is true. + /// - experimentalFeatures: The set of experimental features that should be enabled in the + /// parser. /// - file: The file in which failure occurred. Defaults to the file name of the test case in /// which this function was called. /// - line: The line number on which failure occurred. Defaults to the line number on which this @@ -29,6 +31,7 @@ class PrettyPrintTestCase: DiagnosingTestCase { configuration: Configuration = Configuration.forTesting, whitespaceOnly: Bool = false, findings: [FindingSpec] = [], + experimentalFeatures: Parser.ExperimentalFeatures = [], file: StaticString = #file, line: UInt = #line ) { @@ -44,6 +47,7 @@ class PrettyPrintTestCase: DiagnosingTestCase { configuration: configuration, selection: markedInput.selection, whitespaceOnly: whitespaceOnly, + experimentalFeatures: experimentalFeatures, findingConsumer: { emittedFindings.append($0) } ) assertStringsEqualWithDiff( @@ -76,6 +80,7 @@ class PrettyPrintTestCase: DiagnosingTestCase { configuration: configuration, selection: markedInput.selection, whitespaceOnly: whitespaceOnly, + experimentalFeatures: experimentalFeatures, findingConsumer: { _ in } // Ignore findings during the idempotence check. ) assertStringsEqualWithDiff( @@ -95,6 +100,8 @@ class PrettyPrintTestCase: DiagnosingTestCase { /// - configuration: The formatter configuration. /// - whitespaceOnly: If true, the pretty printer should only apply whitespace changes and omit /// changes that insert or remove non-whitespace characters (like trailing commas). + /// - experimentalFeatures: The set of experimental features that should be enabled in the + /// parser. /// - findingConsumer: A function called for each finding that is emitted by the pretty printer. /// - Returns: The pretty-printed text, or nil if an error occurred and a test failure was logged. private func prettyPrintedSource( @@ -102,11 +109,14 @@ class PrettyPrintTestCase: DiagnosingTestCase { configuration: Configuration, selection: Selection, whitespaceOnly: Bool, + experimentalFeatures: Parser.ExperimentalFeatures = [], findingConsumer: @escaping (Finding) -> Void ) -> (String, Context) { // Ignore folding errors for unrecognized operators so that we fallback to a reasonable default. let sourceFileSyntax = - OperatorTable.standardOperators.foldAll(Parser.parse(source: source)) { _ in } + OperatorTable.standardOperators.foldAll( + Parser.parse(source: source, experimentalFeatures: experimentalFeatures) + ) { _ in } .as(SourceFileSyntax.self)! let context = makeContext( sourceFileSyntax: sourceFileSyntax, diff --git a/Tests/SwiftFormatTests/PrettyPrint/ValueGenericsTests.swift b/Tests/SwiftFormatTests/PrettyPrint/ValueGenericsTests.swift new file mode 100644 index 00000000..54dec83a --- /dev/null +++ b/Tests/SwiftFormatTests/PrettyPrint/ValueGenericsTests.swift @@ -0,0 +1,46 @@ +@_spi(ExperimentalLanguageFeatures) import SwiftParser + +final class ValueGenericsTests: PrettyPrintTestCase { + func testValueGenericDeclaration() { + let input = "struct Foo { static let bar = n }" + let expected = """ + struct Foo< + let n: Int + > { + static let bar = n + } + + """ + assertPrettyPrintEqual( + input: input, + expected: expected, + linelength: 20, + experimentalFeatures: [.valueGenerics] + ) + } + + func testValueGenericTypeUsage() { + let input = + """ + let v1: Vector<100, Int> + let v2 = Vector<100, Int>() + """ + let expected = """ + let v1: + Vector< + 100, Int + > + let v2 = + Vector< + 100, Int + >() + + """ + assertPrettyPrintEqual( + input: input, + expected: expected, + linelength: 15, + experimentalFeatures: [.valueGenerics] + ) + } +} diff --git a/Tests/SwiftFormatTests/Rules/LintOrFormatRuleTestCase.swift b/Tests/SwiftFormatTests/Rules/LintOrFormatRuleTestCase.swift index 9ae0050e..dc69fbef 100644 --- a/Tests/SwiftFormatTests/Rules/LintOrFormatRuleTestCase.swift +++ b/Tests/SwiftFormatTests/Rules/LintOrFormatRuleTestCase.swift @@ -1,7 +1,7 @@ import SwiftFormat @_spi(Rules) @_spi(Testing) import SwiftFormat import SwiftOperators -import SwiftParser +@_spi(ExperimentalLanguageFeatures) import SwiftParser import SwiftSyntax import XCTest @_spi(Testing) import _SwiftFormatTestSupport @@ -16,18 +16,21 @@ class LintOrFormatRuleTestCase: DiagnosingTestCase { /// where findings are expected to be emitted. /// - findings: A list of `FindingSpec` values that describe the findings that are expected to /// be emitted. + /// - experimentalFeatures: The set of experimental features that should be enabled in the + /// parser. /// - file: The file the test resides in (defaults to the current caller's file). /// - line: The line the test resides in (defaults to the current caller's line). final func assertLint( _ type: LintRule.Type, _ markedSource: String, findings: [FindingSpec] = [], + experimentalFeatures: Parser.ExperimentalFeatures = [], file: StaticString = #file, line: UInt = #line ) { let markedText = MarkedText(textWithMarkers: markedSource) let unmarkedSource = markedText.textWithoutMarkers - let tree = Parser.parse(source: unmarkedSource) + let tree = Parser.parse(source: unmarkedSource, experimentalFeatures: experimentalFeatures) let sourceFileSyntax = try! OperatorTable.standardOperators.foldAll(tree).as(SourceFileSyntax.self)! @@ -80,6 +83,8 @@ class LintOrFormatRuleTestCase: DiagnosingTestCase { /// - findings: A list of `FindingSpec` values that describe the findings that are expected to /// be emitted. /// - configuration: The configuration to use when formatting (or nil to use the default). + /// - experimentalFeatures: The set of experimental features that should be enabled in the + /// parser. /// - file: The file the test resides in (defaults to the current caller's file) /// - line: The line the test resides in (defaults to the current caller's line) final func assertFormatting( @@ -88,12 +93,13 @@ class LintOrFormatRuleTestCase: DiagnosingTestCase { expected: String, findings: [FindingSpec] = [], configuration: Configuration? = nil, + experimentalFeatures: Parser.ExperimentalFeatures = [], file: StaticString = #file, line: UInt = #line ) { let markedInput = MarkedText(textWithMarkers: input) let originalSource: String = markedInput.textWithoutMarkers - let tree = Parser.parse(source: originalSource) + let tree = Parser.parse(source: originalSource, experimentalFeatures: experimentalFeatures) let sourceFileSyntax = try! OperatorTable.standardOperators.foldAll(tree).as(SourceFileSyntax.self)! @@ -134,7 +140,7 @@ class LintOrFormatRuleTestCase: DiagnosingTestCase { printTokenStream: false, whitespaceOnly: false ).prettyPrint() - let prettyPrintedTree = Parser.parse(source: prettyPrintedSource) + let prettyPrintedTree = Parser.parse(source: prettyPrintedSource, experimentalFeatures: experimentalFeatures) XCTAssertEqual( whitespaceInsensitiveText(of: actual), whitespaceInsensitiveText(of: prettyPrintedTree), From 8c68ec3e98376cf89ea4a1126e993d6f6e7c351e Mon Sep 17 00:00:00 2001 From: TTOzzi Date: Sat, 16 Nov 2024 08:00:27 +0900 Subject: [PATCH 10/14] Add indentBlankLines configuration --- Documentation/Configuration.md | 5 + .../API/Configuration+Default.swift | 1 + Sources/SwiftFormat/API/Configuration.swift | 14 + .../SwiftFormat/PrettyPrint/PrettyPrint.swift | 6 +- .../PrettyPrint/PrettyPrintBuffer.swift | 15 +- .../PrettyPrint/TokenStreamCreator.swift | 7 + .../Configuration+Testing.swift | 1 + .../PrettyPrint/IndentBlankLinesTests.swift | 294 ++++++++++++++++++ 8 files changed, 339 insertions(+), 4 deletions(-) create mode 100644 Tests/SwiftFormatTests/PrettyPrint/IndentBlankLinesTests.swift diff --git a/Documentation/Configuration.md b/Documentation/Configuration.md index d31f98a0..14072d84 100644 --- a/Documentation/Configuration.md +++ b/Documentation/Configuration.md @@ -94,6 +94,11 @@ top-level keys and values: * `multiElementCollectionTrailingCommas` _(boolean)_: Determines whether multi-element collection literals should have trailing commas. Defaults to `true`. + +* `indentBlankLines` _(boolean)_: Determines whether blank lines should be modified + to match the current indentation. When this setting is true, blank lines will be modified + to match the indentation level, adding indentation whether or not there is existing whitespace. + When false (the default), all whitespace in blank lines will be completely removed. > TODO: Add support for enabling/disabling specific syntax transformations in > the pipeline. diff --git a/Sources/SwiftFormat/API/Configuration+Default.swift b/Sources/SwiftFormat/API/Configuration+Default.swift index d18164f3..1af06a12 100644 --- a/Sources/SwiftFormat/API/Configuration+Default.swift +++ b/Sources/SwiftFormat/API/Configuration+Default.swift @@ -41,5 +41,6 @@ extension Configuration { self.noAssignmentInExpressions = NoAssignmentInExpressionsConfiguration() self.multiElementCollectionTrailingCommas = true self.reflowMultilineStringLiterals = .never + self.indentBlankLines = false } } diff --git a/Sources/SwiftFormat/API/Configuration.swift b/Sources/SwiftFormat/API/Configuration.swift index 2586024a..20b491a0 100644 --- a/Sources/SwiftFormat/API/Configuration.swift +++ b/Sources/SwiftFormat/API/Configuration.swift @@ -46,6 +46,7 @@ public struct Configuration: Codable, Equatable { case noAssignmentInExpressions case multiElementCollectionTrailingCommas case reflowMultilineStringLiterals + case indentBlankLines } /// A dictionary containing the default enabled/disabled states of rules, keyed by the rules' @@ -260,6 +261,13 @@ public struct Configuration: Codable, Equatable { public var reflowMultilineStringLiterals: MultilineStringReflowBehavior + /// Determines whether to add indentation whitespace to blank lines or remove it entirely. + /// + /// If true, blank lines will be modified to match the current indentation level: + /// if they contain whitespace, the existing whitespace will be adjusted, and if they are empty, spaces will be added to match the indentation. + /// If false (the default), the whitespace in blank lines will be removed entirely. + public var indentBlankLines: Bool + /// Creates a new `Configuration` by loading it from a configuration file. public init(contentsOf url: URL) throws { let data = try Data(contentsOf: url) @@ -368,6 +376,12 @@ public struct Configuration: Codable, Equatable { self.reflowMultilineStringLiterals = try container.decodeIfPresent(MultilineStringReflowBehavior.self, forKey: .reflowMultilineStringLiterals) ?? defaults.reflowMultilineStringLiterals + self.indentBlankLines = + try container.decodeIfPresent( + Bool.self, + forKey: .indentBlankLines + ) + ?? defaults.indentBlankLines // If the `rules` key is not present at all, default it to the built-in set // so that the behavior is the same as if the configuration had been diff --git a/Sources/SwiftFormat/PrettyPrint/PrettyPrint.swift b/Sources/SwiftFormat/PrettyPrint/PrettyPrint.swift index f9e9246e..607364ee 100644 --- a/Sources/SwiftFormat/PrettyPrint/PrettyPrint.swift +++ b/Sources/SwiftFormat/PrettyPrint/PrettyPrint.swift @@ -441,7 +441,7 @@ public class PrettyPrinter { outputBuffer.enqueueSpaces(size) outputBuffer.write("\\") } - outputBuffer.writeNewlines(newline) + outputBuffer.writeNewlines(newline, shouldIndentBlankLines: configuration.indentBlankLines) lastBreak = true } else { if outputBuffer.isAtStartOfLine { @@ -458,6 +458,10 @@ public class PrettyPrinter { // Print out the number of spaces according to the size, and adjust spaceRemaining. case .space(let size, _): + if configuration.indentBlankLines, outputBuffer.isAtStartOfLine { + // An empty string write is needed to add line-leading indentation that matches the current indentation on a line that contains only whitespaces. + outputBuffer.write("") + } outputBuffer.enqueueSpaces(size) // Print any indentation required, followed by the text content of the syntax token. diff --git a/Sources/SwiftFormat/PrettyPrint/PrettyPrintBuffer.swift b/Sources/SwiftFormat/PrettyPrint/PrettyPrintBuffer.swift index 6c3402a0..af9d02f1 100644 --- a/Sources/SwiftFormat/PrettyPrint/PrettyPrintBuffer.swift +++ b/Sources/SwiftFormat/PrettyPrint/PrettyPrintBuffer.swift @@ -70,8 +70,11 @@ struct PrettyPrintBuffer { /// subtract the previously written newlines during the second call so that we end up with the /// correct number overall. /// - /// - Parameter newlines: The number and type of newlines to write. - mutating func writeNewlines(_ newlines: NewlineBehavior) { + /// - Parameters: + /// - newlines: The number and type of newlines to write. + /// - shouldIndentBlankLines: A Boolean value indicating whether to insert spaces + /// for blank lines based on the current indentation level. + mutating func writeNewlines(_ newlines: NewlineBehavior, shouldIndentBlankLines: Bool) { let numberToPrint: Int switch newlines { case .elective: @@ -86,7 +89,13 @@ struct PrettyPrintBuffer { } guard numberToPrint > 0 else { return } - writeRaw(String(repeating: "\n", count: numberToPrint)) + for number in 0..= 1 { + writeRaw(currentIndentation.indentation()) + } + writeRaw("\n") + } + lineNumber += numberToPrint isAtStartOfLine = true consecutiveNewlineCount += numberToPrint diff --git a/Sources/SwiftFormat/PrettyPrint/TokenStreamCreator.swift b/Sources/SwiftFormat/PrettyPrint/TokenStreamCreator.swift index 3832d687..03da7d4b 100644 --- a/Sources/SwiftFormat/PrettyPrint/TokenStreamCreator.swift +++ b/Sources/SwiftFormat/PrettyPrint/TokenStreamCreator.swift @@ -3521,6 +3521,12 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor { leadingIndent = nil case .newlines(let count), .carriageReturns(let count), .carriageReturnLineFeeds(let count): + if config.indentBlankLines, + let leadingIndent, leadingIndent.count > 0 + { + requiresNextNewline = true + } + leadingIndent = .spaces(0) guard !isStartOfFile else { break } @@ -3557,6 +3563,7 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor { case .spaces(let n): guard leadingIndent == .spaces(0) else { break } leadingIndent = .spaces(n) + case .tabs(let n): guard leadingIndent == .spaces(0) else { break } leadingIndent = .tabs(n) diff --git a/Sources/_SwiftFormatTestSupport/Configuration+Testing.swift b/Sources/_SwiftFormatTestSupport/Configuration+Testing.swift index 8d095767..a3c59372 100644 --- a/Sources/_SwiftFormatTestSupport/Configuration+Testing.swift +++ b/Sources/_SwiftFormatTestSupport/Configuration+Testing.swift @@ -41,6 +41,7 @@ extension Configuration { config.spacesAroundRangeFormationOperators = false config.noAssignmentInExpressions = NoAssignmentInExpressionsConfiguration() config.multiElementCollectionTrailingCommas = true + config.indentBlankLines = false return config } } diff --git a/Tests/SwiftFormatTests/PrettyPrint/IndentBlankLinesTests.swift b/Tests/SwiftFormatTests/PrettyPrint/IndentBlankLinesTests.swift new file mode 100644 index 00000000..31f8a119 --- /dev/null +++ b/Tests/SwiftFormatTests/PrettyPrint/IndentBlankLinesTests.swift @@ -0,0 +1,294 @@ +import SwiftFormat + +final class IndentBlankLinesTests: PrettyPrintTestCase { + func testIndentBlankLinesEnabled() { + let input = + """ + class A { + func foo() -> Int { + return 1 + } + \u{0020}\u{0020} + func bar() -> Int { + return 2 + } + } + """ + + let expected = + """ + class A { + func foo() -> Int { + return 1 + } + \u{0020}\u{0020} + func bar() -> Int { + return 2 + } + } + + """ + var config = Configuration.forTesting + config.indentBlankLines = true + assertPrettyPrintEqual(input: input, expected: expected, linelength: 80, configuration: config) + } + + func testIndentBlankLinesDisabled() { + let input = + """ + class A { + func foo() -> Int { + return 1 + } + \u{0020}\u{0020} + func bar() -> Int { + return 2 + } + } + """ + + let expected = + """ + class A { + func foo() -> Int { + return 1 + } + + func bar() -> Int { + return 2 + } + } + + """ + var config = Configuration.forTesting + config.indentBlankLines = false + assertPrettyPrintEqual(input: input, expected: expected, linelength: 80, configuration: config) + } + + func testLineWithMoreWhitespacesThanIndentation() { + let input = + """ + class A { + func foo() -> Int { + return 1 + } + \u{0020}\u{0020}\u{0020}\u{0020}\u{0020} + func bar() -> Int { + return 2 + } + } + """ + + let expected = + """ + class A { + func foo() -> Int { + return 1 + } + \u{0020}\u{0020} + func bar() -> Int { + return 2 + } + } + + """ + var config = Configuration.forTesting + config.indentBlankLines = true + assertPrettyPrintEqual(input: input, expected: expected, linelength: 80, configuration: config) + } + + func testLineWithFewerWhitespacesThanIndentation() { + let input = + """ + class A { + func foo() -> Int { + return 1 + } + \u{0020} + func bar() -> Int { + return 2 + } + } + """ + + let expected = + """ + class A { + func foo() -> Int { + return 1 + } + \u{0020}\u{0020} + func bar() -> Int { + return 2 + } + } + + """ + var config = Configuration.forTesting + config.indentBlankLines = true + assertPrettyPrintEqual(input: input, expected: expected, linelength: 80, configuration: config) + } + + func testLineWithoutWhitespace() { + let input = + """ + class A { + func foo() -> Int { + return 1 + } + + func bar() -> Int { + return 2 + } + } + """ + + let expected = + """ + class A { + func foo() -> Int { + return 1 + } + \u{0020}\u{0020} + func bar() -> Int { + return 2 + } + } + + """ + var config = Configuration.forTesting + config.indentBlankLines = true + assertPrettyPrintEqual(input: input, expected: expected, linelength: 80, configuration: config) + } + + func testConsecutiveLinesWithMoreWhitespacesThanIndentation() { + let input = + """ + class A { + func foo() -> Int { + return 1 + } + \u{0020}\u{0020}\u{0020}\u{0020}\u{0020} + \u{0020}\u{0020}\u{0020}\u{0020} + func bar() -> Int { + return 2 + } + } + """ + + let expected = + """ + class A { + func foo() -> Int { + return 1 + } + \u{0020}\u{0020} + func bar() -> Int { + return 2 + } + } + + """ + var config = Configuration.forTesting + config.indentBlankLines = true + assertPrettyPrintEqual(input: input, expected: expected, linelength: 80, configuration: config) + } + + func testConsecutiveLinesWithFewerWhitespacesThanIndentation() { + let input = + """ + class A { + func foo() -> Int { + return 1 + } + \u{0020} + + func bar() -> Int { + return 2 + } + } + """ + + let expected = + """ + class A { + func foo() -> Int { + return 1 + } + \u{0020}\u{0020} + func bar() -> Int { + return 2 + } + } + + """ + var config = Configuration.forTesting + config.indentBlankLines = true + assertPrettyPrintEqual(input: input, expected: expected, linelength: 80, configuration: config) + } + + func testConsecutiveLinesWithoutWhitespace() { + let input = + """ + class A { + func foo() -> Int { + return 1 + } + + + func bar() -> Int { + return 2 + } + } + """ + + let expected = + """ + class A { + func foo() -> Int { + return 1 + } + \u{0020}\u{0020} + func bar() -> Int { + return 2 + } + } + + """ + var config = Configuration.forTesting + config.indentBlankLines = true + assertPrettyPrintEqual(input: input, expected: expected, linelength: 80, configuration: config) + } + + func testExpressionsWithUnnecessaryWhitespaces() { + let input = + """ + class A { + func foo() -> Int { + return 1 + } + \u{0020}\u{0020} + func bar() -> Int { + return 2 + } + } + """ + + let expected = + """ + class A { + func foo() -> Int { + return 1 + } + \u{0020}\u{0020} + func bar() -> Int { + return 2 + } + } + + """ + var config = Configuration.forTesting + config.indentBlankLines = true + assertPrettyPrintEqual(input: input, expected: expected, linelength: 80, configuration: config) + } +} From 2cd032cdac94c41bdae8175ea623e3c8250ce47a Mon Sep 17 00:00:00 2001 From: Bernd Kolb Date: Sat, 23 Nov 2024 20:18:34 +0100 Subject: [PATCH 11/14] PrettyPrinter reports wrong line LineNumbersTests The reason for the wrong line number were multiline comments. In to accomodate for this, we now check the string while writing for new lines and increment the line count accordingly. Issue: #882 --- .../PrettyPrint/PrettyPrintBuffer.swift | 15 +++- .../PrettyPrint/LineNumbersTests.swift | 84 +++++++++++++++++++ 2 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 Tests/SwiftFormatTests/PrettyPrint/LineNumbersTests.swift diff --git a/Sources/SwiftFormat/PrettyPrint/PrettyPrintBuffer.swift b/Sources/SwiftFormat/PrettyPrint/PrettyPrintBuffer.swift index af9d02f1..4b02d372 100644 --- a/Sources/SwiftFormat/PrettyPrint/PrettyPrintBuffer.swift +++ b/Sources/SwiftFormat/PrettyPrint/PrettyPrintBuffer.swift @@ -118,7 +118,20 @@ struct PrettyPrintBuffer { writeRaw(text) consecutiveNewlineCount = 0 pendingSpaces = 0 - column += text.count + + // In case of comments, we may get a multi-line string. + // To account for that case, we need to correct the lineNumber count. + // The new column is only the position within the last line. + let lines = text.split(separator: "\n") + lineNumber += lines.count - 1 + if lines.count > 1 { + // in case we have inserted new lines, we need to reset the column + column = lines.last?.count ?? 0 + } else { + // in case it is an end of line comment or a single line comment, + // we just add to the current column + column += lines.last?.count ?? 0 + } } /// Request that the given number of spaces be printed out before the next text token. diff --git a/Tests/SwiftFormatTests/PrettyPrint/LineNumbersTests.swift b/Tests/SwiftFormatTests/PrettyPrint/LineNumbersTests.swift new file mode 100644 index 00000000..1bb58f7a --- /dev/null +++ b/Tests/SwiftFormatTests/PrettyPrint/LineNumbersTests.swift @@ -0,0 +1,84 @@ +import SwiftFormat +import _SwiftFormatTestSupport + +final class LineNumbersTests: PrettyPrintTestCase { + func testLineNumbers() { + let input = + """ + final class A { + @Test func b() throws { + doSomethingInAFunctionWithAVeryLongName() 1️⃣// Here we have a very long comment that should not be here because it is far too long + } + } + """ + + let expected = + """ + final class A { + @Test func b() throws { + doSomethingInAFunctionWithAVeryLongName() // Here we have a very long comment that should not be here because it is far too long + } + } + + """ + + assertPrettyPrintEqual( + input: input, + expected: expected, + linelength: 120, + whitespaceOnly: true, + findings: [ + FindingSpec("1️⃣", message: "move end-of-line comment that exceeds the line length") + ] + ) + } + + func testLineNumbersWithComments() { + let input = + """ + // Copyright (C) 2024 My Coorp. All rights reserved. + // + // This document is the property of My Coorp. + // It is considered confidential and proprietary. + // + // This document may not be reproduced or transmitted in any form, + // in whole or in part, without the express written permission of + // My Coorp. + + final class A { + @Test func b() throws { + doSomethingInAFunctionWithAVeryLongName() 1️⃣// Here we have a very long comment that should not be here because it is far too long + } + } + """ + + let expected = + """ + // Copyright (C) 2024 My Coorp. All rights reserved. + // + // This document is the property of My Coorp. + // It is considered confidential and proprietary. + // + // This document may not be reproduced or transmitted in any form, + // in whole or in part, without the express written permission of + // My Coorp. + + final class A { + @Test func b() throws { + doSomethingInAFunctionWithAVeryLongName() // Here we have a very long comment that should not be here because it is far too long + } + } + + """ + + assertPrettyPrintEqual( + input: input, + expected: expected, + linelength: 120, + whitespaceOnly: true, + findings: [ + FindingSpec("1️⃣", message: "move end-of-line comment that exceeds the line length") + ] + ) + } +} From 8fec655e4b91689d895533fe0433e9fb29102bc6 Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Wed, 4 Dec 2024 10:35:07 -0800 Subject: [PATCH 12/14] Use dockerless Windows jobs Dockerless Windows is 5-10 minutes faster than Docker on Windows --- .github/workflows/pull_request.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index d49113a8..5a9f7543 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -8,6 +8,8 @@ jobs: tests: name: Test uses: swiftlang/github-workflows/.github/workflows/swift_package_test.yml@main + with: + enable_windows_docker: false # Dockerless Windows is 5-10 minutes faster than Docker on Windows soundness: name: Soundness uses: swiftlang/github-workflows/.github/workflows/soundness.yml@main From 23709e812cf252cd99157433a32a363bc3899c7c Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Wed, 4 Dec 2024 16:13:08 -0800 Subject: [PATCH 13/14] Fix issue in publish_release pipeline testing swift-format in debug configuration --- .github/workflows/publish_release.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/publish_release.yml b/.github/workflows/publish_release.yml index 03c4f04f..a360c3f9 100644 --- a/.github/workflows/publish_release.yml +++ b/.github/workflows/publish_release.yml @@ -116,8 +116,8 @@ jobs: # fatal: empty ident name (for <>) not allowed cmd /c "type $env:TEMP\patch.diff | git am || (exit /b 1)" # We require that releases of swift-format build without warnings - linux_build_command: swift test -Xswiftc -warnings-as-errors ${{ matrix.release && '-c release' }} - windows_build_command: swift test -Xswiftc -warnings-as-errors ${{ matrix.release && '-c release' }} + linux_build_command: swift test -Xswiftc -warnings-as-errors ${{ matrix.release && '-c release' || '' }} + windows_build_command: swift test -Xswiftc -warnings-as-errors ${{ matrix.release && '-c release' || '' }} create_tag: name: Create Tag runs-on: ubuntu-latest From 4db71dbfb9f24ce83c02b15c426b6f39405739eb Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Wed, 4 Dec 2024 16:14:11 -0800 Subject: [PATCH 14/14] Run Windows tests dockerless --- .github/workflows/publish_release.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/publish_release.yml b/.github/workflows/publish_release.yml index a360c3f9..5c1985c7 100644 --- a/.github/workflows/publish_release.yml +++ b/.github/workflows/publish_release.yml @@ -99,6 +99,7 @@ jobs: matrix: release: [true, false] with: + enable_windows_docker: false # Dockerless Windows is 5-10 minutes faster than Docker on Windows linux_pre_build_command: | git config --global --add safe.directory "$(realpath .)" git config --local user.name 'swift-ci'