Skip to content
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

Skip braces in regex literals #4091

Closed

Conversation

SimplyDanny
Copy link
Collaborator

@SimplyDanny SimplyDanny commented Aug 14, 2022

This is a workaround for the false positive reported in #4090.

Regex literals will only be supported in Swift 6. For that reason, SourceKit does not yet know anything about the syntax type "Regex Literal". The correct fix would be skipping regex literals completely in the rule like it is done for strings and comments.

The lookarounds in the regex to find the braces make the rule significantly slower as we see in the report below.

@SwiftLintBot
Copy link

12 Messages
📖 Linting Aerial with this PR took 0.46s vs 0.42s on master (9% slower)
📖 Linting Alamofire with this PR took 0.39s vs 0.38s on master (2% slower)
📖 Linting Firefox with this PR took 1.6s vs 1.54s on master (3% slower)
📖 Linting Kickstarter with this PR took 2.26s vs 2.16s on master (4% slower)
📖 Linting Moya with this PR took 0.23s vs 0.22s on master (4% slower)
📖 Linting Nimble with this PR took 0.24s vs 0.23s on master (4% slower)
📖 Linting Quick with this PR took 0.16s vs 0.15s on master (6% slower)
📖 Linting Realm with this PR took 1.34s vs 1.27s on master (5% slower)
📖 Linting SourceKitten with this PR took 0.23s vs 0.22s on master (4% slower)
📖 Linting Sourcery with this PR took 0.63s vs 0.57s on master (10% slower)
📖 Linting Swift with this PR took 1.08s vs 0.97s on master (11% slower)
📖 Linting WordPress with this PR took 2.84s vs 2.43s on master (16% slower)

Generated by 🚫 Danger

@jpsim
Copy link
Collaborator

jpsim commented Aug 14, 2022

Alternatively, here's a SwifSyntax implementation that I think mostly works, I just haven't implemented corrections yet: https://github.com/realm/SwiftLint/compare/jp-closure-spacing-rule-swift-syntax

Feel free to pick it up if you have time, otherwise I can probably finish it later this week.

@SimplyDanny
Copy link
Collaborator Author

Alternatively, here's a SwifSyntax implementation that I think mostly works, I just haven't implemented corrections yet: https://github.com/realm/SwiftLint/compare/jp-closure-spacing-rule-swift-syntax

Looks good to me so far!

Feel free to pick it up if you have time, otherwise I can probably finish it later this week.

I'll let you know once I find the time to continue with it.

@jpsim
Copy link
Collaborator

jpsim commented Aug 15, 2022

#4092 is ready to go now

jpsim added a commit that referenced this pull request Aug 15, 2022
@SimplyDanny SimplyDanny deleted the skip-braces-in-regex-literals branch August 17, 2022 20:14
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.

3 participants