Rewrite void_return rule using SwiftSyntax #4158

marcelofabri · 2022-09-04T10:58:03Z

No description provided.

SwiftLintBot · 2022-09-04T11:04:45Z

	2 Warnings
⚠️	If this is a user-facing change, please include a CHANGELOG entry to credit yourself! You can find it at CHANGELOG.md.
⚠️	This PR may need tests.

	35 Messages
📖	Linting Aerial with this PR took 0.26s vs 0.33s on main (21% faster)
📖	Linting Alamofire with this PR took 0.18s vs 0.33s on main (45% faster)
📖	Linting Firefox with this PR took 0.58s vs 1.39s on main (58% faster)
📖	Linting Kickstarter with this PR took 0.76s vs 1.92s on main (60% faster)
📖	Linting Moya with this PR took 0.2s vs 0.21s on main (4% faster)
📖	Linting Nimble with this PR took 0.16s vs 0.22s on main (27% faster)
📖	Linting Quick with this PR took 0.12s vs 0.14s on main (14% faster)
📖	Linting Realm with this PR took 0.7s vs 1.18s on main (40% faster)
📖	Linting SourceKitten with this PR took 0.17s vs 0.21s on main (19% faster)
📖	Linting Sourcery with this PR took 0.35s vs 0.53s on main (33% faster)
📖	Linting Swift with this PR took 0.78s vs 0.94s on main (17% faster)
📖	Linting WordPress with this PR took 0.84s vs 2.17s on main (61% faster)
📖	This PR fixed a violation in Firefox: /ThirdParty/Deferred/DeferredTests/LockProtectedTests.swift:34:63: warning: Void Return Violation: Prefer `-> Void` over `-> ()`. (void_return)
📖	This PR fixed a violation in Firefox: /ThirdParty/Deferred/DeferredTests/LockProtectedTests.swift:52:62: warning: Void Return Violation: Prefer `-> Void` over `-> ()`. (void_return)
📖	This PR fixed a violation in SourceKitten: /Tests/SourceKittenFrameworkTests/Fixtures/Subscript.swift:3:31: warning: Void Return Violation: Prefer `-> Void` over `-> ()`. (void_return)
📖	This PR fixed a violation in Sourcery: /SourceryTests/Stub/Performance-Code/Kiosk/Auction Listings/ListingsCountdownManager.swift:39:63: warning: Void Return Violation: Prefer `-> Void` over `-> ()`. (void_return)
📖	This PR fixed a violation in Sourcery: /SourceryTests/Stub/Performance-Code/Kiosk/Auction Listings/ListingsViewController.swift:16:96: warning: Void Return Violation: Prefer `-> Void` over `-> ()`. (void_return)
📖	This PR fixed a violation in Sourcery: /SourceryTests/Stub/Performance-Code/Kiosk/Auction Listings/ListingsViewController.swift:23:103: warning: Void Return Violation: Prefer `-> Void` over `-> ()`. (void_return)
📖	This PR fixed a violation in Sourcery: /SourceryTests/Stub/Performance-Code/Kiosk/Bid Fulfillment/Models/BidDetails.swift:18:64: warning: Void Return Violation: Prefer `-> Void` over `-> ()`. (void_return)
📖	This PR fixed a violation in Swift: /benchmark/single-source/DataBenchmarks.swift:371:73: warning: Void Return Violation: Prefer `-> Void` over `-> ()`. (void_return)
📖	This PR fixed a violation in Swift: /benchmark/single-source/IterateData.swift:26:71: warning: Void Return Violation: Prefer `-> Void` over `-> ()`. (void_return)
📖	This PR fixed a violation in Swift: /benchmark/utils/DriverUtils.swift:350:55: warning: Void Return Violation: Prefer `-> Void` over `-> ()`. (void_return)
📖	This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/Me/App Settings/AppSettingsViewController.swift:178:70: warning: Void Return Violation: Prefer `-> Void` over `-> ()`. (void_return)
📖	This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/Me/App Settings/AppSettingsViewController.swift:223:77: warning: Void Return Violation: Prefer `-> Void` over `-> ()`. (void_return)
📖	This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/Me/App Settings/AppSettingsViewController.swift:338:67: warning: Void Return Violation: Prefer `-> Void` over `-> ()`. (void_return)
📖	This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/People/PersonViewController.swift:275:53: warning: Void Return Violation: Prefer `-> Void` over `-> ()`. (void_return)
📖	This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/People/PersonViewController.swift:292:84: warning: Void Return Violation: Prefer `-> Void` over `-> ()`. (void_return)
📖	This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/People/PersonViewController.swift:328:53: warning: Void Return Violation: Prefer `-> Void` over `-> ()`. (void_return)
📖	This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/Post/AbstractPostListViewController.swift:670:57: warning: Void Return Violation: Prefer `-> Void` over `-> ()`. (void_return)
📖	This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/Post/AbstractPostListViewController.swift:721:38: warning: Void Return Violation: Prefer `-> Void` over `-> ()`. (void_return)
📖	This PR fixed a violation in WordPress: /WordPress/Classes/ViewRelated/Views/RichTextView/RichTextView.swift:185:63: warning: Void Return Violation: Prefer `-> Void` over `-> ()`. (void_return)
📖	This PR fixed a violation in WordPress: /WordPress/WordPressTest/PushAuthenticationServiceTests.swift:35:92: warning: Void Return Violation: Prefer `-> Void` over `-> ()`. (void_return)
📖	This PR fixed a violation in WordPress: /WordPress/WordPressTest/PushAuthenticationServiceTests.swift:41:92: warning: Void Return Violation: Prefer `-> Void` over `-> ()`. (void_return)
📖	This PR fixed a violation in WordPress: /WordPress/WordPressTest/PushAuthenticationServiceTests.swift:48:92: warning: Void Return Violation: Prefer `-> Void` over `-> ()`. (void_return)
📖	This PR fixed a violation in WordPress: /WordPress/WordPressTest/PushAuthenticationServiceTests.swift:59:92: warning: Void Return Violation: Prefer `-> Void` over `-> ()`. (void_return)

Here's an example of your CHANGELOG entry:

* Rewrite void_return rule using SwiftSyntax.  
  [marcelofabri](https://github.com/marcelofabri)
  [#issue_number](https://github.com/realm/SwiftLint/issues/issue_number)

note: There are two invisible spaces after the entry's text.

Generated by 🚫 Danger

jpsim

Lovely

jpsim · 2022-09-05T00:23:05Z

Source/SwiftLintFramework/Rules/Style/VoidReturnRule.swift

+
+        correctionPositions.append(tuple.positionAfterSkippingLeadingTrivia)
+
+        var returnType = SyntaxFactory.makeTypeIdentifier("Void")


TIL about SyntaxFactory

jpsim · 2022-09-07T19:16:51Z

@marcelofabri can we merge this?

marcelofabri · 2022-09-08T06:30:03Z

@marcelofabri can we merge this?

I need to think about what to do with these violations. My current thinking is that it's fine to have them fixed here and update redundant_void_return to catch them, wdyt?

jpsim · 2022-09-08T15:01:13Z

Looking at the OSSCheck report, it looks like all the violations are safe because they're in closures with enough parent type context to infer the void return value. That's not the case with the current implementation of redundant_void_return in your PR.

ZevEisenberg · 2022-09-17T19:34:23Z

+1! I can't speak to the specifics of this rewrite, but it does fix a false positive that currently exists on main, and which might be good to add to the test cases:

func foo() -> () async -> Int

Failure on main:

testWithDefaultConfiguration(): failed - nonTriggeringExample violated: 
```
/* 👨‍👩‍👧‍👦👨‍👩‍👧‍👦👨‍👩‍👧‍👦 */
func foo() -> () async -> Int
              ^ warning: Void Return Violation: Prefer `-> Void` over `-> ()`. (void_return)
```

ZevEisenberg · 2022-11-29T18:58:16Z

bump - I just rediscovered this thread because I dusted off an old project and found that this was still the case

lo1tuma · 2023-02-17T13:27:11Z

Is there anything missing or something I could help with to get this PR merged?

SimplyDanny · 2023-12-09T23:34:05Z

Rule has been rewritten in #5351.

ZevEisenberg · 2023-12-10T01:05:40Z

I'm not at a computer right now so I can't test it, but do you know if it now correctly handles the case I mentioned above?

SimplyDanny · 2023-12-10T08:27:37Z

I'm not at a computer right now so I can't test it, but do you know if it now correctly handles the case I mentioned above?

Yes, the example is handled correctly now. See #5381.

Rewrite void_return rule using SwiftSyntax

2004967

marcelofabri force-pushed the marcelo/void_return-swift-syntax branch from 6f1b76d to 2004967 Compare September 4, 2022 11:09

Remove import

5e82f79

jpsim approved these changes Sep 4, 2022

View reviewed changes

jpsim reviewed Sep 5, 2022

View reviewed changes

marcelofabri marked this pull request as ready for review September 8, 2022 12:34

lo1tuma mentioned this pull request Feb 17, 2023

Fix void_return rule to support async and async throws functions #4772

Merged

SimplyDanny closed this Dec 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrite void_return rule using SwiftSyntax #4158

Rewrite void_return rule using SwiftSyntax #4158

marcelofabri commented Sep 4, 2022

SwiftLintBot commented Sep 4, 2022 •

edited

Loading

jpsim left a comment

jpsim Sep 5, 2022

jpsim commented Sep 7, 2022

marcelofabri commented Sep 8, 2022

jpsim commented Sep 8, 2022 •

edited

Loading

ZevEisenberg commented Sep 17, 2022 •

edited

Loading

ZevEisenberg commented Nov 29, 2022

lo1tuma commented Feb 17, 2023

SimplyDanny commented Dec 9, 2023

ZevEisenberg commented Dec 10, 2023

SimplyDanny commented Dec 10, 2023


		correctionPositions.append(tuple.positionAfterSkippingLeadingTrivia)

		var returnType = SyntaxFactory.makeTypeIdentifier("Void")

Rewrite void_return rule using SwiftSyntax #4158

Rewrite void_return rule using SwiftSyntax #4158

Conversation

marcelofabri commented Sep 4, 2022

SwiftLintBot commented Sep 4, 2022 • edited Loading

jpsim left a comment

Choose a reason for hiding this comment

jpsim Sep 5, 2022

Choose a reason for hiding this comment

jpsim commented Sep 7, 2022

marcelofabri commented Sep 8, 2022

jpsim commented Sep 8, 2022 • edited Loading

ZevEisenberg commented Sep 17, 2022 • edited Loading

ZevEisenberg commented Nov 29, 2022

lo1tuma commented Feb 17, 2023

SimplyDanny commented Dec 9, 2023

ZevEisenberg commented Dec 10, 2023

SimplyDanny commented Dec 10, 2023

SwiftLintBot commented Sep 4, 2022 •

edited

Loading

jpsim commented Sep 8, 2022 •

edited

Loading

ZevEisenberg commented Sep 17, 2022 •

edited

Loading