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

Migrate ClosureSpacingRule to SwiftSyntax #4092

Merged
merged 15 commits into from
Aug 15, 2022

Conversation

jpsim
Copy link
Collaborator

@jpsim jpsim commented Aug 14, 2022

Fixes #4090. Continued from #4091.

To do:

  • Review OSSCheck results
  • Benchmark (9x faster: 123.413s to 13.163s cumulative)
  • Implement corrections
  • Clean up
  • Changelog entry

@jpsim jpsim changed the title WIP: Start migrating ClosureSpacingRule to SwiftSyntax WIP: Migrate ClosureSpacingRule to SwiftSyntax Aug 14, 2022
@jpsim jpsim force-pushed the jp-closure-spacing-rule-swift-syntax branch from 337c168 to b861ffe Compare August 14, 2022 19:08
@realm realm deleted a comment from SwiftLintBot Aug 14, 2022
@SwiftLintBot
Copy link

SwiftLintBot commented Aug 14, 2022

4 Warnings
⚠️ This PR may need tests.
⚠️ This PR introduced a violation in Firefox: /Sync/Synchronizers/HistorySynchronizer.swift:198:16: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
⚠️ This PR introduced a violation in Firefox: /Tests/SharedTests/NSURLExtensionsTests.swift:308:25: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
⚠️ This PR introduced a violation in Quick: /Tests/QuickTests/QuickTests/FunctionalTests/ItTests.swift:100:105: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
60 Messages
📖 Linting Aerial with this PR took 1.21s vs 1.15s on master (5% slower)
📖 Linting Alamofire with this PR took 1.38s vs 1.35s on master (2% slower)
📖 Linting Firefox with this PR took 5.82s vs 5.66s on master (2% slower)
📖 Linting Kickstarter with this PR took 32.94s vs 8.32s on master (295% slower)
📖 Linting Moya with this PR took 0.67s vs 0.63s on master (6% slower)
📖 Linting Nimble with this PR took 0.6s vs 0.58s on master (3% slower)
📖 Linting Quick with this PR took 0.29s vs 0.29s on master (0% slower)
📖 Linting Realm with this PR took 11.59s vs 9.36s on master (23% slower)
📖 Linting SourceKitten with this PR took 0.49s vs 0.48s on master (2% slower)
📖 Linting Sourcery with this PR took 2.2s vs 2.13s on master (3% slower)
📖 Linting Swift with this PR took 4.11s vs 3.94s on master (4% slower)
📖 Linting WordPress with this PR took 10.08s vs 9.66s on master (4% slower)
📖 This PR fixed a violation in Aerial: /Resources/MainUI/Settings panels/NowPlayingViewController.swift:173:87: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in Firefox: /Client/Frontend/Browser/MailProviders.swift:11:33: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in Firefox: /Client/Frontend/Browser/MailProviders.swift:12:36: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in Firefox: /Client/Frontend/Settings/AppSettingsOptions.swift:585:45: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in Firefox: /Client/Frontend/Settings/SettingsTableViewController.swift:53:30: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in Firefox: /Client/Frontend/Theme/LegacyThemeManager/LegacyTheme.swift:137:31: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in Firefox: /Client/Frontend/Theme/LegacyThemeManager/LegacyTheme.swift:193:54: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in Firefox: /Tests/ClientTests/Mocks/MockableHistory.swift:26:66: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in Firefox: /Tests/XCUITests/LibraryTests.swift:10:25: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in Quick: /Sources/Quick/World.swift:72:45: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in Sourcery: /Templates/Tests/Context/AutoEquatable.swift:20:24: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in Sourcery: /Templates/Tests/Context/AutoHashable.swift:8:24: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in Swift: /stdlib/private/OSLog/OSLogStringAlignment.swift:56:48: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in Swift: /stdlib/private/StdlibCollectionUnittest/CheckMutableCollectionType.swift:72:25: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in Swift: /stdlib/private/StdlibCollectionUnittest/CheckMutableCollectionType.swift:73:25: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in Swift: /stdlib/private/StdlibCollectionUnittest/CheckMutableCollectionType.swift:74:25: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in Swift: /stdlib/public/core/SetVariant.swift:114:25: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/Utility/Spotlight/SearchableActivityConvertable.swift:50:36: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/Utility/Spotlight/SearchableActivityConvertable.swift:54:37: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/Utility/Spotlight/SearchableActivityConvertable.swift:60:55: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/Utility/Spotlight/SearchableActivityConvertable.swift:65:54: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/Utility/Spotlight/SearchableActivityConvertable.swift:69:60: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/Utility/Spotlight/SearchableActivityConvertable.swift:73:53: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/Utility/Spotlight/SearchableItemConvertable.swift:37:40: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/Utility/Spotlight/SearchableItemConvertable.swift:41:28: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/Utility/Spotlight/SearchableItemConvertable.swift:45:35: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/Utility/Spotlight/SearchableItemConvertable.swift:49:31: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/Utility/Spotlight/SearchableItemConvertable.swift:53:30: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/Utility/Spotlight/SearchableItemConvertable.swift:57:36: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/Utility/Spotlight/SearchableItemConvertable.swift:63:50: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/Utility/Spotlight/SearchableItemConvertable.swift:67:50: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/Utility/Spotlight/SearchableItemConvertable.swift:73:52: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/Cells/MediaSizeSliderCell.swift:10:23: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/Cells/MediaSizeSliderCell.swift:11:19: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/Cells/MediaSizeSliderCell.swift:12:27: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/Cells/MediaSizeSliderCell.swift:13:32: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/Cells/MediaSizeSliderCell.swift:14:28: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/Cells/MediaSizeSliderCell.swift:15:35: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/Cells/MediaSizeSliderCell.swift:16:35: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/Cells/MediaSizeSliderCell.swift:8:20: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/Cells/MediaSizeSliderCell.swift:9:23: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/Gutenberg/Layout Picker/GutenbergLayoutPickerViewController.swift:34:63: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/Reader/ReaderStreamHeader.swift:8:47: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/Site Creation/Design Selection/RemoteSiteDesign+Thumbnail.swift:6:28: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/Site Creation/Shared/ModelSettableCell.swift:16:26: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/Views/WPRichText/WPRichTextMediaAttachment.swift:4:26: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/Views/WPRichText/WPRichTextMediaAttachment.swift:5:23: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)
📖 This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/Views/WPRichText/WPRichTextMediaAttachment.swift:6:23: warning: Closure Spacing Violation: Closure expressions should have a single space inside each brace. (closure_spacing)

Generated by 🚫 Danger

@jpsim
Copy link
Collaborator Author

jpsim commented Aug 15, 2022

I've reviewed all the OSSCheck results and the new violations introduced are all legitimate violations that we previously missed. The violations that were removed are all false positives in the sense that none of them are closures, even though if you have this rule enabled you probably also want to lint for these cases too, like for single-line function bodies or protocol {get set}, but I think they should be handled by separate rules.

@jpsim jpsim marked this pull request as ready for review August 15, 2022 16:35
@jpsim jpsim changed the title WIP: Migrate ClosureSpacingRule to SwiftSyntax Migrate ClosureSpacingRule to SwiftSyntax Aug 15, 2022
@jpsim jpsim requested a review from SimplyDanny August 15, 2022 16:35
@jpsim jpsim enabled auto-merge (squash) August 15, 2022 17:58
@jpsim jpsim merged commit 3037946 into master Aug 15, 2022
@jpsim jpsim deleted the jp-closure-spacing-rule-swift-syntax branch August 15, 2022 18:27
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.

closure_spacing false positive when using Swift Regex literals involving brackets
2 participants