dart-lang / linter

Linter for Dart.
https://dart.dev/tools/linter-rules
BSD 3-Clause "New" or "Revised" License
630 stars 170 forks source link

Strict `void` lint request #3471

Open lrhn opened 2 years ago

lrhn commented 2 years ago

A void expression should be treated as not having a value. The language is permissive for backwards compatibility reasons, and allow you to assign void to void and dynamic, and assign void, dynamic and null to void.

I'd like to restrict doing so as much as possible. With a strict_void lint (or possible multiple separate lints, if that makes more sense), it would be an error to:

Which might be summarized as:

a14n commented 2 years ago

@lrhn : didn't this issue rather intend to be filed on https://github.com/dart-lang/linter ?

bwilkerson commented 2 years ago

I don't have an objection to this being implemented as a lint, but it might make more sense for it to be implemented as a language option, along the same lines as strict-casts, strict-inference, and strict-raw-types, possibly called strict-void.

@leafpetersen @srawlins

lrhn commented 2 years ago

I'm not sure what a "language option" really is. Couldn't it just be a named set of lints? I don't see any need for more categories of user-enabled/configured source analyses.

@a14n Absolutely should be in linter repo. My bad. (So that's why I couldn't find it!)

bwilkerson commented 2 years ago

I'm not sure what a "language option" really is.

As I understand it, the notion of a language option was originally designed to be a way for users to further tighten the semantics of the language. It was motivated by a desire to clean up the strong_mode option that we had before we had introduced the notion of experiments.

In the analysis options file it looks something like the following:

analyzer:
  language:
    strict-casts: true

Couldn't it just be a named set of lints?

Yes, I believe that we could replace each of the current language options with a warning that is disabled by default. Whether there's a compelling reason to support a different way of enabling these warnings, especially in light of the proposed simplification of errors and warnings, is definitely something we should consider. I hadn't really thought about that question, I was merely pointing out that this is very similar to what we now call language options.

eernstg commented 1 year ago

Is this issue a duplicate (or a further development) of #1830?

rakudrama commented 1 year ago

I think this goes too far and throws out some good things with the bad. Chaining explicit-void-to-explicit-void is a nice language feature.

Writing methods that delegate with => without worrying about the 'type' is a nice affordance, allowing the delegating pattern be shorter and have visual uniformity between void and non-void methods. It is make-work, i.e. an impediment to coding velocity, to have to flip between a method definition that has an expression body vs a full body, and having to avoid => is especially ugly in closures used as arguments.

The current rules allow annotations of expression-statements by annotating a variable.

@pragma('dart2js:disable-inlining')
void _ = print('hello');

If the above becomes illegal then we will need another way of annotating an expression-statement. (The pattern is not yet in common usage but dart2js is heading towards finely scoped annotations to restrict the scope of non-spec behaviour (https://github.com/dart-lang/sdk/issues/49475). As the different annotations become more finely scoped, using this pattern would become the recommended practice.)