Skip to content

Inconsistent behaviour when getter that returns a function is called with/without .call() #48635

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

Open
daadu opened this issue Mar 22, 2022 · 7 comments
Labels
area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). type-question A question about expected behavior or functionality

Comments

@daadu
Copy link

daadu commented Mar 22, 2022

Dart-pad demo: https://dartpad.dev/?id=a0f88b47e1d05bd8f2347442f867de84

When a "getter" returns a function - depending on how it is called - obj.adder(fetchNum()) vs obj.adder.call(fetchNum()) - the execution-path is different, check the demo link above.

This is an inconsistent behaviour, when the getter has a side-effect.

PS: Not sure if this is a bug or feature (as intended). According to me, it is a bug - since the behaviour is inconsitent.

@daadu daadu added the bug label Mar 22, 2022
@daadu daadu changed the title Inconsistent behaviour when getter returns a function Inconsistent behaviour when getter that returns a function is called with/without .call Mar 22, 2022
@daadu daadu changed the title Inconsistent behaviour when getter that returns a function is called with/without .call Inconsistent behaviour when getter that returns a function is called with/without .call() Mar 22, 2022
@eernstg
Copy link
Member

eernstg commented Mar 22, 2022

@daadu, thanks for the concise stand-alone example, that's always a great way to point to the exact topic of interest!

The getter adder should be executed before the actual arguments are evaluated, both with obj.adder(fetchNum()) and with obj.adder.call(fetchNum()), cf. this location in the spec. This is also the consistent approach, based on the overall informal decision to have left-to-right evaluation.

So I'll transfer this to the SDK repository.

We may need to reconsider this particular rule, however, if it turns out to be a significantly breaking change. At least, we'd have to run it via a breaking change process.

@eernstg eernstg transferred this issue from dart-lang/language Mar 22, 2022
@eernstg
Copy link
Member

eernstg commented Mar 22, 2022

It looks like a change that could give rise to a rather small amount of breakage (because getters returning functions could be a somewhat esoteric design choice), but based on the fact that it just changes the order of evaluation it might cause some really subtle bugs that aren't noticed for a while. And, of course, it might also break a lot of code, who knows.

I can see that the actual arguments are evaluated before the getter with dart and dart2js (the CFE might be making that decision and lowering the code).

@johnniwinther, we have had discussions about the evaluation order of similar constructs previously. What do you think? And is it true that this decision is made by the CFE (for all or some configurations)? @sigmundch, @mkustermann, WDYT?

@eernstg eernstg added area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). type-question A question about expected behavior or functionality and removed bug labels Mar 22, 2022
@daadu
Copy link
Author

daadu commented Mar 22, 2022

Just for note, I discovered it when writing mock_plugin lib - with a feature called "dynamic stubbing" - the following test shows how it is used:

test("dynamic stubbing", () async {
    // arrange
    final connectivityPlugin = setUpMockPlugin("dev.fluttercommunity.plus/connectivity");
    final checkStub = connectivityPlugin.stubFor.call(await Connectivity().checkConnectivity());
    checkStub.toResult((call) async => "mobile");
    // act
    final result = await Connectivity().checkConnectivity();
    // assert
    expect(result, ConnectivityResult.mobile);
    checkStub.verify.calledOnce();
});

Here, the stubFor is a getter that returns functions - with a side-effect that enables capturing any subsequent call to plugin, this then can be stubbed and verified. stubFor(...) misbehaves (await Connectivity().checkConnectivity() called before the side-effect inside the getter) but stubFor.call works as expected (so the above test passes). I designed it in such a way - to make it easier for user to stub with format like - mockPlugin.stubFor(...).toReturn(() => ...).

I think similar mechanism is done by mockito as well - with when and verify.

@eernstg
Copy link
Member

eernstg commented Mar 22, 2022

Just discussed the matter with @johnniwinther, confirming that this topic has been an issue for several years, and there is no easy path to "just fix it", but we are in a better position to do it than we used to be (because various stages of the compilation process have more type information available today). We'll discuss how to deal with it, but it is likely to take some time.

@leafpetersen
Copy link
Member

See previous discussion here as well.

@leafpetersen
Copy link
Member

Also here.

@leafpetersen
Copy link
Member

Discussion of the state of the implementations as of 2019 here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). type-question A question about expected behavior or functionality
Projects
None yet
Development

No branches or pull requests

3 participants