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

ConstantReader.revive creates a broken reference for Curves.ease #533

Closed
srawlins opened this issue Jun 9, 2021 · 5 comments
Closed

ConstantReader.revive creates a broken reference for Curves.ease #533

srawlins opened this issue Jun 9, 2021 · 5 comments

Comments

@srawlins
Copy link
Member

srawlins commented Jun 9, 2021

(User reported this at dart-lang/mockito#425; mockito's code generation uses source_gen to revive parameter default values)

When trying to revive the default value of the curve parameter of TabController.animateTo, ConstantReader.revive gives back Cubic.ease, which is not a real thing.

The default value of this parameter is Curves.ease, declared as:

static const Cubic ease = Cubic(0.25, 0.1, 0.25, 1.0);

When I do some print-debugging, I see that reviveInstance gets all the way down to checking fields, where clazz is Cubic and e is Curves.ease. So the result is Revivable._(source: ..., accessor: 'Cubic.ease') (again, not a real thing).

I think the fix might be as easy as adjusting the for-loop here to hold on to t, the type that e is found on, and using that rather than clazz.

As a stand-alone example, I have this:

class Foo {
  void f({Bar a = Bars.b}) {}
}
class Bars {
  static const Baz b = Baz();
}
class Bar {}
class Baz implements Bar {
  const Baz();
}
@kuhnroyal
Copy link

I think I run into this problem or something related when trying to mock a database generated with https://github.com/simolus3/moor

There is a TableUpdateQuery class here: https://github.com/simolus3/moor/blob/develop/moor/lib/src/runtime/api/stream_updates.dart#L122 with a factory
const factory TableUpdateQuery.any() = AnyUpdateQuery;.

The AnyUpdateQuery class is declared in another file here: https://github.com/simolus3/moor/blob/develop/moor/lib/src/runtime/executor/stream_queries.dart#L279

When the mock for the database is generated it contains this fragment for a method where the const factory is used as default value for a parameter:

import 'package:moor/src/runtime/api/runtime_api.dart' as _i29;
import 'package:moor/src/runtime/executor/stream_queries.dart' as _i32;

  @override
  _i35.Stream<Set<_i29.TableUpdate>> tableUpdates(
          [_i29.TableUpdateQuery? query = const _i32.TableUpdateQuery.any()]) =>
      (super.noSuchMethod(Invocation.method(#tableUpdates, [query]),
              returnValue: Stream<Set<_i29.TableUpdate>>.empty())
          as _i35.Stream<Set<_i29.TableUpdate>>);

The _i32 import prefix is obviously wrong and doesn't point to the file containing the factory function but instead to the type that is being returned by the factory function. This should be _i29 as well.

@simolus3 FYI

@kuhnroyal
Copy link

Actually meant to post this in the linked mockito issue :/
But may also be related to the ConstantReader used in mockito.

@natebosch
Copy link
Member

I suspect we may still have some gaps here, but #546 got us a lot closer. I filed #547 to track the remaining bug.

@natebosch
Copy link
Member

@srawlins - I just realized I failed to confirm whether the Curves.ease case is fixed or is still hit by #547

Do you have a repro set up to confirm whether this fixed it?

@srawlins
Copy link
Member Author

Yes I just confirmed with a unit test that the correct text is revived, with this issue fixed. Thanks!

(This is with a minimal repro which aimed to emulate the Curves bug. I don't have a repro set up which uses Curves.)

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

3 participants