Sdk: Analyzer CLI should support generated sources

[matanlurey]

Thanks to the work of @MichaelRFairhurst (and others!), the analysis _server_ now understands generated sources from package:build_*. That makes the IDE experience of users much better, thanks!

The next step though is to make generated sources work for non-IDE purposes, i.e. continuous integration using the dartanalyzer CLI tool. For now we are skipping analysis, which is less than desirable (https://github.com/dart-lang/angular/issues/1508).

[dentra]

Any chance to get it fixed?

[evanweible-wf]

Is there any work planned to address this issue? And if not, would it be something you'd accept a contribution for? It's impacting us on our transition from Dart 1 to Dart 2 and our workarounds are essentially the same as yours were for angular – skip analysis during CI or add an ignore rule for the analyzer error. Unfortunately our issue is not with the uri_has_not_been_generated rule but the undefined_setter rule because of how we're using this generated code.

Skipping analysis altogether during CI is most likely a non-starter for us, and our concern is that ignoring the undefined_setter rule for whole files or for the entire package has a high chance of swallowing actual errors. For some additional context, this is related to our conversion of over_react from transformers to builders and has a significant impact on our internal consumers. It's tough for us to convert 50+ over_react consumers to Dart 2 and the builder pattern with a caveat like this.

@matanlurey @kevmoo

[kevmoo]

@stereotype441 @devoncarew ?

[stereotype441]

@MichaelRFairhurst what would be involved in getting this to work with the command-line analyzer?

[matanlurey]

Of note, this will start to impact Flutter and related projects as they start to adopt builders.

/cc @jonahwilliams

... we are having to start ignoring other errors now, such as invalid_assignment, due to the lack of generated file support on the command-line.=, similar to @evanweible-wf has noted in https://github.com/dart-lang/sdk/issues/34098#issuecomment-460331317.

[devoncarew]

I believe Flutter will be fine here - flutter analyze is based on the analysis server, not the cli analyzer command.

For non-flutter projects, one option for analysis on CI systems is to use a CLI based on the analysis server. pub global activate tuneup and then pub global run tuneup analyze will analyze the project, _should_ support generated code, and will return a non-zero exit code if any analysis issues are found.

[evanweible-wf]

@devoncarew thanks for suggesting tuneup, we've been able to use that during CI successfully.

Unfortunately, this still presents a pretty big issue for us because the pana tool that scans every uploaded package uses dartanalyzer directly and fails on our over_react package for this same reason, with the result being that our package receives a health score of 0 (and prevents it from recognizing that it is Dart 2 compatible).

Here's an example pana run: https://pub.dartlang.org/packages/over_react/versions/2.0.0-alpha#-analysis-tab-

@kevmoo is there any way to get pana to successfully analyze a package that leverages build-to-cache? We were hoping to release 2.0.0 today, but we don't want it to appear as though the package is broken.

[kevmoo]

@evanweible-wf – you're publishing a package that's incomplete without source gen being run? Uh...we should discuss if there is something else you could do here...

[evanweible-wf]

@kevmoo It sounds like our only option is to build to source, but we had originally shied away from that option because of the documentation around source vs cache:

Use build_to: source when:

• The generated code _does not_ depend on details outside of the containing
  package that might change with a different version of its dependencies. Any
  details from the _generating_ package, or any other dependency used by the
  generated code must not change without a major version bump.
• The generated code should be pub published with the package and used by
  downstream packages without running a build themselves.

Use build_to: cache when:

• The generated code uses details from outside of the package with the generated
  code, including from the _generating_ package which may change without a major
  version bump.
• The builder generates lots of files that the user might not expect to see.
• The builder targets use cases for "application" packages that are unlikely to
  be published rather than packages which will be published and become
  dependencies.

Our thought was that if we build to source, we'd be unable to propagate fixes or changes to the builder that consumers would be able to consume _just by updating over_react_ itself (which would be possible with build-to-cache). If we build to source, then we are at the mercy of all dependencies that have generated over_react code. Maybe that's fine and maybe that's realistically the only option we have. Does that sound right?

[aaronlademann-wf]

I would also add that, if the use of build_to: cache results in "a package that's incomplete without source gen being run" - why is it available?

To me - a package that relies on the tool that the dart team designed as a replacement for transformers - which analyzed to perfection using a transformer - should not be seen as "incomplete". It should be seen as a package that needs to be built to be complete since we can no longer rely on the transformer.

Also, committing generated files is a big bummer / overhead for consumers.

[MichaelRFairhurst]

if the use of build_to: cache results in "a package that's incomplete without source gen being run" - why is it available?

See the last two bullets of each. Cache is best for "application" packages that are unlikely to be published" and source is best for "code should be pub published."

There are some unknowns here about what we should do in the case of "generated code uses details from outside of the package." That's the problem you're hitting with over_react. The problem is that we don't have hardly any infrastructure here....should we issue warnings when using such packages without a proper build config, for instance? Should this affect package health at all in the meantime before we have such infrastructure (granted, a score of 0 is of course way too extreme), ie, since it relies on a not-fully-supported use case, do we want to give full health for this?

I'll add another unknown. Does anyone know if pub runs the build before analysis? Ie, even if this issue were solved, would the files exist for the CLI to pick up? I'm pretty sure pana uses the analysis server and not the CLI, so I'm pretty sure the issue here is related to the build not running before analysis rather than it being an analyzer issue in this case.

@stereotype441 Regarding the question of how long it would take to support in the analyzer -- which would have been a benefit regardless so that you didn't need to switch your CI tools to use tuneup -- there are two paths:

• a week or so to refactor analyzer_cli to use ContextBuilder instead of its own context building code. Then that can be plugged into the existing PackageBuildWorkspace used by server, or
• 3 weeks? or so to rewrite cli as a layer on top of server and get this for free

[evanweible-wf]

@MichaelRFairhurst Any update on this? Is there a separate issue for updating the analyzer CLI to use the analysis server?