Add ability to use a resolved AST for complex codemods that require static types

[TimWhiting]

Motivation

I'm working on a codemod package for package:riverpod, and with some of the upcoming breaking changes I need to not only know the AST structure, but also the static types for some things. I tried a few things, but realized I would be reimplementing the resolving logic from package:analyzer if I tried to resolve things manually, in addition to the fact that resolving the AST requires understanding of multiple files.

Sorry for not seeing the contributing process earlier. But I think this should be pretty easy to review and clean. I've done some manual and testing with the example, and created an automated test for it. Let me know if this requires additional testing.

Might resolve #7

Changes

As listed in the updated changelog:

• Breaking Change: runInteractiveCodemod is now async, and returns Future<int> instead of int

• Adds a ResolvedSuggestor interface for Suggestors that require static analysis context

• Adds ResolvedAstVisitingSuggestorMixin which should be used for codemods that require static analysis

• Add example of such a codemod: see example/codemod_analysis_required.dart

Release Notes

^ The above should be good I think.

Review

See CONTRIBUTING.md for more details on review types (+1 / QA +1 / +10) and code review process.

@Workiva/app-frameworks

QA Checklist

• [x] Tests were updated and provide good coverage of the changeset and other affected code
• [x] Manual testing was performed if needed
  • [x] Steps from PR author: Run the new example cd example && dart run codemod_analysis_required.dart
  • [x] Anything falling under manual testing criteria outlined in CONTRIBUTING.md

Merge Checklist

While we perform many automated checks before auto-merging, some manual checks are needed:

• [ ] A Client Platform member has reviewed these changes
• [ ] There are no unaddressed comments - this check can be automated if reviewers use the "Request Changes" feature
• [ ] For release PRs - Version metadata in Rosie comment is correct

[aviary2-wf]

Security Insights

No security relevant content was detected by automated scans.

Action Items

• Review PR for security impact; comment "security review required" if needed or unsure

• Verify aviary.yaml coverage of security relevant code

  Questions or Comments? Reach out on Slack: #support-infosec.

[evanweible-wf]

This is great, thank you @TimWhiting! I had started a similar branch towards #7, with the main difference being that I also made Suggestor.generatePatches() async so that it could do async work, as well. I think I'd still like to make that change, and then pass in the AnalysisContext to that method instead of the resolved CompilationUnit. That way, the suggestor can do whatever it needs to do with the analysis context, which may be helpful for future use cases. (My only concern is how much work it is to create the initial AnalysisContextCollection for very large repos - need to test that out.)

For the time being, I'm going to merge this into an integration branch, and then hopefully follow up soon with the above changes. Thanks again for the contribution!

[TimWhiting]

Yeah, sounds good. Thanks!

I wondered about doing that, but didn't want to make generatePatches async for some reason (probably because it is an iterator, and streams feel less efficient since most of the work is not async). I wonder if you just make shouldSkip async and make ResolvedSuggestor resolve the ast if the Subclass returns false for shouldSkip.

My understanding is that by using AnalysisContextCollection it reuses analysis that it already has done for any file, so that it won't have to do more analysis than required for each file if it has already resolved libraries/files that that file depends on. When you have a Suggestor that requires analysis it will essentially have to be run anyways on all of the files that aren't skipped. I guess the main thing that could reduce analysis in your approach is the shouldSkip method might be able to completely skip out on analysis for some files that aren't being analyzed by others.

[evanweible-wf]

Following up here, I'm playing around with a few more refactors and breaking changes in this branch if you're interested in taking a look: https://github.com/Workiva/dart_codemod/pull/39