Suggested code from omit_local_variable_types linter fails at runtime

[TheSpydog]

Take this example:

import 'dart:ffi';
import 'package:ffi/ffi.dart';

class MyStruct extends Struct {
  @Uint32()
  int blah;
}

void main(List<String> arguments) {
  Pointer<MyStruct> a = allocate();
  print("Allocated successfully!");
  free(a);
}

This code works as expected, but the omit_local_variable_types linter doesn't like it because a has an explicit type. In order to fix this, I used Visual Studio Code's "Quick Fix" tool, which changed the contents of main to the following:

  var a = allocate();
  print("Allocated successfully!");
  free(a);

Notice the loss of type information. The linter is happy with this change, but when I run the code, it throws the following exception:

Unhandled exception:
Invalid argument(s): NativeType does not have a predefined size (@unsized). Unsized NativeTypes do not support [sizeOf] because their size is unknown. Consequently, [allocate], [Pointer.load], [Pointer.store], and [Pointer.elementAt] are not available.
#0      _sizeOf (dart:ffi-patch/ffi_patch.dart:42:56)
dart-lang/sdk#1      sizeOf (dart:ffi-patch/ffi_patch.dart:39:10)
dart-lang/sdk#2      allocate (package:ffi/src/allocation.dart:47:33)
dart-lang/sdk#3      main (file:///<snip>/test.dart:11:11)

Which makes complete sense, given that it's trying to allocate an unknown type of native object. My best guess of what it was supposed to have suggested is this: var a = allocate<MyStruct>().

Dart VM version: 2.8.4 (stable) (Wed Jun 3 12:26:04 2020 +0200) on "macos_x64"
Tested on macOS and Windows.

[osa1]

This also happens with functions like List.filled. Example:

void f() {
  List x = List.filled(10, null);
  print(x.runtimeType);
}

dart lint complains about List x, and dart fix --apply turns it into var x, but that also changes program semantics. Originally it prints List<dynamic>, with the fix it prints List<Null>. You can't add anything to List<Null> other than null (and maybe other values with bottom type(s)) so subsequent add calls start to fail.

void f() {
  var x = List.filled(10, null);
  print(x.runtimeType);
  x.add(123); // compile time type error
}

[eernstg]

I think this issue fits better as a linter issue, so I'm transferring it there.

The proposal would then be that omit_local_variable_types should stop flagging local variable declarations with a type annotation in the case where the type annotation changes the outcome of all language processing steps (like type inference, generic function instantiation, integer-literal-with-double-value coercion, etc.), or that the associated quick fix should transform additional parts of the declaration such that all properties (static and dynamic) are preserved.

[bwilkerson]

I believe the recommended style is to use var and move the type information to the initializer expression. @munificent can confirm or refute that premise.

If that's the case, then this becomes an issue with the fix, rather than with the lint.

[eernstg]

I believe the recommended style is to use var and move the type information to the initializer expression

That recommendation has indeed been put forward several times.

However, we might note that this is to some extent a recommendation to avoid type inference, because it requires the developer to provide feed-forward type arguments in various locations in the initializing expression, whereas the variable with an explicit type annotation (that is SomeType<Maybe, With, Arguments> x = e;) will declaratively state the desired result, and leave it up to the type inferencer to compute ("backwards") which type arguments in the initializing expression are needed in order to reach that goal.

I usually push the idea that omit_local_variable_types should only flag declarations where the initializing expression has an "obvious" type, which is basically the same thing as saying: If you are using non-trivial type inference then it's OK to declare the type.

[munificent]

I believe the recommended style is to use var and move the type information to the initializer expression. @munificent can confirm or refute that premise.

That's right, and the "Effective Dart" rule for it links to this lint.

I usually push the idea that omit_local_variable_types should only flag declarations where the initializing expression has an "obvious" type, which is basically the same thing as saying: If you are using non-trivial type inference then it's OK to declare the type.

That's what we say for fields and top-level variables. For locals, we felt it was better to have a simple blanket rule that can be applied mechanically. Since a local variable never bleeds out into a library's exported API, pinning down the precise type is less important.

[eernstg]

@munificent wrote:

Since a local variable never bleeds out into a library's exported API, pinning down the precise type is less important

Of course, all these considerations are not relevant to var x = C<Some, Type, Arguments>(...); where C is a class, because we can easily see that the type of x is C<Some, Type, Arguments>. But omit_local_variable_types should flag C<Some, Type, Arguments> x = C(...); exactly because the type is already obvious using var.

