Skip to content

Updates from PackageDSLKit (#89)#165

Closed
leogdion wants to merge 0 commit into
v0.0.5from
packagedslkit
Closed

Updates from PackageDSLKit (#89)#165
leogdion wants to merge 0 commit into
v0.0.5from
packagedslkit

Conversation

@leogdion

Copy link
Copy Markdown
Member

No description provided.

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f041f637-af88-4810-bbb8-77c8b68a9458

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch packagedslkit

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecov Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.04%. Comparing base (61fb486) to head (bb78546).
⚠️ Report is 5 commits behind head on v0.0.5.

Files with missing lines Patch % Lines
Sources/SyntaxKit/Declarations/Struct.swift 0.00% 9 Missing ⚠️
Sources/SyntaxKit/Expressions/Closure.swift 0.00% 6 Missing ⚠️
Sources/SyntaxKit/Declarations/Init.swift 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           v0.0.5     #165      +/-   ##
==========================================
- Coverage   73.61%   73.04%   -0.57%     
==========================================
  Files         117      117              
  Lines        5613     5632      +19     
==========================================
- Hits         4132     4114      -18     
- Misses       1481     1518      +37     
Flag Coverage Δ
spm 72.42% <0.00%> (-0.36%) ⬇️
swift-6.1 72.60% <0.00%> (-0.84%) ⬇️
swift-6.2 72.94% <0.00%> (+<0.01%) ⬆️
ubuntu 73.04% <0.00%> (-0.57%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@claude

claude Bot commented Jun 11, 2026

Copy link
Copy Markdown

Code Review - PR 165: Updates from PackageDSLKit

OVERVIEW
Three areas: dependency/tooling updates, XCTest-to-Swift-Testing migration, new array-based DSL API overloads.

BUGS / CORRECTNESS

  1. Dead assertion in invalidRaw test (MappedValueCollectionRepresentedTests.swift)
    The replaced code leaves '_ = decodingError' which is a complete no-op. The original XCTAssertNotNil was trivially true at that point, but at least it communicated intent. Replace with a real assertion such as: #expect(decodingError is DecodingError)

  2. Missing 'any' keyword in Struct.init (Struct.swift)
    The new public init uses '[CodeBlock]' while the existing internal init uses '[any CodeBlock]'. Since CodeBlock is a protocol, Swift 5.7+ requires the 'any' keyword for existentials. In Swift 6 strict mode this will likely generate a warning or error. Should be declared as 'members: [any CodeBlock]'.

  3. Test.Skip may not be the correct Swift Testing API (SKSampleMacroTests.swift)
    'throw Test.Skip(...)' -- Swift Testing does not expose a directly throwable Test.Skip type equivalent to XCTest XCTSkip. Please verify this compiles and produces a skip not a failure. The idiomatic alternative is a withKnownIssue block.

CODE QUALITY

  1. Prefer #expect(throws:) for failure tests
    The rawValueFailure and stringFailure tests in both MappedValueCollectionRepresentedTests and MappedValueDictionaryRepresentedTests retain the old do/catch pattern. Swift Testing has a cleaner path via #expect(throws: SomeError.case).

  2. Non-throwing tests catching errors manually
    EnumSetTests.decoder() and EnumSetTests.encoder() use do/catch + Issue.record + early return despite @test supporting 'throws' natively. Adding 'throws' would let try propagate naturally, consistent with MappedEnumTests.decoder() throws in this same PR.

MISSING TEST COVERAGE

  1. No tests for new public API additions
    Init, Struct, and Closure each get a new array-based initializer, and Struct gets a new inherits(String array) overload. Per CLAUDE.md, new syntax elements should have corresponding unit tests in Tests/SyntaxKitTests/Unit/. None were added here.

POSITIVE CHANGES

  • swift-syntax URL change (apple -> swiftlang org) is correct; packages moved to the swiftlang GitHub org
  • XCTest to Swift Testing migration is consistent with the project stated preference
  • Array-based DSL overloads follow existing patterns and are useful for programmatic code generation
  • inherits(String array) is a natural complement to the existing variadic overload
  • swift-docc-plugin 1.4.4 to 1.4.5 is a safe patch update

SUMMARY
Priority items before merging: dead assertion in invalidRaw (1), missing any keyword (2), verifying Test.Skip behavior (3). Items 4-6 are quality improvements but not blockers.

@leogdion

Copy link
Copy Markdown
Member Author

Superseded by v0.0.5 (#166) — all changes from this branch (bb78546) are already present in v0.0.5. Rebasing packagedslkit onto v0.0.5 produced an empty diff. Original commit preserved on the packagedslkit-backup branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant