Skip to content

Install: follow-up style cleanup for InstallSafeDITool + Xcode build plugin#277

Merged
dfed merged 1 commit intomainfrom
claude/install-plugin-style-cleanup
Apr 21, 2026
Merged

Install: follow-up style cleanup for InstallSafeDITool + Xcode build plugin#277
dfed merged 1 commit intomainfrom
claude/install-plugin-style-cleanup

Conversation

@dfed
Copy link
Copy Markdown
Owner

@dfed dfed commented Apr 20, 2026

Summary

Style-guide follow-up to #273. CLAUDE.md bars bare `if { return|throw }` patterns with implicit fall-through — the non-returning path has to sit in an explicit `else`.

Changes

  • `Plugins/InstallSafeDITool/InstallSafeDITool.swift` — HTTP status check for the downloaded asset. The old
    ```swift
    if let httpResponse = response as? HTTPURLResponse,
    !(200..<300).contains(httpResponse.statusCode)
    {
    throw DownloadFailedError(...)
    }
    ```
    falls through on 2xx. Restructured to
    ```swift
    if let httpResponse = response as? HTTPURLResponse {
    guard (200..<300).contains(httpResponse.statusCode) else { throw ... }
    }
    ```
    Same runtime behavior.

  • `Plugins/SafeDIGenerator/SafeDIGenerateDependencyTree.swift` — Xcode variant's real-scan / `PluginScanner` split. The old `if runsRealScan { do-return-catch } ...` block returned from inside the `if` and fell through on `SafeDIToolLaunchError`. Extracted:

    • `buildCommandsUsingRealScan(...)` — returns `nil` on recoverable launch failure, lets `SafeDIToolProcessError` propagate
    • `buildCommandsUsingPluginScanner(...)` — regex-based fallback

    Caller is now
    ```swift
    if runsRealScan, let commands = try buildCommandsUsingRealScan(...) {
    return commands
    } else {
    return buildCommandsUsingPluginScanner(...)
    }
    ```
    Both branches return. Runtime behavior unchanged: `ProcessError` still surfaces, `LaunchError` still warns + falls to scanner.

What this doesn't do

  • Binary checksum / code signature verification for the downloaded tool (codex P1 on Plugins: restore InstallSafeDITool download command for Xcodeproj users #273). Current controls are Gatekeeper (macOS signing + notarization at `Process.run` time) and the `--version` match in `verifiedDownloadedToolLocation`. A full checksum-pin would mirror `SafeDIToolBinary`'s checksum and needs publish-workflow coordination — separate PR.

Test plan

  • `swift build --traits sourceBuild` — builds clean
  • `./CLI/lint.sh`
  • Plugin code is non-unit-testable; no test changes. Existing CI covers the `buildCommandsUsingPluginScanner` path via `spm-project-integration` and `spm-multi-project-integration` (they exercise the Xcode build plugin through `xcodebuild`).

🤖 Generated with Claude Code

… build plugin

Follow-up to #273 covering style-guide violations surfaced by the
second-pass self review. CLAUDE.md bars bare \`if { throw|return }\`
patterns with implicit fall-through — the non-returning path must
sit in an explicit \`else\`.

- **InstallSafeDITool.swift: HTTP-status check.** The
  \`if httpResponse...,!(200..<300).contains(...) { throw ... }\` block
  used fall-through when the status was 2xx. Restructured to
  \`if let httpResponse { guard (200..<300).contains(...) else { throw } }\`.
- **SafeDIGenerateDependencyTree.swift: Xcode real-scan vs scanner
  split.** The \`if runsRealScan { do-return-catch } ... PluginScanner
  fallback\` block returned from inside the \`if\` and fell through on
  the \`SafeDIToolLaunchError\` catch. Extracted the two paths into
  \`buildCommandsUsingRealScan\` (nil on recoverable launch failure)
  and \`buildCommandsUsingPluginScanner\`; the caller is now an
  \`if ... let commands { return commands } else { return scanner }\`
  with both branches returning.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (75846d3) to head (52d2a24).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##              main      #277   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           41        41           
  Lines         6796      6796           
=========================================
  Hits          6796      6796           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dfed dfed merged commit e6f113a into main Apr 21, 2026
18 checks passed
@dfed dfed deleted the claude/install-plugin-style-cleanup branch April 21, 2026 01:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant