Add SymbolLocator dependency and OpenSwiftUISymbolDualTests #263
Conversation
WalkthroughThis update restructures the Swift package manifest ( Changes
Sequence Diagram(s)sequenceDiagram
participant TestRunner
participant CGSize_ExtensionTests
participant CGSize
participant SwiftUIStub
TestRunner->>CGSize_ExtensionTests: Run hasZero test
CGSize_ExtensionTests->>CGSize: Call hasZero (OpenSwiftUI)
CGSize_ExtensionTests->>CGSize: Call swiftUIHasZero (via @_silgen_name)
CGSize->>SwiftUIStub: Access OpenSwiftUITestStub_CGSizeHasZero
SwiftUIStub-->>CGSize: Return result
CGSize_ExtensionTests->>TestRunner: Assert results match expected
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
Tests/OpenSwiftUISymbolDualTests/README.md (1)
3-4: Minor wording tweak for clarityMissing article “a/the” makes the sentence read a little abruptly.
-Test non-public API of SwiftUI via SymbolLocator. +Test a non-public SwiftUI API via the SymbolLocator framework.🧰 Tools
🪛 LanguageTool
[uncategorized] ~3-~3: You might be missing the article “the” here.
Context: ## OpenSwiftUISymbolDualTests Test non-public API of SwiftUI via SymbolLocator...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
Tests/OpenSwiftUISymbolDualTests/Extension/CGSize+ExtensionTests.swift (1)
18-18: Rename test type to adhere to Swift naming guidelinesSwiftLint rightfully warns that
CGSize_ExtensionTestscontains an underscore.
CGSizeHasZeroTestsorCGSizeExtensionTestsreads cleaner and avoids the violation.-struct CGSize_ExtensionTests { +struct CGSizeHasZeroTests {🧰 Tools
🪛 SwiftLint (0.57.0)
[Error] 18-18: Type name 'CGSize_ExtensionTests' should only contain alphanumeric and other allowed characters
(type_name)
Package.swift (1)
249-259: Helper target is always built but contains only Darwin stubs
OpenSwiftUISymbolDualTestsHelperhas C sources that are effectively empty on non-Darwin after the previous suggestion.
SwiftPM will still build (and distribute) the target on Linux, pulling theSymbolLocatordependency unnecessarily.Consider limiting the target with a platform condition to speed up cross-platform builds:
-let openSwiftUISymbolDualTestsHelperTarget = Target.target( +let openSwiftUISymbolDualTestsHelperTarget = Target.target( + // Only meaningful on Apple OSes where SwiftUI exists + // (Linux & Windows can still compile the main package without the helper) name: "OpenSwiftUISymbolDualTestsHelper", dependencies: [ .product(name: "SymbolLocator", package: "SymbolLocator"), ], @@ -) + swiftSettings: sharedSwiftSettings, + linkerSettings: [], + cSettings: [], + cxxSettings: [], + // 🔽 + // 📌 Make this target Apple-only. + // .when(platforms: [.darwinPlatforms]) +)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Package.resolved(0 hunks)Package.swift(7 hunks)Sources/OpenSwiftUISymbolDualTestsHelper/Extension/CGSize+ExtensionTestsStub.c(1 hunks)Tests/OpenSwiftUISymbolDualTests/Extension/CGSize+ExtensionTests.swift(1 hunks)Tests/OpenSwiftUISymbolDualTests/README.md(1 hunks)
💤 Files with no reviewable changes (1)
- Package.resolved
🧰 Additional context used
🪛 LanguageTool
Tests/OpenSwiftUISymbolDualTests/README.md
[uncategorized] ~3-~3: You might be missing the article “the” here.
Context: ## OpenSwiftUISymbolDualTests Test non-public API of SwiftUI via SymbolLocator...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🪛 SwiftLint (0.57.0)
Tests/OpenSwiftUISymbolDualTests/Extension/CGSize+ExtensionTests.swift
[Error] 18-18: Type name 'CGSize_ExtensionTests' should only contain alphanumeric and other allowed characters
(type_name)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Execute tests on Ubuntu (6.0.1)
- GitHub Check: Execute compatibility tests for macOS (macos-15, 16.0, 2024)
- GitHub Check: Execute tests on macOS (macos-15, 16.0, 2024)
- GitHub Check: Execute tests on macOS (macos-15, 16.0, 2021)
- GitHub Check: Execute tests on iOS (macos-15, 16.0, 2021, 18.0)
- GitHub Check: Execute tests on iOS (macos-15, 16.0, 2024, 18.0)
🔇 Additional comments (2)
Tests/OpenSwiftUISymbolDualTests/Extension/CGSize+ExtensionTests.swift (1)
17-33: Attribute argument syntax likely to fail compilationThe second parameter is passed as
.enabled { … }, but the@Testmacro expects either
enabled:labeled argument or.enabled{}inside an options array depending on the overload.Please double-check the syntax against the current
TestingAPI. A minimal change that compiles on 6.1 snapshots is:- .enabled { + enabled: { #available(iOS 18, macOS 14, *) },Also consider removing the trailing comma before the closing
)to silence “unexpected argument” diagnostics.🧰 Tools
🪛 SwiftLint (0.57.0)
[Error] 18-18: Type name 'CGSize_ExtensionTests' should only contain alphanumeric and other allowed characters
(type_name)
Package.swift (1)
300-303: Confirm third-party licence compatibility forSymbolLocatorBefore merging, verify that
SymbolLocator’s licence is compatible with OpenSwiftUI’s distribution model (especially if you plan to publish binaries).
| #include "OpenSwiftUIBase.h" | ||
|
|
||
| #if OPENSWIFTUI_TARGET_OS_DARWIN | ||
|
|
||
| #import <SymbolLocator.h> | ||
|
|
||
| DEFINE_SL_STUB_SLF(OpenSwiftUITestStub_CGSizeHasZero, SwiftUI, $sSo6CGSizeV7SwiftUIE7hasZeroSbvg); | ||
|
|
||
| #endif |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard the Darwin-only code earlier to avoid build failures on non-Darwin platforms
OpenSwiftUIBase.h may itself pull in Darwin-specific SDK headers. Including it unconditionally means this file will still be compiled on Linux/WASI and will fail during preprocessing even though the body is wrapped in #if OPENSWIFTUI_TARGET_OS_DARWIN.
-#include "OpenSwiftUIBase.h"
-
-#if OPENSWIFTUI_TARGET_OS_DARWIN
-#import <SymbolLocator.h>
-DEFINE_SL_STUB_SLF(OpenSwiftUITestStub_CGSizeHasZero, SwiftUI, $sSo6CGSizeV7SwiftUIE7hasZeroSbvg);
-#endif
+#if OPENSWIFTUI_TARGET_OS_DARWIN
+#include "OpenSwiftUIBase.h"
+#import <SymbolLocator.h>
+DEFINE_SL_STUB_SLF(OpenSwiftUITestStub_CGSizeHasZero, SwiftUI, $sSo6CGSizeV7SwiftUIE7hasZeroSbvg);
+#endifThis keeps the entire translation unit empty on non-Darwin platforms and avoids accidental header leakage.
📝 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.
| #include "OpenSwiftUIBase.h" | |
| #if OPENSWIFTUI_TARGET_OS_DARWIN | |
| #import <SymbolLocator.h> | |
| DEFINE_SL_STUB_SLF(OpenSwiftUITestStub_CGSizeHasZero, SwiftUI, $sSo6CGSizeV7SwiftUIE7hasZeroSbvg); | |
| #endif | |
| #if OPENSWIFTUI_TARGET_OS_DARWIN | |
| #include "OpenSwiftUIBase.h" | |
| #import <SymbolLocator.h> | |
| DEFINE_SL_STUB_SLF(OpenSwiftUITestStub_CGSizeHasZero, SwiftUI, $sSo6CGSizeV7SwiftUIE7hasZeroSbvg); | |
| #endif |
| extension CGSize { | ||
| var swiftUIHasZero: Bool { | ||
| @_silgen_name("OpenSwiftUITestStub_CGSizeHasZero") | ||
| get | ||
| } | ||
| } |
There was a problem hiding this comment.
Prefer an external C-style thunk instead of applying @_silgen_name directly to a property
@_silgen_name is intended for functions; applying it to a property accessor is unsupported and relies on undefined behaviour.
A safer pattern is to expose the stub as a private C-style function and wrap it in a Swift computed property:
-extension CGSize {
- var swiftUIHasZero: Bool {
- @_silgen_name("OpenSwiftUITestStub_CGSizeHasZero")
- get
- }
-}
+@_silgen_name("OpenSwiftUITestStub_CGSizeHasZero")
+private func _swiftUIHasZero(_ size: CGSize) -> Bool
+
+extension CGSize {
+ var swiftUIHasZero: Bool { _swiftUIHasZero(self) }
+}This avoids ABI guess-work and makes the call-site explicit.
📝 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.
| extension CGSize { | |
| var swiftUIHasZero: Bool { | |
| @_silgen_name("OpenSwiftUITestStub_CGSizeHasZero") | |
| get | |
| } | |
| } | |
| @_silgen_name("OpenSwiftUITestStub_CGSizeHasZero") | |
| private func _swiftUIHasZero(_ size: CGSize) -> Bool | |
| extension CGSize { | |
| var swiftUIHasZero: Bool { _swiftUIHasZero(self) } | |
| } |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #263 +/- ##
==========================================
+ Coverage 20.94% 20.96% +0.02%
==========================================
Files 337 337
Lines 15486 15486
==========================================
+ Hits 3243 3247 +4
+ Misses 12243 12239 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
Tests/OpenSwiftUISymbolDualTests/Extension/CGSize+ExtensionTests.swift (1)
10-15: Prefer an external C-style thunk instead of applying@_silgen_namedirectly to a property
@_silgen_nameis intended for functions; applying it to a property accessor is unsupported and relies on undefined behavior.
A safer pattern is to expose the stub as a private C-style function and wrap it in a Swift computed property:-extension CGSize { - var swiftUIHasZero: Bool { - @_silgen_name("OpenSwiftUITestStub_CGSizeHasZero") - get - } -} +@_silgen_name("OpenSwiftUITestStub_CGSizeHasZero") +private func _swiftUIHasZero(_ size: CGSize) -> Bool + +extension CGSize { + var swiftUIHasZero: Bool { _swiftUIHasZero(self) } +}This avoids ABI guess-work and makes the call-site explicit.
🧹 Nitpick comments (2)
Tests/OpenSwiftUISymbolDualTests/README.md (1)
3-3: Consider adding "the" before "non-public API"The sentence would read better as "Test the non-public API of SwiftUI via SymbolLocator."
-Test non-public API of SwiftUI via SymbolLocator. +Test the non-public API of SwiftUI via SymbolLocator.🧰 Tools
🪛 LanguageTool
[uncategorized] ~3-~3: You might be missing the article “the” here.
Context: ## OpenSwiftUISymbolDualTests Test non-public API of SwiftUI via SymbolLocator...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
Tests/OpenSwiftUISymbolDualTests/Extension/CGSize+ExtensionTests.swift (1)
18-18: Use camelCase for type names instead of snake_caseSwift naming conventions recommend using camelCase for type names. Rename
CGSize_ExtensionTeststoCGSizeExtensionTeststo follow this convention.-struct CGSize_ExtensionTests { +struct CGSizeExtensionTests {🧰 Tools
🪛 SwiftLint (0.57.0)
[Error] 18-18: Type name 'CGSize_ExtensionTests' should only contain alphanumeric and other allowed characters
(type_name)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Package.resolved(0 hunks)Package.swift(7 hunks)Sources/OpenSwiftUISymbolDualTestsHelper/Extension/CGSize+ExtensionTestsStub.c(1 hunks)Tests/OpenSwiftUISymbolDualTests/Extension/CGSize+ExtensionTests.swift(1 hunks)Tests/OpenSwiftUISymbolDualTests/README.md(1 hunks)
💤 Files with no reviewable changes (1)
- Package.resolved
🚧 Files skipped from review as they are similar to previous changes (1)
- Sources/OpenSwiftUISymbolDualTestsHelper/Extension/CGSize+ExtensionTestsStub.c
🧰 Additional context used
🪛 SwiftLint (0.57.0)
Tests/OpenSwiftUISymbolDualTests/Extension/CGSize+ExtensionTests.swift
[Error] 18-18: Type name 'CGSize_ExtensionTests' should only contain alphanumeric and other allowed characters
(type_name)
🪛 LanguageTool
Tests/OpenSwiftUISymbolDualTests/README.md
[uncategorized] ~3-~3: You might be missing the article “the” here.
Context: ## OpenSwiftUISymbolDualTests Test non-public API of SwiftUI via SymbolLocator...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Execute compatibility tests for iOS (macos-15, 16.0, 2024, 18.0)
- GitHub Check: Execute tests on Ubuntu (6.0.1)
🔇 Additional comments (11)
Tests/OpenSwiftUISymbolDualTests/README.md (1)
5-23: Code examples look good!The code examples clearly demonstrate how to use the SymbolLocator for testing SwiftUI internal symbols. The C macro for defining the stub and the Swift extension with the silgen name annotations are well structured.
Tests/OpenSwiftUISymbolDualTests/Extension/CGSize+ExtensionTests.swift (1)
19-40: Well-structured test with comprehensive test casesThe test is well-designed with multiple test cases covering different combinations of width and height values. The test correctly verifies that both the OpenSwiftUI and SwiftUI implementations behave identically and match the expected results.
Package.swift (9)
104-118: Clean restructuring of targetsThe CoreGraphicsShims targets are now properly organized with a main target and corresponding test target, which improves the package structure.
132-143: Good test target organizationThe OpenSwiftUI_SPITests target is properly structured with appropriate dependencies, including the main target and Numerics for testing.
161-170: Well-structured test target with appropriate dependenciesThe OpenSwiftUICoreTests target includes the necessary dependencies for testing, with a proper comment explaining the OpenSwiftUI import requirement.
174-181: Clean COpenSwiftUI target declarationThe COpenSwiftUI target is well-structured with appropriate header search paths and settings.
204-220: Good test target reorganizationThe OpenSwiftUITests and OpenSwiftUICompatibilityTests targets are properly restructured with appropriate dependencies.
228-234: Proper source specification for OpenSwiftUIBridgeThe OpenSwiftUIBridge target now explicitly specifies its sources, which improves clarity and maintainability.
247-269: Well-designed SymbolDualTests infrastructureThe new OpenSwiftUISymbolDualTestsHelper target and its corresponding test target are properly structured with the necessary SymbolLocator dependency. This provides a clean separation between the test support code and the actual tests.
301-301: Successfully added SymbolLocator dependencyThe SymbolLocator package dependency is properly specified with an appropriate version constraint.
315-334: Well-organized target listThe target list is now well-organized and includes all the new targets in a logical order, making the package structure easier to understand.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores