[tool] Combine code analysis commands into analyze
#9860
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the code analysis tooling by consolidating the lint-android and xcode-analyze commands into a single, more flexible analyze command. This new command allows running Dart and native code analysis for various platforms (Android, iOS, macOS) together or separately, controlled by flags. The changes include updating the AnalyzeCommand to handle different analysis types, removing the old command files, and updating CI configurations and documentation to use the new command. The test suite has also been significantly updated to cover the new unified command's functionality. My review found one critical issue in the CI configuration that needs to be addressed.
|
|
||
| /// Runs Gradle lint analysis for the given package, and returns the result | ||
| /// that applies to that analysis. | ||
| Future<PackageResult> _runGradleLintForPackage( |
There was a problem hiding this comment.
This is runForPackage from lint_android_command.dart, with essentially no changes beyond renaming.
| } | ||
|
|
||
| /// Analyzes [plugin] for [targetPlatform]. | ||
| Future<PackageResult> _runXcodeAnalysisForPackage( |
There was a problem hiding this comment.
This is runForPackage from xcode_analyze_command.dart with some minor changes to make it return a PackageResult, and to include the skip logic that was a level higher in the old command. The result aggregation logic that xcode-analyze had is gone since analyze now has more generic aggregation logic that makes it unnecessary.
| final RepositoryPackage plugin1 = createFakePlugin('a', packagesDir); | ||
|
|
||
| await runCapturingPrint(runner, <String>['analyze']); | ||
| group('result aggregation', () { |
There was a problem hiding this comment.
This small test group is new.
| @@ -35,482 +36,551 @@ void main() { | |||
| runner.addCommand(analyzeCommand); | |||
| }); | |||
|
|
|||
| test('analyzes all packages', () async { | |||
There was a problem hiding this comment.
All the "removed" code is actually just wrapped in a new 'dart analyze' group.
|
|
||
| test('skips commands if all files should be ignored', () async { | ||
| createFakePackage('package_a', packagesDir); | ||
| group('gradle lint', () { |
There was a problem hiding this comment.
This group is the old lint-android tests, with minor string changes where I changed some logging messages.
| }); | ||
|
|
||
| // TODO(stuartmorgan): Separate iOS and macOS. | ||
| group('Xcode analyze', () { |
There was a problem hiding this comment.
This is the old xcode-analyze tests with very minor tweaks.
This comment was marked as spam.
This comment was marked as spam.
Folds native code analysis commands into
analyzeto simplify the set of commands:lint-androidis nowanalyze --no-dart --androidxcode-analyze --iosis nowanalyze --no-dart --iosxcode-analyze --macosis nowanalyze --no-dart --macosIn practice
--no-dartis mostly there for CI; Dart analysis is fast enough that for local plugin development the new expected use case would likely beanalyze --whateverplatorm, and it will run analysis on all relevant code in the package.Part of flutter/flutter#173413
Pre-Review Checklist
[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or I have commented below to indicate which version change exemption this PR falls under1.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style, or I have commented below to indicate which CHANGELOG exemption this PR falls under1.///).Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2 ↩3