Plugin: strip comments and strings before PluginScanner regex match#271
Merged
Plugin: strip comments and strings before PluginScanner regex match#271
Conversation
`PluginScanner.fileContainsRoot` and `fileContainsGenerateMockTrue` ran their `@Instantiable(...)` regex against raw source, which false-positives when the pattern appears inside comments or string literals. With the `sourceBuild` trait, SafeDICore's sources get scanned transitively through the plugin's `SafeDITool` dependency — and `Sources/SafeDICore/Models/SafeDIToolManifest.swift` has "`@Instantiable(isRoot: true)`" in a doc comment explaining the manifest format. The scanner recorded `SafeDIToolManifest+SafeDI.swift` as an expected output of every downstream module's `SafeDIGenerator`; at build time SafeDITool didn't produce that file, and SwiftDriver failed trying to compile it as an input. Only Xcode triggered this: `swift build` takes the primary plugin path (real `SafeDITool scan`). Xcode takes the `createBuildCommandsWithPluginScanner` fallback because `context.tool(named:).url` returns an unresolved build- variable path at plugin-setup time. Strip line comments, block comments (nested), and string literals from the content before regex matching. Preserves line structure so regex position semantics are stable. The real parser in SafeDITool is still authoritative; this scan only predicts output files. Also add an `xcodebuild` step to the existing Package Integration CI job so the plugin's fallback path stays exercised. The pre-existing `swift build` step continues to cover the primary path. Manually verified: `xcodebuild -scheme ExamplePackageIntegration -destination "name=iPhone 17"` → BUILD SUCCEEDED after fresh DerivedData wipe. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d55ebc9668
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Extract `stripSwiftCommentsAndStrings` from `Plugins/PluginScanner.swift` into its own file `Plugins/PluginScannerStringUtilities.swift`, symlinked into both `Plugins/SafeDIGenerator/` (so the plugin target still compiles it) and a new `Tests/SafeDIGeneratorPluginTests/` test target. SPM plugin targets can't be test dependencies (they can only depend on executable/binary targets), so the symlink pattern matches the existing one for `Plugins/Shared.swift` — both targets share the same source file with no duplication. 12 new tests cover: line comments, triple-slash doc comments, block comments, nested block comments, multi-line block comments (line-count preservation), double-quoted strings, triple-quoted strings, escaped quotes inside strings, and the regression case that motivated PR #271 (the `SafeDIToolManifest.swift` doc-comment false positive). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`stripSwiftCommentsAndStrings` naively treated every `/*` as a block- comment opener. Swift regex literals can contain `/*` (e.g., `/\/*/` matches zero or more slashes); a file containing such a regex with no later `*/` would trip the stripper into block-comment mode and run away through real `@Instantiable(...)` annotations — causing the plugin's fallback scanner to miss roots and register wrong build-command outputs. Two fixes: 1. **Lookahead guard on block-comment entry.** Before entering block-comment mode on `/*`, scan ahead for a matching `*/` at balanced depth. If none exists, treat the `/*` as regular text. Defangs runaway consumption for unterminated `/*` (including the simple-regex case — `/.../` forms remain ambiguous with division in a tokenless scanner, so we can't detect them directly). 2. **Recognize extended regex literals.** `#/…/#` is delimited unambiguously; skip its span whole, preserving newlines. Three new tests cover: `#/.../#` containing an @INSTANTIABLE mention; `/\/*/` preceding a real @INSTANTIABLE (preserves it); bare unterminated `/*` (preserves subsequent code). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: acbeccd5b4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #271 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 41 41
Lines 6796 6796
=========================================
Hits 6796 6796 🚀 New features to boost your workflow:
|
Follow-up to the previous codex P1 fix. `blockCommentHasCloser`'s naive scan still counted `*/` tokens that appear inside string literals, so a simple regex `/\/*/` followed later by `let s = "*/"` would still enter block-comment mode and eat real `@Instantiable` declarations between them. Fix: the lookahead now walks through string literals (single and triple-quoted), line comments, and extended regex literals the same way the main stripper does, only counting `*/` when at code-level depth. This matches codex's reported failing input. Test added: `preservesInstantiable_whenSimpleRegexContainsSlashStarAndLaterStringContainsStarSlash` pins the exact shape codex cited. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Xcode builds of the Example Package Integration (and any downstream using
sourceBuildtrait) failed with:swift buildsucceeded — only Xcode failed.Root cause
PluginScanner.fileContainsRootran its@Instantiable\s*\(...isRoot\s*:\s*trueregex against raw Swift source, including comments and string literals. When thesourceBuildtrait is active, SafeDICore's sources get transitively scanned, andSources/SafeDICore/Models/SafeDIToolManifest.swift:42contains the literal text`@Instantiable(isRoot: true)`inside a doc comment. The scanner markedSafeDIToolManifest.swiftas containing a root, registeredSafeDIToolManifest+SafeDI.swiftas an expected output of every module'sSafeDIGenerator, and the downstream compile failed when that file wasn't produced.Why only Xcode:
swift buildtakes the primary plugin path (runs the realSafeDITool scanduring plugin setup). Xcode'scontext.tool(named:).urlreturns an unresolved${BUILD_DIR}/${CONFIGURATION}/SafeDIToolpath at plugin-setup time, so the setuprunSafeDIToolcall throws and the plugin falls back tocreateBuildCommandsWithPluginScanner, which uses the regex.Fix
Strip line comments (
//), block comments (/* */, nesting supported), and string literals (single- and triple-quoted) from the source before matching. Newlines are preserved so regex position semantics stay consistent. The real parser inSafeDIToolremains authoritative — this scanner only predicts output files.Added a
stripSwiftCommentsAndStringshelper and routed bothfileContainsRootandfileContainsGenerateMockTruethrough it.Regression guard
Added an
xcodebuildstep to the existingBuild Package Integration on Xcode 26CI job. The pre-existingswift buildstep continues to cover the primary plugin path; the new step covers the fallback (where this bug lived).Manual verification
Test plan
swift test --traits sourceBuild— 885 tests pass./CLI/lint.shxcodebuildon the example — BUILD SUCCEEDED🤖 Generated with Claude Code