feat(swift-pm): add Swift Package Manager support#148
Conversation
- Add `Package.swift` parser using tree-sitter-swift; extracts `from:`, `exact:`, `.upToNextMajor(from:)`, `.upToNextMinor(from:)`, half-open (`A ..< B`), and closed (`A ... B`) range constraints; skips branch/revision pins - Add `SwiftPmVersionMatcher` reusing caret semantics for bare versions and evaluating compound ranges produced by the parser - Reuse `GitHubRegistry` for SPM version fetches; add `with_registry_type()` builder so the same HTTP backend can report `RegistryType::SwiftPm` keeping cache namespaces separate from GitHub Actions - Add `RegistryType::SwiftPm` variant with `as_str` / `FromStr` round-trip and `detect_parser_type` routing for `Package.swift` URIs - Wire `registries.swiftPm` config block (`enabled`, `url`) into `LspConfig`; `swiftPm.url` doubles as a GitHub Enterprise API base URL override and adds the URL's bare host to the parser's allow-list for private mirror support - Register `PackageSwiftParser` in `create_resolvers`; expose `swift_pm_parser_from` and `swift_pm_registry_from` helpers - Add E2E test file, resolver unit tests, `host_from_url` extraction tests, and test helper wiring for the new registry type - Update README, CLAUDE.md, and ARCHITECTURE.md to document the new registry
There was a problem hiding this comment.
Thanks for adding Swift Package Manager support! The overall quality of the implementation, tests, and docs is high. That said, there are two correctness concerns around version-constraint semantics (the handling of .upToNextMinor, and a caret mismatch for 0.x versions) — I left inline comments at the relevant spots. I also noted a few minor points and one question.
| let prefix = first_child_of_kind(call, "prefix_expression")?; | ||
| let ident_node = first_child_of_kind(prefix, "simple_identifier")?; | ||
| let ident = &content[ident_node.byte_range()]; | ||
| if ident != "upToNextMajor" && ident != "upToNextMinor" { |
There was a problem hiding this comment.
[Issue] .upToNextMinor is treated the same as .upToNextMajor
This branch distinguishes upToNextMajor / upToNextMinor, but the following extract_string_field only returns the bare version string from from: (e.g. "2.4.0"), so the information about which operator was used is discarded.
As a result, SwiftPmVersionMatcher evaluates both as a Cargo caret, making .upToNextMinor effectively behave like .upToNextMajor.
- Expected:
.upToNextMinor(from: "2.4.0")→>=2.4.0, <2.5.0 - Actual:
>=2.4.0, <3.0.0(misses minor-only updates)
SPM itself computes different upper bounds for the two (PackageRequirement.swift):
upToNextMajor(from:) -> version ..< Version(major + 1, 0, 0)
upToNextMinor(from:) -> version ..< Version(major, minor + 1, 0)For upToNextMinor, I'd suggest emitting a tilde-equivalent constraint (a ~ prefix, or a >=A, <major.(minor+1).0 range string) so the two are kept distinct.
| // SPM constraint versions are bare (e.g. `4.92.0`). Strip the `v` prefix | ||
| // from available versions so the Cargo matcher's semver comparison works. | ||
| let normalized = strip_v_prefix(available_versions); | ||
| CratesVersionMatcher.version_exists(version_spec, &normalized) |
There was a problem hiding this comment.
[Issue] caret semantics diverge between SPM and Cargo for 0.x versions
Delegating to CratesVersionMatcher does not match SPM, because Cargo's caret treats 0.x specially.
- SPM
from: "0.2.3"(=upToNextMajor) →>=0.2.3, <1.0.0 - Cargo caret
^0.2.3→>=0.2.3, <0.3.0
SPM computes the upper bound purely as major + 1, with no special-casing for 0.x:
upToNextMajor(from:) -> version ..< Version(major + 1, 0, 0)So for a 0.x package (common in the Swift ecosystem), when the latest is >= 0.3.0, it is compatible per SPM but this code falsely reports it as outdated. The compare_to_latest just below uses the same delegation and is affected the same way.
The robust fix is to normalize from: / upToNextMajor in the parser into an explicit >=A, <(major+1).0.0 range so it doesn't depend on Cargo's caret special-casing (the range expressions already work this way).
| "..." => "<=", | ||
| _ => return None, | ||
| }; | ||
| let version = format!(">={}, {}{}", lower, upper_op, upper); |
There was a problem hiding this comment.
[Minor] diagnostics show the internal representation instead of the user's original notation
The >=1.0.0, <5.0.0 produced here goes straight into version, so the diagnostic ends up as Update available: >=1.0.0, <5.0.0 -> 5.1.0 (see the e2e test half_open_range_warns_when_latest_exceeds_upper_bound).
What the user actually wrote is "1.0.0" ..< "5.0.0", so it would be friendlier to preserve the original source notation (e.g. in extra_info) for display. Not blocking.
|
|
||
| /// Extract the host portion of an HTTP(S) URL (no port handling required — | ||
| /// the hosts we compare against are bare hostnames like `github.example.com`). | ||
| fn host_from_url(url: &str) -> Option<String> { |
There was a problem hiding this comment.
[Minor] host-extraction logic is duplicated with the parser
This host_from_url (for the config URL) and split_host_and_path in package_swift.rs (for the dependency URL) share similar logic for stripping credentials (user@) and ports (:443). The purposes differ, but there's a risk of them drifting if only one is changed later. Consider extracting a shared helper, or noting the relationship in a comment (optional).
| Some(RegistryType::PyPI) | ||
| } else if is_compose_file(uri) { | ||
| Some(RegistryType::Docker) | ||
| } else if uri.ends_with("/Package.swift") { |
There was a problem hiding this comment.
[Minor / question]
Two things:
-
ends_with("/Package.swift")assumes a leading slash. This is consistent with the existing parsers, so it's fine, but a bare relativePackage.swiftwon't be matched (editors usually sendfile://URIs, so the practical impact is small). -
SwiftPM supports version-specific manifests (named like
Package@swift-5.9.swift). These aren't matched by the current check. I can't tell whether supporting them is worthwhile, so if you think it would improve the UX, it'd be great if you could handle it. If not, just calling it out as out of scope would be helpful.
Package.swiftparser using tree-sitter-swift; extractsfrom:,exact:,.upToNextMajor(from:),.upToNextMinor(from:), half-open (A ..< B), and closed (A ... B) range constraints; skips branch/revision pinsSwiftPmVersionMatcherreusing caret semantics for bare versions and evaluating compound ranges produced by the parserGitHubRegistryfor SPM version fetches; addwith_registry_type()builder so the same HTTP backend can reportRegistryType::SwiftPmkeeping cache namespaces separate from GitHub ActionsRegistryType::SwiftPmvariant withas_str/FromStrround-trip anddetect_parser_typerouting forPackage.swiftURIsregistries.swiftPmconfig block (enabled,url) intoLspConfig;swiftPm.urldoubles as a GitHub Enterprise API base URL override and adds the URL's bare host to the parser's allow-list for private mirror supportPackageSwiftParserincreate_resolvers; exposeswift_pm_parser_fromandswift_pm_registry_fromhelpershost_from_urlextraction tests, and test helper wiring for the new registry type