From 6e500139b23b3b50f96b64b88dcd8de9c294cd79 Mon Sep 17 00:00:00 2001 From: Shawn Shi Date: Sun, 19 Jul 2026 21:41:38 -0400 Subject: [PATCH 1/2] Add SwiftLint, pre-commit hooks, and CI workflow - .swiftlint.yml: opt into a handful of stricter rules (force unwrapping, implicitly unwrapped optionals, empty_count, etc.), cap line length, and whitelist the small set of short/underscored identifiers already in use rather than loosening the rule globally. - .pre-commit-config.yaml: local SwiftLint hook scoped to staged Swift files via the pre-commit framework. - .github/workflows/ci.yml: build, test, and lint on macOS on push/PR. - Autocorrect the resulting mechanical violations (trailing commas, trailing newlines, colon spacing) across existing files; everything else surfaces as an advisory warning rather than blocking. Co-Authored-By: Claude Sonnet 4.6 --- .github/workflows/ci.yml | 49 +++++++++++++++++++ .pre-commit-config.yaml | 10 ++++ .swiftlint.yml | 44 +++++++++++++++++ README.md | 16 ++++++ .../Bootstrap/BuildServerBootstrap.swift | 1 - .../Service/BuildServiceProviding.swift | 2 +- .../Watcher/WorkspaceChangeFilter.swift | 6 +-- .../SourceKitXcodeBSP/Xcode/XcodePaths.swift | 2 +- .../SourcekitXcodeBspCommand.swift | 2 +- Sources/test-ipc/main.swift | 2 +- .../Watcher/WorkspaceWatcherTests.swift | 14 +++--- 11 files changed, 133 insertions(+), 15 deletions(-) create mode 100644 .github/workflows/ci.yml create mode 100644 .pre-commit-config.yaml create mode 100644 .swiftlint.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..d385af0 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,49 @@ +name: CI + +on: + push: + branches: [main] + pull_request: + workflow_dispatch: + +concurrency: + group: ci-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + build-and-test: + runs-on: macos-latest + steps: + - uses: actions/checkout@v4 + + - name: Select Xcode + uses: maxim-lobanov/setup-xcode@v1 + with: + # This project requires Xcode 26+ (see README Requirements). Pin an exact + # version here if `latest-stable` ever resolves below that floor. + xcode-version: latest-stable + + - name: Cache SwiftPM dependencies + uses: actions/cache@v4 + with: + path: .build + key: ${{ runner.os }}-spm-${{ hashFiles('Package.resolved') }} + restore-keys: | + ${{ runner.os }}-spm- + + - name: Build + run: swift build + + - name: Run tests + run: swift test + + lint: + runs-on: macos-latest + steps: + - uses: actions/checkout@v4 + + - name: Install SwiftLint + run: brew install swiftlint + + - name: Lint + run: swiftlint lint diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..f6098f0 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,10 @@ +repos: + - repo: local + hooks: + - id: swiftlint + name: SwiftLint + description: Lint staged Swift files with SwiftLint. + entry: swiftlint lint + language: system + files: \.swift$ + exclude: ^\.build/ diff --git a/.swiftlint.yml b/.swiftlint.yml new file mode 100644 index 0000000..ed74607 --- /dev/null +++ b/.swiftlint.yml @@ -0,0 +1,44 @@ +included: + - Sources + - Tests + +excluded: + - .build + - .swiftpm + +opt_in_rules: + - empty_count + - closure_spacing + - contains_over_first_not_nil + - fatal_error_message + - first_where + - force_unwrapping + - implicitly_unwrapped_optional + - last_where + - redundant_nil_coalescing + - sorted_first_last + - unneeded_parentheses_in_closure_argument + +disabled_rules: + - todo + +line_length: + warning: 120 + error: 200 + +identifier_name: + excluded: + - id + - s + - t + - fm + - op + - v26_2 + - v16_0 + - v16_2_1 + +force_unwrapping: + severity: warning + +implicitly_unwrapped_optional: + severity: warning diff --git a/README.md b/README.md index b30b524..48a3c58 100644 --- a/README.md +++ b/README.md @@ -110,8 +110,24 @@ swift test # Build release binary swift build -c release + +# Lint +swiftlint lint ``` +### Pre-commit hooks + +This repo uses [pre-commit](https://pre-commit.com) to lint staged Swift files with +[SwiftLint](https://github.com/realm/SwiftLint) before each commit. + +```bash +brew install pre-commit swiftlint +pre-commit install +``` + +After that, `git commit` runs SwiftLint automatically against whatever `.swift` files are +staged. Run it manually against everything with `pre-commit run --all-files`. + ### Project layout ``` diff --git a/Sources/SourceKitXcodeBSP/Bootstrap/BuildServerBootstrap.swift b/Sources/SourceKitXcodeBSP/Bootstrap/BuildServerBootstrap.swift index 14c12f2..977d7ab 100644 --- a/Sources/SourceKitXcodeBSP/Bootstrap/BuildServerBootstrap.swift +++ b/Sources/SourceKitXcodeBSP/Bootstrap/BuildServerBootstrap.swift @@ -323,4 +323,3 @@ public extension BuildServerBootstrap { ) } } - diff --git a/Sources/SourceKitXcodeBSP/Service/BuildServiceProviding.swift b/Sources/SourceKitXcodeBSP/Service/BuildServiceProviding.swift index 6e7c637..7001bcb 100644 --- a/Sources/SourceKitXcodeBSP/Service/BuildServiceProviding.swift +++ b/Sources/SourceKitXcodeBSP/Service/BuildServiceProviding.swift @@ -105,7 +105,7 @@ public struct BuildServiceProviderFactory: Sendable { // Always set explicitly so our config wins over any inherited environment value. value: synchronousBuildDescriptionSerialization ? "YES" : "NO", overwrite: true - ), + ) ] if let serviceBundlePath { // An explicit path from the config is authoritative — overwrite any inherited value. diff --git a/Sources/SourceKitXcodeBSP/Watcher/WorkspaceChangeFilter.swift b/Sources/SourceKitXcodeBSP/Watcher/WorkspaceChangeFilter.swift index b16ceba..6ace2ed 100644 --- a/Sources/SourceKitXcodeBSP/Watcher/WorkspaceChangeFilter.swift +++ b/Sources/SourceKitXcodeBSP/Watcher/WorkspaceChangeFilter.swift @@ -19,7 +19,7 @@ public struct WorkspaceChangeFilter: Sendable { "xcuserdata", "Index.noindex", "ModuleCache.noindex", - "CompilationCache.noindex", + "CompilationCache.noindex" ] public init(allowedPaths: Set) { @@ -90,7 +90,7 @@ public struct WorkspaceChangeFilter: Sendable { .appendingPathComponent("project.xcworkspace") .appendingPathComponent("xcshareddata") .appendingPathComponent("swiftpm") - .appendingPathComponent("Package.resolved").path, + .appendingPathComponent("Package.resolved").path ] } @@ -101,7 +101,7 @@ public struct WorkspaceChangeFilter: Sendable { workspaceURL .appendingPathComponent("xcshareddata") .appendingPathComponent("swiftpm") - .appendingPathComponent("Package.resolved").path, + .appendingPathComponent("Package.resolved").path ] } diff --git a/Sources/SourceKitXcodeBSP/Xcode/XcodePaths.swift b/Sources/SourceKitXcodeBSP/Xcode/XcodePaths.swift index d1854fd..737d6a2 100644 --- a/Sources/SourceKitXcodeBSP/Xcode/XcodePaths.swift +++ b/Sources/SourceKitXcodeBSP/Xcode/XcodePaths.swift @@ -174,7 +174,7 @@ public struct XcodeVersion: Sendable, CustomStringConvertible { public init(versionString: String) { let parts = versionString.split(separator: ".").compactMap { Int($0) } - major = parts.count > 0 ? parts[0] : 0 + major = !parts.isEmpty ? parts[0] : 0 minor = parts.count > 1 ? parts[1] : 0 patch = parts.count > 2 ? parts[2] : 0 } diff --git a/Sources/sourcekit-xcode-bsp/SourcekitXcodeBspCommand.swift b/Sources/sourcekit-xcode-bsp/SourcekitXcodeBspCommand.swift index 5b19aa5..1238f52 100644 --- a/Sources/sourcekit-xcode-bsp/SourcekitXcodeBspCommand.swift +++ b/Sources/sourcekit-xcode-bsp/SourcekitXcodeBspCommand.swift @@ -59,4 +59,4 @@ struct Serve: AsyncParsableCommand { serviceProvider: serviceProvider ) } -} \ No newline at end of file +} diff --git a/Sources/test-ipc/main.swift b/Sources/test-ipc/main.swift index 5f62f9d..9323727 100644 --- a/Sources/test-ipc/main.swift +++ b/Sources/test-ipc/main.swift @@ -3,7 +3,7 @@ import SwiftBuild private final class NopDelegate: SWBPlanningOperationDelegate, @unchecked Sendable { func provisioningTaskInputs(targetGUID: String, provisioningSourceData: SWBProvisioningTaskInputsSourceData) async -> SWBProvisioningTaskInputs { .init() } - func executeExternalTool(commandLine: [String], workingDirectory: String?, environment: [String : String]) async throws -> SWBExternalToolResult { + func executeExternalTool(commandLine: [String], workingDirectory: String?, environment: [String: String]) async throws -> SWBExternalToolResult { print(" executeExternalTool: \(commandLine.first ?? "?") (returning .deferred)") return .deferred } diff --git a/Tests/SourceKitXcodeBSPTests/Watcher/WorkspaceWatcherTests.swift b/Tests/SourceKitXcodeBSPTests/Watcher/WorkspaceWatcherTests.swift index c97662d..f844f50 100644 --- a/Tests/SourceKitXcodeBSPTests/Watcher/WorkspaceWatcherTests.swift +++ b/Tests/SourceKitXcodeBSPTests/Watcher/WorkspaceWatcherTests.swift @@ -58,7 +58,7 @@ struct WorkspaceChangeFilterTests { allowedPaths: [ "/Projects/App/App.xcodeproj/project.pbxproj", "/Projects/App/Package.swift", - "/Projects/App/Package.resolved", + "/Projects/App/Package.resolved" ] ) @@ -70,7 +70,7 @@ struct WorkspaceChangeFilterTests { func rejectsOtherProjects() { let filter = WorkspaceChangeFilter( allowedPaths: [ - "/Projects/App/App.xcodeproj/project.pbxproj", + "/Projects/App/App.xcodeproj/project.pbxproj" ] ) @@ -83,7 +83,7 @@ struct WorkspaceChangeFilterTests { let filter = WorkspaceChangeFilter( allowedPaths: [ "/Projects/App/App.xcodeproj/project.pbxproj", - "/Projects/App/Package.swift", + "/Projects/App/Package.swift" ] ) @@ -96,7 +96,7 @@ struct WorkspaceChangeFilterTests { "/Projects/App/SourcePackages/checkouts/Foo/Package.swift", "/Projects/App/SourcePackages/checkouts/Foo/Foo.xcodeproj/project.pbxproj", "/Projects/App/.swiftpm/configuration/registries.json", - "/Projects/App/App.xcodeproj/xcuserdata/user.xcuserdatad/xcschemes/xcschememanagement.plist", + "/Projects/App/App.xcodeproj/xcuserdata/user.xcuserdatad/xcschemes/xcschememanagement.plist" ] for path in noise { @@ -110,7 +110,7 @@ struct WorkspaceChangeFilterTests { // Even if a checkout path were mistakenly allowlisted, ignore components win. let filter = WorkspaceChangeFilter( allowedPaths: [ - "/Projects/App/SourcePackages/checkouts/Dep/Package.swift", + "/Projects/App/SourcePackages/checkouts/Dep/Package.swift" ] ) #expect( @@ -126,7 +126,7 @@ struct WorkspaceChangeFilterTests { // be an ignore component or reloads never fire. let filter = WorkspaceChangeFilter( allowedPaths: [ - "/Users/me/checkouts/MyApp/MyApp.xcodeproj/project.pbxproj", + "/Users/me/checkouts/MyApp/MyApp.xcodeproj/project.pbxproj" ] ) #expect( @@ -228,7 +228,7 @@ struct WatcherContextDebounceTests { context.handle(paths: [ "/Projects/App/.git/HEAD", "/Projects/App/.build/debug/output", - "/Projects/App/SourcePackages/checkouts/Dep/Package.swift", + "/Projects/App/SourcePackages/checkouts/Dep/Package.swift" ]) try await Task.sleep(for: .milliseconds(120)) From 9344302b972a169902b160b066d56b7580dfa1e8 Mon Sep 17 00:00:00 2001 From: Shawn Shi Date: Wed, 22 Jul 2026 21:43:21 -0400 Subject: [PATCH 2/2] Address PR review feedback: cache key, SwiftLint pin, trailing commas MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Cache key off Package.swift instead of the uncommitted Package.resolved (which always hashed to an empty match), and scope the cache to fetched dependency sources rather than the whole .build tree so it can't silently restore stale build products across dependency or toolchain changes. - Pin the lint job's SwiftLint to an exact version (0.63.3, matching what contributors install locally) via the portable release binary, instead of `brew install swiftlint` tracking whatever Homebrew's formula currently points at. - Flip trailing_comma to mandatory_comma: true per review — trailing commas on multiline collection literals keep future list edits to single-line diffs. Co-Authored-By: Claude Sonnet 4.6 --- .github/workflows/ci.yml | 23 +++++++++++++++---- .swiftlint.yml | 3 +++ .../Service/BuildServiceProviding.swift | 2 +- .../Watcher/WorkspaceChangeFilter.swift | 6 ++--- .../Watcher/WorkspaceWatcherTests.swift | 8 +++---- .../Xcode/XcodePathsTests.swift | 2 +- 6 files changed, 31 insertions(+), 13 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d385af0..4de1ced 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,11 +23,20 @@ jobs: # version here if `latest-stable` ever resolves below that floor. xcode-version: latest-stable - - name: Cache SwiftPM dependencies + - name: Cache SwiftPM dependency sources uses: actions/cache@v4 with: - path: .build - key: ${{ runner.os }}-spm-${{ hashFiles('Package.resolved') }} + # Package.resolved isn't committed (swift-build/swift-tools-protocols float + # on `branch: main`), so it always hashes to the same empty match — key off + # Package.swift instead. Cache only fetched sources, not .build's compiled + # objects, so a stale cache can't silently serve outdated build products + # across dependency or toolchain changes; sources just get an incremental + # `git fetch` instead of a full clone. + path: | + ~/Library/Caches/org.swift.swiftpm + .build/checkouts + .build/repositories + key: ${{ runner.os }}-spm-${{ hashFiles('Package.swift') }} restore-keys: | ${{ runner.os }}-spm- @@ -43,7 +52,13 @@ jobs: - uses: actions/checkout@v4 - name: Install SwiftLint - run: brew install swiftlint + env: + SWIFTLINT_VERSION: "0.63.3" # keep in sync with the version contributors install locally + run: | + curl -sL "https://github.com/realm/SwiftLint/releases/download/${SWIFTLINT_VERSION}/portable_swiftlint.zip" -o swiftlint.zip + unzip -q swiftlint.zip -d "$RUNNER_TEMP/swiftlint-bin" + chmod +x "$RUNNER_TEMP/swiftlint-bin/swiftlint" + echo "$RUNNER_TEMP/swiftlint-bin" >> "$GITHUB_PATH" - name: Lint run: swiftlint lint diff --git a/.swiftlint.yml b/.swiftlint.yml index ed74607..f2aa6cc 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -26,6 +26,9 @@ line_length: warning: 120 error: 200 +trailing_comma: + mandatory_comma: true + identifier_name: excluded: - id diff --git a/Sources/SourceKitXcodeBSP/Service/BuildServiceProviding.swift b/Sources/SourceKitXcodeBSP/Service/BuildServiceProviding.swift index 7001bcb..6e7c637 100644 --- a/Sources/SourceKitXcodeBSP/Service/BuildServiceProviding.swift +++ b/Sources/SourceKitXcodeBSP/Service/BuildServiceProviding.swift @@ -105,7 +105,7 @@ public struct BuildServiceProviderFactory: Sendable { // Always set explicitly so our config wins over any inherited environment value. value: synchronousBuildDescriptionSerialization ? "YES" : "NO", overwrite: true - ) + ), ] if let serviceBundlePath { // An explicit path from the config is authoritative — overwrite any inherited value. diff --git a/Sources/SourceKitXcodeBSP/Watcher/WorkspaceChangeFilter.swift b/Sources/SourceKitXcodeBSP/Watcher/WorkspaceChangeFilter.swift index 6ace2ed..b16ceba 100644 --- a/Sources/SourceKitXcodeBSP/Watcher/WorkspaceChangeFilter.swift +++ b/Sources/SourceKitXcodeBSP/Watcher/WorkspaceChangeFilter.swift @@ -19,7 +19,7 @@ public struct WorkspaceChangeFilter: Sendable { "xcuserdata", "Index.noindex", "ModuleCache.noindex", - "CompilationCache.noindex" + "CompilationCache.noindex", ] public init(allowedPaths: Set) { @@ -90,7 +90,7 @@ public struct WorkspaceChangeFilter: Sendable { .appendingPathComponent("project.xcworkspace") .appendingPathComponent("xcshareddata") .appendingPathComponent("swiftpm") - .appendingPathComponent("Package.resolved").path + .appendingPathComponent("Package.resolved").path, ] } @@ -101,7 +101,7 @@ public struct WorkspaceChangeFilter: Sendable { workspaceURL .appendingPathComponent("xcshareddata") .appendingPathComponent("swiftpm") - .appendingPathComponent("Package.resolved").path + .appendingPathComponent("Package.resolved").path, ] } diff --git a/Tests/SourceKitXcodeBSPTests/Watcher/WorkspaceWatcherTests.swift b/Tests/SourceKitXcodeBSPTests/Watcher/WorkspaceWatcherTests.swift index f844f50..810c96a 100644 --- a/Tests/SourceKitXcodeBSPTests/Watcher/WorkspaceWatcherTests.swift +++ b/Tests/SourceKitXcodeBSPTests/Watcher/WorkspaceWatcherTests.swift @@ -58,7 +58,7 @@ struct WorkspaceChangeFilterTests { allowedPaths: [ "/Projects/App/App.xcodeproj/project.pbxproj", "/Projects/App/Package.swift", - "/Projects/App/Package.resolved" + "/Projects/App/Package.resolved", ] ) @@ -83,7 +83,7 @@ struct WorkspaceChangeFilterTests { let filter = WorkspaceChangeFilter( allowedPaths: [ "/Projects/App/App.xcodeproj/project.pbxproj", - "/Projects/App/Package.swift" + "/Projects/App/Package.swift", ] ) @@ -96,7 +96,7 @@ struct WorkspaceChangeFilterTests { "/Projects/App/SourcePackages/checkouts/Foo/Package.swift", "/Projects/App/SourcePackages/checkouts/Foo/Foo.xcodeproj/project.pbxproj", "/Projects/App/.swiftpm/configuration/registries.json", - "/Projects/App/App.xcodeproj/xcuserdata/user.xcuserdatad/xcschemes/xcschememanagement.plist" + "/Projects/App/App.xcodeproj/xcuserdata/user.xcuserdatad/xcschemes/xcschememanagement.plist", ] for path in noise { @@ -228,7 +228,7 @@ struct WatcherContextDebounceTests { context.handle(paths: [ "/Projects/App/.git/HEAD", "/Projects/App/.build/debug/output", - "/Projects/App/SourcePackages/checkouts/Dep/Package.swift" + "/Projects/App/SourcePackages/checkouts/Dep/Package.swift", ]) try await Task.sleep(for: .milliseconds(120)) diff --git a/Tests/SourceKitXcodeBSPTests/Xcode/XcodePathsTests.swift b/Tests/SourceKitXcodeBSPTests/Xcode/XcodePathsTests.swift index 2c1b400..1441eaf 100644 --- a/Tests/SourceKitXcodeBSPTests/Xcode/XcodePathsTests.swift +++ b/Tests/SourceKitXcodeBSPTests/Xcode/XcodePathsTests.swift @@ -83,7 +83,7 @@ struct XcodePathsTests { .infoPlistInvalid, .versionNotFound, .unsupportedVersion(XcodeVersion(major: 15, minor: 0)), - .swbBuildServiceNotFound + .swbBuildServiceNotFound, ] for error in errors {