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

remove prefer_void_to_null from recommended #154

Closed
pq opened this issue Sep 26, 2023 · 6 comments
Closed

remove prefer_void_to_null from recommended #154

pq opened this issue Sep 26, 2023 · 6 comments

Comments

@pq
Copy link
Member

pq commented Sep 26, 2023

In dart-lang/sdk#59309, @eernstg proposes we eliminate prefer_void_to_null and I'm inclined to agree. Unless there are strong reservations here, I'd propose that as a change to make before publishing 3.0.

/cc @devoncarew @lrhn @natebosch @jakemac53 @goderbauer

@pq pq added the type-lint label Sep 26, 2023
@github-project-automation github-project-automation bot moved this to Triage backlog in Lints Triage Sep 26, 2023
@pq pq moved this from Triage backlog to Under consideration in Lints Triage Sep 26, 2023
@devoncarew
Copy link
Member

For other readers, the docs for the lint are here:

https://dart.dev/tools/linter-rules/prefer_void_to_null

And in-lined from the dart-lang/linter discussion, from @srawlins:

I agree I think it has outlived its usefulness; I was getting analysis_server compliant and it was pretty awkward; there were plenty of valid uses of Null.

Which sounds like the lint may be producing more false positives than we'd want.

@jakemac53
Copy link

jakemac53 commented Sep 26, 2023

It is worth noting that in particular for return types, void can be annoying, because it can interact poorly with LUB and other similar computations:

var x = switch(1) {
  1 => true,
  _ => print('weird!'), // because this returns void, `x` is now inferred as `void`.
};
print(x); // Error, can't use void

This is actually a simplification of a real situation that came up recently 🤷

@goderbauer
Copy link
Contributor

I am not familiar with all the details here, but I have no objection to removing this lint. While it hasn't caused any trouble in our code base, it also hasn't been all that useful.

@devoncarew devoncarew changed the title [remove] prefer_void_to_null from recommended remove prefer_void_to_null from recommended Sep 26, 2023
@lrhn
Copy link
Member

lrhn commented Sep 28, 2023

I agree with removing this. As discussed elsewhere, there are cases where you should prefer void to Null, but with null safety, there are more cases where you'd actually choose Null deliberately, and you're less likely to choose the wrong one by accident.
Also null is no longer so special that it deserves the extra treatment. The same argument applies to every singleton type, whether () or a one-element enum (why pass a value to look at, if you know ahead of time what the value will be, it's better to say void so that you're not encuraged to actually look at the value.

The problem that this lint solves is now small enough, and well known enough, that it can be handled in code-review instead.

@natebosch
Copy link
Member

LGTM. I have slight reservations that the same patterns this lint was intended to avoid might start coming back, but I do think it's less useful with null safety, and I do think I recall some places where I wanted to violate this lint.

@devoncarew
Copy link
Member

This proposal is accepted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

6 participants