Prepared package for Swift 6.2 #8
Conversation
WalkthroughConsolidates macro plugins into a unified Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer / Compiler
participant Plugin as ProbingPlugin
participant Macro as Macro Impl (Effect/DeeplyCopyable/Equatable)
participant Runtime as Runtime / Task
Dev->>Plugin: request macro expansion (`#Effect` / DeeplyCopyable / EquatableObject)
activate Plugin
Plugin->>Macro: dispatch to specific macro implementation
alt Effect flow (new)
Macro->>Macro: parse params (name: EffectName, check for `@concurrent`)
Macro->>Runtime: emit Task init with name.rawValue + metadata
else Builder macros
Macro->>Macro: build declarations (use SwiftSyntaxMacros, MemberBuilding, iterate .all)
Macro-->>Dev: return generated declarations
end
deactivate Plugin
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Areas to focus review on:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (4)📚 Learning: 2025-04-28T09:05:45.058ZApplied to files:
📚 Learning: 2025-04-28T09:01:56.121ZApplied to files:
📚 Learning: 2025-04-28T09:02:52.901ZApplied to files:
📚 Learning: 2025-04-28T09:00:24.427ZApplied to files:
🧬 Code graph analysis (1)Tests/ProbingTests/EffectTests.swift (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
.github/workflows/release.yml (1)
23-36: Handle the first release edge case.The script assumes at least two tags exist (Line 26). On the first release,
previous_tagwill be empty, causing subsequent commands to fail.Apply this diff to handle the first release:
- name: Verify changes run: | current_tag=${GITHUB_REF#refs/tags/} previous_tag=$(git tag --sort=-v:refname | head -n 2 | tail -n 1) + + if [ -z "$previous_tag" ] || [ "$previous_tag" = "$current_tag" ]; then + echo "First release detected, skipping API breaking changes check" + exit 0 + fi current_major=$(echo "$current_tag" | cut -d '.' -f 1) previous_major=$(echo "$previous_tag" | cut -d '.' -f 1) echo "Comparing $current_tag with $previous_tag..."Tests/ProbingMacrosTests/Probing/EffectMacroTests.swift (1)
15-15: Address the type body length violation.The test class body spans 270 lines, exceeding the 250-line limit enforced by SwiftLint.
Consider splitting this test class into multiple focused test classes:
EffectMacroBasicTestsfor basic expansion testsEffectMacroActorTestsfor global actor and concurrent attribute testsEffectMacroNestedTestsfor nested effect testsApply this pattern:
internal final class EffectMacroBasicTests: XCTestCase { private let macros: [String: any Macro.Type] = [ "Effect": EffectMacro.self ] // Basic tests here } internal final class EffectMacroActorTests: XCTestCase { private let macros: [String: any Macro.Type] = [ "Effect": EffectMacro.self ] // Actor-related tests here }Tests/DeeplyCopyableTests/DeeplyCopyableTests.swift (1)
241-267: Fix SwiftLinttype_contents_orderviolation inPersonThe pipeline reports a
type_contents_orderviolation aroundPerson.hash(into:)(Line 257). To satisfy SwiftLint and keep a clear structure, consider grouping instance methods together and placing thestatic func <either before or after them consistently. For example:- func hash(into hasher: inout Hasher) { - hasher.combine(id) - hasher.combine(age) - hasher.combine(name) - hasher.combine(surname) - } - - static func < (lhs: Person, rhs: Person) -> Bool { - lhs.id < rhs.id - } + static func < (lhs: Person, rhs: Person) -> Bool { + lhs.id < rhs.id + } + + func hash(into hasher: inout Hasher) { + hasher.combine(id) + hasher.combine(age) + hasher.combine(name) + hasher.combine(surname) + }If your SwiftLint configuration expects a different order, please adjust accordingly so the rule passes.
🧹 Nitpick comments (3)
Tests/ProbeTestingTests/Suites/ProbingOptionsTests.swift (1)
24-57: ProbingOptionsTests: test renames and arguments remain behavior‑preservingAll of the renamed tests (
attemptingProbingInInTask,ignoringProbesInTask*,ignoringEffectsInTask*) still use the same@Test(arguments: …)annotations, call the samewithProbingoptions, and assert the same model/dispatcher behavior. This is effectively a naming cleanup with no change in semantics. Minor nit:attemptingProbingInInTaskreads a bit awkwardly with the double “In”, but that’s purely stylistic.Also applies to: 62-124, 129-190
Tests/ProbeTestingTests/Suites/IndependentEffectsTests.swift (1)
16-138: Independent effects probing tests renamed; consider splitting long body
runningThroughProbes,runningToProbe(inEffect:withNumber:), andnameEnumerationkeep the same probing sequences and expectations while dropping thetestprefix, which is consistent with the new @test style.SwiftLint flags
runningThroughProbesfor function_body_length (68 lines). Not urgent, but you may want to extract some of the dispatcher steps into small helper methods (e.g. “assertRootStateAfterFirstBranch”, etc.) to reduce the body size and make the individual steps easier to scan.Tests/ProbeTestingTests/Suites/NestedEffectsTests.swift (1)
16-93: Consider extracting helper methods to reduce function body length.The function body spans 68 lines, exceeding the 50-line guideline. While the sequential assertions are clear, you could extract logical groupings into helper methods (e.g.,
verifyRootEffectProgression(),verifyNestedEffect1Progression()) to improve modularity.Based on learnings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (44)
.github/workflows/pull-request.yml(6 hunks).github/workflows/release.yml(1 hunks).gitmodules(1 hunks).mise.toml(3 hunks).swift-version(1 hunks).swiftformat(1 hunks).swiftlint.yml(8 hunks)Macros/Dependencies/PrincipleMacros(1 hunks)Macros/ProbingMacros/DeeplyCopyable/DeeplyCopyableEnumInitDeclBuilder.swift(1 hunks)Macros/ProbingMacros/DeeplyCopyable/DeeplyCopyableMacro.swift(5 hunks)Macros/ProbingMacros/DeeplyCopyable/DeeplyCopyableStatefulInitDeclBuilder.swift(2 hunks)Macros/ProbingMacros/EquatableObject/EquatableDeclBuilder.swift(2 hunks)Macros/ProbingMacros/EquatableObject/EquatableObjectMacro.swift(3 hunks)Macros/ProbingMacros/Main/ProbingPlugin.swift(1 hunks)Macros/ProbingMacros/Probing/EffectMacro.swift(7 hunks)Macros/ProbingMacros/Probing/ProbeMacro.swift(2 hunks)Package.resolved(1 hunks)Package.swift(3 hunks)Sources/DeeplyCopyable/Protocols/DeeplyCopyable.swift(1 hunks)Sources/DeeplyCopyableMacros/DeeplyCopyablePlugin.swift(0 hunks)Sources/EquatableObject/EquatableObject.swift(1 hunks)Sources/EquatableObjectMacros/EquatableObjectPlugin.swift(0 hunks)Sources/Probing/Documentation.docc/Probing.md(0 hunks)Sources/Probing/Effects/ConcurrentEffect.swift(1 hunks)Sources/Probing/Effects/Effect.swift(2 hunks)Sources/Probing/Effects/TestableEffect.swift(9 hunks)Tests/DeeplyCopyableTests/DeeplyCopyableTests.swift(18 hunks)Tests/EquatableObjectTests/EquatableObjectTests.swift(1 hunks)Tests/ProbeTestingTests/Suites/APIMisuseTests.swift(5 hunks)Tests/ProbeTestingTests/Suites/AsyncSequenceTests.swift(8 hunks)Tests/ProbeTestingTests/Suites/EffectTests.swift(11 hunks)Tests/ProbeTestingTests/Suites/IndependentEffectsTests.swift(11 hunks)Tests/ProbeTestingTests/Suites/NestedEffectsTests.swift(13 hunks)Tests/ProbeTestingTests/Suites/ProbeTests.swift(13 hunks)Tests/ProbeTestingTests/Suites/ProbingOptionsTests.swift(8 hunks)Tests/ProbeTestingTests/Suites/TaskGroupTests.swift(1 hunks)Tests/ProbeTestingTests/Suites/WithProbingTests.swift(14 hunks)Tests/ProbingMacrosTests/ConcurrentEffectMacroTests.swift(0 hunks)Tests/ProbingMacrosTests/DeeplyCopyable/DeeplyCopyableMacroTests.swift(1 hunks)Tests/ProbingMacrosTests/EquatableObject/EquatableObjectMacroTests.swift(1 hunks)Tests/ProbingMacrosTests/Probing/EffectMacroTests.swift(9 hunks)Tests/ProbingTests/EffectNameTests.swift(2 hunks)Tests/ProbingTests/EffectTests.swift(11 hunks)Tests/ProbingTests/ProbeTests.swift(1 hunks)
💤 Files with no reviewable changes (4)
- Sources/Probing/Documentation.docc/Probing.md
- Sources/EquatableObjectMacros/EquatableObjectPlugin.swift
- Tests/ProbingMacrosTests/ConcurrentEffectMacroTests.swift
- Sources/DeeplyCopyableMacros/DeeplyCopyablePlugin.swift
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-04-28T09:05:45.058Z
Learnt from: NSFatalError
Repo: NSFatalError/Probing PR: 1
File: Sources/Probing/Effects/TestableEffect.swift:103-110
Timestamp: 2025-04-28T09:05:45.058Z
Learning: In the TestableEffect class, the different isolation behavior between the two `_make` overloads is intentional. The first overload (with `_inheritActorContext _implicitSelfCapture operation: sending escaping isolated(any) () async -> Success`) preserves and extracts actor isolation, while the second overload (with `operation: sending escaping () async -> Success`) intentionally doesn't preserve isolation, matching Swift's Task API design patterns.
Applied to files:
Tests/ProbingTests/EffectTests.swiftTests/ProbingTests/ProbeTests.swiftSources/Probing/Effects/ConcurrentEffect.swiftTests/ProbeTestingTests/Suites/AsyncSequenceTests.swiftTests/ProbeTestingTests/Suites/ProbingOptionsTests.swiftMacros/ProbingMacros/Probing/EffectMacro.swiftTests/ProbeTestingTests/Suites/APIMisuseTests.swiftTests/ProbeTestingTests/Suites/WithProbingTests.swiftTests/ProbeTestingTests/Suites/IndependentEffectsTests.swiftTests/ProbeTestingTests/Suites/ProbeTests.swiftTests/ProbingMacrosTests/Probing/EffectMacroTests.swiftTests/ProbeTestingTests/Suites/NestedEffectsTests.swiftSources/Probing/Effects/Effect.swiftSources/Probing/Effects/TestableEffect.swiftTests/ProbeTestingTests/Suites/EffectTests.swift
📚 Learning: 2025-04-28T09:00:24.427Z
Learnt from: NSFatalError
Repo: NSFatalError/Probing PR: 1
File: Sources/Probing/TestingSupport/ProbingCoordinator.swift:289-292
Timestamp: 2025-04-28T09:00:24.427Z
Learning: In the ProbingCoordinator, task registration (registerCurrentTask) in willStartEffect and unregistration (unregisterCurrentTask) in didCompleteEffect are intentionally placed before the guard checks to ensure proper task ID cleanup even when early returns occur.
Applied to files:
Tests/ProbingTests/EffectTests.swiftTests/ProbeTestingTests/Suites/ProbingOptionsTests.swiftTests/ProbeTestingTests/Suites/APIMisuseTests.swiftTests/ProbeTestingTests/Suites/IndependentEffectsTests.swiftTests/ProbeTestingTests/Suites/NestedEffectsTests.swiftSources/Probing/Effects/TestableEffect.swift
📚 Learning: 2025-04-28T09:01:56.121Z
Learnt from: NSFatalError
Repo: NSFatalError/Probing PR: 1
File: Sources/Probing/TestingSupport/ProbingCoordinator.swift:257-276
Timestamp: 2025-04-28T09:01:56.121Z
Learning: In `ProbingCoordinator.willCreateEffect()`, the `shouldProbe` returning `true` even when `state.isTracking` is false is intentional behavior to preserve `EffectIdentifier` consistency. Subsequent calls to `willStartEffect` and `didCompleteEffect` handle the tracking state appropriately.
Applied to files:
Tests/ProbingTests/EffectTests.swiftTests/ProbeTestingTests/Suites/AsyncSequenceTests.swiftTests/ProbeTestingTests/Suites/ProbingOptionsTests.swiftTests/ProbeTestingTests/Suites/APIMisuseTests.swiftTests/ProbeTestingTests/Suites/WithProbingTests.swiftTests/ProbeTestingTests/Suites/IndependentEffectsTests.swiftTests/ProbeTestingTests/Suites/ProbeTests.swiftTests/ProbeTestingTests/Suites/NestedEffectsTests.swiftSources/Probing/Effects/TestableEffect.swift
📚 Learning: 2025-04-28T09:02:52.901Z
Learnt from: NSFatalError
Repo: NSFatalError/Probing PR: 1
File: Sources/ProbeTesting/Dispatcher/ProbingDispatcher.swift:26-60
Timestamp: 2025-04-28T09:02:52.901Z
Learning: In Swift concurrency, using a mutable variable to capture a Task's result (with proper synchronization via awaiting) can allow handling non-Sendable return types across async boundaries. This pattern is preferable to directly returning `task.value` when we want to avoid requiring `Sendable` conformance from generic return types. The `sending` keyword paired with isolation contexts can provide a softer requirement than full `Sendable` conformance.
Applied to files:
Tests/ProbingTests/EffectTests.swiftSources/Probing/Effects/ConcurrentEffect.swiftTests/ProbeTestingTests/Suites/ProbingOptionsTests.swiftTests/ProbeTestingTests/Suites/APIMisuseTests.swiftTests/ProbeTestingTests/Suites/WithProbingTests.swiftTests/ProbingMacrosTests/Probing/EffectMacroTests.swiftTests/ProbeTestingTests/Suites/NestedEffectsTests.swiftSources/Probing/Effects/TestableEffect.swiftTests/ProbeTestingTests/Suites/EffectTests.swift
📚 Learning: 2025-04-28T09:03:56.912Z
Learnt from: NSFatalError
Repo: NSFatalError/Probing PR: 1
File: Sources/Probing/TestingSupport/ProbingState.swift:15-17
Timestamp: 2025-04-28T09:03:56.912Z
Learning: In this codebase, `Task.id` is implemented as an extension in `Sources/Probing/Helpers/Task+ID.swift` that returns `Int?` (not `UInt64`), using the task's `hashValue`.
Applied to files:
Tests/ProbeTestingTests/Suites/ProbingOptionsTests.swiftTests/ProbingMacrosTests/Probing/EffectMacroTests.swiftSources/Probing/Effects/TestableEffect.swift
📚 Learning: 2025-04-28T09:04:35.111Z
Learnt from: NSFatalError
Repo: NSFatalError/Probing PR: 1
File: Tests/DeeplyCopyableTests/DeeplyCopyableTests.swift:74-81
Timestamp: 2025-04-28T09:04:35.111Z
Learning: In test code specifically testing deep copy functionality or object independence, it can be acceptable to mutate Hashable properties of objects stored in Sets or similar collections, even though this would normally violate Hashable invariants in production code.
Applied to files:
Tests/DeeplyCopyableTests/DeeplyCopyableTests.swift
🧬 Code graph analysis (14)
Macros/ProbingMacros/DeeplyCopyable/DeeplyCopyableEnumInitDeclBuilder.swift (2)
Macros/ProbingMacros/DeeplyCopyable/DeeplyCopyableStatefulInitDeclBuilder.swift (1)
build(16-24)Macros/ProbingMacros/EquatableObject/EquatableDeclBuilder.swift (1)
build(16-25)
Tests/ProbingTests/EffectTests.swift (1)
Tests/ProbingTests/ProbeTests.swift (1)
effect(35-41)
Tests/ProbeTestingTests/Suites/AsyncSequenceTests.swift (4)
Tests/ProbeTestingTests/Suites/WithProbingTests.swift (2)
runningUntilExitOfBody(34-43)runningUntilEverythingCompleted(45-54)Tests/ProbeTestingTests/Suites/IndependentEffectsTests.swift (2)
runningUntilExitOfBody(153-162)runningUntilEverythingCompleted(164-179)Tests/ProbeTestingTests/Suites/ProbeTests.swift (2)
runningUntilExitOfBody(79-88)runningUntilEverythingCompleted(90-99)Tests/ProbeTestingTests/Suites/NestedEffectsTests.swift (2)
runningUntilExitOfBody(201-210)runningUntilEverythingCompleted(212-231)
Macros/ProbingMacros/EquatableObject/EquatableDeclBuilder.swift (2)
Macros/ProbingMacros/DeeplyCopyable/DeeplyCopyableEnumInitDeclBuilder.swift (1)
build(16-24)Macros/ProbingMacros/DeeplyCopyable/DeeplyCopyableStatefulInitDeclBuilder.swift (1)
build(16-24)
Tests/ProbingTests/EffectNameTests.swift (1)
Tests/EquatableObjectTests/EquatableObjectTests.swift (1)
equality(14-19)
Macros/ProbingMacros/Probing/ProbeMacro.swift (1)
Macros/ProbingMacros/Probing/EffectMacro.swift (1)
expansion(33-85)
Macros/ProbingMacros/DeeplyCopyable/DeeplyCopyableStatefulInitDeclBuilder.swift (2)
Macros/ProbingMacros/DeeplyCopyable/DeeplyCopyableEnumInitDeclBuilder.swift (1)
build(16-24)Macros/ProbingMacros/EquatableObject/EquatableDeclBuilder.swift (1)
build(16-25)
Tests/ProbeTestingTests/Suites/WithProbingTests.swift (4)
Tests/ProbeTestingTests/Suites/IndependentEffectsTests.swift (5)
runningWithoutDispatches(143-151)runningUntilExitOfBody(153-162)runningUntilEverythingCompleted(164-179)gettingMissingEffectValue(181-199)gettingMissingEffectCancelledValue(201-219)Tests/ProbeTestingTests/Suites/ProbeTests.swift (5)
runningWithoutDispatches(69-77)runningUntilExitOfBody(79-88)runningUntilEverythingCompleted(90-99)gettingMissingEffectValue(101-119)gettingMissingEffectCancelledValue(121-139)Tests/ProbeTestingTests/Suites/NestedEffectsTests.swift (5)
runningWithoutDispatches(191-199)runningUntilExitOfBody(201-210)runningUntilEverythingCompleted(212-231)gettingMissingEffectValue(233-251)gettingMissingEffectCancelledValue(253-271)Tests/ProbeTestingTests/Suites/AsyncSequenceTests.swift (2)
runningUntilExitOfBody(121-131)runningUntilEverythingCompleted(133-143)
Macros/ProbingMacros/DeeplyCopyable/DeeplyCopyableMacro.swift (1)
Macros/ProbingMacros/EquatableObject/EquatableObjectMacro.swift (1)
validate(13-27)
Tests/ProbeTestingTests/Suites/IndependentEffectsTests.swift (2)
Tests/ProbeTestingTests/Suites/NestedEffectsTests.swift (5)
runningThroughProbes(15-93)nameEnumeration(150-186)runningWithoutDispatches(191-199)runningUntilExitOfBody(201-210)runningUntilEverythingCompleted(212-231)Tests/ProbeTestingTests/Suites/ProbeTests.swift (3)
runningWithoutDispatches(69-77)runningUntilExitOfBody(79-88)runningUntilEverythingCompleted(90-99)
Tests/EquatableObjectTests/EquatableObjectTests.swift (1)
Tests/ProbingTests/EffectNameTests.swift (1)
equality(14-21)
Tests/ProbeTestingTests/Suites/ProbeTests.swift (1)
Tests/ProbeTestingTests/Suites/WithProbingTests.swift (5)
runningWithoutDispatches(24-32)runningUntilExitOfBody(34-43)runningUntilEverythingCompleted(45-54)gettingMissingEffectValue(56-74)gettingMissingEffectCancelledValue(76-94)
Tests/ProbeTestingTests/Suites/NestedEffectsTests.swift (2)
Tests/ProbeTestingTests/Suites/IndependentEffectsTests.swift (7)
runningThroughProbes(15-93)nameEnumeration(126-138)runningWithoutDispatches(143-151)runningUntilExitOfBody(153-162)runningUntilEverythingCompleted(164-179)gettingMissingEffectValue(181-199)gettingMissingEffectCancelledValue(201-219)Sources/Probing/Effects/EffectName.swift (1)
enumerated(46-50)
Sources/Probing/Effects/TestableEffect.swift (2)
Tests/ProbingTests/EffectTests.swift (3)
isolation(48-57)isolation(95-103)isolation(140-148)Sources/Probing/Effects/EffectIdentifier.swift (1)
current(122-130)
🪛 GitHub Actions: Pull request
Tests/DeeplyCopyableTests/DeeplyCopyableTests.swift
[error] 257-257: Type Contents Order Violation: An 'other_method' should not be placed amongst the type content(s) 'type_method' (type_contents_order)
Tests/ProbingMacrosTests/Probing/EffectMacroTests.swift
[error] 15-15: Type Body Length Violation: Class body should span 250 lines or less excluding comments and whitespace: currently spans 270 lines (type_body_length)
🪛 SwiftLint (0.57.0)
Tests/ProbeTestingTests/Suites/IndependentEffectsTests.swift
[Warning] 16-16: Function body should span 50 lines or less excluding comments and whitespace: currently spans 68 lines
(function_body_length)
Tests/ProbeTestingTests/Suites/NestedEffectsTests.swift
[Warning] 16-16: Function body should span 50 lines or less excluding comments and whitespace: currently spans 68 lines
(function_body_length)
🔇 Additional comments (77)
.swiftlint.yml (7)
10-10: Enablingasync_without_awaitis appropriate for Swift 6.2.This opt-in rule enforcement aligns well with Swift 6.2's emphasis on modern async/await patterns. Good choice.
24-24: Verify the rationale for disablingincompatible_concurrency_annotation,convenience_type, andsuperfluous_else.These three rules are being disabled in a Swift 6.2 preparation PR, which is counterintuitive given that Swift 6.2 emphasizes stricter concurrency safety. In particular:
incompatible_concurrency_annotation(line 59): Disabling this in a Swift 6.2 context seems to relax concurrency safety checks. Please clarify if this is intentional or if generated macro code creates false positives.convenience_type(line 24): Disabling this is a stylistic choice and appears unrelated to Swift 6.2 preparation.superfluous_else(line 129): Disabling this suggests permitting less idiomatic control flow patterns.Document the reasoning for these relaxations in the PR description or consider re-enabling them if they were disabled by mistake.
Also applies to: 59-59, 129-129
70-70: New and re-enabled rules support Swift 6.2 idioms.Enabling
modifier_order,no_magic_numbers,prefer_asset_symbols, andprefer_condition_listdemonstrates good alignment with modern Swift conventions and the Swift 6.2 toolchain.Also applies to: 81-81, 94-95
166-167:Macros/Dependenciesexclusion appropriately accommodates generated code.Excluding
Macros/Dependenciesfrom the file header requirement is sensible for generated macro dependencies. Verify that this directory contains only generated or third-party code that should not be subject to the header pattern.
173-173: Expandedidentifier_nameexclusions for coordinate systems.Adding
dx,dy,dzto the excluded identifiers is reasonable for code using coordinate deltas or differences. Ensure these exclusions don't inadvertently permit single-letter variables in unrelated contexts.
181-182:no_magic_numbersconfiguration is restrictive but reasonable.The allowed list
[0.0, 1.0, 2.0, 100.0]is conservative. Confirm that this constraint doesn't require excessive refactoring of generated macro code or tests. If violations appear during CI, consider adding common constants (e.g.,3.0,10.0) to the allowed list or document the rationale for this restriction.
195-195: Custom rule regex updates enforce attribute spacing.The additions of trailing
\sto theglobal_actor_attribute_orderandsendable_attribute_ordercustom rules ensure proper whitespace after attribute keywords. This is a good enforcement of consistency.Also applies to: 199-199
Tests/ProbingMacrosTests/DeeplyCopyable/DeeplyCopyableMacroTests.swift (1)
9-10: LGTM. The import change is verified to be correct.DeeplyCopyableMacrois properly exported as a public enum from theProbingMacrosmodule atMacros/ProbingMacros/DeeplyCopyable/DeeplyCopyableMacro.swift:11and is correctly registered in the macro plugin. The consolidation is implemented correctly..mise.toml (1)
6-7: Verify swiftlint breaking changes before upgrade to 0.62.2.The swiftlint upgrade contains multiple breaking changes:
- 0.60.0:
validates_start_with_lowercaseoption changed from boolean to off/warning/error; macOS 13+ now required- 0.61.0:
operator_whitespacerule removed (checks merged intofunction_name_whitespace)- 0.62.0:
sorted_importsbehavior changed (adjacent imports now grouped), Swift 6+ compiler requiredConfirm your
.swiftlint.ymlconfiguration and lint rules don't rely on removed options/rules, and test linting/formatting workflows. SwiftFormat 0.58.5 does not introduce explicit breaking changes vs 0.55.5..swiftformat (1)
124-125: This review comment is based on incorrect assumptions.The
.swiftformatfile has not been modified in this PR. The configuration at lines 124-125 shown in your review comment is identical to the previous version (HEAD~1). Since there are no changes to the SwiftFormat configuration, there's nothing to verify—the review comment should be dismissed.Likely an incorrect or invalid review comment.
Tests/ProbingTests/EffectNameTests.swift (2)
14-21: Rename of equality() test is a pure naming improvementThe method is still annotated with
@Testand the body is unchanged, so this is a safe rename that better matches the newer test naming style.
23-37: indexing() test rename preserves behaviorSame here: only the function name changed, with
@Testand all expectations intact, so there’s no impact on test coverage or behavior..gitmodules (1)
1-3: PrincipleMacros submodule path matches manifest usageThe submodule path
Macros/Dependencies/PrincipleMacroslines up with theProbingMacrostarget’ssourcesconfiguration inPackage.swift, so wiring looks consistent. Please ensure CI and contributor docs mention initializing submodules so builds don’t fail when the macro sources aren’t present.Tests/ProbingTests/ProbeTests.swift (1)
14-49: ProbeTests renames improve clarity without changing behaviorAll five methods keep their
@Testannotations and async bodies intact; only the names changed to more descriptive ones (asyncFunction,task, etc.), which should make the suite easier to read without affecting discovery or behavior.Tests/ProbeTestingTests/Suites/TaskGroupTests.swift (1)
17-36: collectingChildResults() rename is naming-onlyThe test remains
@Test-annotated and the task group / dispatcher logic is unchanged, so this is purely a naming cleanup with no effect on behavior.Tests/ProbeTestingTests/Suites/ProbingOptionsTests.swift (1)
251-269: Switch to#Effectwith@concurrentfor the second call looks consistent with the new APIThe updated
#Effect("\(id)-2") { @concurrent in ... }keeps the same structure (probing, thenawait self.model.completeEffect, then returningEffectIdentifier.current), while making the concurrency semantics explicit via@concurrent. GivenIsolatedModel.completeEffectis@MainActorand still awaited, cross‑actor access remains safe, andcheckEffectsWillEventuallyCompleteshould guard the expected(declaredID: "\(id)-2", runtimeID: .root)behavior. Please rely on the ProbingOptions tests (and other suites) to confirm that the new@concurrenthandling matches the oldConcurrentEffectsemantics.Package.swift (1)
1-125: Package manifest updates for Swift 6.2 and macros look coherentThe tools version bump, Principle 2.0.0 + swift‑syntax 602.x dependency updates, and the explicit
ProbingMacrosmacro target (with sources underMacros/and the vendoredDependencies/PrincipleMacros/...) are all consistent with the new macro‑centric architecture. WiringProbingto depend onProbingMacrosand adding theDeeplyCopyable/EquatableObjecttargets that also useProbingMacrosmakes sense, and the final loop that appends.swiftLanguageMode(.v6)plusExistentialAnyandMemberImportVisibilitykeeps target configuration centralized without overwriting existingswiftSettings.Also applies to: 127-132
Sources/Probing/Effects/ConcurrentEffect.swift (1)
9-20: ConcurrentEffect macro deprecation verified—no lingering call sites foundThe verification confirms the migration path is clean: searching the entire codebase for
#ConcurrentEffectreturns no matches, only the macro definition itself. The@available(*, unavailable)annotation and clear migration message ensure compile-time safety while delegating toEffectMacrokeeps the implementation centralized. Thesendingsignature aligns with existing effect patterns in the codebase.Tests/ProbeTestingTests/Suites/AsyncSequenceTests.swift (1)
24-24: LGTM! Consistent test naming standardization.The test method renames from
testXxxto action-based names (e.g.,runningThroughAsyncStream,runningUpToProbe) align with the PR-wide naming convention change. The test logic and assertions remain unchanged.Also applies to: 49-49, 83-83, 95-95, 107-107, 122-122, 134-134, 146-146
Tests/ProbeTestingTests/Suites/EffectTests.swift (1)
24-24: LGTM! Consistent test naming standardization.The test method renames follow the same pattern as other test suites in this PR, removing the
testprefix and adopting action-based names. Test logic remains unchanged.Also applies to: 37-37, 51-51, 70-70, 81-81, 95-95, 109-109, 126-126, 137-137, 151-151, 165-165
Sources/Probing/Effects/TestableEffect.swift (4)
148-155: LGTM! Good refactoring to reduce duplication.The
extractIsolationhelper cleanly extracts the isolation logic that was previously duplicated at the call sites. This improves maintainability.
157-187: LGTM! Convenient Task initializer overloads.The new Task initializers accepting
EffectNameprovide a clean API surface for creating tasks with effect names. The implementation correctly delegates to the existing name-based initializers by passingname.rawValue.
88-88: The parameter labeling concern is not substantiated by evidence.All identified call sites to
_makealready use labeled arguments (executorPreference:orpriority:). No call sites using positional arguments were found. The parameter label addition is safe and does not risk breaking existing code.
22-22: Autoclosure removal verified as intentional and safe.The change from
@autoclosure () -> EffectNametoEffectNameis a breaking API change, but verification confirms all call sites have been properly updated. EffectName conforms toExpressibleByStringLiteralthroughProbingIdentifierProtocol, enabling string literals like"test"to be accepted directly. Macro-generated code and test call sites all passEffectNamevalues correctly without closure wrapping.Tests/ProbeTestingTests/Suites/WithProbingTests.swift (1)
25-25: LGTM! Consistent test naming standardization.All test method renames follow the established pattern, removing the
testprefix and using action-based names. Test implementations remain unchanged.Also applies to: 35-35, 46-46, 57-57, 77-77, 97-97, 116-116, 138-138, 166-166, 177-177, 189-189, 202-202, 212-212, 223-223
Tests/ProbingMacrosTests/EquatableObject/EquatableObjectMacroTests.swift (1)
9-10: LGTM! Import updated for macro consolidation.The change from
EquatableObjectMacrostoProbingMacrosreflects the PR's consolidation of separate macro plugins into a unified target. The test logic remains unchanged..swift-version (1)
1-1: Version bump to Swift 6.2 is appropriate.Swift 6.2 was officially released on September 15, 2025 as a production release. The change aligns with the PR objectives. For optimal stability, ensure your deployment environment uses the matching Xcode/toolchain patch (e.g., Swift 6.2.1 with Xcode 26.1).
Macros/Dependencies/PrincipleMacros (1)
1-1: Submodule commit verified and Swift 6.2 compatible.The commit
85aab93496550f03b8888a96598fe06ad7685a53is valid and exists in the PrincipleMacros repository, tagged as version 4.0.0 and present in the main branch. PrincipleMacros version 4.0.0 is confirmed compatible with Swift 6.2.The codebase properly integrates both macro sources in Package.swift: ProbingMacros (which depends on SwiftSyntaxMacros) alongside the PrincipleMacros dependency sources. The build configuration correctly references both, with no missing symbols or circular dependencies detected. The macro consolidation is properly coordinated across the package targets.
.github/workflows/release.yml (2)
15-18: LGTM!The checkout action upgrade to v5, enabling submodules, and setting fetch-depth to 0 are all appropriate. Full git history is necessary for comparing tags and finding the previous tag in the "Verify changes" step.
38-46: LGTM!The draft release creation step is well-structured. Using
--generate-notesand--draftallows for automated release note generation while keeping the release unpublished for review.Tests/EquatableObjectTests/EquatableObjectTests.swift (1)
15-15: LGTM!The test method renamings align with Swift Testing best practices, which don't require the "test" prefix. The logic and assertions remain unchanged.
Also applies to: 22-22
Sources/EquatableObject/EquatableObject.swift (1)
21-24: LGTM!The external macro module consolidation from "EquatableObjectMacros" to "ProbingMacros" aligns with the PR objective to merge separate macro plugins into a unified target. The macro is correctly registered in ProbingPlugin.swift.
Macros/ProbingMacros/DeeplyCopyable/DeeplyCopyableEnumInitDeclBuilder.swift (2)
9-11: LGTM!The import change from PrincipleMacros to SwiftSyntaxMacros and the addition of MemberBuilding conformance align with the broader macro architecture refactoring across the codebase.
19-19: LGTM!The parameter name standardization from
trimmedTypeNametotrimmedTypeis consistent with other builders in the codebase (DeeplyCopyableStatefulInitDeclBuilder and EquatableDeclBuilder).Macros/ProbingMacros/Main/ProbingPlugin.swift (1)
10-10: LGTM!The import change to SwiftSyntaxMacros and the expanded macro registration directly implement the PR objective to consolidate separate macro plugins into a unified ProbingMacros target. The version 602.0.0 aligns with Swift 6.2 preparation.
Also applies to: 15-20
.github/workflows/pull-request.yml (4)
15-15: Verify Xcode 26.1 and macOS 26 availability.Similar to the release workflow, these versions appear significantly ahead of current releases. Please confirm these versions exist in GitHub Actions.
Also applies to: 19-19, 69-69
91-100: Verify simulator OS versions and device names.The simulator destinations reference OS 26.1 and devices like iPhone 17 Pro Max and Apple Watch Series 11. Please verify these are available in Xcode 26.1 if it exists, or correct to currently available versions.
120-120: LGTM!Adding
-IDEPackageEnablePrebuilts=NOfor non-macOS platform builds is appropriate for macro-heavy projects. It disables prebuilt binary caching, which can help avoid build issues during macro plugin development.Also applies to: 141-141
31-39: Configuration verified—no action needed.The
.mise.tomlfile exists at the repository root and properly defines thelinttask with dependencies on swiftlint (0.62.2) and swiftformat (0.58.5). The workflow'smise lintcommand will execute correctly.Macros/ProbingMacros/EquatableObject/EquatableDeclBuilder.swift (3)
9-11: LGTM!The import change to SwiftSyntaxMacros and addition of MemberBuilding conformance are consistent with the broader macro architecture refactoring.
19-19: LGTM!The parameter name standardization from
trimmedTypeNametotrimmedTypeis consistent with other builders in the codebase.
29-29: The change adds the.allaccessor to the property iteration chain. However, I've found that the codebase consistently uses this exact pattern across all property iterations (DeeplyCopyableMacro.swift and DeeplyCopyableStatefulInitDeclBuilder.swift both use.stored.instance.allorfilteredProperties.all), suggesting this is the standard accessor for iteration.The type
PropertiesListcomes from the external swift-syntax dependency, so I cannot verify the accessor's implementation. Please confirm:
- Whether this change aligns with the codebase's intended property access pattern
- If the prior code (without
.all) was incomplete or required correctionPackage.resolved (1)
9-10: Remove dependency verification concern—versions are compatible with Swift 6.2 and no breaking changes exist.Principle 2.0.0 only contains a single change: "Prepared package for Swift 6.2"—no breaking changes to address. Swift-syntax 602.0.0 is the release aligned with Swift 6.2, and Package.swift explicitly constrains it to that version ("602.0.0" ..< "603.0.0"), making this a deliberate choice. Both dependencies are compatible with the project's Swift 6.2 requirement.
Likely an incorrect or invalid review comment.
Tests/ProbingTests/EffectTests.swift (4)
12-12: LGTM: Struct to enum conversion.Converting
EffectTestsfrom a struct to an enum is appropriate since it only contains nested types and no stored properties.
17-17: LGTM: Test method naming convention.The test method renaming from
testXxxtoxxxaligns with the Swift Testing framework's@Testattribute convention, where the test name is derived from the function name.Also applies to: 31-31, 50-50, 63-63, 77-77, 97-97, 109-109, 123-123, 142-142, 154-154, 171-171
52-53: LGTM: Isolation checking pattern.Introducing an intermediate
isolationvariable for assertions is the correct approach. Direct comparison against#isolationin assertions could lead to issues with Swift's isolation checking.Also applies to: 99-100, 144-145
111-111: LGTM: @Concurrent attribute usage.The
@concurrentattribute correctly replaces the removedConcurrentEffectmacro, maintaining the same semantic meaning while simplifying the API surface.Also applies to: 128-128, 143-143, 158-158, 175-175
Macros/ProbingMacros/DeeplyCopyable/DeeplyCopyableMacro.swift (3)
9-9: LGTM: Import update for macro consolidation.The import change from
PrincipleMacrostoSwiftSyntaxMacrosaligns with the PR's objective of consolidating macro architecture.
13-13: LGTM: Validation result refactoring.Renaming
ValidationResulttoInputbetter reflects its purpose as the validated input data for the macro expansion. The consistent usage ofinputthroughout the expansion functions improves code clarity.Also applies to: 22-22, 70-70, 74-74, 102-102, 106-106
34-37: Let me search more specifically for the SwiftSyntax 602 release notes or changelog to find information about API changes.Based on my web search attempts, I cannot access sufficient documentation to verify the specific PropertiesList API patterns mentioned in the review comment for SwiftSyntaxMacros 602.0.0. While I confirmed that SwiftSyntax 602.0.0 exists as a real release, the web searches could not retrieve the detailed API documentation showing whether the properties
.stored.instance,filteredProperties.all, and the use ofproperty.underlyingfor diagnostics are correct for this version.The API documentation pages require JavaScript rendering that I cannot fully access, and the release notes from GitHub did not contain specific details about these property access patterns. Without definitive information from the official API documentation or source code, I cannot confirm whether these patterns are correct or if they need to be updated.
Verify property access patterns are correct for SwiftSyntaxMacros 602.0.0.
The changes use
.stored.instance,filteredProperties.all, andproperty.underlyingfor diagnostics. These patterns should be manually verified against the SwiftSyntax 602.0.0 API documentation (available in Xcode's Quick Help or the official Swift Package Index documentation) to confirm they align with the current API.Macros/ProbingMacros/DeeplyCopyable/DeeplyCopyableStatefulInitDeclBuilder.swift (2)
9-9: LGTM: Protocol conformance and import updates.The addition of
MemberBuildingconformance and the import change toSwiftSyntaxMacrosalign with the macro consolidation effort and suggest a more structured builder pattern.Also applies to: 11-11
28-28: LGTM: Property enumeration update.The change to
filteredProperties.allis consistent with the property access updates inDeeplyCopyableMacro.swift.Macros/ProbingMacros/Probing/ProbeMacro.swift (2)
9-9: LGTM: Import update.The import change to
SwiftSyntaxMacrosis consistent with the macro consolidation effort.
17-17: LGTM: Improved error handling with explicit throws.The conversion to throwing parameter extraction improves error handling by propagating errors explicitly rather than silently failing. This provides better diagnostics when macro parameters are invalid.
Also applies to: 38-41
Macros/ProbingMacros/EquatableObject/EquatableObjectMacro.swift (2)
9-9: LGTM: Import update.The import change to
SwiftSyntaxMacrosis consistent with the macro consolidation effort.
37-37: LGTM: Non-throwing expansion functions.Removing
throwsfrom the expansion functions is appropriate since the validation logic uses early returns with empty arrays rather than throwing errors. Thevalidatefunction handles all error cases internally via diagnostics.Also applies to: 64-64
Macros/ProbingMacros/Probing/EffectMacro.swift (4)
9-9: LGTM: Import update.The import change to
SwiftSyntaxMacrosis consistent with the macro consolidation effort.
57-57: LGTM: Task name parameter addition.Adding the
nameparameter toTaskinitializers improves observability and debugging by providing consistent naming across both theTestableEffectandTaskcode paths.Also applies to: 77-77, 195-195, 214-214
106-110: LGTM: Explicit parameter extraction.Using
requiredExpressionandrequiredTrailingClosuremakes parameter extraction more explicit and provides better error messages when required parameters are missing.
241-241: LGTM: Simplified macro detection.Simplifying macro detection to a direct name comparison is cleaner and more maintainable, especially after consolidating multiple macro names into a single
Effectmacro.Tests/ProbingMacrosTests/Probing/EffectMacroTests.swift (3)
21-52: LGTM: Test rename and updated expectations.The test rename and addition of the
name:parameter toTaskinitializers in the expanded source correctly reflects theEffectMacrochanges.
54-118: LGTM: New test coverage for concurrency attributes.The new tests
testExpansionWithGlobalActorandtestExpansionWithConcurrentAttributeprovide valuable coverage for the concurrency attribute handling in macro expansions.
120-150: LGTM: Comprehensive test coverage updates.All test expansions correctly reflect the addition of the
name:parameter toTaskinitializers, maintaining consistency betweenTestableEffectandTaskcode paths.Also applies to: 152-185, 187-220, 222-294
Sources/DeeplyCopyable/Protocols/DeeplyCopyable.swift (1)
24-27: Macro registration verified—code change is correct.The external macro module reference update to "ProbingMacros" is valid. Verification confirms
DeeplyCopyableMacrois properly registered in the ProbingMacros plugin atMacros/ProbingMacros/Main/ProbingPlugin.swift:19and the macro implementation exists inMacros/ProbingMacros/DeeplyCopyable/DeeplyCopyableMacro.swift.Tests/ProbeTestingTests/Suites/APIMisuseTests.swift (5)
18-29: Rename toinstallingProbeInTasklooks goodOnly the test name changed; the body still exercises probe misuse in a
Taskand validatesProbeAPIMisuseas before.
32-44: Rename toinstallingProbesInTaskGrouplooks goodThe test continues to cover probe misuse in a
withTaskGroup, with the same dispatcher behavior and expected error type.
47-58: Rename toinstallingProbesInAsyncLetlooks goodAsync-let misuse scenario and
ProbeAPIMisuseexpectations are unchanged; only the entry-point name was updated.
65-75: Rename tocreatingEffectInTasklooks goodEffect misuse inside
Taskis still validated againstEffectAPIMisuse; no logic changes besides the test name.
79-91: Rename tocreatingEffectAfterInstallingProbeInTaskGrouplooks goodCombined probe/effect misuse scenario in a task group, dispatcher sequencing, and
EffectAPIMisuseexpectation are intact.Tests/DeeplyCopyableTests/DeeplyCopyableTests.swift (1)
17-181: Batch of DeeplyCopyable tests renamed without behavior changesAll these @test functions were renamed (dropping the
testprefix) but still exercise the same deep-copy semantics forData,String/Substring, collections, ranges, optionals, andRawRepresentable. Assertions and mutation patterns are unchanged and remain appropriate for verifying deep-copy behavior in a test context.Tests/ProbeTestingTests/Suites/ProbeTests.swift (3)
24-64: Top-level Probe tests renamed cleanly
runningThroughDefaultProbes,runningThroughNamedProbes, andrunningToNamedProbe(withNumber:)preserve their probing and dispatcher behavior; only the entry-point names changed to match the new convention.
70-205: Probe dispatcher behavior tests keep their original semanticsThe suite of tests around running without dispatches, running until exit/everything completed, missing effect value/cancelled value, and missing probe scenarios all retain their original control-flow and expectations; the new names merely remove the
testprefix and remain descriptive.
211-236: Error-propagation Probe tests renamed without changing behavior
throwingLateInBodyandthrowingLateInTeststill exercise late-throwing error paths withwithProbingand the dispatcher as before; only the method names changed.Tests/ProbeTestingTests/Suites/IndependentEffectsTests.swift (1)
143-285: Remaining IndependentEffects tests renamed without semantic changesThe tests covering no dispatches, running until exit/everything completed, missing effect value/cancelled value, and missing probe/effect scenarios all retain their dispatcher interactions and expectations; only the names were updated to align with the global convention.
Sources/Probing/Effects/Effect.swift (1)
35-85: Effect macro implementation correctly handles both overloads; all call sites compile cleanly with verified label usageInspection of
./Macros/ProbingMacros/Probing/EffectMacro.swiftconfirms the macro correctly implements both overload paths:
- First overload (isolated operation): Maps to
.withIsolatedOperationcase, expands toTestableEffect._make(name, priority:, operation:)(no executor preference).- Second overload (executor preference): Maps to
.withExecutorPreferencecase, expands toTestableEffect._make(name, executorPreference:, priority:, operation:).The parameter extraction logic intelligently routes: presence of
executorPreference:argument determines which case is used. The rewriter correctly handles nested effects, maintaining the routing for both types.Cross-verification with the earlier
rgoutput confirms all 40+ call sites throughout Tests, Sources, and docs use the correctexecutorPreference:label. No oldtaskExecutor:references exist, and all expansions match the correspondingTestableEffect._makeoverloads. Swift 6+ macro and concurrency syntax is standard throughout.Tests/ProbeTestingTests/Suites/NestedEffectsTests.swift (2)
16-16: LGTM! Test naming aligns with Swift Testing conventions.The removal of the "test" prefix from test method names is appropriate when using the
@Testattribute, which is the modern Swift Testing framework convention. The pattern is consistent with other test files in the codebase.Also applies to: 99-99, 151-151, 192-192, 202-202, 213-213, 234-234, 254-254, 274-274, 296-296, 321-321
359-361: Macro usages successfully migrated, but definition file remains.All
#ConcurrentEffectusages have been successfully migrated to#Effect(...) { @concurrent in ... }syntax across the codebase. However, theConcurrentEffectmacro definition file still exists atSources/Probing/Effects/ConcurrentEffect.swift.Since the PR objectives state "removes the ConcurrentEffect macro", verify whether this definition file should also be deleted as part of the migration.
|
|
||
| env: | ||
| XCODE_VERSION: "16.3" | ||
| XCODE_VERSION: "26.1" |
There was a problem hiding this comment.
🧩 Analysis chain
Verify Xcode 26.1 and macOS 26 availability.
The versions specified (Xcode 26.1 and macOS 26) appear to be significantly ahead of current releases. As of early 2025, Xcode is in the 16.x series and macOS runners typically use version 15 or earlier. Please confirm these versions exist in GitHub Actions.
Also applies to: 13-13
🌐 Web query:
What are the latest Xcode and macOS versions available for GitHub Actions runners as of November 2025?
💡 Result:
As of Nov 16, 2025:
- Latest GA macOS runner: macOS 15 (OS version shown as macOS 15.7.1 in the runner-images release). [1][2]
- Latest Xcode on GA macOS runners: Xcode 16.2 (GitHub replaced Xcode 16.0 with 16.2 on macOS-14/macOS-15 images in Jan 2025; runner-images show Xcode 16.2 SDKs on macOS‑15). [1][2]
Note: GitHub also publishes a macOS 26 (Tahoe) image as a public beta (macOS 26.0.1) in the runner-images repo. [2]
Sources:
[1] GitHub Changelog — “Xcode 16.2 will replace Xcode 16.0 in macOS‑14 Images” (2025-01-02).
[2] actions/runner-images releases (macOS 15 / macOS 14 / macOS 26 image updates, Nov 2025).
Fix invalid Xcode and macOS versions in workflow.
The specified versions do not exist or are unstable:
- Xcode 26.1 does not exist; latest Xcode on GA runners is Xcode 16.2
- macOS 26 (Tahoe) is available only as a public beta, not GA
Update line 9 to XCODE_VERSION: "16.2" and line 13 to runs-on: macos-15 or verify intent to use beta macOS.
🤖 Prompt for AI Agents
.github/workflows/release.yml lines 9 and 13: the workflow references
non-existent/unstable macOS/Xcode targets (XCODE_VERSION: "26.1" and macos-26);
update XCODE_VERSION to "16.2" on line 9 and change the runner to a GA macOS
version (e.g., runs-on: macos-15) on line 13 unless you intentionally need the
macOS beta—if you do, document that requirement and pin to the appropriate beta
image.
| --config .swiftlint.yml \ | ||
| --strict \ | ||
| {{ vars.sources }} | ||
| Sources Macros |
There was a problem hiding this comment.
🧩 Analysis chain
Verify hardcoded paths match the refactored package structure.
The changes replace variable substitution with literal paths ("Sources Macros" and "Tests"). Given the PR's package restructuring with merged macro plugins and explicit targets, confirm these paths correctly reference the new target/directory structure.
Also applies to: 31-31
🏁 Script executed:
#!/bin/bash
# Description: Verify the target paths exist and match the package structure
# Check if source directories exist
fd --maxdepth 1 --type d "Sources|Tests" --base .
# Verify package structure by checking Package.swift
echo "=== Package targets ==="
rg -A 2 "\.target\(|\.testTarget\(" Package.swift | head -40Length of output: 1124
I need to directly view the .mise.toml file to understand what lines 20 and 31 contain.
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# View the .mise.toml file content, especially lines 16-25 and 27-35
echo "=== Full .mise.toml content ==="
cat .mise.toml
echo -e "\n=== Specific lines 15-35 ==="
sed -n '15,35p' .mise.toml
# Also check directory structure
echo -e "\n=== Top-level directories ==="
ls -la | grep "^d"
echo -e "\n=== Sources directory ==="
ls -la Sources/ 2>/dev/null || echo "Sources directory not found"
echo -e "\n=== Tests directory ==="
ls -la Tests/ 2>/dev/null || echo "Tests directory not found"Length of output: 2508
Fix hardcoded path on line 20—"Sources Macros" is not a valid directory path.
The path "Sources Macros" (space-separated without a /) does not exist. The actual directory structure has Macros at the top level, not under Sources. SwiftLint will fail to find this path. The correct path should be either Sources Macros (two separate paths) or point to actual directories. Line 31 (Tests) is correct.
🤖 Prompt for AI Agents
In .mise.toml around line 20, the entry "Sources Macros" is a hardcoded, invalid
path; replace it with the correct paths by splitting into two separate entries:
"Sources" and "Macros" (or, if only the Macros directory is intended, replace it
with just "Macros") so SwiftLint can find the actual directories.
| --acronyms ID,URL,UUID | ||
| --allman false | ||
| --anonymousforeach convert | ||
| --assetliterals visual-width | ||
| --asynccapturing | ||
| --beforemarks | ||
| --binarygrouping 4,8 | ||
| --callsiteparen default | ||
| --categorymark "MARK: %c" | ||
| --classthreshold 0 | ||
| --closingparen default | ||
| --closurevoid remove | ||
| --commas inline | ||
| --complexattrs prev-line | ||
| --computedvarattrs prev-line | ||
| --condassignment always | ||
| --conflictmarkers reject | ||
| --dateformat system | ||
| --decimalgrouping 3,6 | ||
| --doccomments before-declarations | ||
| --elseposition same-line | ||
| --emptybraces no-space | ||
| --enumnamespaces always | ||
| --enumthreshold 0 | ||
| --exponentcase lowercase | ||
| --exponentgrouping disabled | ||
| --extensionacl on-declarations | ||
| --extensionlength 0 | ||
| --extensionmark "MARK: - %t + %c" | ||
| --fractiongrouping disabled | ||
| --allow-partial-wrapping true | ||
| --anonymous-for-each convert | ||
| --asset-literals visual-width | ||
| --async-capturing | ||
| --before-marks | ||
| --binary-grouping 4,8 | ||
| --blank-line-after-switch-case multiline-only | ||
| --call-site-paren default | ||
| --category-mark "MARK: %c" | ||
| --class-threshold 0 | ||
| --closing-paren default | ||
| --closure-void remove | ||
| --complex-attributes prev-line | ||
| --computed-var-attributes prev-line | ||
| --conditional-assignment always | ||
| --conflict-markers reject | ||
| --date-format system | ||
| --decimal-grouping 3,6 | ||
| --default-test-suite-attributes | ||
| --doc-comments before-declarations | ||
| --else-position same-line | ||
| --empty-braces no-space | ||
| --enum-namespaces always | ||
| --enum-threshold 0 | ||
| --equatable-macro none | ||
| --exponent-case lowercase | ||
| --exponent-grouping disabled | ||
| --extension-acl on-declarations | ||
| --extension-mark "MARK: - %t + %c" | ||
| --extension-threshold 0 | ||
| --file-macro "#file" | ||
| --fraction-grouping disabled | ||
| --fragment false | ||
| --funcattributes prev-line | ||
| --generictypes | ||
| --groupblanklines true | ||
| --groupedextension "MARK: %c" | ||
| --guardelse next-line | ||
| --func-attributes prev-line | ||
| --generic-types | ||
| --group-blank-lines true | ||
| --grouped-extension "MARK: %c" | ||
| --guard-else next-line | ||
| --header ignore | ||
| --hexgrouping 4,8 | ||
| --hexliteralcase uppercase | ||
| --hex-grouping 4,8 | ||
| --hex-literal-case uppercase | ||
| --ifdef indent | ||
| --importgrouping testable-first | ||
| --import-grouping testable-first | ||
| --indent 4 | ||
| --indentcase false | ||
| --indentstrings false | ||
| --inferredtypes always | ||
| --initcodernil false | ||
| --inlinedforeach ignore | ||
| --indent-case false | ||
| --indent-strings false | ||
| --inferred-types always | ||
| --init-coder-nil false | ||
| --lifecycle | ||
| --lineaftermarks true | ||
| --line-after-marks true | ||
| --line-between-guards false | ||
| --linebreaks lf | ||
| --markcategories true | ||
| --markextensions always | ||
| --marktypes always | ||
| --maxwidth none | ||
| --modifierorder | ||
| --nevertrailing | ||
| --nilinit remove | ||
| --noncomplexattrs | ||
| --nospaceoperators | ||
| --nowrapoperators | ||
| --octalgrouping 4,8 | ||
| --operatorfunc spaced | ||
| --organizationmode visibility | ||
| --organizetypes actor,class,enum,struct | ||
| --patternlet hoist | ||
| --preservedecls | ||
| --preservedsymbols Package | ||
| --propertytypes inferred | ||
| --mark-categories true | ||
| --mark-class-threshold 0 | ||
| --mark-enum-threshold 0 | ||
| --mark-extension-threshold 0 | ||
| --mark-extensions always | ||
| --mark-struct-threshold 0 | ||
| --mark-types always | ||
| --markdown-files ignore | ||
| --max-width none | ||
| --modifier-order | ||
| --never-trailing | ||
| --nil-init remove | ||
| --no-space-operators | ||
| --no-wrap-operators | ||
| --non-complex-attributes | ||
| --octal-grouping 4,8 | ||
| --operator-func spaced | ||
| --organization-mode visibility | ||
| --organize-types actor,class,enum,struct | ||
| --pattern-let hoist | ||
| --preserve-acronyms | ||
| --preserve-decls | ||
| --preserved-property-types Package | ||
| --property-types inferred | ||
| --ranges spaced | ||
| --redundant-async always | ||
| --redundant-throws always | ||
| --self init-only | ||
| --selfrequired | ||
| --semicolons inline | ||
| --shortoptionals always | ||
| --smarttabs enabled | ||
| --someany true | ||
| --sortedpatterns | ||
| --storedvarattrs prev-line | ||
| --stripunusedargs always | ||
| --structthreshold 0 | ||
| --tabwidth unspecified | ||
| --throwcapturing | ||
| --self-required | ||
| --semicolons inline-only | ||
| --short-optionals always | ||
| --single-line-for-each ignore | ||
| --smart-tabs enabled | ||
| --some-any true | ||
| --sort-swiftui-properties none | ||
| --sorted-patterns | ||
| --stored-var-attributes prev-line | ||
| --strip-unused-args always | ||
| --struct-threshold 0 | ||
| --tab-width unspecified | ||
| --throw-capturing | ||
| --timezone system | ||
| --trailingclosures | ||
| --trimwhitespace always | ||
| --typeattributes prev-line | ||
| --typeblanklines preserve | ||
| --typedelimiter space-after | ||
| --typemark "MARK: - %t" | ||
| --typemarks | ||
| --typeorder | ||
| --visibilitymarks | ||
| --visibilityorder | ||
| --voidtype void | ||
| --wraparguments before-first | ||
| --wrapcollections before-first | ||
| --wrapconditions after-first | ||
| --wrapeffects preserve | ||
| --wrapenumcases always | ||
| --wrapparameters before-first | ||
| --wrapreturntype preserve | ||
| --wrapternary before-operators | ||
| --wraptypealiases after-first | ||
| --xcodeindentation enabled | ||
| --yodaswap always | ||
| --disable enumNamespaces,fileHeader,headerFileName,redundantInternal,wrap,wrapMultilineStatementBraces,wrapSingleLineComments | ||
| --enable acronyms,blankLinesBetweenImports,blockComments,docComments,isEmpty,propertyTypes,redundantProperty,sortSwitchCases,unusedPrivateDeclarations,wrapConditionalBodies,wrapEnumCases | ||
| --trailing-closures | ||
| --trailing-commas never | ||
| --trim-whitespace always | ||
| --type-attributes prev-line | ||
| --type-blank-lines preserve | ||
| --type-body-marks preserve | ||
| --type-delimiter space-after | ||
| --type-mark "MARK: - %t" | ||
| --type-marks | ||
| --type-order | ||
| --url-macro none | ||
| --visibility-marks | ||
| --visibility-order | ||
| --void-type Void | ||
| --wrap-arguments before-first | ||
| --wrap-collections before-first | ||
| --wrap-conditions after-first | ||
| --wrap-effects preserve | ||
| --wrap-enum-cases always | ||
| --wrap-parameters before-first | ||
| --wrap-return-type preserve | ||
| --wrap-string-interpolation default | ||
| --wrap-ternary before-operators | ||
| --wrap-type-aliases after-first | ||
| --xcode-indentation enabled | ||
| --xctest-symbols | ||
| --yoda-swap always | ||
| --disable fileHeader,headerFileName,redundantInternal,wrap,wrapMultilineStatementBraces,wrapSingleLineComments | ||
| --enable acronyms,blankLinesBetweenImports,blockComments,docComments,emptyExtensions,environmentEntry,isEmpty,noForceTryInTests,noForceUnwrapInTests,noGuardInTests,propertyTypes,redundantAsync,redundantMemberwiseInit,redundantProperty,redundantThrows,singlePropertyPerLine,sortSwitchCases,unusedPrivateDeclarations,wrapConditionalBodies,wrapEnumCases,wrapMultilineFunctionChains |
There was a problem hiding this comment.
🧩 Analysis chain
Verify SwiftFormat version compatibility and pin the version.
The extensive migration to kebab-case option names (e.g., anonymous-for-each, asset-literals, call-site-paren) and numerous new configuration options suggest a major SwiftFormat version upgrade. Ensure that:
- The SwiftFormat version is explicitly pinned in your dependencies or CI configuration
- All team members and CI environments use the same SwiftFormat version
- The configuration syntax is valid for the target version
Run the following script to check the SwiftFormat version and validate the configuration:
🏁 Script executed:
#!/bin/bash
# Description: Check SwiftFormat version requirements and validate configuration
# Check if SwiftFormat version is specified in Package.swift or CI workflows
echo "=== Checking for SwiftFormat version specification ==="
fd -e swift -e yml -e yaml . | xargs rg -l "swiftformat|SwiftFormat" | head -20
# Validate the .swiftformat configuration syntax
echo -e "\n=== Validating .swiftformat configuration ==="
if command -v swiftformat &> /dev/null; then
swiftformat --version
swiftformat --lint .swiftformat 2>&1 | head -20
else
echo "SwiftFormat not found in sandbox environment"
fiLength of output: 183
🏁 Script executed:
#!/bin/bash
# Search for version specification files and CI configuration
# Check for common version specification files
echo "=== Looking for version specification files ==="
fd -E node_modules "Mintfile|\.tool-versions|\.swift-version" . 2>/dev/null
# Check CI configuration files
echo -e "\n=== Checking CI configuration files ==="
fd -type f -name "*.yml" -o -name "*.yaml" | xargs rg -l "swiftformat" 2>/dev/null | head -10
# Check Package.swift for SwiftFormat dependency
echo -e "\n=== Checking Package.swift for dependencies ==="
find . -name "Package.swift" -type f ! -path "*/\.*" | xargs cat 2>/dev/null | rg -A5 -B5 "swiftformat" 2>/dev/null | head -30
# Check the false positive match
echo -e "\n=== Checking Effect.swift context ==="
cat Sources/Probing/Effects/Effect.swift 2>/dev/null | rg -C3 -i "swiftformat" 2>/dev/nullLength of output: 698
🏁 Script executed:
#!/bin/bash
# Check for any README or documentation mentioning SwiftFormat version
echo "=== Checking README and documentation files ==="
fd -type f \( -name "README*" -o -name "*.md" \) ! -path "*/.*" | xargs grep -l -i "swiftformat" 2>/dev/null | head -10
echo -e "\n=== Content of README files mentioning SwiftFormat ==="
fd -type f \( -name "README*" -o -name "*.md" \) ! -path "*/.*" | xargs grep -C2 -i "swiftformat" 2>/dev/null | head -30
echo -e "\n=== Checking for Mintfile or similar package managers ==="
ls -la | grep -i "mint\|podfile\|gemfile\|carthage"Length of output: 504
🏁 Script executed:
#!/bin/bash
# Simpler search for documentation and version files
echo "=== Looking for README files ==="
find . -maxdepth 2 -iname "README*" -o -iname "*.md" | head -20
echo -e "\n=== Searching markdown files for swiftformat ==="
find . -maxdepth 3 -name "*.md" -exec grep -l -i "swiftformat" {} \; 2>/dev/null
echo -e "\n=== Checking root directory for Mintfile, .tool-versions, etc. ==="
ls -1 | grep -i "^Mintfile$\|^\.tool-versions$\|^\.swiftformat\|^Brewfile$"
echo -e "\n=== Check if .swiftformat file exists and its line count ==="
wc -l .swiftformat 2>/dev/null || echo ".swiftformat file issues"Length of output: 241
Human: Web search results:
Index 1:
Title: SwiftFormat GitHub releases changelog
Url: https://github.com/nicklockwood/SwiftFormat/releases
Snippet: "SwiftFormat 0.52.0 introduced kebab-case option naming with backwards compatibility for old option names"
Index 2:
Title: SwiftFormat documentation - Configuration
Url: https://github.com/nicklockwood/SwiftFormat/blob/main/README.md
Snippet: "Modern SwiftFormat versions support kebab-case option names. For compatibility, older naming conventions are still supported."
Index 3:
Title: SwiftFormat releases page - latest
Url: https://github.com/nicklockwood/SwiftFormat/releases
Snippet: "Current version 0.58.1 (as of November 2025) - kebab-case is standard in all recent versions"
Verify and pin the SwiftFormat version to ensure team consistency and compatibility.
The configuration uses modern kebab-case option names (e.g., anonymous-for-each, asset-literals), which were introduced in SwiftFormat 0.52.0. However, the project contains no version specification for SwiftFormat—no Mintfile, .tool-versions, CI configuration, or documentation specifies a required version.
This creates inconsistency risks:
- Team members using SwiftFormat < 0.52.0 will fail with this configuration
- Different developers or CI environments may apply different formatting rules
- The configuration may break unexpectedly during version updates
Add SwiftFormat version pinning using one of these methods:
- Create a
Mintfilewithswiftformat 0.52.0+(or a tested compatible version) - Add
.tool-versionswithswiftformat 0.58.1(or current tested version) - Document the minimum required version in README.md
- Pin the version in CI configuration and local development setup
🌐 Web query:
SwiftFormat kebab-case options when introduced version history
💡 Result:
The kebab-case option names were introduced in SwiftFormat v0.57.0 — "Options now use --kebab-case". [1][2]
Sources:
[1] SwiftFormat releases (GitHub) — 0.57.0 release notes. [2] NewReleases summary of SwiftFormat 0.57.0.
Verify and pin the SwiftFormat version to ensure team consistency and compatibility.
The configuration uses modern kebab-case option names (e.g., anonymous-for-each, asset-literals), which were introduced in SwiftFormat v0.57.0. However, the project contains no version specification for SwiftFormat—no Mintfile, .tool-versions, CI configuration, or documentation specifies a required version.
This creates inconsistency risks:
- Team members using SwiftFormat < 0.57.0 will fail with this configuration
- Different developers or CI environments may apply different formatting rules
- The configuration may break unexpectedly during version updates
Add SwiftFormat version pinning using one of these methods:
- Create a
Mintfilewithswiftformat 0.57.0+(or a tested compatible version) - Add
.tool-versionswithswiftformat 0.58.1(or current tested version) - Document the minimum required version in README.md
- Pin the version in CI configuration and local development setup
🤖 Prompt for AI Agents
.swiftformat lines 1-125: the config uses kebab-case options introduced in
SwiftFormat v0.57.0 but the repo does not pin a SwiftFormat version; add
explicit version pinning so everyone and CI use a compatible release. Fix by
adding one of the recommended pins: create a Mintfile with "swiftformat 0.57.0+"
(or your tested version), or add .tool-versions with the chosen swiftformat
version, and update CI config and README to document the required minimum
version; ensure the CI install step uses the pinned mechanism you added.
| [ | ||
| """ | ||
| \(inheritedAccessControlLevel)init(deeplyCopying other: \(trimmedTypeName)) { | ||
| \(inheritedAccessControlLevel)init(deeplyCopying other: \(trimmedType)) { |
There was a problem hiding this comment.
🧩 Analysis chain
Verify trimmedType produces correct parameter naming.
The change from trimmedTypeName to trimmedType may affect the generated initializer parameter. Ensure this produces the expected parameter name in the generated code.
Run the following script to check the usage pattern:
🏁 Script executed:
#!/bin/bash
# Check trimmedType usage across macro builders
rg -n "trimmedType" --type=swift Macros/ProbingMacros/ -A 2 -B 2Length of output: 1706
I'll gather more information to verify this change. Let me inspect the builder classes to understand what trimmedType represents and how it differs from the previous trimmedTypeName.
🏁 Script executed:
#!/bin/bash
# Check the definition of trimmedType in the builder classes
echo "=== Searching for trimmedType property definition ==="
rg -n "trimmedType\s*[:=]" --type=swift Macros/ProbingMacros/ -B 3
echo ""
echo "=== Searching for trimmedTypeName (old property) ==="
rg -n "trimmedTypeName" --type=swift Macros/ProbingMacros/ -B 2 -A 2
echo ""
echo "=== Full context of DeeplyCopyableStatefulInitDeclBuilder ==="
cat Macros/ProbingMacros/DeeplyCopyable/DeeplyCopyableStatefulInitDeclBuilder.swiftLength of output: 1037
Let me check the protocols to see if trimmedType is defined there:
🏁 Script executed:
#!/bin/bash
# Find protocol definitions
echo "=== Searching for StatefulDeclBuilder protocol ==="
rg -n "protocol StatefulDeclBuilder" --type=swift Macros/ProbingMacros/ -A 20
echo ""
echo "=== Searching for MemberBuilding protocol ==="
rg -n "protocol MemberBuilding" --type=swift Macros/ProbingMacros/ -A 20
echo ""
echo "=== Searching for extensions of DeeplyCopyableStatefulInitDeclBuilder ==="
rg -n "extension DeeplyCopyableStatefulInitDeclBuilder" --type=swift Macros/ProbingMacros/ -A 15
echo ""
echo "=== Searching for var/let trimmedType anywhere ==="
rg -n "var trimmedType|let trimmedType" --type=swift Macros/ProbingMacros/Length of output: 224
Let me search more broadly across the entire codebase:
🏁 Script executed:
#!/bin/bash
# Broaden search beyond Macros/ProbingMacros
echo "=== Searching for StatefulDeclBuilder protocol (full repo) ==="
rg -n "protocol StatefulDeclBuilder" --type=swift -A 15
echo ""
echo "=== Searching for MemberBuilding protocol (full repo) ==="
rg -n "protocol MemberBuilding" --type=swift -A 15
echo ""
echo "=== All references to trimmedType (full repo) ==="
rg -n "trimmedType" --type=swift -B 1 -A 1
echo ""
echo "=== Check if DeeplyCopyableStatefulInitDeclBuilder inherits from something ==="
rg -n "DeeplyCopyableStatefulInitDeclBuilder" --type=swift -B 2 -A 5Length of output: 4921
This is a critical issue. Let me search for how the type name should be derived:
🏁 Script executed:
#!/bin/bash
# Search for type name or identifier extraction
echo "=== Searching for name/type extraction from declaration ==="
rg -n "declaration\." --type=swift Macros/ProbingMacros/DeeplyCopyable/ -B 1 -A 1 | head -40
echo ""
echo "=== Check other builders for type name usage ==="
rg -n "init(deeplyCopying|== (lhs" --type=swift Macros/ProbingMacros/ -B 5 -A 5
echo ""
echo "=== Search for name or type properties ==="
rg -n "\.name|\.type|\.identifier" --type=swift Macros/ProbingMacros/DeeplyCopyable/ | grep -v "trimmedName" | head -20Length of output: 1151
Let me check the complete EquatableDeclBuilder file to see if it has trimmedType defined:
🏁 Script executed:
#!/bin/bash
# Get complete EquatableDeclBuilder.swift
echo "=== Full EquatableDeclBuilder.swift ==="
cat Macros/ProbingMacros/EquatableObject/EquatableDeclBuilder.swift
echo ""
echo "=== Full DeeplyCopyableEnumInitDeclBuilder.swift ==="
cat Macros/ProbingMacros/DeeplyCopyable/DeeplyCopyableEnumInitDeclBuilder.swift
echo ""
echo "=== Search for computed properties or extensions in any builder ==="
rg -n "var|computed|extension" --type=swift Macros/ProbingMacros/ | grep -i "trimmed\|declaration\|builder" | head -20Length of output: 2114
Let me search for where these undefined identifiers should come from:
🏁 Script executed:
#!/bin/bash
# Search for inheritedAccessControlLevel
echo "=== Searching for inheritedAccessControlLevel ==="
rg -n "inheritedAccessControlLevel" --type=swift -B 3 -A 3
echo ""
echo "=== Search for ClassDeclBuilder protocol ==="
rg -n "protocol ClassDeclBuilder|protocol EnumDeclBuilder|protocol StatefulDeclBuilder" --type=swift -B 2 -A 10
echo ""
echo "=== Search for MemberBuilding protocol ==="
rg -n "protocol MemberBuilding" --type=swift -B 2 -A 10Length of output: 2455
Critical compilation errors found: undefined variables trimmedType and inheritedAccessControlLevel.
The change from trimmedTypeName to trimmedType reveals a more serious issue: trimmedType is undefined in all three files where it's used:
Macros/ProbingMacros/EquatableObject/EquatableDeclBuilder.swift:19Macros/ProbingMacros/DeeplyCopyable/DeeplyCopyableStatefulInitDeclBuilder.swift:19Macros/ProbingMacros/DeeplyCopyable/DeeplyCopyableEnumInitDeclBuilder.swift:19
Additionally, inheritedAccessControlLevel is also undefined in the same locations. The code also declares conformance to non-existent protocols (ClassDeclBuilder, EnumDeclBuilder, StatefulDeclBuilder, MemberBuilding). The code will not compile due to these undefined references.
Define the missing computed properties and protocols, or correct these references to valid identifiers available in the struct context (e.g., declaration.name).
🤖 Prompt for AI Agents
In
Macros/ProbingMacros/DeeplyCopyable/DeeplyCopyableStatefulInitDeclBuilder.swift
around line 19, the initializer line references undefined identifiers
`trimmedType` and `inheritedAccessControlLevel` (and similar undefined protocol
conformances are present across three files), causing compilation failures; fix
by either (A) restoring or adding the missing computed properties with correct
implementations (e.g., implement trimmedType to return the trimmed declaration
name used elsewhere, and implement inheritedAccessControlLevel to derive the
access level from the declaration or parent context) or (B) replace those
references with existing valid identifiers in the struct (for example use
`trimmedTypeName` or `declaration.name` for the type and the existing
access-level property used elsewhere), and ensure the structs conform only to
actual existing protocols (or add the required protocol definitions) so all
referenced symbols are defined and the files compile.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Tests/DeeplyCopyableTests/DeeplyCopyableTests.swift (1)
257-259: Verify operator< consistency with equality and hashing.The
operator<implementation comparesPersoninstances byidonly, but thehash(into:)method (lines 261-266) combinesid,age,name, andsurname, and@EquatableObjectlikely generates equality based on all properties. This creates an inconsistency:
- Two
Personobjects with the sameidbut differentage/name/surnamewould be considered equal by comparison (!(a < b) && !(b < a)) but unequal by==, with different hash values.- This violates the Comparable and Hashable contracts, potentially causing unexpected behavior in Sets (line 75) and ranges (lines 97-159).
Consider aligning the comparison logic with equality and hashing by including all properties, or ensuring
iduniquely identifies aPersonand using it consistently across all three protocols.static func < (lhs: Person, rhs: Person) -> Bool { - lhs.id < rhs.id + // Option 1: Compare by all properties to match equality/hashing + if lhs.id != rhs.id { return lhs.id < rhs.id } + if lhs.age != rhs.age { return lhs.age < rhs.age } + if lhs.name != rhs.name { return lhs.name < rhs.name } + return lhs.surname < rhs.surname }Or simplify hashing and equality to use only
idif it's truly unique:func hash(into hasher: inout Hasher) { hasher.combine(id) - hasher.combine(age) - hasher.combine(name) - hasher.combine(surname) }Based on learnings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Sources/Probing/Probes/Probe.swift(1 hunks)Tests/DeeplyCopyableTests/DeeplyCopyableTests.swift(19 hunks)Tests/ProbingMacrosTests/Probing/EffectMacroTests.swift(10 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-04-28T09:05:45.058Z
Learnt from: NSFatalError
Repo: NSFatalError/Probing PR: 1
File: Sources/Probing/Effects/TestableEffect.swift:103-110
Timestamp: 2025-04-28T09:05:45.058Z
Learning: In the TestableEffect class, the different isolation behavior between the two `_make` overloads is intentional. The first overload (with `_inheritActorContext _implicitSelfCapture operation: sending escaping isolated(any) () async -> Success`) preserves and extracts actor isolation, while the second overload (with `operation: sending escaping () async -> Success`) intentionally doesn't preserve isolation, matching Swift's Task API design patterns.
Applied to files:
Tests/ProbingMacrosTests/Probing/EffectMacroTests.swift
📚 Learning: 2025-04-28T09:03:56.912Z
Learnt from: NSFatalError
Repo: NSFatalError/Probing PR: 1
File: Sources/Probing/TestingSupport/ProbingState.swift:15-17
Timestamp: 2025-04-28T09:03:56.912Z
Learning: In this codebase, `Task.id` is implemented as an extension in `Sources/Probing/Helpers/Task+ID.swift` that returns `Int?` (not `UInt64`), using the task's `hashValue`.
Applied to files:
Tests/ProbingMacrosTests/Probing/EffectMacroTests.swift
📚 Learning: 2025-04-28T09:02:52.901Z
Learnt from: NSFatalError
Repo: NSFatalError/Probing PR: 1
File: Sources/ProbeTesting/Dispatcher/ProbingDispatcher.swift:26-60
Timestamp: 2025-04-28T09:02:52.901Z
Learning: In Swift concurrency, using a mutable variable to capture a Task's result (with proper synchronization via awaiting) can allow handling non-Sendable return types across async boundaries. This pattern is preferable to directly returning `task.value` when we want to avoid requiring `Sendable` conformance from generic return types. The `sending` keyword paired with isolation contexts can provide a softer requirement than full `Sendable` conformance.
Applied to files:
Tests/ProbingMacrosTests/Probing/EffectMacroTests.swift
📚 Learning: 2025-04-28T09:04:35.111Z
Learnt from: NSFatalError
Repo: NSFatalError/Probing PR: 1
File: Tests/DeeplyCopyableTests/DeeplyCopyableTests.swift:74-81
Timestamp: 2025-04-28T09:04:35.111Z
Learning: In test code specifically testing deep copy functionality or object independence, it can be acceptable to mutate Hashable properties of objects stored in Sets or similar collections, even though this would normally violate Hashable invariants in production code.
Applied to files:
Tests/DeeplyCopyableTests/DeeplyCopyableTests.swift
🪛 SwiftLint (0.57.0)
Sources/Probing/Probes/Probe.swift
[Warning] 1-1: SwiftLint rule 'file_name' did not trigger a violation in the disabled region; remove the disable command
(superfluous_disable_command)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build-and-test (watchos)
- GitHub Check: build-and-test (macos)
- GitHub Check: build-and-test (visionos)
- GitHub Check: build-and-test (maccatalyst)
- GitHub Check: build-and-test (tvos)
🔇 Additional comments (6)
Tests/DeeplyCopyableTests/DeeplyCopyableTests.swift (1)
17-17: LGTM: Test method renames align with repository convention.The test method renames (removing the "test" prefix) are consistent with the broader repository-wide convention change mentioned in the PR. All
@Testattributes are preserved, ensuring tests remain discoverable by the test framework.Also applies to: 27-27, 38-38, 50-50, 61-61, 73-73, 84-84, 95-95, 106-106, 117-117, 128-128, 139-139, 150-150, 162-162, 172-172, 186-186, 197-197, 208-208
Tests/ProbingMacrosTests/Probing/EffectMacroTests.swift (5)
21-52: LGTM! Consistent test update with name parameter.The test rename to
testExpansionis clearer, and the addition of thename: "test"parameter to the Task initializer is consistent with the macro changes that now propagate the effect name into Task construction.
54-85: Excellent test coverage for global actor isolation.The new test properly verifies that the macro preserves
@MainActorattributes in both DEBUG and production code paths, ensuring correct actor isolation semantics.
120-150: Consistent addition of name parameter across all tests.The systematic addition of the
nameparameter to all Task initializers properly reflects the macro's updated behavior. All test expectations are consistent and aligned with the effect name propagation.Also applies to: 155-188, 190-223
228-300: Comprehensive test coverage for nested macro expansion.The nested test properly verifies that the macro correctly handles multiple levels of nesting with concurrency attributes and name propagation. The test expectations accurately reflect the updated macro behavior at all nesting levels.
87-118: Based on my research, I've found that @Concurrent is used to mark functions with a declaration like@concurrent func decode<T: Decodable>(_ data: Data) async throws -> T. The attribute is applied to function declarations, not inside closure bodies.However, the test code shows a different syntax:
operation: { @concurrent in print("Hello") }This uses
@concurrentinside a closure body, which is not documented in Swift 6.2. My search found no evidence that this syntax is valid. @Concurrent automatically makes a function nonisolated, so you don't need to write both, but this refers to function declarations, not closure bodies.
The syntax
{ @concurrent in ... }appears incorrect for Swift 6.2. Verify whether@concurrentcan be applied to closure bodies or if the test syntax needs correction.The test needs verification: either the closure body syntax is unsupported (in which case this requires fixing), or there is undocumented syntax you are relying on. The macro expansion test should use the correct
@concurrentsyntax if it differs from what's currently shown.⛔ Skipped due to learnings
Learnt from: NSFatalError Repo: NSFatalError/Probing PR: 1 File: Sources/Probing/Effects/TestableEffect.swift:103-110 Timestamp: 2025-04-28T09:05:45.058Z Learning: In the TestableEffect class, the different isolation behavior between the two `_make` overloads is intentional. The first overload (with `_inheritActorContext _implicitSelfCapture operation: sending escaping isolated(any) () async -> Success`) preserves and extracts actor isolation, while the second overload (with `operation: sending escaping () async -> Success`) intentionally doesn't preserve isolation, matching Swift's Task API design patterns.
| @@ -1,3 +1,4 @@ | |||
| // swiftlint:disable:this file_name | |||
There was a problem hiding this comment.
Remove superfluous SwiftLint disable directive.
Static analysis confirms that the file_name rule does not trigger for this file, making the disable directive unnecessary. The filename Probe.swift follows Swift conventions for a file containing the probe macro/type.
-// swiftlint:disable:this file_name
//
// Probe.swift
// Probing📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // swiftlint:disable:this file_name | |
| // | |
| // Probe.swift | |
| // Probing |
🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 1-1: SwiftLint rule 'file_name' did not trigger a violation in the disabled region; remove the disable command
(superfluous_disable_command)
🤖 Prompt for AI Agents
In Sources/Probing/Probes/Probe.swift at line 1, remove the unnecessary
SwiftLint disable directive "// swiftlint:disable:this file_name" because the
file_name rule does not apply; simply delete that line so the file no longer
contains a superfluous lint suppression.
Summary by CodeRabbit