Skip to content

Migrate ExpiringTodoRule from SourceKit to SwiftSyntax #6113

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

jpsim
Copy link
Collaborator

@jpsim jpsim commented Jun 20, 2025

Summary

Convert ExpiringTodoRule to use SwiftSyntax for improved performance and better multiline comment handling.

Key Technical Improvements

  • Full file content matching to handle multiline TODO/FIXME comments
  • Enhanced comment detection supporting comments across multiple lines
  • Improved performance with SwiftSyntax visitor pattern over SourceKit AST parsing

Migration Details

  • Replaced ASTRule with @SwiftSyntaxRule(optIn: true) annotation
  • Implemented ViolationsSyntaxVisitor pattern for file traversal
  • Changed from per-comment processing to full file regex matching
  • Added robust position-in-comment verification using trivia analysis
  • Maintained full compatibility with existing rule behavior and test cases

@jpsim jpsim force-pushed the migrate-expiringtodorule-from-sourcekit-to-swiftsyntax branch from d02cf16 to 3369649 Compare June 20, 2025 02:05
@jpsim
Copy link
Collaborator Author

jpsim commented Jun 20, 2025

Needs #6114 to have Regex support everywhere.

@jpsim jpsim force-pushed the migrate-expiringtodorule-from-sourcekit-to-swiftsyntax branch from 3369649 to 088b822 Compare June 20, 2025 11:48
@SwiftLintBot
Copy link

SwiftLintBot commented Jun 20, 2025

1 Warning
⚠️ This PR may need tests.
18 Messages
📖 Building this branch resulted in a binary size of 25178.18 KiB vs 25174.48 KiB when built on main (0% larger).
📖 Linting Aerial with this PR took 0.23 s vs 0.29 s on main (20% faster).
📖 Linting Alamofire with this PR took 0.18 s vs 0.36 s on main (50% faster).
📖 Linting Brave with this PR took 0.77 s vs 2.19 s on main (64% faster).
📖 Linting DuckDuckGo with this PR took 5.48 s vs 6.46 s on main (15% faster).
📖 Linting Firefox with this PR took 1.03 s vs 2.93 s on main (64% faster).
📖 Linting Kickstarter with this PR took 0.75 s vs 2.19 s on main (65% faster).
📖 Linting Moya with this PR took 0.14 s vs 0.2 s on main (30% faster).
📖 Linting NetNewsWire with this PR took 0.31 s vs 0.77 s on main (59% faster).
📖 Linting Nimble with this PR took 0.15 s vs 0.27 s on main (44% faster).
📖 Linting PocketCasts with this PR took 0.72 s vs 2.04 s on main (64% faster).
📖 Linting Quick with this PR took 0.13 s vs 0.19 s on main (31% faster).
📖 Linting Realm with this PR took 0.43 s vs 1.07 s on main (59% faster).
📖 Linting Sourcery with this PR took 0.28 s vs 0.62 s on main (54% faster).
📖 Linting Swift with this PR took 0.46 s vs 1.26 s on main (63% faster).
📖 Linting VLC with this PR took 0.21 s vs 0.41 s on main (48% faster).
📖 Linting Wire with this PR took 1.92 s vs 5.55 s on main (65% faster).
📖 Linting WordPress with this PR took 1.03 s vs 3.03 s on main (66% faster).

Generated by 🚫 Danger

jpsim added 4 commits June 21, 2025 15:37
## Summary

Convert ExpiringTodoRule to use SwiftSyntax and native Swift Regex
instead of SourceKit and NSRegularExpression for improved performance
and better multiline comment handling.

## Key Technical Improvements

- **Native Swift Regex** replacing NSRegularExpression for modern Swift patterns
- **Full file content matching** to handle multiline TODO/FIXME comments
- **Typed regex output** using Regex<(Substring, Substring)> for type safety
- **Enhanced comment detection** supporting comments across multiple lines
- **Improved performance** with SwiftSyntax visitor pattern over SourceKit AST parsing

## Migration Details

- Replaced `ASTRule` with `@SwiftSyntaxRule(optIn: true)` annotation
- Implemented `ViolationsSyntaxVisitor` pattern for file traversal
- Changed from per-comment processing to full file regex matching
- Added robust position-in-comment verification using trivia analysis
- Migrated from NSRegularExpression to native Swift Regex with typed captures
- Maintained full compatibility with existing rule behavior and test cases
Turns out that the Regex approach was extremely slow.
@jpsim jpsim force-pushed the migrate-expiringtodorule-from-sourcekit-to-swiftsyntax branch from b7206e9 to e869ac1 Compare June 21, 2025 19:38
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.

4 participants