Skip to content

Commit 5d785a0

Browse files
[tool] Combine code analysis commands into analyze (#9860)
Folds native code analysis commands into `analyze` to simplify the set of commands: - `lint-android` is now `analyze --no-dart --android` - `xcode-analyze --ios` is now `analyze --no-dart --ios` - `xcode-analyze --macos` is now `analyze --no-dart --macos` In practice `--no-dart` is mostly there for CI; Dart analysis is fast enough that for local plugin development the new expected use case would likely be `analyze --whateverplatorm`, and it will run analysis on all relevant code in the package. Part of flutter/flutter#173413 ## Pre-Review Checklist **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot 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. [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.
1 parent 3db5adc commit 5d785a0

12 files changed

+1659
-1637
lines changed

.ci/targets/android_platform_tests.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ tasks:
1111
args: ["build-examples", "--apk"]
1212
- name: lint
1313
script: .ci/scripts/tool_runner.sh
14-
args: ["lint-android"]
14+
args: ["analyze", "--android", "--no-dart"]
1515
# Native unit and native integration are split into two steps to allow for
1616
# different exclusions.
1717
# TODO(stuartmorgan): Eliminate the native unit test exclusion, and combine

.ci/targets/ios_platform_tests.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,11 @@ tasks:
1414
args: ["build-examples", "--ios", "--swift-package-manager"]
1515
- name: xcode analyze
1616
script: .ci/scripts/tool_runner.sh
17-
args: ["xcode-analyze", "--ios", "--exclude=script/configs/xcode_warnings_exceptions.yaml"]
17+
args: ["analyze", "--no-dart", "--ios", "--exclude=script/configs/xcode_warnings_exceptions.yaml"]
1818
- name: xcode analyze deprecation
1919
# Ensure we don't accidentally introduce deprecated code.
2020
script: .ci/scripts/tool_runner.sh
21-
args: ["xcode-analyze", "--ios", "--ios-min-version=14.0", "--exclude=script/configs/exclude_xcode_deprecation.yaml,script/configs/xcode_warnings_exceptions.yaml"]
21+
args: ["analyze", "--no-dart", "--ios", "--ios-min-version=14.0", "--exclude=script/configs/exclude_xcode_deprecation.yaml,script/configs/xcode_warnings_exceptions.yaml"]
2222
- name: native test
2323
script: .ci/scripts/tool_runner.sh
2424
# Simulator name and version must match name and version in create_simulator.sh

.ci/targets/macos_platform_tests.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,11 @@ tasks:
1111
args: ["build-examples", "--macos", "--swift-package-manager"]
1212
- name: xcode analyze
1313
script: .ci/scripts/tool_runner.sh
14-
args: ["xcode-analyze", "--macos", "--exclude=script/configs/xcode_warnings_exceptions.yaml"]
14+
args: ["analyze", "--no-dart", "--macos", "--exclude=script/configs/xcode_warnings_exceptions.yaml"]
1515
- name: xcode analyze deprecation
1616
# Ensure we don't accidentally introduce deprecated code.
1717
script: .ci/scripts/tool_runner.sh
18-
args: ["xcode-analyze", "--macos", "--macos-min-version=14.0", "--exclude=script/configs/exclude_xcode_deprecation.yaml,script/configs/xcode_warnings_exceptions.yaml"]
18+
args: ["analyze", "--no-dart", "--macos", "--macos-min-version=14.0", "--exclude=script/configs/exclude_xcode_deprecation.yaml,script/configs/xcode_warnings_exceptions.yaml"]
1919
- name: native test
2020
script: .ci/scripts/tool_runner.sh
2121
args: ["native-test", "--macos", "--xcode-warnings-exceptions=script/configs/xcode_warnings_exceptions.yaml"]

script/tool/README.md

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,12 +49,25 @@ dart run script/tool/bin/flutter_plugin_tools.dart format --packages package_nam
4949

5050
The `flutter/packages` repository uses clang version `15.0.0` . Newer versions of clang may format code differently.
5151

52-
### Run the Dart Static Analyzer
52+
### Run the Static Analysis
53+
54+
To analyze only Dart code:
5355

5456
```sh
5557
dart run script/tool/bin/flutter_plugin_tools.dart analyze --packages package_name
5658
```
5759

60+
To include native code, include the relevant platform flag(s). For example:
61+
62+
```sh
63+
# Analyze Dart and Android Java/Kotlin code:
64+
dart run script/tool/bin/flutter_plugin_tools.dart analyze --android --packages package_name
65+
# Analyze Dart and iOS+macOS Objective-C/Swift code:
66+
dart run script/tool/bin/flutter_plugin_tools.dart analyze --ios --macos --packages package_name
67+
```
68+
69+
Dart analysis can be excluded with `--no-dart`.
70+
5871
### Run Dart Unit Tests
5972

6073
```sh

script/tool/lib/src/analyze_command.dart

Lines changed: 258 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,15 @@ import 'dart:io' as io;
66

77
import 'package:file/file.dart';
88

9+
import 'common/core.dart';
910
import 'common/file_filters.dart';
11+
import 'common/flutter_command_utils.dart';
12+
import 'common/gradle.dart';
1013
import 'common/output_utils.dart';
1114
import 'common/package_looping_command.dart';
15+
import 'common/plugin_utils.dart';
1216
import 'common/repository_package.dart';
17+
import 'common/xcode.dart';
1318

1419
/// A command to run Dart analysis on packages.
1520
class AnalyzeCommand extends PackageLoopingCommand {
@@ -20,6 +25,17 @@ class AnalyzeCommand extends PackageLoopingCommand {
2025
super.platform,
2126
super.gitDir,
2227
}) {
28+
// Platform options.
29+
// By default, only Dart analysis is run.
30+
argParser.addFlag(_dartFlag, help: "Runs 'dart analyze'", defaultsTo: true);
31+
argParser.addFlag(platformAndroid,
32+
help: "Runs 'gradle lint' on Android code");
33+
argParser.addFlag(platformIOS,
34+
help: "Runs 'xcodebuild analyze' on iOS code");
35+
argParser.addFlag(platformMacOS,
36+
help: "Runs 'xcodebuild analyze' on macOS code");
37+
38+
// Dart options.
2339
argParser.addMultiOption(_customAnalysisFlag,
2440
help:
2541
'Directories (comma separated) that are allowed to have their own '
@@ -42,13 +58,27 @@ class AnalyzeCommand extends PackageLoopingCommand {
4258
'intended to be used with pathified analysis, where a resolver '
4359
'failure indicates that no out-of-band failure can result anyway.',
4460
hide: true);
61+
62+
// Xcode options.
63+
argParser.addOption(_minIOSVersionArg,
64+
help: 'Sets the minimum iOS deployment version to use when compiling, '
65+
'overriding the default minimum version. This can be used to find '
66+
'deprecation warnings that will affect the plugin in the future.');
67+
argParser.addOption(_minMacOSVersionArg,
68+
help:
69+
'Sets the minimum macOS deployment version to use when compiling, '
70+
'overriding the default minimum version. This can be used to find '
71+
'deprecation warnings that will affect the plugin in the future.');
4572
}
4673

74+
static const String _dartFlag = 'dart';
4775
static const String _customAnalysisFlag = 'custom-analysis';
4876
static const String _downgradeFlag = 'downgrade';
4977
static const String _libOnlyFlag = 'lib-only';
5078
static const String _analysisSdk = 'analysis-sdk';
5179
static const String _skipIfResolvingFailsFlag = 'skip-if-resolving-fails';
80+
static const String _minIOSVersionArg = 'ios-min-version';
81+
static const String _minMacOSVersionArg = 'macos-min-version';
5282

5383
late String _dartBinaryPath;
5484

@@ -61,9 +91,6 @@ class AnalyzeCommand extends PackageLoopingCommand {
6191
final String description = 'Analyzes all packages using dart analyze.\n\n'
6292
'This command requires "dart" and "flutter" to be in your path.';
6393

64-
@override
65-
final bool hasLongOutput = false;
66-
6794
/// Checks that there are no unexpected analysis_options.yaml files.
6895
bool _hasUnexpectedAnalysisOptions(RepositoryPackage package) {
6996
final List<FileSystemEntity> files =
@@ -95,9 +122,32 @@ class AnalyzeCommand extends PackageLoopingCommand {
95122

96123
@override
97124
bool shouldIgnoreFile(String path) {
98-
return isRepoLevelNonCodeImpactingFile(path) ||
99-
isNativeCodeFile(path) ||
100-
isPackageSupportFile(path);
125+
// Support files don't affect any analysis.
126+
if (isRepoLevelNonCodeImpactingFile(path) || isPackageSupportFile(path)) {
127+
return true;
128+
}
129+
130+
// For native code, it depends on the flags.
131+
if (path.endsWith('.dart')) {
132+
return !getBoolArg(_dartFlag);
133+
}
134+
if (path.endsWith('.java') || path.endsWith('.kt')) {
135+
return !getBoolArg(platformAndroid);
136+
}
137+
if (path.endsWith('.c') ||
138+
path.endsWith('.cc') ||
139+
path.endsWith('.cpp') ||
140+
path.endsWith('.h')) {
141+
// If C/C++ linting is added, Windows and Linux should be added here.
142+
return !(getBoolArg(platformIOS) || getBoolArg(platformMacOS));
143+
}
144+
if (path.endsWith('.m') ||
145+
path.endsWith('.mm') ||
146+
path.endsWith('.swift')) {
147+
return !(getBoolArg(platformIOS) || getBoolArg(platformMacOS));
148+
}
149+
150+
return false;
101151
}
102152

103153
@override
@@ -112,6 +162,92 @@ class AnalyzeCommand extends PackageLoopingCommand {
112162

113163
@override
114164
Future<PackageResult> runForPackage(RepositoryPackage package) async {
165+
final Map<String, PackageResult> subResults = <String, PackageResult>{};
166+
if (getBoolArg(_dartFlag)) {
167+
_printSectionHeading('Running dart analyze.');
168+
subResults['Dart'] = await _runDartAnalysisForPackage(package);
169+
}
170+
if (getBoolArg(platformAndroid)) {
171+
_printSectionHeading('Running gradle lint.');
172+
subResults['Android'] = await _runGradleLintForPackage(package);
173+
}
174+
if (getBoolArg(platformIOS)) {
175+
_printSectionHeading('Running iOS xcodebuild analyze.');
176+
final String minIOSVersion = getStringArg(_minIOSVersionArg);
177+
subResults['iOS'] = await _runXcodeAnalysisForPackage(
178+
package,
179+
FlutterPlatform.ios,
180+
extraFlags: <String>[
181+
'-destination',
182+
'generic/platform=iOS Simulator',
183+
if (minIOSVersion.isNotEmpty)
184+
'IPHONEOS_DEPLOYMENT_TARGET=$minIOSVersion',
185+
],
186+
);
187+
}
188+
if (getBoolArg(platformMacOS)) {
189+
_printSectionHeading('Running macOS xcodebuild analyze.');
190+
final String minMacOSVersion = getStringArg(_minMacOSVersionArg);
191+
subResults['macOS'] = await _runXcodeAnalysisForPackage(
192+
package,
193+
FlutterPlatform.macos,
194+
extraFlags: <String>[
195+
if (minMacOSVersion.isNotEmpty)
196+
'MACOSX_DEPLOYMENT_TARGET=$minMacOSVersion',
197+
],
198+
);
199+
}
200+
201+
// Make sure at least one analysis option was requested.
202+
if (subResults.isEmpty) {
203+
printError('At least one analysis option flag must be provided.');
204+
throw ToolExit(exitInvalidArguments);
205+
}
206+
// If only one analysis was requested, just return its result.
207+
if (subResults.length == 1) {
208+
return subResults.values.first;
209+
}
210+
// Otherwise, aggregate the messages, with the least positive status.
211+
final Map<String, PackageResult> failedResults =
212+
Map<String, PackageResult>.of(subResults)
213+
..removeWhere((String key, PackageResult value) =>
214+
value.state != RunState.failed);
215+
final Map<String, PackageResult> skippedResults =
216+
Map<String, PackageResult>.of(subResults)
217+
..removeWhere((String key, PackageResult value) =>
218+
value.state != RunState.skipped);
219+
// If anything failed, collect all the failure messages, prefixed by type.
220+
if (failedResults.isNotEmpty) {
221+
return PackageResult.fail(<String>[
222+
for (final MapEntry<String, PackageResult> entry
223+
in failedResults.entries)
224+
'${entry.key}${entry.value.details.isEmpty ? '' : ': ${entry.value.details.join(', ')}'}'
225+
]);
226+
}
227+
// If everything was skipped, mark as skipped with all of the explanations.
228+
if (skippedResults.length == subResults.length) {
229+
return PackageResult.skip(skippedResults.entries
230+
.map((MapEntry<String, PackageResult> entry) =>
231+
'${entry.key}: ${entry.value.details.first}')
232+
.join(', '));
233+
}
234+
// For all succes, or a mix of success and skip, log any skips but mark as
235+
// success.
236+
for (final MapEntry<String, PackageResult> skip in skippedResults.entries) {
237+
printSkip('Skipped ${skip.key}: ${skip.value.details.first}');
238+
}
239+
return PackageResult.success();
240+
}
241+
242+
void _printSectionHeading(String heading) {
243+
print('\n$heading');
244+
print('--------------------');
245+
}
246+
247+
/// Runs Dart analysis for the given package, and returns the result that
248+
/// applies to that analysis.
249+
Future<PackageResult> _runDartAnalysisForPackage(
250+
RepositoryPackage package) async {
115251
final bool libOnly = getBoolArg(_libOnlyFlag);
116252

117253
if (libOnly && !package.libDirectory.existsSync()) {
@@ -176,4 +312,120 @@ class AnalyzeCommand extends PackageLoopingCommand {
176312
workingDir: package.directory);
177313
return exitCode == 0;
178314
}
315+
316+
/// Runs Gradle lint analysis for the given package, and returns the result
317+
/// that applies to that analysis.
318+
Future<PackageResult> _runGradleLintForPackage(
319+
RepositoryPackage package) async {
320+
if (!pluginSupportsPlatform(platformAndroid, package,
321+
requiredMode: PlatformSupport.inline)) {
322+
return PackageResult.skip(
323+
'Package does not contain native Android plugin code');
324+
}
325+
326+
for (final RepositoryPackage example in package.getExamples()) {
327+
final GradleProject project = GradleProject(example,
328+
processRunner: processRunner, platform: platform);
329+
330+
if (!project.isConfigured()) {
331+
final bool buildSuccess = await runConfigOnlyBuild(
332+
example, processRunner, platform, FlutterPlatform.android);
333+
if (!buildSuccess) {
334+
printError('Unable to configure Gradle project.');
335+
return PackageResult.fail(<String>['Unable to configure Gradle.']);
336+
}
337+
}
338+
339+
final String packageName = package.directory.basename;
340+
341+
// Only lint one build mode to avoid extra work.
342+
// Only lint the plugin project itself, to avoid failing due to errors in
343+
// dependencies.
344+
//
345+
// TODO(stuartmorgan): Consider adding an XML parser to read and summarize
346+
// all results. Currently, only the first three errors will be shown
347+
// inline, and the rest have to be checked via the CI-uploaded artifact.
348+
final int exitCode = await project.runCommand('$packageName:lintDebug');
349+
if (exitCode != 0) {
350+
return PackageResult.fail();
351+
}
352+
}
353+
354+
return PackageResult.success();
355+
}
356+
357+
/// Analyzes [plugin] for [targetPlatform].
358+
Future<PackageResult> _runXcodeAnalysisForPackage(
359+
RepositoryPackage package,
360+
FlutterPlatform targetPlatform, {
361+
List<String> extraFlags = const <String>[],
362+
}) async {
363+
final String platformString =
364+
targetPlatform == FlutterPlatform.ios ? 'iOS' : 'macOS';
365+
if (!pluginSupportsPlatform(targetPlatform.name, package,
366+
requiredMode: PlatformSupport.inline)) {
367+
return PackageResult.skip(
368+
'Package does not contain native $platformString plugin code');
369+
}
370+
371+
final Xcode xcode = Xcode(processRunner: processRunner, log: true);
372+
final List<String> errors = <String>[];
373+
for (final RepositoryPackage example in package.getExamples()) {
374+
// See https://github.com/flutter/flutter/issues/172427 for discussion of
375+
// why this is currently necessary.
376+
print('Disabling Swift Package Manager...');
377+
setSwiftPackageManagerState(example, enabled: false);
378+
379+
// Unconditionally re-run build with --debug --config-only, to ensure that
380+
// the project is in a debug state even if it was previously configured,
381+
// and that SwiftPM is disabled.
382+
print('Running flutter build --config-only...');
383+
final bool buildSuccess = await runConfigOnlyBuild(
384+
example,
385+
processRunner,
386+
platform,
387+
targetPlatform,
388+
buildDebug: true,
389+
);
390+
if (!buildSuccess) {
391+
printError('Unable to prepare native project files.');
392+
errors.add(
393+
'Unable to build ${getRelativePosixPath(example.directory, from: package.directory)}.');
394+
continue;
395+
}
396+
397+
// Running tests and static analyzer.
398+
final String examplePath = getRelativePosixPath(example.directory,
399+
from: package.directory.parent);
400+
print('Running $platformString tests and analyzer for $examplePath...');
401+
final int exitCode = await xcode.runXcodeBuild(
402+
example.directory,
403+
platformString,
404+
// Clean before analyzing to remove cached swiftmodules from previous
405+
// runs, which can cause conflicts.
406+
actions: <String>['clean', 'analyze'],
407+
workspace: '${platformString.toLowerCase()}/Runner.xcworkspace',
408+
scheme: 'Runner',
409+
configuration: 'Debug',
410+
hostPlatform: platform,
411+
extraFlags: <String>[
412+
...extraFlags,
413+
'GCC_TREAT_WARNINGS_AS_ERRORS=YES',
414+
],
415+
);
416+
if (exitCode == 0) {
417+
printSuccess('$examplePath ($platformString) passed analysis.');
418+
} else {
419+
printError('$examplePath ($platformString) failed analysis.');
420+
errors.add(
421+
'${getRelativePosixPath(example.directory, from: package.directory)} failed analysis.');
422+
}
423+
424+
print('Removing Swift Package Manager override...');
425+
setSwiftPackageManagerState(example, enabled: null);
426+
}
427+
return errors.isEmpty
428+
? PackageResult.success()
429+
: PackageResult.fail(errors);
430+
}
179431
}

script/tool/lib/src/common/output_utils.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,11 @@ void printError(String message) {
3737
print(_colorizeIfAppropriate(message, Styles.RED));
3838
}
3939

40+
/// Prints [message] in dark grey, if the environment supports color.
41+
void printSkip(String message) {
42+
print(_colorizeIfAppropriate(message, Styles.DARK_GRAY));
43+
}
44+
4045
/// Returns [message] with escapes to print it in [color], if the environment
4146
/// supports color.
4247
String colorizeString(String message, Styles color) {

0 commit comments

Comments
 (0)