Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cascade_invocations false negative #4873

Closed
FMorschel opened this issue Feb 8, 2024 · 8 comments
Closed

cascade_invocations false negative #4873

FMorschel opened this issue Feb 8, 2024 · 8 comments

Comments

@FMorschel
Copy link
Contributor

Describe the issue
cascade_invocations false negative,

To Reproduce
I'm not sure what is triggering on the last case (it should always trigger), maybe you can reduce the code to reproduce it, I couldn't, sorry. Here is the full example I found:

import 'package:test/test.dart';

extension ObjectExt<T extends Object> on T {
  T? when(bool Function(T self) predicate, {T? Function(T self)? orElse}) =>
      predicate(this) ? this : orElse?.call(this);
}

void main() {
  // BOTH DON'T TRIGGER
  test('predicate parameter is the same object as the caller', () {
    final object = Object();
    object.when((self) {
      expect(self, same(object));
      return false;
    });
  });
  test('orElse parameter is the same object as the caller', () {
    final object = Object();
    object.when(
      (_) => false,
      orElse: (self) {
        expect(self, same(object));
        return false;
      },
    );
  });

  // THIS ONE TRIGGERS, THIS ONE IS RIGHT
  test('orElse is not called if predicate is true', () {
    final object = Object();
    object.when(
      (_) => true,
      orElse: (_) {
        throw AssertionError('orElse should not be called');
      },
    );
  });
}

Expected behavior
All cases should trigger.

@FMorschel
Copy link
Contributor Author

FMorschel commented Feb 8, 2024

I got it wrong, sorry. If the when method was called as cascade, then there would be no way of referring to object.

But I tested something else:

late final Object object;
object = Object();
object.when(
  (_) => false,
  orElse: (self) {
    expect(self, same(object));
    return false;
  },
);

Shouldn't this be triggering, though?

Edit

late final Object object;
object = Object()..when(
  (_) => false,
  orElse: (self) {
    expect(self, same(object));
    return false;
  },
);

This works just fine.

@lrhn
Copy link
Member

lrhn commented Feb 8, 2024

late final Object object;
object = Object();
object.when(
  (_) => false,
  orElse: (self) {
    expect(self, same(object));
    return false;
  },
);

This one is potentially dangerous.
You know that .when doesn't call its argument immediately, but the analyzer can't see that, so it should assume that changing the code to object = Object()..when( .... object...); could move the reference to object to before the assignment.

@FMorschel
Copy link
Contributor Author

Wouldn't it process the call to when only after initializing object though? I'm not sure I understand what you mean.

Is there an example that would call it first? If so, how would it know what properties object has to use inside this method call?

@lrhn
Copy link
Member

lrhn commented Feb 9, 2024

Wouldn't it process the call to when only after initializing object though?

This code would, because when doesn't call its argument eagerly, but the analyzer can't be expected to know that.
Consider this code:

late final Object object;
object = Object()..banana(() {
   print(object);
});

If we define banana as:

extension on Object {
  void banana(void Function() action) {
    action();
  }
}

then it will throw an uninitialized late variable access error, because object is read before it's assigned.
If banana had delayed the call to its action until later, then object would be assigned first. That's what when does, but the analyzer doesn't know that.

So if the analyzer sees an access to object in the expression of the object....., it won't suggest moving it up before the assignment to object. And object = Object()..something() is equivalent to let tmp = Object(); tmp.something(); object = tmp;, so making it ..something() moves that expression before the assignment.

@FMorschel
Copy link
Contributor Author

I see what you mean, thank you for the answer. Is there any documentation that I could read on that? I'd like to understand better how in my example, that call to object would maybe not be a problem.

Also, if you agree then, I believe this issue should be closed since this behaviour was intended.

@bwilkerson
Copy link
Member

@parlough In case you know of any documentation.

I'll go ahead and close this. We can re-open if it should remain open.

@parlough
Copy link
Member

Is there any documentation that I could read on that?

Is there a specific portion of the earlier discussion you are looking for documentation of? The behavior of cascade notation, considerations for accessing late variables, etc?

As for late, we have some docs, but it's quite limited. I've opened dart-lang/site-www#5532 to track some improvements here as well as for other issues I've come across.

@FMorschel
Copy link
Contributor Author

TBH when I asked it, both. But since then I think I understand now what I didn't about cascade annotations, and from what you showed me on late I'll run some tests for me to understand them better. Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants