Dartanalyzer Type Checking

I enjoyed exploring function signature parameters in Dart yesterday. It seemed pretty powerful stuff, with just one bit that did not work quite as expected. There was one case in which the dartanalyzer tool was unable to identify a mismatched function parameter. This did not bother me too much, but Kasper Lund asked that I file a bug report. Those folks are pretty intense about getting everything tracked so it can be fixed.

Bug reports really are a simple thing. If you supply every last detail, then you have done a good job. The trick about bug reports is making sure that every last detail amounts to very little information. In other words, boil bug reports down to the simplest case in which the bug can be demonstrated (if possible).

My explanation from last night is an example of a poor bug report. It was little more than, "when I write a method with a function signature parameter, then dartanalyzer does not correctly identify non-function arguments." The example call that I gave is a good example of being the opposite of helpful:

db.close(' *** this is not a function *** ');

It provides no clue as to the context in which it was called (it was of a test) or the manner in which the db variable was created. Now, I could spend a lot of time explaining all of this and describing the changes to dart dirty that were necessary to create the problem, but that is a lot of work for me. As much work as it is for me, it is compounded significantly for the project. Someone has to read it enough to triage it. Someone else will have to read it to fix it. Someone else will have to give the LGTM signal on the fix. And there are probably others.

So instead, I try to boil it down to the simplest version of the bug for submission. I never really tried my problem with a function, so I start there. In a new file, I define a function with a function signature parameter and call it with a non-function:

// function_signature_test.dart
main() {
  doSomething();
  doSomething(()=>print('callback'));
  doSomething(' *** this is not a function *** ');
}

doSomething([cb()]) {
  print('something');
  if (cb != null) cb();
}

In the main entry point, I call the doSomething() function three times with different arguments. The doSomething() function accepts a single, optional parameter (square brackets around parameters are one way to mark parameters as option in Dart). The empty parentheses in the cb() parameter indicate that this optional parameter needs to be a function that accepts zero arguments.

The second call to doSomething() in the main entry point is an example of calling it with a callback function with the necessary zero parameters. When the first call is made, only the “something” string in the doSomething() function prints. When the second call is made, both the “something” string from doSomething() and the “callback” string from the supplied callback are printed. The output of the first two function calls is then:

something
something
callback

When doSomething() tries to “call” the string supplied by the third call, the code blows up with a runtime error. The cb parameter was assigned the value of " *** this is not a function *** " which makes no sense when called as cb().

Now, if this this the simplest version of the dartanalyzer bug that I found yesterday, then dartanalyzer will not be able to tell me that there is a problem with the third doSomething() call:

➜  tmp  dartanalyzer test_function.dart
Analyzing test_function.dart...
[warning] The argument type 'String' cannot be assigned to the parameter type '() -> dynamic' (/home/chris/tmp/test_function.dart, line 4, col 15)
1 warning found.

Interesting! So dartanalyzer is able to find these kinds of problems after all. Why then, was I not seeing warnings yesterday?

It turns out to be manner in which I declared objects whose methods contain function signatures. Again, I setup a simple test case, this time a class with a single method accepting an optional method signature:

class Thing {
  act([cb()]) {
    print("Thing#act");
    if (cb != null) cb();
  }
}

I find that, if I declare instances of the Thing class with the var keywords (as I did yesterday), then dartanalyzer does not warn me about problems. But, if I declare the object with the explicit Thing type, then dartanalyzer can identify the problem. By way of example, if I change the main entry point to:

main() {
  var var_thing = new Thing();
  var_thing.act(' *** this is not a function *** ');

  Thing known_thing = new Thing();
  known_thing.act(' *** this is not a function *** ');
}

Then dartanalyzer will find fault only with the second of the two calls:

➜  tmp  dartanalyzer test_function.dart     
Analyzing test_function.dart...
[warning] The argument type 'String' cannot be assigned to the parameter type '() -> dynamic' (/home/chris/tmp/test_function.dart, line 6, col 19)
1 warning found.

Even though the objects are instantiated exactly the same way and the methods are invoked identically to each other, dartanalyzer only does its work if the variable is declared with an explicit type.

To which I say, bleh.

I love the power of dartanalyzer to find problems in my code, but I'll be darned if I am going to start writing all of my code like:

Thing thing = new Thing();

Or even worse:

Thing thing = new ThingFactory().thing;

In my mind, readability trumps tooling, so I am going to stick with

var thing = new Thing();

The redundancy of “thing” is already too much for me in that final example so I would usually take some time to see if either the class or instance needs a better name. I am not going to then turn around and introduce yet more redundancy into my code—even if it does buy me some admittedly useful type analysis.

I admit that an argument could be made that, since I am declaring thing as a variable with the var keyword, I am, by definition, stating that my variable can change type. I would argue that this is almost never a good practice and is worthy of a dartanalyzer warning. Now that factory constructors must return types of the enclosing class, I would further argue that it always possible to infer the type of any new assignment.

Regardless, if the concern in dartanlyzer is that the type is variable when declared with var, what about replacing it with final? If I declare a final_thing, the type will, by definition, not change:

main() {
  final final_thing = new Thing();
  final_thing.act(' *** this is not a function *** ');
  final_thing = 42;
}

In this case, both the call to the act() method with the non-function and the assignment of a final variable should draw the ire of dartanalyzer, right?

Nope:

➜  tmp  dartanalyzer test_function.dart
Analyzing test_function.dart...
[warning] Final variables cannot be assigned a value (/home/chris/tmp/test_function.dart, line 4, col 3)
1 warning found.

So in the end, I do not think that I have a bug on my hand. Last night's work on function signatures as parameters was correct.

Instead, I believe that I have identified a limitation of dartanalyzer. I would certainly prefer it work as I expected—inferring types from the new assignment—and hopefully someday this behavior will change. Until then, it is good to have a better understanding of the limitations of the tool.

And it is all thanks to very small test cases!


Day #852

‹prev | My Chain | next›