-
Notifications
You must be signed in to change notification settings - Fork 2
Added Memoized macro #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Warning Rate limit exceeded@NSFatalError has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 24 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
WalkthroughProject renamed and restructured from Publishable → Relay; added Memoized macro and AccessControlLevel; replaced legacy macro builders with new Relay macro implementations and plugin; removed old Publishable sources; updated package, submodule, CI/tooling pins and runners; added docs and extensive macro and integration tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Fn as Method
participant Macro as `@Memoized`
participant Builder as MemoizedDeclBuilder
participant Cache as Generated _property
participant Obs as Observation System
Fn->>Macro: Annotated with `@Memoized`
Macro->>Builder: validate signature & request peer generation
Builder->>Cache: emit cache storage + computed property
Cache->>Obs: wrap computation with observation tracking
Obs-->>Cache: on change → invalidate cache
Cache-->>Fn: return cached or recomputed result
sequenceDiagram
autonumber
participant Class as Annotated Class (`@Publishable`)
participant Macro as PublishableMacro
participant PB as PropertyPublisherDeclBuilder
participant OR as ObservationRegistrarDeclBuilder
participant Pub as Generated PropertyPublisher
Class->>Macro: Attach `@Publishable`
Macro->>PB: Generate PropertyPublisher + publisher member
Macro->>OR: Generate ObservationRegistrar and publish routing
PB->>Pub: create publishers for stored/computed/memoized properties
OR->>Pub: provide willSet/didSet/access/withMutation implementations
Class-->>Pub: gains `.publisher` and registrar-backed behavior
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas needing focused review:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 21
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
README.md (2)
158-158: Update documentation link to reflect repository rename.The documentation link still references the old
NSFatalError/Publishablerepository path.-[Full documentation is available on the Swift Package Index.](https://swiftpackageindex.com/NSFatalError/Publishable/documentation/publishable) +[Full documentation is available on the Swift Package Index.](https://swiftpackageindex.com/NSFatalError/Relay/documentation/relay)
163-166: Update installation package URL to reflect repository rename.The installation example still references the old
Publishablerepository URL. Update it to point to the newRelayrepository:.package( - url: "https://github.com/NSFatalError/Publishable", + url: "https://github.com/NSFatalError/Relay", from: "1.0.0" ).swiftlint.yml (1)
156-164: Update file header pattern to reflect package rename to "Relay".The
file_headerpattern on line 160 still references "Publishable", but the PR is renaming the package to "Relay". This will cause new/modified files to fail header validation.Apply this diff to update the file header pattern:
file_header: required_pattern: | // // SWIFTLINT_CURRENT_FILENAME - // Publishable + // Relay // // Created by .+ on \d{2}/\d{2}/\d{4}\. // Copyright © \d{4} .+\. All rights reserved\. //Sources/Relay/Imports.swift (1)
1-7: Address the file header violation.The pipeline reports that header comments should be consistent with project patterns. Please ensure the header format matches the project's linting rules.
Sources/Relay/Publishable/SwiftObservationRegistrar.swift (1)
1-7: Address the file header violation.The pipeline reports that header comments should be consistent with project patterns. Please ensure the header format matches the project's linting rules.
Sources/Relay/Publishable/AnyPropertyPublisher.swift (1)
1-7: Update.swiftlint.ymlto reflect the project rename from "Publishable" to "Relay"The
file_headerrule in.swiftlint.ymlexpects "Publishable" on line 3 of all Swift files, but the project has been renamed to "Relay". All 27 Swift files now use "Relay" in their headers, causing SwiftLint violations throughout the codebase. Update therequired_patternin.swiftlint.ymlto replace "Publishable" with "Relay" to resolve this issue and allow CI to pass.Tests/RelayTests/Publishable/SwiftDataPublishableTests.swift (1)
2-231: Tests look consistent with the other Publishable suites; fix header for SwiftLint.The switch to
observationsQueue: [Bool]withappend(true)inonChangeand the updated expectations (== true/== nil) match the pattern used in the other Publishable tests. The@Publishable @Model final class Persondefinition also aligns with how the macros are exercised elsewhere.The only actionable issue is the
file_headerSwiftLint error on this file. Please update the header comment to match your existing convention (copying from a passing test file is usually safest) so the CI stops failing.Tests/RelayTests/Publishable/AnyPropertyPublisherTests.swift (1)
3-208: Bool‑based observation queues look good; fix the header for SwiftLint.Switching
observationsQueuefrom[Void]to[Bool]and updating theonChangeclosures/expectations keeps the test behavior the same while making it more explicit when an observation occurred. The patterns in all four tests align with the other Publishable test suites.The outstanding issue is the
file_headerSwiftLint violation; please adjust this file’s header comment to match your standard Relay test header so the pipeline passes.Sources/RelayMacros/Publishable/PublishableMacro.swift (1)
3-121: Publishable macro wiring is cohesive; just fix the header to satisfy SwiftLint.The
PublishableMacroimplementation is well‑structured:
validateenforces that the macro is only applied to final@Observable/@Modelclasses.- The
MemberMacroexpansion shares parsed properties and the preferred global actor acrossPublisherDeclBuilder,PropertyPublisherDeclBuilder, andObservationRegistrarDeclBuilder, which keeps the generatedpublisher,PropertyPublisher, and registrar coherent.- The
ExtensionMacroexpansion reuses the same validation and builds a@GlobalActor Publishableconformance that matches the actor isolation resolved for the class.I don’t see functional issues here.
The remaining blocker is the
file_headerSwiftLint error; please update this file’s header comment to match your existing Relay macro files so the CI check passes.
🧹 Nitpick comments (5)
Sources/Relay/Publishable/AnyPropertyPublisher.swift (1)
49-61: Consider guarding against unbalanced begin/end modificationsThe new logic (send
willChangewhenpendingModificationsbecomes 1, anddidChangewhen it’s 1 on end) nicely models outermost-change semantics. The only risk is that an extraendModifications()call would drivependingModificationsnegative.If you want extra robustness, you could add an assertion/guard to prevent underflow, e.g. early‑exit or
precondition(pendingModifications > 0)before decrementing..swiftformat (1)
1-125: Ensure CI uses a SwiftFormat version that understands these optionsThis config switches to newer kebab‑case option names and enables a broad set of rules (via
--disable/--enable). It looks internally consistent, but older SwiftFormat versions will silently ignore or misinterpret unknown flags. Please confirm that your local and CI SwiftFormat versions are new enough to support all these options.Package.swift (1)
7-21: Package manifest and target wiring look consistentThe rename to
Relay, target graph, and updatedPrincipleMacros/swift-syntaxranges all look coherent with the macro + test targets and the new Swift 6 settings. No functional issues spotted here. You may optionally address the SwiftLint hints about explicit ACL and top‑level constant naming if you want a fully clean lint run.Also applies to: 34-47, 49-75, 78-85
Sources/RelayMacros/Publishable/PublisherDeclBuilder.swift (1)
11-15: Consider removing or using the unusedpropertiesfield
propertiesis stored but never read. If it’s not needed by this builder, consider removing it from the type and its initializer call sites to avoid confusion; otherwise, add a brief comment indicating its intended future use.Tests/RelayTests/Memoized/ObservationMemoizedTests.swift (1)
12-375: Observation‑based memoized tests look correct; other SwiftLint warnings are optionalThe Independent/Dependent groups thoroughly cover access, tracking, and async observation of memoized
baseAreaandvolume, and they match the patterns in the other memoized test suites. The additional SwiftLint warnings (magic numbers, function body length) are expected in assertion‑heavy tests and don’t indicate correctness issues—treat them as optional cleanups if you want a zero‑warning test suite.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (45)
.github/workflows/pull-request.yml(4 hunks).github/workflows/release.yml(1 hunks).mise.toml(1 hunks).spi.yml(1 hunks).swift-version(1 hunks).swiftformat(1 hunks).swiftlint.yml(8 hunks)Package.resolved(1 hunks)Package.swift(4 hunks)README.md(3 hunks)Sources/Publishable/Documentation.docc/Publishable.md(0 hunks)Sources/Publishable/PropertyPublisher/Publishable.swift(0 hunks)Sources/Publishable/Registrars/PublishableObservationRegistrar.swift(0 hunks)Sources/PublishableMacros/Builders/ObservationRegistrarDeclBuilder.swift(0 hunks)Sources/PublishableMacros/Builders/PropertyPublisherDeclBuilder.swift(0 hunks)Sources/PublishableMacros/Builders/PublisherDeclBuilder.swift(0 hunks)Sources/Relay/Documentation.docc/MemoizedMacros.md(1 hunks)Sources/Relay/Documentation.docc/PublishableMacros.md(1 hunks)Sources/Relay/Documentation.docc/Relay.md(1 hunks)Sources/Relay/Imports.swift(1 hunks)Sources/Relay/Memoized/Memoized.swift(1 hunks)Sources/Relay/Publishable/AnyPropertyPublisher.swift(3 hunks)Sources/Relay/Publishable/Publishable.swift(1 hunks)Sources/Relay/Publishable/PublishableObservationRegistrar.swift(1 hunks)Sources/Relay/Publishable/SwiftObservationRegistrar.swift(1 hunks)Sources/RelayMacros/Main/RelayPlugin.swift(2 hunks)Sources/RelayMacros/Memoized/MemoizedDeclBuilder.swift(1 hunks)Sources/RelayMacros/Memoized/MemoizedMacro.swift(1 hunks)Sources/RelayMacros/Publishable/ObservationRegistrarDeclBuilder.swift(1 hunks)Sources/RelayMacros/Publishable/PropertyPublisherDeclBuilder.swift(1 hunks)Sources/RelayMacros/Publishable/PublishableMacro.swift(5 hunks)Sources/RelayMacros/Publishable/PublisherDeclBuilder.swift(1 hunks)Tests/RelayMacrosTests/Memoized/MainActorMemoizedMacroTests.swift(1 hunks)Tests/RelayMacrosTests/Memoized/MemoizedMacroTests.swift(1 hunks)Tests/RelayMacrosTests/Publishable/MainActorPublishableMacroTests.swift(1 hunks)Tests/RelayMacrosTests/Publishable/PublishableMacroTests.swift(5 hunks)Tests/RelayTests/Helpers/Array+PopFirst.swift(1 hunks)Tests/RelayTests/Memoized/MainActorMemoizedTests.swift(1 hunks)Tests/RelayTests/Memoized/ObservationMemoizedTests.swift(1 hunks)Tests/RelayTests/Memoized/PublishableMemoizedTests.swift(1 hunks)Tests/RelayTests/Memoized/SwiftDataMemoizedTests.swift(1 hunks)Tests/RelayTests/Publishable/AnyPropertyPublisherTests.swift(13 hunks)Tests/RelayTests/Publishable/MainActorPublishableTests.swift(1 hunks)Tests/RelayTests/Publishable/ObservationPublishableTests.swift(14 hunks)Tests/RelayTests/Publishable/SwiftDataPublishableTests.swift(14 hunks)
💤 Files with no reviewable changes (6)
- Sources/Publishable/Registrars/PublishableObservationRegistrar.swift
- Sources/PublishableMacros/Builders/PropertyPublisherDeclBuilder.swift
- Sources/PublishableMacros/Builders/ObservationRegistrarDeclBuilder.swift
- Sources/PublishableMacros/Builders/PublisherDeclBuilder.swift
- Sources/Publishable/Documentation.docc/Publishable.md
- Sources/Publishable/PropertyPublisher/Publishable.swift
🧰 Additional context used
🧬 Code graph analysis (17)
Sources/RelayMacros/Publishable/PublisherDeclBuilder.swift (3)
Sources/RelayMacros/Publishable/PropertyPublisherDeclBuilder.swift (1)
build(17-32)Sources/RelayMacros/Memoized/MemoizedDeclBuilder.swift (1)
build(21-43)Sources/RelayMacros/Publishable/ObservationRegistrarDeclBuilder.swift (1)
build(21-41)
Tests/RelayTests/Publishable/SwiftDataPublishableTests.swift (2)
Tests/RelayTests/Publishable/MainActorPublishableTests.swift (8)
storedProperty(16-54)computedProperty(56-99)willChange(104-151)observe(28-34)observe(68-74)observe(116-125)observe(165-174)didChange(153-200)Tests/RelayTests/Helpers/Array+PopFirst.swift (1)
popFirst(11-16)
Tests/RelayTests/Publishable/AnyPropertyPublisherTests.swift (4)
Tests/RelayTests/Helpers/Array+PopFirst.swift (1)
popFirst(11-16)Tests/RelayTests/Publishable/MainActorPublishableTests.swift (1)
storedProperty(16-54)Tests/RelayTests/Publishable/ObservationPublishableTests.swift (1)
storedProperty(15-53)Tests/RelayTests/Publishable/SwiftDataPublishableTests.swift (1)
storedProperty(16-54)
Sources/RelayMacros/Publishable/PublishableMacro.swift (1)
Sources/RelayMacros/Memoized/MemoizedMacro.swift (1)
validate(19-77)
Sources/RelayMacros/Memoized/MemoizedMacro.swift (1)
Sources/RelayMacros/Memoized/MemoizedDeclBuilder.swift (1)
build(21-43)
Tests/RelayTests/Memoized/MainActorMemoizedTests.swift (2)
Tests/RelayTests/Memoized/ObservationMemoizedTests.swift (14)
access(16-48)access(168-207)trackWhenCached(50-78)trackWhenCached(209-243)observe(55-61)observe(85-91)observe(214-220)observe(250-256)trackWhenNotCached(80-108)trackWhenNotCached(245-279)macOS(110-160)macOS(281-343)calculateBaseArea(363-367)calculateVolume(369-373)Tests/RelayTests/Helpers/Array+PopFirst.swift (1)
popFirst(11-16)
Tests/RelayTests/Memoized/SwiftDataMemoizedTests.swift (3)
Tests/RelayTests/Memoized/MainActorMemoizedTests.swift (14)
access(17-49)access(170-209)trackWhenCached(51-79)trackWhenCached(211-245)observe(56-62)observe(86-92)observe(216-222)observe(252-258)trackWhenNotCached(81-109)trackWhenNotCached(247-281)macOS(111-161)macOS(283-345)calculateBaseArea(365-369)calculateVolume(371-375)Tests/RelayTests/Memoized/ObservationMemoizedTests.swift (14)
access(16-48)access(168-207)trackWhenCached(50-78)trackWhenCached(209-243)observe(55-61)observe(85-91)observe(214-220)observe(250-256)trackWhenNotCached(80-108)trackWhenNotCached(245-279)macOS(110-160)macOS(281-343)calculateBaseArea(363-367)calculateVolume(369-373)Tests/RelayTests/Helpers/Array+PopFirst.swift (1)
popFirst(11-16)
Tests/RelayTests/Memoized/PublishableMemoizedTests.swift (1)
Tests/RelayTests/Helpers/Array+PopFirst.swift (1)
popFirst(11-16)
Sources/RelayMacros/Publishable/ObservationRegistrarDeclBuilder.swift (3)
Sources/RelayMacros/Publishable/PropertyPublisherDeclBuilder.swift (1)
build(17-32)Sources/RelayMacros/Memoized/MemoizedDeclBuilder.swift (2)
build(21-43)assumeIsolatedIfNeededFunction(69-94)Sources/RelayMacros/Publishable/PublisherDeclBuilder.swift (1)
build(16-29)
Sources/RelayMacros/Memoized/MemoizedDeclBuilder.swift (2)
Sources/RelayMacros/Memoized/MemoizedMacro.swift (1)
trimmedReturnType(86-88)Sources/RelayMacros/Publishable/ObservationRegistrarDeclBuilder.swift (1)
assumeIsolatedIfNeededFunction(149-181)
Tests/RelayTests/Publishable/MainActorPublishableTests.swift (1)
Tests/RelayTests/Publishable/ObservationPublishableTests.swift (8)
storedProperty(15-53)observe(27-33)observe(67-73)observe(115-124)observe(164-173)computedProperty(55-98)willChange(103-150)didChange(152-199)
Tests/RelayMacrosTests/Memoized/MainActorMemoizedMacroTests.swift (2)
Tests/RelayMacrosTests/Memoized/MemoizedMacroTests.swift (2)
testExpansion(20-80)testExpansionWithParameters(82-142)Tests/RelayMacrosTests/Publishable/MainActorPublishableMacroTests.swift (1)
testExpansion(20-246)
Sources/RelayMacros/Publishable/PropertyPublisherDeclBuilder.swift (3)
Sources/RelayMacros/Memoized/MemoizedDeclBuilder.swift (1)
build(21-43)Sources/RelayMacros/Publishable/PublisherDeclBuilder.swift (1)
build(16-29)Sources/RelayMacros/Memoized/MemoizedMacro.swift (2)
trimmedReturnType(86-88)defaultPropertyName(79-84)
Tests/RelayMacrosTests/Memoized/MemoizedMacroTests.swift (1)
Tests/RelayMacrosTests/Memoized/MainActorMemoizedMacroTests.swift (1)
testExpansion(20-86)
Tests/RelayTests/Memoized/ObservationMemoizedTests.swift (2)
Tests/RelayTests/Memoized/MainActorMemoizedTests.swift (6)
access(17-49)access(170-209)macOS(111-161)macOS(283-345)calculateBaseArea(365-369)calculateVolume(371-375)Tests/RelayTests/Helpers/Array+PopFirst.swift (1)
popFirst(11-16)
Tests/RelayMacrosTests/Publishable/MainActorPublishableMacroTests.swift (2)
Tests/RelayMacrosTests/Memoized/MainActorMemoizedMacroTests.swift (1)
testExpansion(20-86)Tests/RelayMacrosTests/Publishable/PublishableMacroTests.swift (1)
testExpansion(20-233)
Tests/RelayTests/Publishable/ObservationPublishableTests.swift (2)
Tests/RelayTests/Publishable/MainActorPublishableTests.swift (8)
storedProperty(16-54)computedProperty(56-99)willChange(104-151)observe(28-34)observe(68-74)observe(116-125)observe(165-174)didChange(153-200)Tests/RelayTests/Publishable/SwiftDataPublishableTests.swift (8)
storedProperty(16-54)computedProperty(56-99)willChange(104-151)observe(28-34)observe(68-74)observe(116-125)observe(165-174)didChange(153-200)
🪛 GitHub Actions: Pull request
Sources/Relay/Imports.swift
[error] 1-1: SwiftLint: File Header Violation: Header comments should be consistent with project patterns (file_header)
Sources/Relay/Publishable/AnyPropertyPublisher.swift
[error] 1-1: SwiftLint: File Header Violation: Header comments should be consistent with project patterns (file_header)
Sources/Relay/Publishable/Publishable.swift
[error] 1-1: SwiftLint: File Header Violation: Header comments should be consistent with project patterns (file_header)
Sources/RelayMacros/Publishable/PublisherDeclBuilder.swift
[error] 1-1: SwiftLint: File Header Violation: Header comments should be consistent with project patterns (file_header)
Sources/Relay/Publishable/PublishableObservationRegistrar.swift
[error] 1-1: SwiftLint: File Header Violation: Header comments should be consistent with project patterns (file_header)
Tests/RelayTests/Publishable/SwiftDataPublishableTests.swift
[error] 1-1: SwiftLint: File Header Violation: Header comments should be consistent with project patterns (file_header)
Tests/RelayTests/Publishable/AnyPropertyPublisherTests.swift
[error] 1-1: SwiftLint: File Header Violation: Header comments should be consistent with project patterns (file_header)
Sources/RelayMacros/Publishable/PublishableMacro.swift
[error] 1-1: SwiftLint: File Header Violation: Header comments should be consistent with project patterns (file_header)
Tests/RelayMacrosTests/Publishable/PublishableMacroTests.swift
[error] 1-1: SwiftLint: File Header Violation: Header comments should be consistent with project patterns (file_header)
Sources/Relay/Publishable/SwiftObservationRegistrar.swift
[error] 1-1: SwiftLint: File Header Violation: Header comments should be consistent with project patterns (file_header)
Tests/RelayTests/Memoized/MainActorMemoizedTests.swift
[error] 1-1: SwiftLint: File Header Violation: Header comments should be consistent with project patterns (file_header)
Tests/RelayTests/Memoized/SwiftDataMemoizedTests.swift
[error] 1-1: SwiftLint: File Header Violation: Header comments should be consistent with project patterns (file_header)
[error] 364-364: SwiftLint: No Empty Block Violation: Code blocks should contain at least one statement or comment (no_empty_block)
Sources/RelayMacros/Publishable/ObservationRegistrarDeclBuilder.swift
[error] 1-1: SwiftLint: File Header Violation: Header comments should be consistent with project patterns (file_header)
Sources/RelayMacros/Memoized/MemoizedDeclBuilder.swift
[error] 1-1: SwiftLint: File Header Violation: Header comments should be consistent with project patterns (file_header)
[error] 27-27: SwiftLint: Line Length Violation: Line should be 120 characters or less; currently it has 122 characters (line_length)
Tests/RelayMacrosTests/Memoized/MainActorMemoizedMacroTests.swift
[error] 44-44: SwiftLint: Global actor attribute order Violation: Global actor should be the first attribute. (global_actor_attribute_order)
[error] 112-112: SwiftLint: Global actor attribute order Violation: Global actor should be the first attribute. (global_actor_attribute_order)
[error] 1-1: SwiftLint: File Header Violation: Header comments should be consistent with project patterns (file_header)
[error] 1-1: SwiftLint: File Header Violation: Header comments should be consistent with project patterns (file_header)
Sources/RelayMacros/Publishable/PropertyPublisherDeclBuilder.swift
[error] 1-1: SwiftLint: File Header Violation: Header comments should be consistent with project patterns (file_header)
Tests/RelayMacrosTests/Publishable/MainActorPublishableMacroTests.swift
[error] 1-1: SwiftLint: File Header Violation: Header comments should be consistent with project patterns (file_header)
Tests/RelayTests/Publishable/ObservationPublishableTests.swift
[error] 1-1: SwiftLint: File Header Violation: Header comments should be consistent with project patterns (file_header)
🪛 LanguageTool
README.md
[grammar] ~79-~79: Use a hyphen to join words.
Context: ...e a great way of getting an always-up-to date values derived from other propertie...
(QB_NEW_EN_HYPHEN)
Sources/Relay/Documentation.docc/MemoizedMacros.md
[grammar] ~7-~7: Use a hyphen to join words.
Context: ...e a great way of getting an always-up-to date values derived from other propertie...
(QB_NEW_EN_HYPHEN)
🪛 SwiftLint (0.57.0)
Sources/Relay/Publishable/Publishable.swift
[Warning] 1-1: Header comments should be consistent with project patterns
(file_header)
Sources/Relay/Memoized/Memoized.swift
[Warning] 1-1: Header comments should be consistent with project patterns
(file_header)
Sources/Relay/Publishable/PublishableObservationRegistrar.swift
[Warning] 1-1: Header comments should be consistent with project patterns
(file_header)
Sources/RelayMacros/Memoized/MemoizedMacro.swift
[Warning] 1-1: Header comments should be consistent with project patterns
(file_header)
[Warning] 13-13: Tuples should have at most 2 members
(large_tuple)
Tests/RelayTests/Memoized/SwiftDataMemoizedTests.swift
[Warning] 1-1: Header comments should be consistent with project patterns
(file_header)
[Warning] 284-284: Function body should span 50 lines or less excluding comments and whitespace: currently spans 51 lines
(function_body_length)
[Warning] 364-364: Code blocks should contain at least one statement or comment
(no_empty_block)
[Warning] 23-23: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 28-28: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 33-33: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 37-37: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 42-42: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 43-43: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 47-47: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 75-75: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 105-105: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 126-126: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 131-131: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 136-136: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 137-137: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 138-138: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 141-141: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 143-143: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 146-146: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 148-148: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 150-150: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 153-153: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 154-154: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 155-155: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 182-182: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 189-189: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 190-190: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 194-194: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 195-195: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 198-198: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 199-199: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 201-201: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 205-205: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 206-206: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 238-238: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 274-274: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 297-297: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 304-304: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 311-311: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 312-312: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 313-313: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 315-315: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 318-318: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 320-320: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 322-322: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 325-325: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 327-327: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 329-329: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 331-331: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 334-334: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 335-335: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 336-336: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 338-338: Magic numbers should be replaced by named constants
(no_magic_numbers)
Sources/RelayMacros/Publishable/ObservationRegistrarDeclBuilder.swift
[Warning] 1-1: Header comments should be consistent with project patterns
(file_header)
Sources/RelayMacros/Memoized/MemoizedDeclBuilder.swift
[Warning] 1-1: Header comments should be consistent with project patterns
(file_header)
Package.swift
[Warning] 7-7: Top-level declarations should specify Access Control Level keywords explicitly
(explicit_top_level_acl)
[Warning] 1-1: Header comments should be consistent with project patterns
(file_header)
[Warning] 7-7: Top-level constants should be prefixed by k
(prefixed_toplevel_constant)
Tests/RelayMacrosTests/Memoized/MainActorMemoizedMacroTests.swift
[Warning] 44-44: Global actor should be the first attribute.
(global_actor_attribute_order)
[Warning] 112-112: Global actor should be the first attribute.
(global_actor_attribute_order)
[Warning] 1-1: Header comments should be consistent with project patterns
(file_header)
[Warning] 20-20: Function body should span 50 lines or less excluding comments and whitespace: currently spans 65 lines
(function_body_length)
[Warning] 88-88: Function body should span 50 lines or less excluding comments and whitespace: currently spans 65 lines
(function_body_length)
Tests/RelayTests/Memoized/ObservationMemoizedTests.swift
[Warning] 1-1: Header comments should be consistent with project patterns
(file_header)
[Warning] 283-283: Function body should span 50 lines or less excluding comments and whitespace: currently spans 51 lines
(function_body_length)
[Warning] 22-22: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 27-27: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 32-32: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 36-36: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 41-41: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 42-42: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 46-46: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 74-74: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 104-104: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 125-125: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 130-130: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 135-135: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 136-136: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 137-137: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 140-140: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 142-142: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 145-145: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 147-147: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 149-149: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 152-152: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 153-153: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 154-154: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 181-181: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 188-188: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 189-189: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 193-193: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 194-194: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 197-197: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 198-198: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 200-200: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 204-204: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 205-205: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 237-237: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 273-273: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 296-296: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 303-303: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 310-310: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 311-311: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 312-312: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 314-314: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 317-317: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 319-319: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 321-321: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 324-324: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 326-326: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 328-328: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 330-330: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 333-333: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 334-334: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 335-335: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 337-337: Magic numbers should be replaced by named constants
(no_magic_numbers)
Tests/RelayMacrosTests/Publishable/MainActorPublishableMacroTests.swift
[Warning] 1-1: Header comments should be consistent with project patterns
(file_header)
[Error] 20-20: Function body should span 100 lines or less excluding comments and whitespace: currently spans 225 lines
(function_body_length)
🔇 Additional comments (29)
README.md (1)
77-154: Well-structured Memoized macro documentation.The new Memoized section provides clear, practical examples demonstrating the performance benefits of caching computed property values. The progression from the problem (repeated recomputation) to the solution (using
@Memoizedmacro) is pedagogically sound and mirrors the approach used in the Publishable section..swiftlint.yml (4)
169-170: Addition of coordinate delta exclusions is sensible.The extension of
identifier_nameexclusions to includedx,dy,dzaligns with best practices for coordinate delta variables. This is a reasonable refinement.
10-10: Manual verification required: Run SwiftLint locally to check for rule violations.The configuration file confirms that
async_without_await,modifier_order,prefer_asset_symbols, andprefer_condition_listare enabled as opt-in rules. However, SwiftLint is not available in the sandbox environment, so violations cannot be detected automatically.Before merging, run the provided verification script locally:
swiftlint lint --strict --reporter json | jq '.[] | select(.severity == "error") | {file: .file.path, line: .line, rule: .rule_id, message: .message}' | head -20This will identify any new violations that could cause CI failures with these stricter rules enabled.
178-179: Verify whether tests are exempted from theno_magic_numbersrule and confirm the configuration intent.The codebase analysis reveals that the allowed numbers
[0.0, 1.0, 2.0, 100.0]are partially aligned with usage:
- 0.0, 1.0, 2.0: Legitimate baseline values used throughout tests for initialization
- 100.0: Confirmed used in codebase (
cube.offset = 100.0), but for offset operations—not percentage-based as initially suggested- Unused but present: Values 3.0, 4.0, 6.0, 8.0 appear in tests as computed results but are excluded from the allowed list
Without knowing whether tests are specifically exempted from this rule or whether the excluded values should be allowed, the configuration's correctness cannot be definitively verified. Manual review is needed to confirm:
- Test files are properly configured for (or exempted from)
no_magic_numbers- The allowed list intentionally excludes computed test values or if violations are expected
- The rule configuration meets your team's actual linting standards
188-196: Regexes comply with codebase patterns; trailing whitespace requirement is standard Swift syntax.The trailing
\\sin both patterns is intentional and correctly reflects standard Swift attribute syntax. All examined instances show:
- @sendable usage (12 occurrences): Always followed by whitespace (e.g.,
@Sendable nonisolated func)- @MainActor/@Observable/etc. (multiple attributes): Always separated by spaces when on same line
- Attribute order: No violations found; @sendable and @mainactor attributes appear first when present, never after other attributes
The codebase fully complies with these regex patterns. The trailing
\\srequirement catches cases where attributes violate the "should be first" rule, which the current code does not exhibit..spi.yml (1)
5-5: LGTM!The documentation target update correctly reflects the project rename from Publishable to Relay.
Tests/RelayTests/Helpers/Array+PopFirst.swift (1)
1-7: LGTM!The header comment update correctly reflects the project rename.
Sources/Relay/Documentation.docc/MemoizedMacros.md (1)
1-89: LGTM!The documentation effectively explains the Memoized macro with clear examples demonstrating the performance benefits of caching computed values. The before/after comparison helps readers understand when and how to use the macro.
.mise.toml (1)
8-9: Based on the verification results:Confirm version updates and review for breaking changes in SwiftLint 0.62.2.
The deprecated --path and --in-process-sourcekit arguments have been removed completely in SwiftLint 0.62.2, which is a breaking change from 0.58.2. Verify your project doesn't rely on these command-line options. The private_unit_test rule's deprecated regex configuration option has been removed if you use this rule.
For SwiftFormat 0.58.5: Version 0.58.0 introduced new rules including noForceUnwrapInTests, redundantThrows, redundantAsync, and preferFinalClasses, but these appear to be opt-in or non-breaking additions. This update is straightforward.
Review your SwiftLint configuration and command invocations to ensure compatibility with 0.62.2 before merging.
.swift-version (1)
1-1: No verification issues found — Swift 6.2 is released and Xcode 26.1 compatible.Swift 6.2 was officially released on September 15, 2025, and Xcode 26.1 includes Swift 6.2. The version update is valid and compatible.
Package.resolved (1)
2-20: Dependency pins look consistent; just keep them in sync with Package.swiftThe updated
principlemacrosandswift-syntaxrevisions/versions look fine. Just make sure these pins are generated from, and stay aligned with, yourPackage.swiftconstraints by re‑runningswift package resolvewhen you change those constraints.Sources/Relay/Documentation.docc/PublishableMacros.md (1)
1-62: Nice, clear overview of@PublishableusageThe doc reads well, the example matches the macro’s behavior, and the Topics section lines up with the exposed API surface. No issues from my side.
Sources/Relay/Documentation.docc/Relay.md (1)
1-8: Relay module doc aligns with the new surfaceThe brief description and Topics list correctly reflect the Relay module and its two documented macro groups. Looks good.
.github/workflows/pull-request.yml (1)
15-20: Verify Xcode and simulator identifiers exist onmacos-26runnersThe workflow changes look fine structurally, but
XCODE_VERSION: "26.1"plus the new simulator destinations (e.g.OS=26.1,iPhone 17 Pro Max,Apple Watch Series 11) will only work if that Xcode build and those runtimes/devices are actually installed on themacos-26image. Please double‑check locally or via a throwaway CI run thatxcode-select -s /Applications/Xcode_26.1.appsucceeds and thatxcodebuildrecognizes these destinations.Also applies to: 65-94
Sources/Relay/Publishable/Publishable.swift (2)
9-98:Publishablemacros and protocol surface look well‑shapedThe two macro overloads (
Publishable()andPublishable<Isolation: GlobalActor>(isolation:)) plus the attached member/extension declarations and thePublishableprotocol (withPropertyPublisher: AnyPropertyPublisher<Self>andpublisher) form a coherent, minimal public surface and align with the surrounding documentation. I don’t see API or semantics issues here, assuming theRelayMacros.PublishableMacroimplementation matches these expectations.
1-7: File header is consistent with codebase—review comment appears incorrectThe Publishable.swift header matches the format used throughout the codebase (Imports.swift, Memoized.swift, AnyPropertyPublisher.swift, and 6+ other files all use identical headers). No SwiftLint configuration file exists to validate against, and no header inconsistency was found. The referenced AnyPropertyPublisher.swift has the exact same header format as Publishable.swift.
Likely an incorrect or invalid review comment.
Sources/RelayMacros/Main/RelayPlugin.swift (1)
12-18: Compiler plugin wiring looks correct
RelayPlugincorrectly exposes bothPublishableMacroandMemoizedMacroviaprovidingMacros, matching the new macro surface.Tests/RelayMacrosTests/Memoized/MemoizedMacroTests.swift (1)
9-143: Macro expansion tests align with MemoizedDeclBuilderThe expected expansions (cache storage,
assumeIsolatedIfNeeded,invalidateCache, andwithObservationTrackingblocks) match the implementation inMemoizedDeclBuilder, including the custom-name case. This provides solid regression coverage for the macro.Tests/RelayTests/Publishable/ObservationPublishableTests.swift (1)
13-99: Publishable observation tests match the updated semanticsThe switch to a
[Bool]observationsQueueand the updated expectations instoredProperty,computedProperty,willChange, anddidChangeall align with the intended observation behavior and with the analogous tests in related suites. The nestedPersonvisibility changes are also consistent with the new access model.Also applies to: 101-200
Tests/RelayTests/Memoized/PublishableMemoizedTests.swift (1)
12-193: Memoized + Publishable integration tests look sound
independent,dependent, andsharecollectively cover cache priming, invalidation on dependent property changes, and shared subscription behavior forbaseAreaandvolume. The expected call counts and cache flags (isBaseAreaCached,isVolumeCached) are consistent with the@Memoizedexpansion and the Publishable publisher semantics.Sources/RelayMacros/Memoized/MemoizedDeclBuilder.swift (1)
21-94: MemoizedDeclBuilder implementation matches the tested expansionsThe builder correctly:
- emits a private optional cache backing store,
- generates the computed property using
access(keyPath:)on cache hits,- uses
nonisolated(unsafe) weak var instanceplusinvalidateCachewithwithMutationfor cache clearing, and- handles global-actor vs non-actor cases in
assumeIsolatedIfNeeded.This aligns with the expectations in both the plain and
@MainActormacro tests.Tests/RelayTests/Publishable/MainActorPublishableTests.swift (1)
13-221: Main‑actor Publishable tests mirror existing patterns and look correctThe scenarios for stored/computed properties and willChange/didChange publishers closely match the non‑MainActor tests and exercise deallocation, completion, and observation tracking thoroughly. I don’t see issues here.
Sources/RelayMacros/Publishable/ObservationRegistrarDeclBuilder.swift (1)
11-181: Observation registrar builder logic looks consistent and soundThe generated
ObservationRegistrar(publish/subject/withMutation +assumeIsolatedIfNeeded) matches the patterns used elsewhere (e.g. memoized builder) and correctly gates mutation work through the appropriate global actor when present, while keeping the nonisolated API surface expected byPublishableObservationRegistrar. I don’t see logic or concurrency issues in this builder.Tests/RelayTests/Memoized/MainActorMemoizedTests.swift (1)
12-377: Main‑actor memoized tests provide good coverage and match non‑MainActor variantsThe Independent/Dependent groups exercise cache state, call counts, dependency invalidation between
baseAreaandvolume, and asyncObservationsbehavior in a way that’s consistent with the non‑MainActor tests. The Cube model definition also lines up with what the@Memoizedmacro expects. Looks solid.Tests/RelayTests/Memoized/SwiftDataMemoizedTests.swift (2)
13-343: SwiftData memoized tests are consistent with other suitesThe Independent/Dependent sections exercise the same memoization and observation behaviors as the non‑SwiftData tests, now against a
@ModelCube. Assertions on call counts, cache flags, and asyncObservationsall line up with the expected semantics of@Memoized. No functional issues spotted.
350-377: Incorrect review comment — removinginit()will cause a compiler errorThe primary suggestion is incompatible with SwiftData. @model types require an explicit initializer; omitting it triggers a compiler error, not a resolution of the SwiftLint violation. Removing the empty
init() {}will break the code.The SwiftLint
no_empty_blockviolation can be resolved by:
- Adding a comment inside the block:
init() { /* intentionally empty */ }- Configuring
.swiftlint.ymlto exclude initializer bodies:no_empty_block: { disabled: ["initializer_bodies"] }- Using inline suppression:
// swiftlint:disable:this no_empty_blockKeep the initializer; address the linting violation using one of these approaches instead.
Likely an incorrect or invalid review comment.
Tests/RelayMacrosTests/Publishable/PublishableMacroTests.swift (2)
42-50: Memoized label publisher integration looks coherentUsing
@Memoized(.public)onmakeLabel()and exposing apublic var label: AnyPublisher<String, Never>wired via_computedPropertyPublisher(for: \.label)matches the expected Memoized behavior (function produces a memoized computed property that Publishable then publishes). Makinginitialsprivatewhile still using\.initialswithin the nestedPropertyPublisheris also valid, since nested types can see the outer type’s private members in Swift.Also applies to: 74-83, 117-123
130-223: Observation registrar mutation lifecycle wrapper is well-structuredThe new nonisolated
willSet,didSet, andwithMutationimplementations consistently callobject.publisher.beginModifications(), delegate toSwiftObservationRegistrar, thenpublishandendModifications(), usingassumeIsolatedIfNeededas a hook for isolation-aware behavior. This keeps the non-actor version aligned with the MainActor variant and gives you a single place to evolve isolation behavior later.Tests/RelayMacrosTests/Publishable/MainActorPublishableMacroTests.swift (1)
47-51: Main-actor Publishable + Memoized expansion looks consistentThe main-actor variant mirrors the non-actor test well:
@Memoized(.public)onmakeLabel()is preserved in the expansion, andPropertyPublisherexposes@MainActor public var label: AnyPublisher<String, Never>via_computedPropertyPublisher(for: \.label), matching the Memoized behavior.- All publisher accessors (
age,name,surname,fullName,initials,label) are annotated@MainActor, andObservationRegistrarcorrectly combines@MainActorentrypoints with nonisolated wrappers that delegate throughassumeIsolatedIfNeededandMainActor.shared.assumeIsolated.extension Person: @MainActor Publishablematches the intent of a main-actor-isolated Publishable conformance.This structure looks coherent and in line with the established patterns in your other macro tests.
Also applies to: 79-83, 84-124, 128-237
| XCODE_VERSION: "26.1" | ||
|
|
||
| jobs: | ||
| release: | ||
| runs-on: macos-15 | ||
| runs-on: macos-26 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify Xcode 26.1 and macOS-26 runner availability.
The workflow references Xcode 26.1 and the macOS-26 runner. Please confirm these are available on GitHub Actions to prevent CI failures.
🌐 Web query:
Is Xcode 26.1 available? What macOS runners are available on GitHub Actions?
💡 Result:
Short answer:
- Xcode 26.1 is not yet generally available on GitHub-hosted runners (there are open issues/requests to add it and reports of missing iOS 26.1 platform). [1][2][3]
Which macOS runners are (publicly) available on GitHub Actions:
- macos-14 (macOS 14 Sonoma) — current stable image. [1]
- macos-15 (macOS 15) — available (Intel/arm variants). [1]
- macos-26 (macOS 26 Tahoe) — available as a public beta image (arm64 and related tags). [1][4]
Notes:
- macOS 13 (Ventura) images were announced for deprecation (scheduled removal/unsupported dates noted in the runner-images repo). [1]
- For the latest/image-by-image tool versions (exact Xcode builds, installed iOS platforms/sims), check the actions/runner-images releases and related issues — those pages show per-image tool lists and active issues about Xcode 26.1. [1][2][3]
Sources:
[1] actions/runner-images — releases & image notes.
[2] Issue: "Update to Xcode 26.1" (actions/runner-images).
[3] Issue: "macOS 15 / Xcode 26.1: iOS 26.1 platform not installed" (actions/runner-images).
[4] macOS 26 image release notes (public beta) (actions/runner-images).
Use an available Xcode version; macOS-26 runner is available.
Xcode 26.1 is not yet generally available on GitHub Actions, and the workflow will fail when attempting to use it. macOS-26 (macOS 26 Tahoe) is available as a public beta image.
Update XCODE_VERSION to an available version (e.g., "26.0" or the latest stable release). Check the actions/runner-images releases for the current toolset and available Xcode versions on the macOS-26 image.
🤖 Prompt for AI Agents
.github/workflows/release.yml lines 9-13: XCODE_VERSION is set to "26.1" which
is not available on the macOS-26 runner and will cause the workflow to fail;
change XCODE_VERSION to a supported value (for example "26.0" or the latest
stable Xcode listed for macOS-26 in actions/runner-images releases) and verify
the chosen version exists on the macOS-26 image before committing.
| // | ||
| // Memoized.swift | ||
| // Relay | ||
| // | ||
| // Created by Kamil Strzelecki on 14/11/2025. | ||
| // Copyright © 2025 Kamil Strzelecki. All rights reserved. | ||
| // | ||
|
|
||
| import PrincipleMacrosClientSupport | ||
|
|
||
| /// A macro allowing a method to be used as a computed property, whose value will be automatically cached | ||
| /// and updated when its underlying `Observable` inputs change. | ||
| /// | ||
| /// - Parameters: | ||
| /// - accessControlLevel: Access control level of the generated computed property. | ||
| /// Defaults to `nil`, meaning that no explicit access control level will be applied. | ||
| /// - propertyName: Name of the generated computed property. | ||
| /// Defaults to `nil`, meaning that the name will be derived from the method by trimming its first word. | ||
| /// | ||
| /// - Note: This macro infers the global actor isolation of the method and applies it to the generated declarations. | ||
| /// If this causes compilation errors, use ``Memoized(_:_:isolation:)`` instead. | ||
| /// | ||
| /// - Note: This macro works only with pure methods of classes to which the `@Observable` or `@Model` macro has been applied directly. | ||
| /// | ||
| /// The `@Memoized` macro adds a new computed property to your type that returns the same value as a direct call to the original method. | ||
| /// Unlike a direct method call, this computed property automatically caches its output and returns the cached value on subsequent accesses, | ||
| /// until any of its underlying `Observable` inputs change. After an input changes, the value will be recomputed on the next access. | ||
| /// If the computed property is never accessed again, the original method will not be invoked. | ||
| /// | ||
| /// Like any other property on an `Observable` type, the generated computed property can be tracked with the `Observation` APIs, | ||
| /// as well as `Combine` if the ``Publishable()`` macro has been applied to the enclosing class. | ||
| /// | ||
| @attached(peer, names: arbitrary) | ||
| public macro Memoized( | ||
| _ accessControlLevel: AccessControlLevel? = nil, | ||
| _ propertyName: StaticString? = nil | ||
| ) = #externalMacro( | ||
| module: "RelayMacros", | ||
| type: "MemoizedMacro" | ||
| ) | ||
|
|
||
| /// A macro allowing a method to be used as a computed property, whose value will be automatically cached | ||
| /// and updated when its underlying `Observable` inputs change. | ||
| /// | ||
| /// - Parameters: | ||
| /// - accessControlLevel: Access control level of the generated computed property. | ||
| /// Defaults to `nil`, meaning that no explicit access control level will be applied. | ||
| /// - propertyName: Name of the generated computed property. | ||
| /// Defaults to `nil`, meaning that the name will be derived from the method by trimming its first word. | ||
| /// - isolation: The global actor to which the generated computed property is isolated. | ||
| /// If set to `nil`, the property will be `nonisolated`. | ||
| /// To infer isolation automatically, use the ``Memoized(_:_:)`` macro instead. | ||
| /// | ||
| /// - Note: This macro works only with pure methods of classes to which the `@Observable` or `@Model` macro has been applied directly. | ||
| /// | ||
| /// The `@Memoized` macro adds a new computed property to your type that returns the same value as a direct call to the original method. | ||
| /// Unlike a direct method call, this computed property automatically caches its output and returns the cached value on subsequent accesses, | ||
| /// until any of its underlying `Observable` inputs change. After an input changes, the value will be recomputed on the next access. | ||
| /// If the computed property is never accessed again, the original method will not be invoked. | ||
| /// | ||
| /// Like any other property on an `Observable` type, the generated computed property can be tracked with the `Observation` APIs, | ||
| /// as well as `Combine` if the ``Publishable()`` macro has been applied to the enclosing class. | ||
| /// | ||
| @attached(peer, names: arbitrary) | ||
| public macro Memoized<Isolation: GlobalActor>( | ||
| _ accessControlLevel: AccessControlLevel? = nil, | ||
| _ propertyName: StaticString? = nil, | ||
| isolation: Isolation.Type? | ||
| ) = #externalMacro( | ||
| module: "RelayMacros", | ||
| type: "MemoizedMacro" | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Memoized public macros and docs look good; align header with SwiftLint configuration.
The two Memoized macro overloads cleanly mirror the underlying MemoizedMacro implementation (same parameters and isolation story), and the documentation does a good job explaining caching, invalidation, and the @Observable/@Model constraints. The public surface here looks ready to ship.
As with the other new files, SwiftLint is flagging file_header on this file. Please update the header comment to match your project’s standard Relay header so the pipeline will pass.
🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 1-1: Header comments should be consistent with project patterns
(file_header)
🤖 Prompt for AI Agents
In Sources/Relay/Memoized/Memoized.swift around lines 1–72, the top file header
comment does not match the project's standard header and triggers SwiftLint's
file_header rule; replace the existing header block with the repository's
canonical Relay header (copy the exact header text from another source file in
the repo, e.g. any file under Sources/Relay that already passes linting), update
the year/author fields as appropriate, and ensure there are no extra blank lines
before the import so SwiftLint's file_header check passes.
| // | ||
| // MainActorMemoizedTests.swift | ||
| // Relay | ||
| // | ||
| // Created by Kamil Strzelecki on 15/11/2025. | ||
| // Copyright © 2025 Kamil Strzelecki. All rights reserved. | ||
| // |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Align file header with project SwiftLint rules
This file is failing the file_header rule. Please adjust or remove the header comment to match the project’s standard (e.g. mirror another test file that passes SwiftLint) so CI will succeed.
🧰 Tools
🪛 GitHub Actions: Pull request
[error] 1-1: SwiftLint: File Header Violation: Header comments should be consistent with project patterns (file_header)
🤖 Prompt for AI Agents
In Tests/RelayTests/Memoized/MainActorMemoizedTests.swift around lines 1 to 7,
the file header comment violates the project's SwiftLint file_header rule;
replace or remove this custom header so it matches the project's standard
(mirror a passing test file header), e.g., remove the multi-line
copyright/author block or replace it with the exact header template used
elsewhere in the repo, save the file and run SwiftLint to confirm the rule no
longer fails.
| // | ||
| // ObservationMemoizedTests.swift | ||
| // Relay | ||
| // | ||
| // Created by Kamil Strzelecki on 15/11/2025. | ||
| // Copyright © 2025 Kamil Strzelecki. All rights reserved. | ||
| // |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Update file header to satisfy SwiftLint
SwiftLint is flagging this header as non‑conforming. To keep CI green, adjust/remove the header comment to match whatever pattern the rest of the project uses (or relax the rule for test files, if that’s preferred).
🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 1-1: Header comments should be consistent with project patterns
(file_header)
🤖 Prompt for AI Agents
In Tests/RelayTests/Memoized/ObservationMemoizedTests.swift lines 1-7, the
multi-line file header is flagged by SwiftLint; either remove the header
entirely or change it to the project’s canonical header format used elsewhere
(single-line or SPDX-style) so it conforms to the repo pattern, or alternatively
add the minimal SwiftLint directive to disable the file_header rule for this
test file (e.g., a single-line disable at the top) — pick the repo-preferred
approach and update the file accordingly.
| // | ||
| // PublishableMemoizedTests.swift | ||
| // Relay | ||
| // | ||
| // Created by Kamil Strzelecki on 15/11/2025. | ||
| // Copyright © 2025 Kamil Strzelecki. All rights reserved. | ||
| // |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check this file for the same header lint rule
Given other new test files are tripping file_header, it’s likely this header will be flagged as well. Align it with the project’s standard header to avoid SwiftLint noise.
🤖 Prompt for AI Agents
In Tests/RelayTests/Memoized/PublishableMemoizedTests.swift lines 1-7 the file
header likely violates the project's file_header SwiftLint rule; replace this
custom header with the project's standard header used across other test files
(copy the exact header block from an existing, compliant test file in the repo),
ensuring copyright year, author format, spacing, and comment delimiters match
exactly and that there is a single blank line after the header.
| // | ||
| // SwiftDataMemoizedTests.swift | ||
| // Relay | ||
| // | ||
| // Created by Kamil Strzelecki on 15/11/2025. | ||
| // Copyright © 2025 Kamil Strzelecki. All rights reserved. | ||
| // |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Fix header to comply with SwiftLint file_header rule
As with the other new files, this header doesn’t match the configured pattern and is causing CI to fail. Please update or remove it to match existing files or adjust the rule for tests.
🧰 Tools
🪛 GitHub Actions: Pull request
[error] 1-1: SwiftLint: File Header Violation: Header comments should be consistent with project patterns (file_header)
🪛 SwiftLint (0.57.0)
[Warning] 1-1: Header comments should be consistent with project patterns
(file_header)
🤖 Prompt for AI Agents
In Tests/RelayTests/Memoized/SwiftDataMemoizedTests.swift lines 1-7 the file
header does not match the repository's SwiftLint file_header pattern and is
causing CI to fail; update or remove the header to match the existing files'
header format (copy the exact header used in other test files in
Tests/RelayTests or remove the comment block entirely), ensure the
copyright/year/author fields follow the project's standard, run swiftlint to
verify the file_header rule passes, and commit the change.
| // | ||
| // ObservableTests.swift | ||
| // Publishable | ||
| // ObservationPublishableTests.swift | ||
| // Relay | ||
| // | ||
| // Created by Kamil Strzelecki on 18/01/2025. | ||
| // Copyright © 2025 Kamil Strzelecki. All rights reserved. | ||
| // |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix SwiftLint file header violation
This file is also failing file_header in CI. Please adjust the header to match your existing pattern so the lint job passes.
🧰 Tools
🪛 GitHub Actions: Pull request
[error] 1-1: SwiftLint: File Header Violation: Header comments should be consistent with project patterns (file_header)
🪛 SwiftLint (0.57.0)
[Warning] 1-1: Header comments should be consistent with project patterns
(file_header)
🤖 Prompt for AI Agents
In Tests/RelayTests/Publishable/ObservationPublishableTests.swift around lines 1
to 7, the file header violates SwiftLint's file_header rule; replace the current
header with the project's standard header format used in other files (matching
exact comment layout, author name, copyright year pattern, and any required
blank lines) so the header text and delimiters match the lint configuration
exactly and the CI lint job passes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
README.md (2)
168-168: Update documentation link to reflect repository rename.The documentation link still points to the old
Publishablerepository. Update it to referenceRelay:-[Full documentation is available on the Swift Package Index.](https://swiftpackageindex.com/NSFatalError/Publishable/documentation/publishable) +[Full documentation is available on the Swift Package Index.](https://swiftpackageindex.com/NSFatalError/Relay/documentation/relay)
172-176: Update installation URL to reflect repository rename.The package installation example still references the old
Publishablerepository. Update it to point toRelay:.package( - url: "https://github.com/NSFatalError/Publishable", + url: "https://github.com/NSFatalError/Relay", from: "1.0.0" )
♻️ Duplicate comments (2)
README.md (1)
3-5: Update badge URLs to reflect Relay repository rename (duplicate from previous review).The badges still reference the old
NSFatalError/Publishablerepository, but the project has been renamed to Relay. This creates broken links and inconsistency after the rebranding.Update the badge URLs from
NSFatalError/PublishabletoNSFatalError/Relay:-[](https://swiftpackageindex.com/NSFatalError/Publishable) -[](https://swiftpackageindex.com/NSFatalError/Publishable) -[](https://codecov.io/github/NSFatalError/Publishable) +[](https://swiftpackageindex.com/NSFatalError/Relay) +[](https://swiftpackageindex.com/NSFatalError/Relay) +[](https://codecov.io/github/NSFatalError/Relay)This also adds descriptive alt text for the badges as a bonus fix for the missing alt text flagged by static analysis.
Tests/RelayTests/Memoized/SwiftDataMemoizedTests.swift (1)
1-7: File header violates SwiftLint ruleThis issue has already been flagged in a previous review. The header does not match the configured SwiftLint
file_headerpattern and is causing CI to fail.
🧹 Nitpick comments (1)
Tests/RelayMacrosTests/Memoized/MainActorMemoizedMacroTests.swift (1)
22-88: Test logic correctly validates MainActor memoization expansion.The expanded source accurately reflects the expected behavior for a
@Memoizedmethod on a@MainActor @Observableclass:
- Proper
@MainActorisolation on both cache storage and computed property- Correct use of
weak var instance = selfto prevent retain cycles- The
assumeIsolatedIfNeededhelper correctly usesMainActor.shared.assumeIsolatedto handle isolation boundaries- Cache invalidation via
withMutation(keyPath:)properly notifies the Observable frameworkThe
function_body_lengthwarning could be addressed by extracting the large string literals into separate properties or helper methods, but this is common in macro expansion tests and is fine as-is for test code.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.swiftlint.yml(9 hunks)README.md(3 hunks)Sources/Relay/Documentation.docc/MemoizedMacros.md(1 hunks)Sources/RelayMacros/Memoized/MemoizedDeclBuilder.swift(1 hunks)Sources/RelayMacros/Memoized/MemoizedMacro.swift(1 hunks)Tests/RelayMacrosTests/Memoized/MainActorMemoizedMacroTests.swift(1 hunks)Tests/RelayTests/Memoized/SwiftDataMemoizedTests.swift(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- Sources/RelayMacros/Memoized/MemoizedDeclBuilder.swift
- Sources/RelayMacros/Memoized/MemoizedMacro.swift
- Sources/Relay/Documentation.docc/MemoizedMacros.md
🧰 Additional context used
🧬 Code graph analysis (2)
Tests/RelayMacrosTests/Memoized/MainActorMemoizedMacroTests.swift (2)
Tests/RelayMacrosTests/Memoized/MemoizedMacroTests.swift (1)
testExpansion(20-80)Tests/RelayMacrosTests/Publishable/MainActorPublishableMacroTests.swift (1)
testExpansion(20-246)
Tests/RelayTests/Memoized/SwiftDataMemoizedTests.swift (3)
Tests/RelayTests/Memoized/MainActorMemoizedTests.swift (14)
access(17-49)access(170-209)trackWhenCached(51-79)trackWhenCached(211-245)observe(56-62)observe(86-92)observe(216-222)observe(252-258)trackWhenNotCached(81-109)trackWhenNotCached(247-281)macOS(111-161)macOS(283-345)calculateBaseArea(365-369)calculateVolume(371-375)Tests/RelayTests/Memoized/ObservationMemoizedTests.swift (14)
access(16-48)access(168-207)trackWhenCached(50-78)trackWhenCached(209-243)observe(55-61)observe(85-91)observe(214-220)observe(250-256)trackWhenNotCached(80-108)trackWhenNotCached(245-279)macOS(110-160)macOS(281-343)calculateBaseArea(363-367)calculateVolume(369-373)Tests/RelayTests/Helpers/Array+PopFirst.swift (1)
popFirst(11-16)
🪛 markdownlint-cli2 (0.18.1)
README.md
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
4-4: Images should have alternate text (alt text)
(MD045, no-alt-text)
9-9: Heading levels should only increment by one level at a time
Expected: h2; Actual: h4
(MD001, heading-increment)
🪛 SwiftLint (0.57.0)
Tests/RelayMacrosTests/Memoized/MainActorMemoizedMacroTests.swift
[Warning] 22-22: Function body should span 50 lines or less excluding comments and whitespace: currently spans 65 lines
(function_body_length)
[Warning] 90-90: Function body should span 50 lines or less excluding comments and whitespace: currently spans 65 lines
(function_body_length)
Tests/RelayTests/Memoized/SwiftDataMemoizedTests.swift
[Warning] 284-284: Function body should span 50 lines or less excluding comments and whitespace: currently spans 51 lines
(function_body_length)
[Warning] 23-23: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 28-28: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 33-33: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 37-37: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 42-42: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 43-43: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 47-47: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 75-75: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 105-105: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 126-126: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 131-131: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 136-136: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 137-137: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 138-138: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 141-141: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 143-143: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 146-146: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 148-148: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 150-150: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 153-153: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 154-154: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 155-155: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 182-182: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 189-189: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 190-190: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 194-194: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 195-195: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 198-198: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 199-199: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 201-201: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 205-205: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 206-206: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 238-238: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 274-274: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 297-297: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 304-304: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 311-311: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 312-312: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 313-313: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 315-315: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 318-318: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 320-320: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 322-322: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 325-325: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 327-327: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 329-329: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 331-331: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 334-334: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 335-335: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 336-336: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 338-338: Magic numbers should be replaced by named constants
(no_magic_numbers)
🔇 Additional comments (13)
.swiftlint.yml (6)
10-10: Enable additional opt-in rules for stricter macro code quality.These changes enable rules (
async_without_await,modifier_order,no_magic_numbers,prefer_asset_symbols,prefer_condition_list) that enforce better code hygiene. Given the addition of the@Memoizedmacro and expanded test coverage, this is a sensible enforcement baseline.Also applies to: 70-70, 81-81, 94-95
24-24: Comment out rules that may be overly restrictive.Moving
convenience_type,incompatible_concurrency_annotation, andsuperfluous_elseto commented-out status (rather than enabled) is a pragmatic approach to avoid false positives during the macro migration. This allows future enablement if the codebase matures.Also applies to: 59-59, 129-129
178-179: Addno_magic_numbersconfiguration with sensible defaults.The allowed numbers
[0.0, 1.0, 2.0, 100.0]cover common constants (unity, scaling, percentage). This is appropriate for macro implementations where fixed numeric values are common.Please verify that the allowed numbers don't exclude legitimate constants used in the new
@Memoizedmacro or its tests. If you encounter violations, the allowed list can be expanded.
160-160: Update file header pattern to reflect Relay branding.The pattern change from "Publishable" to "Relay" aligns with the PR's package rename and is consistent across the file.
170-170: Expand identifier name exclusions to include coordinate deltas.Adding
dx, dy, dzto excluded identifiers (id, ui, x, y, z) is a reasonable allowance for mathematical/physics-style code (derivatives, coordinate offsets). This fits well with macro-heavy code that may use such notation.
192-192: Refine custom rule regex patterns to require trailing whitespace.Lines 192 and 196 update the
global_actor_attribute_orderandsendable_attribute_orderregex patterns from@.+[^,\\s]\\s+@.*Actor\\s)and similar to require explicit trailing whitespace after the attribute. This tightens the pattern matching and reduces false positives on adjacent attributes.Also applies to: 196-196
README.md (1)
82-164: Memoized section is well-documented.The new Memoized section provides clear problem context, practical examples demonstrating the issue, and shows how the macro solves it with minimal changes to the call site. The code examples are well-structured and effectively illustrate both the problem and solution.
Tests/RelayTests/Memoized/SwiftDataMemoizedTests.swift (3)
15-162: Comprehensive test coverage for independent memoizationThe Independent test suite thoroughly exercises memoization behavior including cache hits/misses, invalidation on tracked property changes, stability when untracked properties change (line 46-48), and both synchronous and asynchronous observation tracking.
167-345: Excellent test coverage for dependent memoizationThe Dependent test suite correctly verifies that memoized computations with dependencies (volume depends on baseArea) properly invalidate caches through the dependency chain and maintain correct cache states.
The function body length warning on line 284 is acceptable for comprehensive async test scenarios.
350-379: Well-designed test model for memoization verificationThe Cube model is excellently structured for testing:
- Includes an untracked property (
offset) to verify selective invalidation- Exposes call counts and cache state for precise assertions
- Implements a dependency chain (volume → baseArea → x, y, z) to test dependent memoization
Tests/RelayMacrosTests/Memoized/MainActorMemoizedMacroTests.swift (3)
1-21: Previous concerns successfully addressed.The file header is now properly formatted, and the
global_actor_attribute_orderrule has been scoped with disable/enable comments around the test methods. This resolves the critical SwiftLint violations flagged in the previous review.
90-156: Parameterized expansion test is correct.This test properly validates that the
@Memoized(.public, "customName")parameters are respected in the expansion:
- The cache storage and computed property use the custom name
customNameinstead of deriving it from the method name- The computed property has
publicvisibility as specified by the.publicparameter- All isolation and observation tracking logic remains consistent with the basic expansion
158-160: Proper cleanup and closure.The SwiftLint rule is correctly re-enabled, and the conditional compilation block is properly closed.
| Synchronous observation of `Observable` changes through `Combine` | ||
| Essential tools that extend the capabilities of `Observation`. | ||
|
|
||
| #### Contents |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix heading level increment.
Line 9 defines a level-4 heading (####) immediately after a level-1 heading (#), which violates markdown structure. Headings should increment by at most one level at a time.
-#### Contents
+## Contents📝 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.
| #### Contents | |
| ## Contents |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
9-9: Heading levels should only increment by one level at a time
Expected: h2; Actual: h4
(MD001, heading-increment)
🤖 Prompt for AI Agents
In README.md around line 9, the heading jumps from level 1 (#) directly to level
4 (####) which breaks proper markdown heading order; change the "#### Contents"
heading to an appropriate level that increments by at most one from the
preceding heading (e.g., "## Contents" if previous was "#" or "### Contents"
depending on context) so the document uses sequential heading levels.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
README.md (1)
9-9: Restore proper heading level hierarchy.Line 9 uses
#### Contents(h4), which skips heading levels from the h1 above. Headings should increment by at most one level per the markdown structure rule (MD001).-#### Contents +## Contents
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Package.swift(3 hunks)README.md(2 hunks)Sources/Relay/Documentation.docc/MemoizedMacros.md(1 hunks)Sources/RelayMacros/Main/RelayPlugin.swift(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- Sources/RelayMacros/Main/RelayPlugin.swift
- Sources/Relay/Documentation.docc/MemoizedMacros.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
README.md
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
4-4: Images should have alternate text (alt text)
(MD045, no-alt-text)
9-9: Heading levels should only increment by one level at a time
Expected: h2; Actual: h4
(MD001, heading-increment)
⏰ 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 (tvos)
- GitHub Check: build-and-test (macos)
- GitHub Check: build-and-test (watchos)
- GitHub Check: build-and-test (maccatalyst)
- GitHub Check: build-and-test (ios)
🔇 Additional comments (7)
README.md (2)
15-81: Well-documented Publishable section with clear examples.The Publishable section is comprehensive and clearly explains the feature's purpose, usage, and implementation approach. The code examples effectively demonstrate the synchronous observation pattern with Combine, and the technical explanation of how the macro works is accurate and well-articulated.
83-165: Excellent Memoized documentation with clear problem/solution narrative.The new Memoized section is well-crafted with a compelling problem statement, clear code examples showing before/after behavior, and proper usage patterns. The example effectively demonstrates how the macro eliminates redundant recomputation by caching values until Observable inputs change, making the performance benefit evident to readers.
Package.swift (5)
38-41: LGTM: PrincipleMacrosClientSupport dependency added.The addition of PrincipleMacrosClientSupport to the Relay target is appropriate for providing client-side macro support, aligning with the introduction of new macro functionality in this PR.
57-59: Now I need to examine the actual files to understand the full context. Let me retrieve the Package.swift and CI workflow:Let me retrieve the actual files to understand the implementation better:
Based on my research, I now have sufficient information to provide a conclusive rewritten review comment. Xcode tries to build the executable tool for the current platform, not only for the host platform (macOS), which means that when targeting iOS, Xcode will try to compile the executable tool for iOS, which is sometimes not possible. Cross-compiling macro examples doesn't work yet.
The macOS-only restriction for SwiftCompilerPlugin (swift-syntax) is a standard and necessary pattern. Platform-specific package dependencies allow a package to use a certain dependency only when building for a specific platform, which can be declared in the manifest file.
The macOS-only restriction for SwiftCompilerPlugin is correct and requires no changes.
Swift package plugins require compilation for the host platform (macOS) rather than the target platform. This restriction is a known workaround for macro cross-compilation limitations and won't affect non-macOS platform builds—only the macro compilation itself is macOS-restricted, while the main package can still target multiple platforms.
CI pipelines on non-macOS platforms will correctly skip this dependency using the conditional platform restriction, and macro testing is appropriately handled on macOS only, where macro development occurs.
84-85: No issues identified — features are officially supported in Swift 6.2.Both
MemberImportVisibilityandNonisolatedNonsendingByDefaultare available in Swift 6.2 as opt‑in upcoming feature flags. The syntax using.enableUpcomingFeature()is correct for enabling these features.
1-1: ---Swift 6.2 is valid and stable. Swift 6.2 was officially released on September 15, 2025 and is the stable major release, so the version bump is appropriate.
26-26: No compatibility issues found with PrincipleMacros 3.x upgrade.PrincipleMacros 3.0.0 contains no API-breaking changes; the release was "Prepared package for Swift 6.2", indicating a toolchain compatibility bump rather than API changes. The codebase's existing macro usage patterns (
@attached(member, ...),@attached(extension, ...),@attached(peer, ...)) remain valid and require no modifications.
| [](https://swiftpackageindex.com/NSFatalError/Relay) | ||
| [](https://swiftpackageindex.com/NSFatalError/Relay) | ||
| [](https://codecov.io/gh/NSFatalError/Relay) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add alt text to badge images for accessibility.
Badge images on lines 3–5 are missing alternate text. Markdown alt text should describe each badge's purpose to improve accessibility and provide context when images fail to load.
-[](https://swiftpackageindex.com/NSFatalError/Relay)
-[](https://swiftpackageindex.com/NSFatalError/Relay)
-[](https://codecov.io/gh/NSFatalError/Relay)
+[](https://swiftpackageindex.com/NSFatalError/Relay)
+[](https://swiftpackageindex.com/NSFatalError/Relay)
+[](https://codecov.io/gh/NSFatalError/Relay)📝 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.
| [](https://swiftpackageindex.com/NSFatalError/Relay) | |
| [](https://swiftpackageindex.com/NSFatalError/Relay) | |
| [](https://codecov.io/gh/NSFatalError/Relay) | |
| [](https://swiftpackageindex.com/NSFatalError/Relay) | |
| [](https://swiftpackageindex.com/NSFatalError/Relay) | |
| [](https://codecov.io/gh/NSFatalError/Relay) |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
4-4: Images should have alternate text (alt text)
(MD045, no-alt-text)
🤖 Prompt for AI Agents
In README.md around lines 3 to 5 the markdown image badges lack alt text which
harms accessibility; update each image tag to include a concise descriptive alt
string (e.g., "Swift versions badge", "Supported platforms badge", "Codecov
coverage badge") so the square-bracket alt text is present for each badge and
still link the images as before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (5)
Tests/RelayMacrosTests/Publishable/MainActorPublishableMacroTests.swift (1)
20-246: ** Address function_body_length violation.**The
testExpansionmethod violates SwiftLint'sfunction_body_lengthrule (spans 225 lines). As previously noted, this can be resolved by:
Extracting the large string literals into private file-level constants:
private let mainActorPersonInput = #"""..."""# private let mainActorPersonExpanded = #"""..."""#Or adding a local suppression:
// swiftlint:disable:next function_body_length func testExpansion() {Choose the approach consistent with other macro tests in the repository.
Tests/RelayTests/Memoized/SwiftDataMemoizedTests.swift (1)
1-7: Header still likely violates projectfile_headerruleThis custom multi-line header matches the pattern previously flagged by SwiftLint and in earlier review comments; it will likely continue to trip the
file_headerrule unless aligned with the project’s standard header or removed.Tests/RelayTests/Memoized/PublishableMemoizedTests.swift (1)
1-7: Header likely still conflicts withfile_headerlintAs with the other new test files, this custom header is very likely what SwiftLint’s
file_headerrule is complaining about; aligning it with the project-standard header or removing it should clear the warning/error.Tests/RelayTests/Memoized/MainActorMemoizedTests.swift (1)
1-7: File header appears to still violate project conventionThis header matches patterns previously called out by SwiftLint’s
file_headerrule; bringing it in line with the canonical test-file header (or removing it) should resolve the lint.Tests/RelayTests/Memoized/ObservationMemoizedTests.swift (1)
1-7: Header formatting still at odds with SwiftLintfile_headerruleThis header style is the same pattern previously warned about; updating it to match the project-standard header or removing it will avoid ongoing
file_headerlint noise.
🧹 Nitpick comments (6)
Tests/RelayTests/Memoized/SwiftDataMemoizedTests.swift (2)
111-161: Independentobservationsasync test is well-structuredThe async
Observationssequence is used consistently to assert emissions vs. cache state transitions (not-cached, re-compute, cached). The sleep-based scheduling matches other tests in the suite; if you see flakiness, consider factoring out a helper to encapsulate the “wait for next emission” behavior.
282-344: Dependentobservationsbody slightly exceedsfunction_body_lengththresholdStatic analysis flags this function for length; functionally it’s fine and matches the Observation baseline, but if you care about keeping SwiftLint clean, consider extracting some repeated “mutate + assert queue/cache” steps into small helpers to shorten the body.
Tests/RelayTests/Memoized/PublishableMemoizedTests.swift (1)
53-124:dependenttest thoroughly exercises shared cache between base and volume publishersThe choreography between
volumeQueueandbaseAreaQueue(initial baseArea computation, first volume publish, shared invalidation viaxandz, and post-cancel behavior) looks correct and matches the intended semantics for a memoized dependency graph. Static analysis flags this body for length; if that becomes noisy, extracting a couple of helper assertions for the repeated “pop & assert queues + call counts” could shorten it.Tests/RelayTests/Memoized/MainActorMemoizedTests.swift (2)
111-161: Independentobservationsasync test mirrors Observation baselineThe sequence of sleeps, queue pops, and cache flag assertions matches the non-main-actor version and looks correct for verifying Observations behavior with
@MainActorobjects; if you ever need to tweak for flakiness, consider factoring wait-and-pop into a helper.
283-345: Dependentobservationsasync test slightly exceeds linted body lengthFunctionally this is a faithful
@MainActorport of the Observation test and looks correct; SwiftLint flags it for body length, so if you want to quiet that, consider extracting a small helper for the repeated “mutate cube + check queue/cache flags” pattern.Tests/RelayTests/Memoized/ObservationMemoizedTests.swift (1)
281-343: Dependentobservationsasync test is correct but hitsfunction_body_lengthlintThe async Observations test for
volumeis thorough and matches the non-async observation scenarios; SwiftLint flags the body length, so if that’s bothersome, you could extract repeated “mutate cube + assert queue/cache state” logic into shared helpers to shorten this function.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
Sources/Relay/Publishable/AnyPropertyPublisher.swift(3 hunks)Sources/RelayMacros/Publishable/ObservationRegistrarDeclBuilder.swift(1 hunks)Tests/RelayMacrosTests/Publishable/MainActorPublishableMacroTests.swift(1 hunks)Tests/RelayMacrosTests/Publishable/PublishableMacroTests.swift(5 hunks)Tests/RelayTests/Memoized/MainActorMemoizedTests.swift(1 hunks)Tests/RelayTests/Memoized/ObservationMemoizedTests.swift(1 hunks)Tests/RelayTests/Memoized/PublishableMemoizedTests.swift(1 hunks)Tests/RelayTests/Memoized/SwiftDataMemoizedTests.swift(1 hunks)Tests/RelayTests/Publishable/AnyPropertyPublisherTests.swift(13 hunks)Tests/RelayTests/Publishable/MainActorPublishableTests.swift(1 hunks)Tests/RelayTests/Publishable/ObservationPublishableTests.swift(14 hunks)Tests/RelayTests/Publishable/SwiftDataPublishableTests.swift(14 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Tests/RelayTests/Publishable/ObservationPublishableTests.swift
🧰 Additional context used
🧬 Code graph analysis (9)
Tests/RelayTests/Memoized/PublishableMemoizedTests.swift (1)
Tests/RelayTests/Helpers/Array+PopFirst.swift (1)
popFirst(11-16)
Tests/RelayTests/Publishable/AnyPropertyPublisherTests.swift (2)
Tests/RelayTests/Helpers/Array+PopFirst.swift (1)
popFirst(11-16)Tests/RelayTests/Publishable/MainActorPublishableTests.swift (1)
storedProperty(16-54)
Tests/RelayMacrosTests/Publishable/MainActorPublishableMacroTests.swift (2)
Tests/RelayMacrosTests/Publishable/PublishableMacroTests.swift (1)
testExpansion(20-233)Tests/RelayMacrosTests/Memoized/MainActorMemoizedMacroTests.swift (1)
testExpansion(22-88)
Tests/RelayTests/Publishable/SwiftDataPublishableTests.swift (3)
Tests/RelayTests/Publishable/MainActorPublishableTests.swift (8)
storedProperty(16-54)computedProperty(56-99)willChange(104-151)observe(28-34)observe(68-74)observe(116-125)observe(165-174)didChange(153-200)Tests/RelayTests/Publishable/ObservationPublishableTests.swift (8)
storedProperty(15-53)computedProperty(55-98)willChange(103-150)observe(27-33)observe(67-73)observe(115-124)observe(164-173)didChange(152-199)Tests/RelayTests/Helpers/Array+PopFirst.swift (1)
popFirst(11-16)
Tests/RelayTests/Memoized/SwiftDataMemoizedTests.swift (2)
Tests/RelayTests/Memoized/MainActorMemoizedTests.swift (12)
access(17-49)access(170-209)trackWhenCached(51-79)trackWhenCached(211-245)observe(56-62)observe(86-92)observe(216-222)observe(252-258)macOS(111-161)macOS(283-345)calculateBaseArea(365-369)calculateVolume(371-375)Tests/RelayTests/Helpers/Array+PopFirst.swift (1)
popFirst(11-16)
Sources/RelayMacros/Publishable/ObservationRegistrarDeclBuilder.swift (3)
Sources/RelayMacros/Memoized/MemoizedDeclBuilder.swift (2)
build(21-45)assumeIsolatedIfNeededFunction(71-96)Sources/RelayMacros/Publishable/PropertyPublisherDeclBuilder.swift (1)
build(17-32)Sources/RelayMacros/Publishable/PublisherDeclBuilder.swift (1)
build(16-29)
Tests/RelayTests/Publishable/MainActorPublishableTests.swift (2)
Tests/RelayTests/Publishable/ObservationPublishableTests.swift (8)
storedProperty(15-53)observe(27-33)observe(67-73)observe(115-124)observe(164-173)computedProperty(55-98)willChange(103-150)didChange(152-199)Tests/RelayTests/Helpers/Array+PopFirst.swift (1)
popFirst(11-16)
Tests/RelayTests/Memoized/MainActorMemoizedTests.swift (2)
Tests/RelayTests/Memoized/ObservationMemoizedTests.swift (14)
access(16-48)access(168-207)trackWhenCached(50-78)trackWhenCached(209-243)observe(55-61)observe(85-91)observe(214-220)observe(250-256)trackWhenNotCached(80-108)trackWhenNotCached(245-279)macOS(110-160)macOS(281-343)calculateBaseArea(363-367)calculateVolume(369-373)Tests/RelayTests/Helpers/Array+PopFirst.swift (1)
popFirst(11-16)
Tests/RelayTests/Memoized/ObservationMemoizedTests.swift (2)
Tests/RelayTests/Memoized/MainActorMemoizedTests.swift (10)
access(17-49)access(170-209)observe(56-62)observe(86-92)observe(216-222)observe(252-258)macOS(111-161)macOS(283-345)calculateBaseArea(365-369)calculateVolume(371-375)Tests/RelayTests/Helpers/Array+PopFirst.swift (1)
popFirst(11-16)
🪛 SwiftLint (0.57.0)
Tests/RelayTests/Memoized/PublishableMemoizedTests.swift
[Warning] 54-54: Function body should span 50 lines or less excluding comments and whitespace: currently spans 58 lines
(function_body_length)
[Warning] 34-34: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 35-35: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 36-36: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 40-40: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 42-42: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 48-48: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 49-49: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 83-83: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 84-84: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 85-85: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 87-87: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 88-88: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 92-92: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 94-94: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 97-97: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 100-100: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 102-102: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 104-104: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 105-105: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 111-111: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 112-112: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 113-113: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 115-115: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 121-121: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 122-122: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 145-145: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 146-146: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 147-147: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 148-148: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 154-154: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 155-155: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 156-156: Magic numbers should be replaced by named constants
(no_magic_numbers)
Tests/RelayMacrosTests/Publishable/MainActorPublishableMacroTests.swift
[Error] 20-20: Function body should span 100 lines or less excluding comments and whitespace: currently spans 225 lines
(function_body_length)
Tests/RelayTests/Memoized/SwiftDataMemoizedTests.swift
[Warning] 284-284: Function body should span 50 lines or less excluding comments and whitespace: currently spans 51 lines
(function_body_length)
[Warning] 23-23: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 28-28: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 33-33: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 37-37: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 42-42: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 43-43: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 47-47: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 75-75: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 105-105: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 126-126: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 131-131: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 136-136: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 137-137: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 138-138: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 141-141: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 143-143: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 146-146: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 148-148: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 150-150: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 153-153: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 154-154: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 155-155: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 182-182: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 189-189: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 190-190: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 194-194: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 195-195: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 198-198: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 199-199: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 201-201: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 205-205: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 206-206: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 238-238: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 274-274: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 297-297: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 304-304: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 311-311: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 312-312: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 313-313: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 315-315: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 318-318: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 320-320: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 322-322: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 325-325: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 327-327: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 329-329: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 331-331: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 334-334: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 335-335: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 336-336: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 338-338: Magic numbers should be replaced by named constants
(no_magic_numbers)
Tests/RelayTests/Memoized/MainActorMemoizedTests.swift
[Warning] 285-285: Function body should span 50 lines or less excluding comments and whitespace: currently spans 51 lines
(function_body_length)
[Warning] 23-23: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 28-28: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 33-33: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 37-37: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 42-42: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 43-43: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 47-47: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 75-75: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 105-105: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 126-126: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 131-131: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 136-136: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 137-137: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 138-138: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 141-141: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 143-143: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 146-146: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 148-148: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 150-150: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 153-153: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 154-154: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 155-155: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 183-183: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 190-190: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 191-191: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 195-195: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 196-196: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 199-199: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 200-200: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 202-202: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 206-206: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 207-207: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 239-239: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 275-275: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 298-298: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 305-305: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 312-312: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 313-313: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 314-314: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 316-316: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 319-319: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 321-321: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 323-323: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 326-326: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 328-328: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 330-330: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 332-332: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 335-335: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 336-336: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 337-337: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 339-339: Magic numbers should be replaced by named constants
(no_magic_numbers)
Tests/RelayTests/Memoized/ObservationMemoizedTests.swift
[Warning] 283-283: Function body should span 50 lines or less excluding comments and whitespace: currently spans 51 lines
(function_body_length)
[Warning] 22-22: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 27-27: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 32-32: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 36-36: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 41-41: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 42-42: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 46-46: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 74-74: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 104-104: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 125-125: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 130-130: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 135-135: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 136-136: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 137-137: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 140-140: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 142-142: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 145-145: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 147-147: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 149-149: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 152-152: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 153-153: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 154-154: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 181-181: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 188-188: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 189-189: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 193-193: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 194-194: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 197-197: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 198-198: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 200-200: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 204-204: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 205-205: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 237-237: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 273-273: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 296-296: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 303-303: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 310-310: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 311-311: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 312-312: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 314-314: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 317-317: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 319-319: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 321-321: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 324-324: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 326-326: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 328-328: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 330-330: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 333-333: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 334-334: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 335-335: Magic numbers should be replaced by named constants
(no_magic_numbers)
[Warning] 337-337: Magic numbers should be replaced by named constants
(no_magic_numbers)
⏰ 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 (macos)
- GitHub Check: build-and-test (visionos)
- GitHub Check: build-and-test (maccatalyst)
- GitHub Check: build-and-test (ios)
- GitHub Check: build-and-test (tvos)
🔇 Additional comments (32)
Tests/RelayTests/Publishable/AnyPropertyPublisherTests.swift (1)
9-207: LGTM! Clean refactoring to the Relay module.The test changes are straightforward:
- Updated import to Relay
- Simplified test method names
- Changed observationsQueue to use Bool instead of Void for clearer intent
The test logic and coverage remain intact.
Sources/RelayMacros/Publishable/ObservationRegistrarDeclBuilder.swift (1)
96-147: LGTM! Proper lifecycle and actor isolation handling.The generated ObservationRegistrar methods correctly:
- Wrap property changes with
_beginModifications()and_endModifications()- Capture keypaths and mutations as
nonisolated(unsafe)to allow passing across isolation boundaries- Delegate to
assumeIsolatedIfNeededto execute on the correct actor- Call
publish()after modifications to update subscribersThe lifecycle coordination ensures that willChange/didChange events are properly emitted even for nested modifications.
Tests/RelayTests/Publishable/SwiftDataPublishableTests.swift (1)
10-200: LGTM! Consistent test refactoring.The changes align with the broader Relay migration:
- Updated import to Relay module
- Simplified test method names for better readability
- Changed observationsQueue to Bool for clearer test intent
All test scenarios (stored properties, computed properties, willChange, didChange) continue to properly validate the Publishable behavior with SwiftData integration.
Tests/RelayMacrosTests/Publishable/PublishableMacroTests.swift (2)
47-83: LGTM! Memoized macro integration validated.The test now validates that the Publishable macro correctly handles methods annotated with
@Memoized(.public):
- The original
makeLabel()function is preserved in the expansion- A corresponding
labelpublisher is generated in PropertyPublisher- The publisher provides reactive access to the memoized computed property
This ensures that Publishable and Memoized macros work together correctly.
170-223: LGTM! Complete observation lifecycle implementation.The expanded ObservationRegistrar now includes the full lifecycle protocol:
willSetanddidSetfor property change notificationsaccessfor observation trackingwithMutationfor atomic property mutations- Proper
assumeIsolatedIfNeededfor actor isolationThe nonisolated methods correctly capture parameters as
nonisolated(unsafe)and delegate to the isolated context, ensuring concurrency safety.Tests/RelayTests/Publishable/MainActorPublishableTests.swift (1)
1-222: LGTM! Comprehensive MainActor isolation test coverage.This new test file validates that Publishable works correctly with MainActor-isolated types:
- Tests cover stored properties, computed properties, willChange, and didChange publishers
- Proper use of
nonisolated(unsafe)for observation queues in MainActor context- Publisher subscriptions and observation tracking work correctly across isolation boundaries
- Cleanup on deallocation is properly verified
The test structure mirrors the non-isolated tests, providing consistent coverage across isolation contexts.
Tests/RelayMacrosTests/Publishable/MainActorPublishableMacroTests.swift (1)
84-243: LGTM! Correct MainActor isolation propagation.The macro expansion test validates that @mainactor isolation is correctly applied throughout the generated code:
- PropertyPublisher property accessors are isolated to MainActor (lines 102-123)
- ObservationRegistrar struct and its methods inherit @mainactor isolation (line 128)
assumeIsolatedIfNeededproperly usesMainActor.shared.assumeIsolatedwith file/line parameters for debugging (lines 219-236)- The generated extension correctly applies
@MainActorto the Publishable conformance (line 241)This ensures that all publisher access and observation tracking operations respect the MainActor isolation boundary.
Tests/RelayTests/Memoized/SwiftDataMemoizedTests.swift (7)
15-49: Independentaccesstest logic looks solidThe
Independent.accessscenario correctly mirrors the baseline Observation tests: it verifies lazy computation, memoization, invalidation when true dependencies (x,y) change, and stability when a non-dependent property (offset) mutates.
51-79: IndependenttrackWhenCachedbehavior matches expectationsThe use of
withObservationTrackingandnonisolated(unsafe) var queuematches the existing pattern: no change event whilebaseAreastays cached, and a single change after mutatingxand invalidating the cache.
81-109: IndependenttrackWhenNotCachedcovers the complementary caseThis test nicely complements
trackWhenCached, ensuring that when the initial observation runs while the value is computed and cached, the invalidation onxstill triggers exactly one onChange event and cache invalidation.
169-208: Dependentaccesscorrectly exercises base/volume interactionThis test exercises memoization where
volumedepends onbaseArea, and correctly checks that invalidatingxinvalidates both caches, while subsequent accesses recompute and re-cache in the expected order.
210-244: DependenttrackWhenCachedis consistent with Observation testsThe
withObservationTrackingusage forvolumeand the expectations onqueue, call counts, and cache flags align with the non-SwiftData baseline, giving good coverage that SwiftData +@Memoizedbehave identically here.
246-280: DependenttrackWhenNotCachedcomplements the cached caseThis mirrors the cached scenario but ensures correct invalidation behavior when the observation is first established in a non-cached state; the expectations on queue and cache flags look coherent.
350-379: SwiftDataCubemodel and memoized methods look correctThe
@ModelCubewith@MemoizedcalculateBaseArea/calculateVolumemirrors other test cubes, and theisBaseAreaCached/isVolumeCachedhelpers that peek at_baseArea/_volumeare consistent with the macro’s synthesized storage; the dedicatedinitensuresoffsetis initialized for SwiftData.Tests/RelayTests/Memoized/PublishableMemoizedTests.swift (3)
14-51:independentpublishable test cleanly covers basic memoization & publishingThis test nicely checks that the initial synchronous access populates the cache, then a publisher subscriber sees the cached value and subsequent updates only when real dependencies (
x) change; cancellation and post-cancel mutation behavior are also validated.
126-161:sharetest validates shared publisher reuse effectivelyThe test confirms that multiple baseArea subscribers share a single memoized computation and see consistent updates when dependencies change, and that cancelling one subscriber stops its queue while the other continues to receive values.
167-192:Cubepublishable/observable model is consistent with other testsThe
@Publishable @ObservableCubemirrors the other cube types: same default values, same@Memoizedimplementations, and clear cache-state helpers based on_baseArea/_volume; this should give good signal that theMemoizedmacro plays well with the Publishable stack.Tests/RelayTests/Memoized/MainActorMemoizedTests.swift (7)
14-49: Independentaccesstest correctly exercises@MainActorcube cachingThe logic is identical to the baseline Observation tests but under
@MainActorisolation, verifying lazy computation, invalidation onx/y, and stability when onlyoffsetchanges; this gives good coverage of memoization on a main-actor-bound observable object.
51-79: IndependenttrackWhenCachedrespects main-actor isolation patternUsing
nonisolated(unsafe) var queuetogether withwithObservationTrackingunder a@MainActorstruct is consistent with other tests and allows the onChange closure to mutatequeuewithout tripping isolation rules; expectations on whenqueuereceives an event look correct.
81-109: IndependenttrackWhenNotCachedcompletes the coverage matrixThis test covers the complementary “observe while uncached” path and ensures cache invalidation on
xstill yields a single change notification and leaves the counters/cache flags in the expected state.
167-209: Dependentaccesshandles base/volume interaction under@MainActorThis test validates that invalidating
xorzclears the right caches and recomputes in the expected order, consistent with the Observation tests; expectations on call counts and cache flags line up.
211-245: DependenttrackWhenCachedmain-actor test looks consistentThe tracking of
volumechanges viawithObservationTrackingand the expectations onqueue, call counts, and cache flags all align with the baseline; thenonisolated(unsafe)queue usage is appropriate in this context.
247-281: DependenttrackWhenNotCachedfills in remaining observation scenarioThis covers the uncached-observation path and confirms that invalidating
zafter establishing the observation produces a single change notification and the right cache state transitions.
351-376:@MainActor @ObservableCube definition is coherentThe cube’s properties, call counters, cache indicators, and
@Memoizedimplementations all match the other tests; tying the class to@MainActorhere is appropriate for verifying that the macro handles actor-isolated observables.Tests/RelayTests/Memoized/ObservationMemoizedTests.swift (8)
14-48: Independentaccessbaseline test is clear and correctThis is a good canonical test for basic memoization: it verifies zero initial calls, lazy computation, reuse while cached, invalidation when
ychanges, and no invalidation whenoffset(a non-dependent property) changes.
50-78: IndependenttrackWhenCachedcorrectly wires observation trackingThe pattern using
withObservationTrackingandqueuemirrors Apple’s Observation examples: no notification when re-accessing a cached value, and a single notification whenxchanges and invalidates the cache.
80-108: IndependenttrackWhenNotCachedcovers the uncached-observation pathThis ensures that establishing the observation while the value is not yet cached behaves as expected and that a subsequent mutation of
xstill yields a single change event and cache invalidation.
110-160: Independentobservationsasync test provides solid baseline coverageThe Observations-based async test properly distinguishes between emissions when not cached vs cached and checks call counts and cache flags; structure and expectations look correct.
166-207: Dependentaccesstest correctly models base/volume dependencyThe interplay between
baseAreaandvolumeis well-covered: first access viavolume, thenbaseArea, invalidation viax, and recomputation/cache re-establishment are all asserted with coherent call counts and cache flags.
209-243: DependenttrackWhenCacheddepends-on-volume observation test looks goodThis test verifies that observing
volumewhile cached behaves as expected and that changingzinvalidates only the volume cache while leaving base-area cache intact, which is the right dependency behavior.
245-279: DependenttrackWhenNotCachedcloses the observation behavior matrixThe uncached-observation case yields the expected single notification when
zchanges, and cache flags/call counts align with the intended semantics of memoization and dependency tracking.
349-374: Baseline@ObservableCube type is well-shaped for memoization testsThe observable cube’s fields, call counters, cache-state helpers, and
@Memoizedmethods align with other test cubes and provide a clear, minimal surface for exercising theMemoizedmacro in the plain Observation case.
| public func _beginModifications() { | ||
| pendingModifications += 1 | ||
| if pendingModifications == 1 { | ||
| _willChange.send(object) | ||
| } | ||
| pendingModifications += 1 | ||
| } | ||
|
|
||
| func endModifications() { | ||
| pendingModifications -= 1 | ||
| if pendingModifications == 0 { | ||
| public func _endModifications() { | ||
| if pendingModifications == 1 { | ||
| _didChange.send(object) | ||
| } | ||
| pendingModifications -= 1 | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify balanced begin/end modification calls across the codebase.
The new modification lifecycle counter properly handles nested modifications, but relies on callers to balance _beginModifications() and _endModifications() calls. If _endModifications() is called when pendingModifications is already 0, the counter will become negative, leading to incorrect behavior.
Run the following script to verify that all call sites properly balance these methods:
🏁 Script executed:
#!/bin/bash
# Description: Check for balanced _beginModifications and _endModifications calls
echo "=== Searching for _beginModifications calls ==="
rg -n '_beginModifications\(\)' --type swift
echo ""
echo "=== Searching for _endModifications calls ==="
rg -n '_endModifications\(\)' --type swift
echo ""
echo "=== Checking ObservationRegistrar implementations ==="
rg -nA5 -B5 'func (willSet|didSet|withMutation)' --type swift -g '*ObservationRegistrar*'Length of output: 7811
I need to inspect the complete function implementations to verify the balance across the willSet and didSet boundary, and check error handling in withMutation.
---
🏁 Script executed:
```shell
#!/bin/bash
# Get full implementations of willSet, didSet, and withMutation functions
echo "=== Full willSet implementation ==="
sed -n '98,107p' Sources/RelayMacros/Publishable/ObservationRegistrarDeclBuilder.swift
echo ""
echo "=== Full didSet implementation ==="
sed -n '109,118p' Sources/RelayMacros/Publishable/ObservationRegistrarDeclBuilder.swift
echo ""
echo "=== Full withMutation implementation ==="
sed -n '128,148p' Sources/RelayMacros/Publishable/ObservationRegistrarDeclBuilder.swift
echo ""
echo "=== Full code context around these functions ==="
cat Sources/RelayMacros/Publishable/ObservationRegistrarDeclBuilder.swift | sed -n '96,150p'
Length of output: 604
🏁 Script executed:
#!/bin/bash
# Get full implementations with repository access
echo "=== Full context around willSet, didSet, withMutation ==="
cat Sources/RelayMacros/Publishable/ObservationRegistrarDeclBuilder.swift | sed -n '96,150p'Length of output: 2044
Fix error handling in withMutation to prevent counter imbalance
The withMutation implementation has a critical error handling issue. If underlying.withMutation() or publish() throws an exception, _endModifications() will not be called, leaving pendingModifications in an inconsistent state.
Location: Sources/RelayMacros/Publishable/ObservationRegistrarDeclBuilder.swift, lines 128-148
The _endModifications() call must be guaranteed to execute using a defer block or similar mechanism:
nonisolated func withMutation<T>(
of object: \(trimmedType),
keyPath: KeyPath<\(trimmedType), some Any>,
_ mutation: () throws -> T
) rethrows -> T {
nonisolated(unsafe) let mutation = mutation
nonisolated(unsafe) let keyPath = keyPath
nonisolated(unsafe) var result: T!
try assumeIsolatedIfNeeded {
object.publisher._beginModifications()
defer {
object.publisher._endModifications()
}
result = try underlying.withMutation(of: object, keyPath: keyPath, mutation)
publish(object, keyPath: keyPath)
}
return result
}🤖 Prompt for AI Agents
In Sources/RelayMacros/Publishable/ObservationRegistrarDeclBuilder.swift around
lines 128-148, the withMutation implementation can exit early when
underlying.withMutation() or publish() throws, leaving
object.publisher.pendingModifications unbalanced; to fix it, call
object.publisher._beginModifications() before invoking
underlying.withMutation(), and immediately follow it with a defer that calls
object.publisher._endModifications() so _endModifications() always runs even if
an error is thrown, then perform the underlying.withMutation call and publish
inside the protected block and return the result as before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (1)
Tests/RelayMacrosTests/Publishable/MainActorPublishableMacroTests.swift (1)
20-246: Resolve thefunction_body_lengthlint error before merging.
testExpansionis still ~225 lines, so SwiftLint continues to fail. Please either extract the raw input/expected strings into private constants or silence the rule locally (e.g.// swiftlint:disable:next function_body_length) so CI passes.- func testExpansion() { + // swiftlint:disable:next function_body_length + func testExpansion() {(Or pull the literals out—whichever matches the rest of the suite.)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
.gitmodules(1 hunks)Macros/Dependencies/PrincipleMacros(1 hunks)Macros/RelayMacros/Main/RelayPlugin.swift(1 hunks)Macros/RelayMacros/Memoized/MemoizedDeclBuilder.swift(1 hunks)Macros/RelayMacros/Memoized/MemoizedMacro.swift(1 hunks)Macros/RelayMacros/Publishable/ObservationRegistrarDeclBuilder.swift(1 hunks)Macros/RelayMacros/Publishable/PropertyPublisherDeclBuilder.swift(1 hunks)Macros/RelayMacros/Publishable/PublishableMacro.swift(4 hunks)Macros/RelayMacros/Publishable/PublisherDeclBuilder.swift(1 hunks)Package.resolved(1 hunks)Package.swift(3 hunks)Sources/Relay/Memoized/AccessControlLevel.swift(1 hunks)Sources/Relay/Memoized/Memoized.swift(1 hunks)Tests/RelayMacrosTests/Memoized/MainActorMemoizedMacroTests.swift(1 hunks)Tests/RelayMacrosTests/Memoized/MemoizedMacroTests.swift(1 hunks)Tests/RelayMacrosTests/Publishable/MainActorPublishableMacroTests.swift(1 hunks)Tests/RelayMacrosTests/Publishable/PublishableMacroTests.swift(5 hunks)
✅ Files skipped from review due to trivial changes (2)
- .gitmodules
- Macros/Dependencies/PrincipleMacros
🧰 Additional context used
🧬 Code graph analysis (8)
Macros/RelayMacros/Memoized/MemoizedMacro.swift (2)
Macros/RelayMacros/Publishable/PropertyPublisherDeclBuilder.swift (1)
build(18-33)Macros/RelayMacros/Memoized/MemoizedDeclBuilder.swift (1)
build(21-45)
Macros/RelayMacros/Publishable/PropertyPublisherDeclBuilder.swift (1)
Macros/RelayMacros/Memoized/MemoizedMacro.swift (2)
trimmedReturnType(109-111)defaultPropertyName(102-107)
Macros/RelayMacros/Publishable/PublisherDeclBuilder.swift (1)
Macros/RelayMacros/Publishable/PropertyPublisherDeclBuilder.swift (1)
build(18-33)
Macros/RelayMacros/Memoized/MemoizedDeclBuilder.swift (2)
Macros/RelayMacros/Memoized/MemoizedMacro.swift (1)
trimmedReturnType(109-111)Macros/RelayMacros/Publishable/ObservationRegistrarDeclBuilder.swift (1)
assumeIsolatedIfNeededFunction(150-182)
Macros/RelayMacros/Publishable/PublishableMacro.swift (1)
Macros/RelayMacros/Memoized/MemoizedMacro.swift (1)
validate(20-69)
Tests/RelayMacrosTests/Memoized/MainActorMemoizedMacroTests.swift (2)
Tests/RelayMacrosTests/Memoized/MemoizedMacroTests.swift (2)
testExpansion(20-80)testExpansionWithParameters(82-142)Tests/RelayMacrosTests/Publishable/MainActorPublishableMacroTests.swift (1)
testExpansion(20-246)
Macros/RelayMacros/Publishable/ObservationRegistrarDeclBuilder.swift (3)
Macros/RelayMacros/Publishable/PropertyPublisherDeclBuilder.swift (1)
build(18-33)Macros/RelayMacros/Memoized/MemoizedDeclBuilder.swift (2)
build(21-45)assumeIsolatedIfNeededFunction(71-96)Macros/RelayMacros/Publishable/PublisherDeclBuilder.swift (1)
build(16-29)
Tests/RelayMacrosTests/Publishable/MainActorPublishableMacroTests.swift (2)
Tests/RelayMacrosTests/Memoized/MainActorMemoizedMacroTests.swift (1)
testExpansion(22-88)Tests/RelayMacrosTests/Publishable/PublishableMacroTests.swift (1)
testExpansion(20-233)
🪛 GitHub Actions: Pull request
Sources/Relay/Memoized/AccessControlLevel.swift
[error] 1-1: File Header Violation: Header comments should be consistent with project patterns (file_header)
🪛 SwiftLint (0.57.0)
Tests/RelayMacrosTests/Memoized/MemoizedMacroTests.swift
[Warning] 20-20: Function body should span 50 lines or less excluding comments and whitespace: currently spans 59 lines
(function_body_length)
[Warning] 82-82: Function body should span 50 lines or less excluding comments and whitespace: currently spans 59 lines
(function_body_length)
Macros/RelayMacros/Publishable/PropertyPublisherDeclBuilder.swift
[Warning] 35-35: A 'deinitializer' should not be placed amongst the type content(s) 'other_method'
(type_contents_order)
Sources/Relay/Memoized/AccessControlLevel.swift
[Warning] 1-1: Header comments should be consistent with project patterns
(file_header)
Tests/RelayMacrosTests/Publishable/MainActorPublishableMacroTests.swift
[Error] 20-20: Function body should span 100 lines or less excluding comments and whitespace: currently spans 225 lines
(function_body_length)
| \(observationTrackingBlock()) | ||
| } | ||
| """ | ||
| ] | ||
| } | ||
|
|
||
| private func observationTrackingBlock() -> CodeBlockItemSyntax { | ||
| """ | ||
| return withObservationTracking { | ||
| let result = \(declaration.name.trimmed)() | ||
| _\(raw: propertyName) = result | ||
| return result | ||
| } onChange: { | ||
| invalidateCache() | ||
| } | ||
| """ | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Call access before returning freshly computed values.
When _\(propertyName) is still nil, we fall through to the withObservationTracking path and return immediately. Because there’s no access(keyPath:) call in that branch, the first read of the memoized property never registers with Observation, so downstream observers won’t resubscribe after mutations. Only subsequent (cached) reads call access, which is too late. Please store the computed value, invoke access, and then return it so both the cold and warm paths behave identically.
Apply this diff to observationTrackingBlock():
- return withObservationTracking {
- let result = \(declaration.name.trimmed)()
- _\(raw: propertyName) = result
- return result
- } onChange: {
- invalidateCache()
- }
+ let value = withObservationTracking {
+ let computed = \(declaration.name.trimmed)()
+ _\(raw: propertyName) = computed
+ return computed
+ } onChange: {
+ invalidateCache()
+ }
+ access(keyPath: \\._\(raw: propertyName))
+ return value📝 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.
| \(observationTrackingBlock()) | |
| } | |
| """ | |
| ] | |
| } | |
| private func observationTrackingBlock() -> CodeBlockItemSyntax { | |
| """ | |
| return withObservationTracking { | |
| let result = \(declaration.name.trimmed)() | |
| _\(raw: propertyName) = result | |
| return result | |
| } onChange: { | |
| invalidateCache() | |
| } | |
| """ | |
| } | |
| \(observationTrackingBlock()) | |
| } | |
| """ | |
| ] | |
| } | |
| private func observationTrackingBlock() -> CodeBlockItemSyntax { | |
| """ | |
| let value = withObservationTracking { | |
| let computed = \(declaration.name.trimmed)() | |
| _\(raw: propertyName) = computed | |
| return computed | |
| } onChange: { | |
| invalidateCache() | |
| } | |
| access(keyPath: \\._\(raw: propertyName)) | |
| return value | |
| """ | |
| } |
🤖 Prompt for AI Agents
In Macros/RelayMacros/Memoized/MemoizedDeclBuilder.swift around lines 41 to 57,
the observationTrackingBlock returns the freshly computed value without calling
access(keyPath:) so the initial cold read never registers with Observation;
change the block to compute the value into a local let (e.g. value), assign it
to _\(propertyName), call access(keyPath: \.\(propertyName)) (or the correct
keyPath expression for the property) before returning that local value, ensuring
both the uncached and cached paths call access and behave identically.
| private func memoizedPropertiesPublishers() -> MemberBlockItemListSyntax { | ||
| for member in declaration.memberBlock.members { | ||
| if let functionDecl = member.decl.as(FunctionDeclSyntax.self), | ||
| let attribute = functionDecl.attributes.first(like: "@Memoized"), | ||
| let parameters = try? MemoizedMacro.Parameters(from: attribute), | ||
| let trimmedReturnType = MemoizedMacro.trimmedReturnType(of: functionDecl) { | ||
| let globalActor = parameters.preferredGlobalActorIsolation ?? inheritedGlobalActorIsolation | ||
| let accessControlLevel = parameters.preferredAccessControlLevel | ||
| let name = parameters.preferredPropertyName ?? MemoizedMacro.defaultPropertyName(for: functionDecl) | ||
| let type = trimmedReturnType | ||
| """ | ||
| \(globalActor)\(accessControlLevel)var \(raw: name): AnyPublisher<\(type), Never> { | ||
| _computedPropertyPublisher(for: \\.\(raw: name)) | ||
| } | ||
| """ | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix optional access control emission for memoized publishers.
parameters.preferredAccessControlLevel is optional, so when someone writes plain @Memoized func value() -> Int the publisher you emit becomes nilvar value: AnyPublisher<Int, Never>, which is invalid Swift and breaks Publishable expansion for every memoized method that doesn’t override the access level. Please reuse the same fallback logic the Memoized macro itself uses—derive the effective access control level (e.g. defaulting to the class’s member visibility) before interpolating.
A minimal fix is to centralize the memoized metadata so this builder receives a nonoptional AccessControlLevel:
- let accessControlLevel = parameters.preferredAccessControlLevel
+ let metadata = try MemoizedMacro.Metadata(for: functionDecl, attribute: attribute)
+ let accessControlLevel = metadata.accessControlLevel
+ let name = metadata.propertyName
+ let type = metadata.returnType(The helper can live alongside MemoizedMacro.Parameters so both the macro and this builder stay in sync.)
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In Macros/RelayMacros/Publishable/PropertyPublisherDeclBuilder.swift around
lines 82-99, the code interpolates parameters.preferredAccessControlLevel
directly into the generated var declaration but that value is optional,
producing invalid output like "nilvar ..."; change this to compute a
non-optional effective access control level before string interpolation (e.g.
let accessControlLevel = parameters.preferredAccessControlLevel ??
inheritedAccessControlLevelOrDefault) and use that variable in the emitted text,
and preferably extract this fallback logic into a small shared helper (adjacent
to MemoizedMacro.Parameters) so both the Memoized macro and this builder derive
the same effective AccessControlLevel.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/pull-request.yml (1)
119-120: Clarify the purpose of-IDEPackageEnablePrebuilts=NOflag.The flag has been added to both the Build (Xcode) and Test (Xcode) steps. While this may be intentional (e.g., to force compilation from source or work around prebuilt binary issues), document or explain the reason in a comment or commit message. This will help future maintainers understand the constraint.
Add an inline comment explaining the flag's purpose:
xcodebuild build \ -scheme ${{ needs.prepare.outputs.scheme }} \ -destination "${{ steps.destination.outputs.destination }}" \ + # Disable prebuilts to ensure consistent builds from source -IDEPackageEnablePrebuilts=NO | \Also applies to: 140-141
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pull-request.yml(6 hunks)
⏰ 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 (visionos)
- GitHub Check: build-and-test (maccatalyst)
- GitHub Check: build-and-test (tvos)
- GitHub Check: build-and-test (ios)
🔇 Additional comments (5)
.github/workflows/pull-request.yml (5)
48-65: Review scheme extraction logic for robustness.The scheme extraction uses
basename $GITHUB_REPOSITORYto infer the scheme name, then attempts to match it against xcodebuild output. This approach could fail if:
- The repository name contains special characters that aren't valid in Xcode scheme names
- Multiple schemes exist with similar naming patterns
- The repository is a monorepo with a different scheme name
Consider logging the full
xcodebuild -listoutput for debugging, and add explicit handling for edge cases. The current fallback to exit 1 is good, but additional context would help troubleshooting CI failures.Verify the scheme extraction works correctly by running a test build. You may also want to add a step to print the extracted scheme and platform values for visibility in workflow logs.
24-26: Good improvement: Checkout v5 with submodules support.The upgrade from
actions/checkout@v4tov5withsubmodules: trueis a solid enhancement. This ensures submodule-dependent code (if any) is properly cloned and available for linting, building, and testing.Also applies to: 75-77
31-36: Configuration verified successfully.The
.mise.tomlfile exists at the repository root and contains the required[tasks.lint]target with all dependencies properly defined (swiftlint and swiftformat). The mise setup in the workflow is fully supported by the existing configuration.
15-15: Versions are valid but runner is in beta; clarify intended stability requirements.The macOS 26 image is available in public preview for GitHub Actions, and Xcode 26.1.1 has been released. However, the image is marked as beta, meaning some software can be unstable on the new platform, and there could be queueing issues as capacity is balanced.
For production CI, this carries operational risk. If stability is critical, consider reverting to
macos-15(generally available) until macOS 26 reaches GA. If the project can tolerate beta status, the versions are sound—just document this trade-off.
90-100: Simulator destinations verified — configuration is current and valid.All referenced devices (iPhone 17 Pro Max, Apple Watch Series 11, Apple TV 4K 3rd generation, Apple Vision Pro) are real products as of November 2025, and OS 26.1 is the correct version format under Apple's year-based naming scheme (adopted June 9, 2025). The destination mappings will succeed provided the CI environment has the corresponding tvOS 26.1, watchOS 26.1, visionOS 26.1, and iOS 26.1 simulator runtimes installed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (4)
Tests/RelayMacrosTests/Publishable/MainActorPublishableMacroTests.swift (1)
20-252: Duplicate:function_body_lengthviolation already flagged.This issue was already raised in previous review comments with suggested solutions (extract literals or use local disable).
README.md (2)
3-5: Add descriptive alt text to the first two badges for accessibility.The first two badges still omit alt text (
), which markdownlint flags and which also hurts accessibility. Consider adding brief descriptions.-[](https://swiftpackageindex.com/NSFatalError/Relay) -[](https://swiftpackageindex.com/NSFatalError/Relay) +[](https://swiftpackageindex.com/NSFatalError/Relay) +[](https://swiftpackageindex.com/NSFatalError/Relay)
9-9: Fix heading level jump for “Contents”.
#### Contentsimmediately after# Relayviolates the heading‑increment rule; use an H2 here.-#### Contents +## ContentsMacros/RelayMacros/Memoized/MemoizedDeclBuilder.swift (1)
21-45: Callaccesson the cold path so the first memoized read participates in Observation.Right now the getter only calls
access(keyPath:)in the cached branch. On the initial (uncached) read, you fall through toobservationTrackingBlock()which returns directly fromwithObservationTrackingwithout ever callingaccess. That means the first read of the memoized property doesn’t register with Observation, and observers that rely on that first access won’t be wired up correctly.You can align cold and warm paths by computing into a local, then calling
accessand returning it:private func observationTrackingBlock() -> CodeBlockItemSyntax { """ - return withObservationTracking { - let result = \(declaration.name.trimmed)() - _\(raw: propertyName) = result - return result - } onChange: { - invalidateCache() - } + let value = withObservationTracking { + let computed = \(declaration.name.trimmed)() + _\(raw: propertyName) = computed + return computed + } onChange: { + invalidateCache() + } + access(keyPath: \\._\(raw: propertyName)) + return value """ }Also applies to: 47-56
🧹 Nitpick comments (4)
Macros/RelayMacros/Publishable/ObservationRegistrarDeclBuilder.swift (1)
156-188: Consider extracting theassumeIsolatedIfNeededpattern.This function is similar to the one in
MemoizedDeclBuilder.swift(lines 70-95). Both generate an actor-isolation delegation helper, though with slight variations (this one includesfile/lineparameters for debugging).Consider extracting the common pattern into a shared builder helper or protocol extension to reduce duplication, especially if more builders will need this pattern.
Tests/RelayMacrosTests/Memoized/MemoizedMacroTests.swift (1)
20-80: Consider extracting large source/expected strings to helpers to appease SwiftLint.Both
testExpansionandtestExpansionWithParametersexceed thefunction_body_lengththreshold mainly due to inlined multi‑line source and expandedSource strings. If you want to silence these warnings, consider moving those strings into private helpers or fixtures and keeping the test bodies to the assertions.Also applies to: 82-142
Macros/RelayMacros/Memoized/MemoizedMacro.swift (1)
20-39: Clarify whether@Memoizedshould supportVoidand generic methods.
validatecurrently only checks for a present return type, empty parameter list, no effects, and non‑static methods. That allows cases likefunc foo() -> Voidandfunc foo<T>() -> Intto pass, even though the error message claims “non‑void” methods and generic methods likely aren’t meaningful memoization targets.If you don’t intend to support those, consider tightening validation (e.g. rejecting
Void/()return types and methods with agenericParameterClause) or adjusting the diagnostic text so it matches actual behavior.Also applies to: 71-100, 102-111
Tests/RelayMacrosTests/Memoized/MainActorMemoizedMacroTests.swift (1)
22-88: Optionally shorten long test bodies to satisfyfunction_body_length.Both test methods exceed SwiftLint’s
function_body_lengthlimit due to large inline source and expandedSource literals. If you care about this rule, you could move those strings into private helpers or fixtures and keep the test bodies focused on the assertion.Also applies to: 90-156
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
Macros/Dependencies/PrincipleMacros(1 hunks)Macros/RelayMacros/Main/RelayPlugin.swift(1 hunks)Macros/RelayMacros/Memoized/MemoizedDeclBuilder.swift(1 hunks)Macros/RelayMacros/Memoized/MemoizedMacro.swift(1 hunks)Macros/RelayMacros/Publishable/ObservationRegistrarDeclBuilder.swift(1 hunks)README.md(2 hunks)Tests/RelayMacrosTests/Memoized/MainActorMemoizedMacroTests.swift(1 hunks)Tests/RelayMacrosTests/Memoized/MemoizedMacroTests.swift(1 hunks)Tests/RelayMacrosTests/Publishable/MainActorPublishableMacroTests.swift(1 hunks)Tests/RelayMacrosTests/Publishable/PublishableMacroTests.swift(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- Macros/Dependencies/PrincipleMacros
- Macros/RelayMacros/Main/RelayPlugin.swift
🧰 Additional context used
🧬 Code graph analysis (6)
Macros/RelayMacros/Memoized/MemoizedMacro.swift (1)
Macros/RelayMacros/Memoized/MemoizedDeclBuilder.swift (1)
build(21-45)
Macros/RelayMacros/Publishable/ObservationRegistrarDeclBuilder.swift (3)
Macros/RelayMacros/Memoized/MemoizedDeclBuilder.swift (2)
build(21-45)assumeIsolatedIfNeededFunction(71-96)Macros/RelayMacros/Publishable/PublisherDeclBuilder.swift (1)
build(16-29)Macros/RelayMacros/Publishable/PropertyPublisherDeclBuilder.swift (1)
build(18-33)
Tests/RelayMacrosTests/Publishable/MainActorPublishableMacroTests.swift (2)
Tests/RelayMacrosTests/Memoized/MainActorMemoizedMacroTests.swift (1)
testExpansion(22-88)Tests/RelayMacrosTests/Publishable/PublishableMacroTests.swift (1)
testExpansion(20-239)
Tests/RelayMacrosTests/Memoized/MemoizedMacroTests.swift (1)
Tests/RelayMacrosTests/Memoized/MainActorMemoizedMacroTests.swift (2)
testExpansion(22-88)testExpansionWithParameters(90-156)
Tests/RelayMacrosTests/Memoized/MainActorMemoizedMacroTests.swift (2)
Tests/RelayMacrosTests/Memoized/MemoizedMacroTests.swift (2)
testExpansion(20-80)testExpansionWithParameters(82-142)Tests/RelayMacrosTests/Publishable/MainActorPublishableMacroTests.swift (1)
testExpansion(20-252)
Macros/RelayMacros/Memoized/MemoizedDeclBuilder.swift (2)
Macros/RelayMacros/Memoized/MemoizedMacro.swift (1)
trimmedReturnType(109-111)Macros/RelayMacros/Publishable/ObservationRegistrarDeclBuilder.swift (1)
assumeIsolatedIfNeededFunction(156-188)
🪛 GitHub Actions: Pull request
Macros/RelayMacros/Publishable/ObservationRegistrarDeclBuilder.swift
[error] 97-97: SwiftLint: Function Body Length Violation: Function body should span 50 lines or less excluding comments and whitespace: currently spans 56 lines (function_body_length)
🪛 markdownlint-cli2 (0.18.1)
README.md
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
4-4: Images should have alternate text (alt text)
(MD045, no-alt-text)
9-9: Heading levels should only increment by one level at a time
Expected: h2; Actual: h4
(MD001, heading-increment)
🪛 SwiftLint (0.57.0)
Macros/RelayMacros/Publishable/ObservationRegistrarDeclBuilder.swift
[Warning] 97-97: Function body should span 50 lines or less excluding comments and whitespace: currently spans 56 lines
(function_body_length)
Tests/RelayMacrosTests/Publishable/MainActorPublishableMacroTests.swift
[Error] 20-20: Function body should span 100 lines or less excluding comments and whitespace: currently spans 231 lines
(function_body_length)
Tests/RelayMacrosTests/Memoized/MemoizedMacroTests.swift
[Warning] 20-20: Function body should span 50 lines or less excluding comments and whitespace: currently spans 59 lines
(function_body_length)
[Warning] 82-82: Function body should span 50 lines or less excluding comments and whitespace: currently spans 59 lines
(function_body_length)
Tests/RelayMacrosTests/Memoized/MainActorMemoizedMacroTests.swift
[Warning] 22-22: Function body should span 50 lines or less excluding comments and whitespace: currently spans 65 lines
(function_body_length)
[Warning] 90-90: Function body should span 50 lines or less excluding comments and whitespace: currently spans 65 lines
(function_body_length)
🔇 Additional comments (12)
Macros/RelayMacros/Publishable/ObservationRegistrarDeclBuilder.swift (3)
12-42: LGTM!The builder structure and
build()method correctly generate the nestedObservationenum containing theObservationRegistrar. The composition approach using helper methods keeps the code organized.
44-66: LGTM!The
publishmethod generation correctly iterates over unique inferred types and generates keypath casting logic with early returns. This ensures type-safe subject resolution.
68-95: LGTM!The subject accessor methods correctly generate type-specific keypath matching with proper nil fallback for unknown keypaths.
Tests/RelayMacrosTests/Publishable/PublishableMacroTests.swift (4)
9-11: LGTM!The import correctly references
RelayMacros, aligning with the PR's restructuring from Publishable to Relay.
42-50: LGTM!The test correctly expects the
@Memoized(.public)annotation to be preserved in the@Publishableexpansion, as these are separate macros that process independently.Also applies to: 79-83
117-123: LGTM!The publisher visibility correctly matches the source properties:
fileprivatefor the privateinitialscomputed property, andpublicfor the memoizedlabelproperty.
170-230: LGTM!The lifecycle methods (
willSet,didSet,access,withMutation) correctly implement nonisolated wrappers with properassumeIsolatedIfNeededdelegation. Thedeferblock inwithMutationensures cleanup happens on all exit paths.Tests/RelayMacrosTests/Publishable/MainActorPublishableMacroTests.swift (2)
102-123: LGTM!The
@MainActorannotations on all publisher accessors correctly reflect the parent class's@MainActorisolation, ensuring thread-safe access.
225-242: LGTM!The
assumeIsolatedIfNeededimplementation correctly handles@MainActorisolation using thewithoutActuallyEscapingandunsafeBitCastpattern from the Swift standard library. Thefile/lineparameters provide useful debugging context.Tests/RelayMacrosTests/Memoized/MemoizedMacroTests.swift (1)
16-18: Macro registration for tests looks correct.The
macrosdictionary wiring"Memoized"toMemoizedMacro.selfis clear and matches how the tests useassertMacroExpansion.Macros/RelayMacros/Memoized/MemoizedMacro.swift (1)
114-138: Macro expansion pipeline is well-structured.The flow from
Parametersparsing throughvalidate(_:in:with:)intoMemoizedDeclBuilderis cohesive and keeps validation separate from codegen, which should make the macro easy to evolve.Also applies to: 141-155
Tests/RelayMacrosTests/Memoized/MainActorMemoizedMacroTests.swift (1)
16-21: MainActor macro test setup looks solid.The
macrosdictionary wiring and the explicit// swiftlint:disable global_actor_attribute_orderare appropriate here, since the literals are meant to mirror the compiler’s expansion exactly.
| private func publishableObservationRegistrarFunctions() -> MemberBlockItemListSyntax { | ||
| """ | ||
| nonisolated func willSet( | ||
| _ object: \(trimmedType), | ||
| keyPath: KeyPath<\(trimmedType), some Any> | ||
| ) { | ||
| nonisolated(unsafe) let keyPath = keyPath | ||
| assumeIsolatedIfNeeded { | ||
| object.publisher._beginModifications() | ||
| underlying.willSet(object, keyPath: keyPath) | ||
| } | ||
| } | ||
| nonisolated func didSet( | ||
| _ object: \(trimmedType), | ||
| keyPath: KeyPath<\(trimmedType), some Any> | ||
| ) { | ||
| nonisolated(unsafe) let keyPath = keyPath | ||
| assumeIsolatedIfNeeded { | ||
| underlying.didSet(object, keyPath: keyPath) | ||
| publish(object, keyPath: keyPath) | ||
| object.publisher._endModifications() | ||
| } | ||
| } | ||
| nonisolated func access( | ||
| _ object: \(trimmedType), | ||
| keyPath: KeyPath<\(trimmedType), some Any> | ||
| ) { | ||
| underlying.access(object, keyPath: keyPath) | ||
| } | ||
| nonisolated func withMutation<T>( | ||
| of object: \(trimmedType), | ||
| keyPath: KeyPath<\(trimmedType), some Any>, | ||
| _ mutation: () throws -> T | ||
| ) rethrows -> T { | ||
| nonisolated(unsafe) let mutation = mutation | ||
| nonisolated(unsafe) let keyPath = keyPath | ||
| nonisolated(unsafe) var result: T! | ||
| try assumeIsolatedIfNeeded { | ||
| object.publisher._beginModifications() | ||
| defer { | ||
| publish(object, keyPath: keyPath) | ||
| object.publisher._endModifications() | ||
| } | ||
| result = try underlying.withMutation( | ||
| of: object, | ||
| keyPath: keyPath, | ||
| mutation | ||
| ) | ||
| } | ||
| return result | ||
| } | ||
| """ | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extract methods to fix function_body_length violation blocking CI.
SwiftLint reports this function spans 56 lines (limit: 50), causing a CI error. The four lifecycle methods (willSet, didSet, access, withMutation) can be extracted into separate helper methods.
Apply this refactor to resolve the violation:
private func publishableObservationRegistrarFunctions() -> MemberBlockItemListSyntax {
"""
- nonisolated func willSet(
- _ object: \(trimmedType),
- keyPath: KeyPath<\(trimmedType), some Any>
- ) {
- nonisolated(unsafe) let keyPath = keyPath
- assumeIsolatedIfNeeded {
- object.publisher._beginModifications()
- underlying.willSet(object, keyPath: keyPath)
- }
- }
-
- nonisolated func didSet(
- _ object: \(trimmedType),
- keyPath: KeyPath<\(trimmedType), some Any>
- ) {
- nonisolated(unsafe) let keyPath = keyPath
- assumeIsolatedIfNeeded {
- underlying.didSet(object, keyPath: keyPath)
- publish(object, keyPath: keyPath)
- object.publisher._endModifications()
- }
- }
-
- nonisolated func access(
- _ object: \(trimmedType),
- keyPath: KeyPath<\(trimmedType), some Any>
- ) {
- underlying.access(object, keyPath: keyPath)
- }
-
- nonisolated func withMutation<T>(
- of object: \(trimmedType),
- keyPath: KeyPath<\(trimmedType), some Any>,
- _ mutation: () throws -> T
- ) rethrows -> T {
- nonisolated(unsafe) let mutation = mutation
- nonisolated(unsafe) let keyPath = keyPath
- nonisolated(unsafe) var result: T!
-
- try assumeIsolatedIfNeeded {
- object.publisher._beginModifications()
- defer {
- publish(object, keyPath: keyPath)
- object.publisher._endModifications()
- }
- result = try underlying.withMutation(
- of: object,
- keyPath: keyPath,
- mutation
- )
- }
-
- return result
- }
+ \(willSetFunction())
+
+ \(didSetFunction())
+
+ \(accessFunction())
+
+ \(withMutationFunction())
"""
}
+
+private func willSetFunction() -> MemberBlockItemListSyntax {
+ """
+ nonisolated func willSet(
+ _ object: \(trimmedType),
+ keyPath: KeyPath<\(trimmedType), some Any>
+ ) {
+ nonisolated(unsafe) let keyPath = keyPath
+ assumeIsolatedIfNeeded {
+ object.publisher._beginModifications()
+ underlying.willSet(object, keyPath: keyPath)
+ }
+ }
+ """
+}
+
+private func didSetFunction() -> MemberBlockItemListSyntax {
+ """
+ nonisolated func didSet(
+ _ object: \(trimmedType),
+ keyPath: KeyPath<\(trimmedType), some Any>
+ ) {
+ nonisolated(unsafe) let keyPath = keyPath
+ assumeIsolatedIfNeeded {
+ underlying.didSet(object, keyPath: keyPath)
+ publish(object, keyPath: keyPath)
+ object.publisher._endModifications()
+ }
+ }
+ """
+}
+
+private func accessFunction() -> MemberBlockItemListSyntax {
+ """
+ nonisolated func access(
+ _ object: \(trimmedType),
+ keyPath: KeyPath<\(trimmedType), some Any>
+ ) {
+ underlying.access(object, keyPath: keyPath)
+ }
+ """
+}
+
+private func withMutationFunction() -> MemberBlockItemListSyntax {
+ """
+ nonisolated func withMutation<T>(
+ of object: \(trimmedType),
+ keyPath: KeyPath<\(trimmedType), some Any>,
+ _ mutation: () throws -> T
+ ) rethrows -> T {
+ nonisolated(unsafe) let mutation = mutation
+ nonisolated(unsafe) let keyPath = keyPath
+ nonisolated(unsafe) var result: T!
+
+ try assumeIsolatedIfNeeded {
+ object.publisher._beginModifications()
+ defer {
+ publish(object, keyPath: keyPath)
+ object.publisher._endModifications()
+ }
+ result = try underlying.withMutation(
+ of: object,
+ keyPath: keyPath,
+ mutation
+ )
+ }
+
+ return result
+ }
+ """
+}Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Actions: Pull request
[error] 97-97: SwiftLint: Function Body Length Violation: Function body should span 50 lines or less excluding comments and whitespace: currently spans 56 lines (function_body_length)
🪛 SwiftLint (0.57.0)
[Warning] 97-97: Function body should span 50 lines or less excluding comments and whitespace: currently spans 56 lines
(function_body_length)
🤖 Prompt for AI Agents
In Macros/RelayMacros/Publishable/ObservationRegistrarDeclBuilder.swift around
lines 97 to 154, the publishableObservationRegistrarFunctions() string literal
is 56 lines and triggers SwiftLint's function_body_length; extract each
lifecycle method body into separate private helper methods (e.g.,
buildWillSet(), buildDidSet(), buildAccess(), buildWithMutation()) that return
the corresponding MemberBlockItemListSyntax or string snippet, call those
helpers from publishableObservationRegistrarFunctions() to assemble the final
block, and ensure all captured names (trimmedType, underlying, publish,
assumeIsolatedIfNeeded) remain in scope or are passed into the helpers as
parameters so the resulting publishableObservationRegistrarFunctions() falls
under the 50-line limit.
| // MainActorPublishableMacroTests.swift | ||
| // Relay | ||
| // | ||
| // Created by Kamil Strzelecki on 24/08/2025. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the future creation date.
The creation date is 24/08/2025, which is in the future (current PR date is 2025-11-16). This should likely be an earlier date or corrected to match when the file was actually created.
🤖 Prompt for AI Agents
In Tests/RelayMacrosTests/Publishable/MainActorPublishableMacroTests.swift
around line 5, the file header has a creation date "24/08/2025" which is in the
future; update that date to a past date (e.g., the actual creation date from Git
history or a sensible past date like "24/08/2025" -> "24/08/2024") or
remove/replace the header comment entirely so the file no longer claims a future
creation date.
Summary by CodeRabbit
New Features
Refactor
Documentation
Chores
Tests