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

Consider a static code check for the @GenerateMocks annotation before resolving the library #503

Open
evanweible-wf opened this issue Jan 18, 2022 · 5 comments
Labels
P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug

Comments

@evanweible-wf
Copy link

Related to #502 and dart-lang/build#3238. Currently, the mock builder is implemented in such a way that it fully resolves every Dart library. Given that this builder also currently runs on all .dart files by default, this results in a significant performance hit, especially for large projects, and especially considering that this builder won't generate any code for most of its inputs.

Consumers can narrow down the inputs to this builder, but it would be nice to improve the performance of this builder by default. One option to do so would be to perform a check for the presence of a @GenerateMocks annotation in the source file's text before using the builder API/analyzer to resolve that library. Even if the check produced false-positives (for example, finding it in a comment), it would better than resolving every library.

@srawlins
Copy link
Member

srawlins commented Jan 18, 2022

Great suggestion! Do you have any technical details for this idea?

E.g. in the build method, the builder system passes in a BuildStep which has an .inputLibrary, which I believed is a resolved library. Is there something I can do so that build is called less?

@evanweible-wf
Copy link
Author

My understanding is that BuildStep.inputLibrary is evaluated lazily, so if you don't access it you won't incur the perf hit of resolving the library. You could start by reading the input's contents as text, check if @GenerateMocks( shows up in the source, and return early if not:

  Future<void> build(BuildStep buildStep) async {
    final source = await buildStep.readAsString(buildStep.inputId);
    if (!source.contains('@GenerateMocks(')) return;
    ...
  }

@srawlins srawlins added P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug labels Jan 18, 2022
@natebosch
Copy link
Member

Yeah I think it would be sensible to have something like

    if (!(await buildStep.readAsString(buildStep.inputId))
        .contains('GenerateMocks')) {
      return;
    }

@natebosch
Copy link
Member

I'm not sure if we currently allow subclassing GenerateMocks or creating const declarations in one library and annotating in another - if we do the library we are generating for wouldn't have that string.

@srawlins
Copy link
Member

This would prevent mocks from being built from part files, which is supported today.

I'm not super against removing the support, unless we know of a common pattern.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

3 participants