cascade_invocations: Turning successive method invocations into cascading has sometimes unexpected behavior change

[yyoon]

This is related to the cascade_invocations rule.

We recently did a massive refactoring to address the cascade_invocations warnings, and found that one of those changes introduced a bug in our code.

class Foo {
  Bar _bar;

  Foo() {
    _bar = new Bar();
    _bar.addListener(_handleEvent);
    _bar.doSomething();    // This notifies all listeners
  }

  void _handleEvent() {
    // Use _bar variable here...
  }
}

The doSomething() call changes some internal states and then notifies all the listeners, and that includes the _handleEvent() method. This _handleEvent() implementation then reads the internal state of Bar, so it is important that _bar is correctly assigned before this happens.

We changed this to use cascading style instead.

  Foo() {
    _bar = new Bar()
      ..addListener(_handleEvent)
      ..doSomething();
  }

and it started to crash, because _bar is not yet assigned when _handleEvent() is called for the first time due to the doSomething() call.

We fixed this on our side once we found the issue, but it wasn't an obvious problem to find. I would think it would be difficult for the linter to determine whether it is safe to use cascade or not, but just wanted to call this out as a problem. Also related to #787, as it's also about the order between the assignment and the method calls on that object.

[apwilson]

I usually have this problem if I'm passing a function/closure to the thing I'm cascading that references the variable being set due to the cascade:

For instance:

Object _foo;

void bar() {
  _foo = new Foo()..addListener(_useFoo);
}
void _useFoo() {
  _foo.use();
}

In the above, if Foo.addListener calls the function _useFoo then _foo is unexpectedly null.

Doing this is more right in this case:

Object _foo;

void bar() {
  _foo = new Foo();
  _foo.addListener(_useFoo);
}
void _useFoo() {
  _foo.use();
}

[srawlins]

I don't think we could meaningfully catch these cases without overcatching by a lot. Tricky situation.

[srawlins]

On second thought, I think we could catch when the cascade is part of an assignment, and the assignee is a field. (It's valid for when the assignee is a variable too, but less common?)