[wildcards] report a warning on final wildcard variables

[pq]

With wildcard variables, this doesn't make much sense:

void f() {
  final _ = '';
}

I'm thinking we should produce a warning?

/cc @eernstg @lrhn @kallentu

[lrhn]

I don't think it makes less sense than var _ = 0;.

It just doesn't make sense to have a local variable declaration named _ which is not a parameter or pattern (which already does not introduce a variable).

An "unnecessary variable" warning would apply no matter how you declare it, late int _;, const int _ = 1;;, or just plain var _;.

Using var _ = asyncOp(); to avoid "unawaited future" warnings can probably still work, it doesn't matter whether it's var or final, but they should use unawaited instead.

(I guess I can see someone doing

var a = e1, _ = for effect(), b = e3;

but they still shouldn't.)

[pq]

I don't think it makes less sense than var _ = 0;.

Good points!

Following your logic, it's probably sufficient to treat them both as dead and just issue a DEAD_CODE warning.

Thank you!

[lrhn]

Purely philosophically it's not really dead code. It's not code that could be run, but isn't reachable on any path.
It's just noise that does nothing, even though it does run: A variable declaration that does not introduce a variable, all it does is to evaluate its initializer expression, is just the same as that expression as an expression statement.
(Still allows you to format multiple such expressions on one line as var _ = doA(), _ = doB(), _ = doC();, formatter permitting. Not a good reason.)

In practice, I'm fine with labeling it dead code.

[julemand101]

Should the full expression be marked dead code or only the assignment to the variable?

[pq]

Purely philosophically it's not really dead code.

I appreciate this, thanks. You're right.

It's just noise that does nothing, even though it does run

Absolutely and if the initializer is side-effecting...

Should the full expression be marked dead code or only the assignment to the variable?

If we issue a dead code warning, I'd say we should limit it to the variable (and not the initializer).

/fyi @bwilkerson for 2 cents

[bwilkerson]

I agree, we should only mark the variable, not the initializer.

I believe that in the past we've made a distinction between 'dead code', code that can't be reached, and 'unnecessary' code, code that doesn't need to be there because it has no semantic value. I'd opt for using the latter terminology in this case.

[pq]

Thanks! It sounds like we want to ensure that unnecessary_final is properly getting reported here.

[eernstg]

One thing might come up:

String? get maybeHello => null;

void main() {
  // A switch statement isn't checked for exhaustiveness, unless
  // the scrutinee has a type which is always-exhaustive.
  switch (maybeHello) {
    case String():
      print('Got a String');
    // Oops, we forgot the null case, no warning.
  }

  // We can use a switch expression, they are always checked for exhaustiveness.
  // But they can't be used as statements, so we wrap it in `(...)`.
  (switch (maybeHello) {
    String() => print('Got a String'),
    null => print('Got null'),
  });

  // Alternative way to allow for a switch expression. Perhaps some
  // people would prefer this style?
  var _ = switch (maybeHello) {
    String() => print('Got a String'),
    null => print('Got null'),
  };
}

A couple of expressions (switch expressions and map literals, at least) cannot be used as the expression of an expression statement (that is, it is an error to turn them into a statement simply by adding a semicolon at the end). We may need a workaround for those cases, and var _ = ... could be it.

I don't know if it actually matters, but it wouldn't be too hard to omit the lint when the initializing expression is a switch expression.