cascade_invocations is too eager when type promotion is necessary

[ghost]

@Hixie commented on Mar 4, 2018, 10:53 AM UTC:

Enable the cascade_invocations lint, then analyze this:

class A { }
class B extends A { void foo() { } }

void test(A x) {
  if (A is B) {
    final B b = x;
    b.foo();
  }
}

void main() { }

It says: lint • Cascade consecutive method invocations on the same reference at test.dart:7:5 • cascade_invocations

However, if you change it as suggested:

class A { }
class B extends A { void foo() { } }

void test(A x) {
  if (A is B) {
    final B b = x
      ..foo();
  }
}

void main() { }

...then now the code is no longer correct (error • The method 'foo' isn't defined for the class 'A' at test.dart:7:9 • undefined_method).

This issue was moved by pq from dart-lang/sdk#32407.

[ghost]

@lrhn commented on Mar 6, 2018, 10:40 AM UTC:

Probably don't mean to say if (A is B), but if you do if (x is B) the warning goes away.

[ghost]

@Hixie commented on Mar 6, 2018, 7:10 PM UTC:

Oops, sorry about that typo.

I don't think it changes this bug, though. That conditional could be anything that happens to imply, in a way the analyzer can't recognize, that the cast is ok; the cascade operator isn't appropriate in this case.

[ghost]

@bwilkerson commented on Mar 6, 2018, 7:17 PM UTC:

I'm having trouble thinking of a concrete example of the condition you described. I'm assuming that you actually ran into this somewhere. Can you point us at the actual code that caused the problem?

[ghost]

@Hixie commented on Mar 6, 2018, 8:34 PM UTC:

Yeah, it's https://github.com/seamonkeysocial/cruisemonkey/pull/9/files#diff-e3af0e3a6e3dbe50cfde8027a6c541c8R262

[ghost]

@bwilkerson commented on Mar 6, 2018, 9:01 PM UTC:

Ok, thanks. The problem arrises when the variable cannot be promoted. Sounds like the lint might be checking the wrong type at that point.

[ghost]

@bwilkerson commented on Mar 14, 2018, 2:15 PM UTC:

@alexeieleusis

[ghost]

@leafpetersen commented on Mar 15, 2018, 1:01 AM UTC:

The second error message is actually incorrect, see #30542

The intention is that

 final B b = x
      ..foo();

should be treated (after implicit downcast insertion) as:

 final B b = (x as B)
      ..foo();

Neither of the front ends implements this entirely yet though.

[ghost]

@srawlins commented on May 29, 2019, 3:35 PM UTC:

CC @pq move to linter?

[Hixie]

(An example of where you might run into this is where the condition is checking runtimeType instead of using is, which is common e.g. in operator == implementations; or in cases where you have some tag that determines the actual type of some variable, and the condition is checking the tag to determine what to do (e.g. because some of the other cases might be distinguishing "double" meaning "centimeters" vs "double" meaning "inches" or whatever, so in general you can't rely on the type).)

[nstrelow]

Friendly asking, has this been fixed, or is the lint still breaking sometimes?
Thanks for the hard work!