Wednesday, November 7, 2012

Fixing Every Last Dart Analyzer Issue Possible

‹prev | My Chain | next›

With my dartdoc fun done for now, I shift my attention back to Dart Comics, my Dart-based sample application. A while back, I had used the dart_analyzer tool to clean up most of the outstanding issues related to the recent M1 release of Dart. The issues relating to the underlying Hipster MVC framework have all been addressed, but there is at least one more file that needs investigation.

That file is the one that opens the modal dialog in my sample application:


So I run the analyzer only to find far fewer issues than I remembered:
➜  scripts git:(M1) ✗               
➜  scripts git:(M1) ✗ ~/local/dart/dart-sdk/bin/dart_analyzer web/ModalDialog.dart 
file:/home/chris/repos/dart-comics/public/scripts/web/ModalDialog.dart:44: no such type "ElementList"
    43:   get parent => el.parent;
    44:   ElementList queryAll(String selectors) => el.queryAll(selectors);
          ~~~~~~~~~~~
file:/home/chris/repos/dart-comics/public/scripts/web/ModalDialog.dart:68: Optional parameters cannot start with an '_' character
    67: 
    68:   _drawBackground([_]) {
                           ~
file:/home/chris/repos/dart-comics/public/scripts/web/ModalDialog.dart:89: Optional parameters cannot start with an '_' character
    88: 
    89:   _drawElement([_]) {
                        ~
Compilation failed with 2 problems.
Oh well, I suppose I should not be too sad over a dearth of problems with my code.

The first issue is related to an API simplification from a while back. Rather than support a special ElementList class, Dart chooses to make use of the more generic, but equally expressive List<Element> (read list-of-elements). So my queryAll() method definition becomes:
List<Element> queryAll(String selectors) => el.queryAll(selectors);

The next two issues are some odd code that I wrote:
  _drawBackground([_]) {
    // ...
  }

  _drawElement([_]) {
    // ...
  }
At first glance, I seem to have been saying that these methods accept a single, optional argument—and if that argument is supplied, it will be ignored. It takes me a while to recall why I did this, but it was done so that I could call the methods directly when first showing the modal dialog:
  void show() {
    _attachHanders();
    _drawBackground();
    _drawElement();
  }
And I can also make these methods event handlers for window resizes:
window.
      on.
      resize.
      add(_drawBackground);
In this latter capacity, _drawBackground() is invoked with an Event object, so the method definition must accept an optional argument. Unfortunately, I cannot use the convention of an underscore for the ignored parameter because it is optional. The language specification has a detailed explanation for why this is. For now, I am forced to give a name to the ignored parameters:
_drawBackground([event]) {
    // ...
  }

  _drawElement([event]) {
    // ...
  }
I do not mind this too much as it makes for more readable code. As I said, it took me a moment to remember why I had the optional, ignored parameter. With this change, the next time I stumble across this code, I will know exactly what the purpose of optional parameter is.

With that, my modal still works and I have fixed all of my dart_analyzer errors—except I haven't. After addressing those three issues, dart_analyzer now wants:
➜  scripts git:(M1) ✗ ~/local/dart/dart-sdk/bin/dart_analyzer web/ModalDialog.dart
file:/home/chris/repos/dart-comics/public/scripts/web/ModalDialog.dart:3: Concrete class ModalDialog has unimplemented member(s)
    # From Node:
        Collection<Node> nodes
        Collection<Node> nodes
        NamedNodeMap $dom_attributes
        List<Node> $dom_childNodes
        ...
    # From Element:
        Map<String, String> attributes
        Map<String, String> attributes
        Collection<Element> elements
        Collection<Element> elements
        ...
     2: #import("dart:html");
     3: class ModalDialog implements Element {
              ~~~~~~~~~~~
file:/home/chris/repos/dart-comics/public/scripts/web/ModalDialog.dart:33: 'void' is not assignable to 'Node'
    32:     bg.remove();
    33:     return el.remove();
                   ~~~~~~~~~~~
For the first issue, it seems that Dart is no longer satisfied with quacking like an abstract duck. It now needs to quack, waddle, eat, sleep and poop like a duck. I do not really need ModalDialog to be an Element—I had only meant for it to indicate intention—so I choose discretion as the better part of valor. I remove the implements from ModalDialog:
class ModalDialog {
  // ...
}
I cannot get too upset over this change. A sub-class does not really implement a class unless it defines all abstract methods. It was nice being able to say that a subclass is a little like a superclass, but I can simply choose good names to accomplish the same thing.

The last issue arises because Element#remove() no longer retuns the removed element. So instead of declaring the return type from the function as Element, I use void:
  void hide() {
    _removeHandlers();
    bg.remove();
    el.remove();
  }
That was a good find by the analyzer. Had any calling code been relying on a return value, it would have failed when trying to return el.remove(). That is the kind of thing that the analyzer could only do thanks to the type information associated with the el property. Without it, the analyzer would have no way of knowing what el.remove() normally returns and would therefore have no way of identifying the incorrect return type on that function.

With that, I really have eliminated all of my errors:
➜  scripts git:(M1) ✗ ~/local/dart/dart-sdk/bin/dart_analyzer web/ModalDialog.dart
➜  scripts git:(M1) ✗
That is a tad anti-climatic. After all of that effort, I feel like I ought have more of a reward than no errors. Ah well, I can live with it. Especially now that I have that extra assurance that my code is nice and robust.



Day #563

No comments:

Post a Comment