Support Optional<()> returns that use syntactic sugar, ()?

[ryanbooker]

#1761

[SwiftLintBot]

	12 Messages
📖	Linting Aerial with this PR took 0.38s vs 0.35s on master (8% slower)
📖	Linting Alamofire with this PR took 2.57s vs 2.5s on master (2% slower)
📖	Linting Firefox with this PR took 10.6s vs 10.38s on master (2% slower)
📖	Linting Kickstarter with this PR took 16.14s vs 15.38s on master (4% slower)
📖	Linting Moya with this PR took 1.1s vs 1.05s on master (4% slower)
📖	Linting Nimble with this PR took 1.46s vs 1.4s on master (4% slower)
📖	Linting Quick with this PR took 0.49s vs 0.45s on master (8% slower)
📖	Linting Realm with this PR took 2.37s vs 2.19s on master (8% slower)
📖	Linting SourceKitten with this PR took 0.88s vs 0.81s on master (8% slower)
📖	Linting Sourcery with this PR took 3.98s vs 3.69s on master (7% slower)
📖	Linting Swift with this PR took 10.75s vs 10.38s on master (3% slower)
📖	Linting WordPress with this PR took 9.98s vs 9.83s on master (1% slower)

Generated by 🚫 Danger

[marcelofabri]

@ryanbooker Can you share more information about the motivation for this change?

IMO -> ()? and -> ()! should trigger (-> Void? and -> Void! being preferred).

[jpsim]

Moreover, why would you ever want to return Void? or Void!? That binary state is (almost?) always better represented by Bool or a two-member enum.

[ryanbooker]

@jpsim e.g. traverse_ on Optional returns Optional<()>/()?/Void?.

However, whether you should or shouldn't return Optional<()> in any given case is moot. It's not the same type as (), and so just a bug to reject it. It would be similar to rejecting Result<()> as ().

NB: Only the sugared versions have the bug, as they run afoul of the original regex.
:)

[ryanbooker]

Perhaps there is another way I'm supposed to specify the test, but only the ()? and ()! return types, specifically, run afoul of the current regexp. I can add a test specifically for those cases if you like.

And of course there may be better regex to fix it. :)

[ryanbooker]

@marcelofabri In case I wasn't clear. It's simply a bug in the void_return regex. It's irrelevant whether () or Void is preferred. That's a settings in the linter. :)

The bug is that ()? and ()! are rejected as being equivalent to (). They aren't. That's all we really need to know. However to further reinforce the point: there are useful abstractions that can return Optional<()> and it shouldn't require doing a dance with the void_return, redundant_void_return, syntactic_sugar rules in order to return the sugared version of that perfectly valid and non redundant type.

:)

[marcelofabri]

It's irrelevant whether () or Void is preferred. That's a settings in the linter.

Actually, this is the whole purpose of this rule. To make sure Void is preferred.

What I think would be the better approach is to change redundant_void_return to allow returning Void!, Void?, ()? and ()!. Although I agree that's not common to return those types, it's not that rule's job to enforce that (it's only concerned about the cases that the whole return type can be omitted).

Does that make sense, @ryanbooker? Or am I missing something else here?

[ryanbooker]

No we’re on the same page now. That’s precisely what I intended to do. I did it in the wrong rule. Now I understand why you’re talking about irrelevant things. Haha. :)

…

On 10 Aug 2017, at 18:50, Marcelo Fabri ***@***.***> wrote: It's irrelevant whether () or Void is preferred. That's a settings in the linter. Actually, this is the whole purpose of this rule. To make sure Void is preferred. What I think would be the better approach is to change redundant_void_return to allow returning Void!, Void?, ()? and ()!. Although I agree that's not common to return those types, it's not that rule's job to enforce that (it's only concerned about the cases that the whole return type can be omitted). Does that make sense, @ryanbooker? Or am I missing something else here? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[ryanbooker]

Let's try this again. :)

[marcelofabri]

Thanks! 💯

[jpsim]

👍