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

bool.fromEnvironment('dart.library.js_util') (and alike?) constant can't be revived, breaking the codegen #683

Open
dustincatap opened this issue Aug 8, 2023 · 3 comments

Comments

@dustincatap
Copy link

Hi. I don't know if this issue is valid for this repository.

I am having trouble generating mocks when using auto_route.

Running dart run build_runner build --verbose outputs this error:

[INFO] Entrypoint:Generating build script...
[INFO] Entrypoint:Generating build script completed, took 287ms

[INFO] BuildDefinition:Initializing inputs
[INFO] BuildDefinition:Reading cached asset graph...
[INFO] BuildDefinition:Reading cached asset graph completed, took 46ms

[INFO] BuildDefinition:Checking for updates since last build...
[INFO] BuildDefinition:Checking for updates since last build completed, took 464ms

[INFO] Build:Running build...
[INFO] Build:Running build completed, took 9ms

[INFO] Build:Caching finalized dependency graph...
[INFO] Build:Caching finalized dependency graph completed, took 25ms

[SEVERE] mockito:mockBuilder on test/main_test.dart (cached):

Bad state: No element
dart:core                                               List.first
package:source_gen/src/constants/revive.dart 104:21     reviveInstance
package:source_gen/src/constants/reader.dart 278:25     _DartObjectConstant.revive
package:mockito/src/builder.dart 393:34                 _TypeVisitor._addTypesFromConstant
package:mockito/src/builder.dart 289:7                  _TypeVisitor.visitParameterElement
package:analyzer/src/dart/element/element.dart 5560:15  ParameterElementImpl.accept
package:analyzer/src/dart/element/element.dart 2881:13  ElementImpl.visitChildren
package:analyzer/dart/element/visitor.dart 323:13       RecursiveElementVisitor.visitMethodElement
package:mockito/src/builder.dart 276:11                 _TypeVisitor.visitMethodElement
package:analyzer/src/dart/element/element.dart 4896:54  MethodElementImpl.accept
package:analyzer/src/dart/element/element.dart 2881:13  ElementImpl.visitChildren
package:analyzer/dart/element/visitor.dart 233:13       RecursiveElementVisitor.visitClassElement
package:mockito/src/builder.dart 258:11                 _TypeVisitor.visitClassElement
package:analyzer/src/dart/element/element.dart 826:20   ClassElementImpl.accept
package:mockito/src/builder.dart 160:20                 MockBuilder._resolveAssetUris.addTypesFrom
package:mockito/src/builder.dart 168:9                  MockBuilder._resolveAssetUris.addTypesFrom
package:mockito/src/builder.dart 173:7                  MockBuilder._resolveAssetUris
package:mockito/src/builder.dart 76:29                  MockBuilder.build

[SEVERE] Build:
Failed after 63ms

Here is a sample project

@yanok
Copy link
Contributor

yanok commented Aug 8, 2023

I can reproduce with just

class X {
  void m({bool b = kIsWeb}) { print(b); }
}

(package:auto_route uses !kIsWeb as a default value, but just kIsWeb is enough).

The thing is kIsWeb is defined using bool.fromEnvironment and I guess Analyzer has no chance of knowing what the environment is.

@srawlins Could you please confirm this never worked and this is not a regression?

@yanok
Copy link
Contributor

yanok commented Aug 8, 2023

Corr: it's not just bool.fromEnvironment but specifically bool.fromEnvironment('dart.library.js_util') that results in bool (-unknown-) value.

As a quick fix we could probably detect this and try just referencing the constant instead. Won't work with !kIsWeb though.

Longer term I had an idea how we could get rid of the need to revive default values:

  • If think we only need them get the invocations right: if a method is declared with a default value void m({C x = kC}), then calls m() and m(x: kC} should result in the same invocations, so when we override m, we have to also reconstruct kC, which could be a problem, if it involves private things. Or, as we see now, it could also be constant but unknown.
  • We could use a sentinel initializer in the overrides, like void m({C? x = _guardC}). This will make invocations m() and m(x: kC) non equal though, so we would have to check x == _guardC and do something about it.
  • And we can actually get the right value at run time: if we generate _ProxyX class alongside with MockX, but without any overrides, just nSM, we could call it and get the correct default values.

That's just a sketch but I think it could be made to work. At the same time it seems a bit too complex.

So probably even a longer term we should do a breaking API change and separate mocks from their configuration objects as Lasse proposed long ago. So config object will be generated but it won't need any default args. And mocks won't have any overrides except for nSM, so they will get the right default values for free.

@yanok yanok changed the title Can't generate nice mocks bool.fromEnvironment('dart.library.js_util') (and alike?) constant can't be revived, braking the codegen Aug 8, 2023
@yanok yanok changed the title bool.fromEnvironment('dart.library.js_util') (and alike?) constant can't be revived, braking the codegen bool.fromEnvironment('dart.library.js_util') (and alike?) constant can't be revived, breaking the codegen Aug 8, 2023
@yanok
Copy link
Contributor

yanok commented Aug 8, 2023

@dustincatap For you one possible fix will be to wrap RootStackRouter in a class of your own (making sure you don't have kIsWeb as an argument default value) and mock that instead. So far I think that's the only option.

We could come up with some quick fix, but I can't promise that. And a longer term solution won't be implemented any time soon.

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

2 participants