Otherwise, the reason why I keep pushing for an omit_local_variable_types that doesn't forbid helpful declared types is not that anyone a hundred miles away will see and depend on the declared type of the local variable—as you mention, that can't happen anyway.

My main reason is that the placement of the type information as a declared type is a declarative approach that relies on type inference to obtain the actual type arguments at various locations in the initializing expression. So we specify the desired end result, and ask the computer to sort out all the details in order to reach that result.

In contrast, the placement of one or more actual type arguments in the initializing expression requires the developer to (1) sort out all those details themselves (in order to know which type arguments to provide, and where), and (2) redo the information propagation in order to compute the actual declared type of the variable whenever they need to read and understand the code.

In general, I'd consider "state the desired end result and let the computer sort out the details" to be a more rational and useful approach than "sort out the details manually, write some information that will produce the end result after any number of complex steps that aren't visible in the source code, and ask the developers who will ever need to read and understand this code to redo those steps manually".

[munificent]

My main reason is that the placement of the type information as a declared type is a declarative approach that relies on type inference to obtain the actual type arguments at various locations in the initializing expression. So we specify the desired end result, and ask the computer to sort out all the details in order to reach that result.

I definitely see the appeal of that as a consistent approach. Basically, there's two options to avoid users redundantly specifying information:

1. They specify it in types and we push that over into expressions. (What you suggest here.)
2. They specify it in expressions and we push that over into types. (What Effective Dart and this lint suggest.)

For top-level variables and fields we encourage users to do (1) because we want them to think about the type signatures of their API explicitly since a change to those types can have far-reaching effects.

For local variables, we lean towards (2) because expressions carry type information that can be inferred from values. In:

var x = [1];

They don't have to write List or int at all because it can be inferred entirely from the syntax of the list literal and the type of the integer literal inside it. So (2) ends up being maximally terse for most cases—no type arguments needed to be written at all, either in the variable's type annotation or the initializing expression.

Users really like this kind of inference, so we want them to use it when they can. But some initializing expressions don't carry enough information to infer all type arguments:

var x = [];

That leaves us with two options:

var x = <int>[];
List<int> x = [];

We went with the former because that leads to more consistent code:

var x = [1];
var y = <int>[];
var z = [3];

Versus:

var x = [1];
List<int> y;
var z = [3];

In the latter example, y sticks out as being different when really it's just another local variable.

There are still times when you need to annotate a local variable, if the inferred type is not what you want or there is no initializer. But those are rare and stand out deliberately.

Philosophically, I think there's an argument to prefer specifying information in expressions and inferring types from them because expressions are usually more precise than types since they describe an actual value.

[eernstg]

@munificent wrote:

For local variables, we lean towards [not declaring local variable types]

I created #58773 in order to have the outline of a proposal for how to milden omit_local_variable_types such that it will allow developers to write some declared types of local variables (namely the ones that aren't obvious).

Here's the effect on the examples:

var x = [1];

The type of [1] is obvious (as defined in #58773), so List<int> x = [1]; would be linted, and the var form would still prevail.

var x = [];

The type of [] is not obvious, so there's no lint. It is possible that some other lint/hint (about implicit dynamic) would kick in, but otherwise this declaration is allowed (with the current omit_local_variables, and with #58773).

That leaves us with two options:

var x = <int>[];
List<int> x = [];

The type of <int>[] is obvious, so there would be a lint for List<int> x = <int>[];, but both forms shown above would be accepted with the proposal in #58773.

You could say that the type of [] is "almost trivial" and lint List<int> x = []; should be linted. We might recognize how certain forms could move the type information from the declared type into the initializing expression, do that, and determine that the result has a trivial type, but it easily gets complicated. This is a very interesting case!

We went with the former because that leads to more consistent code:

var x = [1];
var y = <int>[];
var z = [3];

Versus:

var x = [1];
List<int> y = [];
var z = [3];

With #58773, both forms would be accepted. However, I'm interested in cases where the type of the initializing expression is not obvious, so the really interesting case is more like this, where the type of x might be inferred as String:

var x = someReceiver.someFunction<int>(on, some, arguments);

In this case I'd very much prefer to allow the developer to proceed if they want to write this:

String x = someReceiver.someFunction<int>(on, some, arguments);

The type String may be technically redundant because it's the type that we would infer anyway, but I believe that it can be helpful (1) to document the type such that we know how to use x later on, and (2) to set up a kind of contract with the maintainers of code that we depend on.

So if we update the version of the package where someFunction is declared, and the type changes to, say, String?, then we'll detect the change at the declaration of x, rather than having confusing type errors further down in the code.