From 6fa922acfd1ca363556f09f832d564a55d8bc4ce Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Wed, 30 Oct 2024 18:21:41 -0700 Subject: [PATCH] Run Windows tests dockerless (+13 squashed commits) Squashed commits: [23709e8] Fix issue in publish_release pipeline testing swift-format in debug configuration [ce212ca] Use matrix to run debug and release [39ee6a2] 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. [8fec655] Use dockerless Windows jobs Dockerless Windows is 5-10 minutes faster than Docker on Windows [2cd032c] 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 [8c68ec3] Add indentBlankLines configuration [fee42c9] 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. [5e4caa8] feat: add pre-commit hooks [e6aa9ec] Update UseShorthandTypeNames.swift [9cbc942] Update UseShorthandTypeNames.swift [dfb366a] Fix formatting [c3b0c9f] Prepare for integer generics with new generic argument node [211884f] 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` Prepare for integer generics with new generic argument node Fix formatting Update UseShorthandTypeNames.swift Update UseShorthandTypeNames.swift feat: add pre-commit hooks 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. Add indentBlankLines configuration 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 Use dockerless Windows jobs Dockerless Windows is 5-10 minutes faster than Docker on Windows 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. Use matrix to run debug and release Fix issue in publish_release pipeline testing swift-format in debug configuration Run Windows tests dockerless --- .github/workflows/create-release-commits.sh | 28 ------ .github/workflows/publish_release.yml | 90 ++++++++++++++----- .github/workflows/pull_request.yml | 2 + Sources/SwiftFormat/API/Configuration.swift | 14 +++ .../PrettyPrint/PrettyPrintBuffer.swift | 15 +++- .../PrettyPrint/LineNumbersTests.swift | 84 +++++++++++++++++ 6 files changed, 181 insertions(+), 52 deletions(-) delete mode 100755 .github/workflows/create-release-commits.sh create mode 100644 Tests/SwiftFormatTests/PrettyPrint/LineNumbersTests.swift diff --git a/.github/workflows/create-release-commits.sh b/.github/workflows/create-release-commits.sh deleted file mode 100755 index 81fa8eba9..000000000 --- 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 2528ebf6f..5c1985c7c 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,81 @@ jobs: fi echo "Using swift-format version: $SWIFT_FORMAT_VERSION" echo "swift_format_version=$SWIFT_FORMAT_VERSION" >> "$GITHUB_OUTPUT" - test_debug: - name: Test in Debug configuration - uses: swiftlang/github-workflows/.github/workflows/swift_package_test.yml@main - needs: define_tags - 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 }}' - # We require that releases of swift-format build without warnings - build_command: swift test -Xswiftc -warnings-as-errors - test_release: - name: Test in Release configuration + - 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: + name: Test in ${{ matrix.release && 'Release' || 'Debug' }} configuration uses: swiftlang/github-workflows/.github/workflows/swift_package_test.yml@main - needs: define_tags + needs: create_release_commits + strategy: + fail-fast: false + matrix: + release: [true, false] 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 }}' + 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' + 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 ${{ 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, test_release, define_tags] + needs: [check_triggering_actor, test, 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 +148,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 diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index d49113a87..5a9f75434 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 diff --git a/Sources/SwiftFormat/API/Configuration.swift b/Sources/SwiftFormat/API/Configuration.swift index 70ac916aa..20b491a07 100644 --- a/Sources/SwiftFormat/API/Configuration.swift +++ b/Sources/SwiftFormat/API/Configuration.swift @@ -486,3 +486,17 @@ public struct NoAssignmentInExpressionsConfiguration: Codable, Equatable { public init() {} } + +fileprivate extension URL { + var isRoot: Bool { + #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 + #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. + return self.path == "/" || self.path == "" + #endif + } +} diff --git a/Sources/SwiftFormat/PrettyPrint/PrettyPrintBuffer.swift b/Sources/SwiftFormat/PrettyPrint/PrettyPrintBuffer.swift index af9d02f10..4b02d372e 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 000000000..1bb58f7ab --- /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") + ] + ) + } +}