diff --git a/.repo_tool_config.yaml b/.repo_tool_config.yaml index 7797e87a90fc..d522aa8e91a3 100644 --- a/.repo_tool_config.yaml +++ b/.repo_tool_config.yaml @@ -20,6 +20,9 @@ allowed_dependencies: # A pin can be either an exact version, or a range with an explicit, inclusive # max version, which must a version that already exists, not a future version. pinned: + # Cyclomatic complexity linter for camera_android_camerax + - dart_code_linter + # Test-only dependency, so does not impact package clients, and # has limited impact so could be easily removed if there are # ever maintenance issues in the future. diff --git a/packages/camera/camera_android_camerax/analysis_options.yaml b/packages/camera/camera_android_camerax/analysis_options.yaml new file mode 100644 index 000000000000..38619134b81a --- /dev/null +++ b/packages/camera/camera_android_camerax/analysis_options.yaml @@ -0,0 +1,16 @@ +include: ../../../analysis_options.yaml + +analyzer: + exclude: + - build/** + - android/** + - ios/** + - web/** + - windows/** + - macos/** + - linux/** + +dart_code_linter: + metrics: + cyclomatic-complexity: 15 + diff --git a/packages/camera/camera_android_camerax/pubspec.yaml b/packages/camera/camera_android_camerax/pubspec.yaml index 257f6fa5d16b..f7496c077e00 100644 --- a/packages/camera/camera_android_camerax/pubspec.yaml +++ b/packages/camera/camera_android_camerax/pubspec.yaml @@ -27,6 +27,7 @@ dependencies: dev_dependencies: build_runner: ^2.2.0 + dart_code_linter: 4.1.5 dart_skills_lint: git: url: https://github.com/flutter/skills.git diff --git a/script/configs/custom_analysis.yaml b/script/configs/custom_analysis.yaml index c0c5b53c1a83..b4539c78a0e1 100644 --- a/script/configs/custom_analysis.yaml +++ b/script/configs/custom_analysis.yaml @@ -34,3 +34,6 @@ - rfw/example # Disables docs requirements, as it is test code. - web_benchmarks/testing/test_app +# Uses custom analysis to enable the cyclomatic complexity linter plugin. +- camera/camera_android_camerax + diff --git a/script/tool/lib/src/analyze_command.dart b/script/tool/lib/src/analyze_command.dart index 3538e3ed5c21..9db7abd6fb5d 100644 --- a/script/tool/lib/src/analyze_command.dart +++ b/script/tool/lib/src/analyze_command.dart @@ -5,6 +5,7 @@ import 'dart:io' as io; import 'package:file/file.dart'; +import 'package:yaml/yaml.dart'; import 'common/core.dart'; import 'common/file_filters.dart'; @@ -330,10 +331,75 @@ class AnalyzeCommand extends PackageLoopingCommand { ...skillsErrors, ]; - if (errors.isNotEmpty) { - return PackageResult.fail(errors); + final customCheckRunners = <_CustomLinter>[ + _CustomLinter(dependencyName: 'dart_code_linter', run: _runDartCodeLinterForPackage), + ]; + + // Skip custom linters during downgrade as metrics are redundant and vulnerable to dependency issues. + if (!getBoolArg(_downgradeFlag)) { + final Pubspec pubspec = package.parsePubspec(); + for (final runner in customCheckRunners) { + final bool hasDependency = pubspec.devDependencies.containsKey(runner.dependencyName); + if (hasDependency) { + errors.addAll(await runner.run(package)); + } + } } - return PackageResult.success(); + + return errors.isEmpty ? PackageResult.success() : PackageResult.fail(errors); + } + + /// Retrieves the configured cyclomatic complexity threshold from the local + /// `analysis_options.yaml` if it exists and is configured. + int? _getLinterThreshold(RepositoryPackage package) { + final File optionsFile = package.directory.childFile('analysis_options.yaml'); + if (!optionsFile.existsSync()) { + return null; + } + try { + final Object? yaml = loadYaml(optionsFile.readAsStringSync()); + if (yaml is YamlMap) { + final Object? linter = yaml['dart_code_linter']; + if (linter is YamlMap) { + final Object? metrics = linter['metrics']; + if (metrics is YamlMap) { + final Object? complexity = metrics['cyclomatic-complexity']; + if (complexity is int) { + return complexity; + } + } + } + } + } catch (_) { + // Ignore errors parsing invalid/incomplete files. + } + return null; + } + + /// Runs the `dart_code_linter` metrics analyzer on the package. + /// + /// Assumes `dart_code_linter` is present in `dev_dependencies`. + Future> _runDartCodeLinterForPackage(RepositoryPackage package) async { + if (!package.libDirectory.existsSync()) { + return []; + } + print('Running dart_code_linter:metrics analysis...'); + final int linterExitCode = await processRunner.runAndStream(_dartBinaryPath, [ + 'run', + 'dart_code_linter:metrics', + 'analyze', + 'lib', + '--set-exit-on-violation-level=warning', + ], workingDir: package.directory); + if (linterExitCode != 0) { + final int? threshold = _getLinterThreshold(package); + final thresholdMessage = threshold != null ? ' (configured threshold: $threshold)' : ''; + return [ + 'Metrics violations found$thresholdMessage. See the package\'s local "analysis_options.yaml" for configured thresholds.', + ]; + } + + return []; } /// Validates that `.agents/skills` contains Dart code if configured for skills analysis. @@ -485,3 +551,17 @@ class AnalyzeCommand extends PackageLoopingCommand { return errors.isEmpty ? PackageResult.success() : PackageResult.fail(errors); } } + +/// Represents a custom linter check that is executed during package analysis. +class _CustomLinter { + const _CustomLinter({required this.dependencyName, required this.run}); + + /// The name of the package dependency that triggers this custom check. + /// + /// The check is only executed if this dependency is listed in the package's + /// `dev_dependencies`. + final String dependencyName; + + /// The runner function that executes the custom check. + final Future> Function(RepositoryPackage) run; +} diff --git a/script/tool/test/analyze_command_test.dart b/script/tool/test/analyze_command_test.dart index c4f5664ec0da..57124c2ebaa5 100644 --- a/script/tool/test/analyze_command_test.dart +++ b/script/tool/test/analyze_command_test.dart @@ -442,6 +442,252 @@ void main() { }); }); + group('dart_code_linter', () { + test('runs dart_code_linter if present in dev_dependencies', () async { + final RepositoryPackage package = createFakePackage( + 'a_package', + packagesDir, + isFlutter: true, + ); + _writeFakePubspecWithLinter(package, inDevDependencies: true); + + _mockCallsForFlutterAnalyze( + processRunner, + extraDartCalls: [ + FakeProcessInfo(MockProcess(), ['run', 'dart_code_linter:metrics']), + ], + ); + + final List output = await runCapturingPrint(runner, ['analyze']); + + expect( + processRunner.recordedCalls, + orderedEquals([ + ProcessCall('flutter', const ['pub', 'get'], package.path), + ProcessCall('dart', const ['analyze', '--fatal-infos'], package.path), + ProcessCall('dart', const [ + 'run', + 'dart_code_linter:metrics', + 'analyze', + 'lib', + '--set-exit-on-violation-level=warning', + ], package.path), + ]), + ); + expect(output, contains('Running dart_code_linter:metrics analysis...')); + }); + + test('does not run dart_code_linter when --downgrade is specified', () async { + final RepositoryPackage package = createFakePackage( + 'a_package', + packagesDir, + isFlutter: true, + ); + _writeFakePubspecWithLinter(package, inDevDependencies: true); + + processRunner.mockProcessesForExecutable['flutter'] = [ + FakeProcessInfo(MockProcess(), ['pub', 'downgrade']), + FakeProcessInfo(MockProcess(), ['pub', 'get']), + ]; + processRunner.mockProcessesForExecutable['dart'] = [ + FakeProcessInfo(MockProcess(), ['analyze', '--fatal-infos']), + ]; + + final List output = await runCapturingPrint(runner, [ + 'analyze', + '--downgrade', + ]); + + expect( + processRunner.recordedCalls, + orderedEquals([ + ProcessCall('flutter', const ['pub', 'downgrade'], package.path), + ProcessCall('flutter', const ['pub', 'get'], package.path), + ProcessCall('dart', const ['analyze', '--fatal-infos'], package.path), + ]), + ); + final String combinedOutput = output.join('\n').toLowerCase(); + expect(combinedOutput, isNot(contains('dart_code_linter'))); + expect(combinedOutput, isNot(contains('metrics'))); + }); + + test('does not run dart_code_linter if present in dependencies', () async { + final RepositoryPackage package = createFakePackage( + 'a_package', + packagesDir, + isFlutter: true, + ); + _writeFakePubspecWithLinter(package, inDependencies: true); + + _mockCallsForFlutterAnalyze(processRunner); + + final List output = await runCapturingPrint(runner, ['analyze']); + + expect( + processRunner.recordedCalls, + orderedEquals([ + ProcessCall('flutter', const ['pub', 'get'], package.path), + ProcessCall('dart', const ['analyze', '--fatal-infos'], package.path), + ]), + ); + final String combinedOutput = output.join('\n').toLowerCase(); + expect(combinedOutput, isNot(contains('dart_code_linter'))); + expect(combinedOutput, isNot(contains('metrics'))); + }); + + test('does not run dart_code_linter if not present', () async { + final RepositoryPackage package = createFakePackage( + 'a_package', + packagesDir, + isFlutter: true, + ); + + _mockCallsForFlutterAnalyze(processRunner); + + final List output = await runCapturingPrint(runner, ['analyze']); + + expect( + processRunner.recordedCalls, + orderedEquals([ + ProcessCall('flutter', const ['pub', 'get'], package.path), + ProcessCall('dart', const ['analyze', '--fatal-infos'], package.path), + ]), + ); + final String combinedOutput = output.join('\n').toLowerCase(); + expect(combinedOutput, isNot(contains('dart_code_linter'))); + expect(combinedOutput, isNot(contains('metrics'))); + }); + + test('runs dart_code_linter using dart for pure Dart packages', () async { + final RepositoryPackage package = createFakePackage('a_package', packagesDir); + _writeFakePubspecWithLinter(package, inDevDependencies: true, includeFlutter: false); + + processRunner.mockProcessesForExecutable['dart'] = [ + FakeProcessInfo(MockProcess(), ['pub', 'get']), + FakeProcessInfo(MockProcess(), ['analyze']), + FakeProcessInfo(MockProcess(), ['run', 'dart_code_linter:metrics']), + ]; + + final List output = await runCapturingPrint(runner, ['analyze']); + + expect( + processRunner.recordedCalls, + orderedEquals([ + ProcessCall('dart', const ['pub', 'get'], package.path), + ProcessCall('dart', const ['analyze', '--fatal-infos'], package.path), + ProcessCall('dart', const [ + 'run', + 'dart_code_linter:metrics', + 'analyze', + 'lib', + '--set-exit-on-violation-level=warning', + ], package.path), + ]), + ); + expect(output, contains('Running dart_code_linter:metrics analysis...')); + }); + + test('skips dart_code_linter if lib/ does not exist', () async { + final RepositoryPackage package = createFakePackage( + 'a_package', + packagesDir, + isFlutter: true, + ); + _writeFakePubspecWithLinter(package, inDevDependencies: true); + package.libDirectory.deleteSync(recursive: true); + + _mockCallsForFlutterAnalyze(processRunner); + + final List output = await runCapturingPrint(runner, ['analyze']); + + expect( + processRunner.recordedCalls, + orderedEquals([ + ProcessCall('flutter', const ['pub', 'get'], package.path), + ProcessCall('dart', const ['analyze', '--fatal-infos'], package.path), + ]), + ); + final String combinedOutput = output.join('\n').toLowerCase(); + expect(combinedOutput, isNot(contains('dart_code_linter'))); + expect(combinedOutput, isNot(contains('metrics'))); + }); + + test('fails if dart_code_linter analysis fails', () async { + final RepositoryPackage package = createFakePackage( + 'a_package', + packagesDir, + isFlutter: true, + ); + _writeFakePubspecWithLinter(package, inDevDependencies: true); + + _mockCallsForFlutterAnalyze( + processRunner, + extraDartCalls: [ + FakeProcessInfo(MockProcess(exitCode: 1), ['run', 'dart_code_linter:metrics']), + ], + ); + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['analyze'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains( + 'Metrics violations found. See the package\'s local "analysis_options.yaml" for configured thresholds.', + ), + ]), + ); + }); + + test('fails with threshold if analysis_options.yaml defines one', () async { + final RepositoryPackage package = createFakePackage( + 'a_package', + packagesDir, + isFlutter: true, + ); + _writeFakePubspecWithLinter(package, inDevDependencies: true); + package.directory.childFile('analysis_options.yaml').writeAsStringSync(''' +dart_code_linter: + metrics: + cyclomatic-complexity: 15 +'''); + + _mockCallsForFlutterAnalyze( + processRunner, + extraDartCalls: [ + FakeProcessInfo(MockProcess(exitCode: 1), ['run', 'dart_code_linter:metrics']), + ], + ); + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['analyze', '--custom-analysis', 'a_package'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains( + 'Metrics violations found (configured threshold: 15). See the package\'s local "analysis_options.yaml" for configured thresholds.', + ), + ]), + ); + }); + }); + group('skills analysis via ci_config.yaml', () { test('fails if package configuration has no dart files in .agents/skills', () async { final RepositoryPackage plugin = createFakePlugin('foo', packagesDir); @@ -551,6 +797,59 @@ void main() { contains(' foo:\n Main package analysis failed\n Skills analysis failed'), ); }); + + test( + 'fails with package analysis, skills analysis, and custom linter violations listed if all fail', + () async { + final RepositoryPackage package = createFakePackage( + 'a_package', + packagesDir, + isFlutter: true, + ); + _writeFakePubspecWithLinter(package, inDevDependencies: true); + package.ciConfigFile.writeAsStringSync('analyze_skills: true'); + + package.directory + .childDirectory('.agents') + .childDirectory('skills') + .childFile('test.dart') + .createSync(recursive: true); + + processRunner.mockProcessesForExecutable['flutter'] = [ + FakeProcessInfo(MockProcess(), ['pub', 'get']), + ]; + processRunner.mockProcessesForExecutable['dart'] = [ + FakeProcessInfo(MockProcess(exitCode: 1), ['analyze']), // main package + FakeProcessInfo(MockProcess(exitCode: 1), ['analyze']), // skills package + FakeProcessInfo(MockProcess(exitCode: 1), [ + 'run', + 'dart_code_linter:metrics', + ]), // custom linter + ]; + + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['analyze', '--custom-analysis', 'a_package'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + final String joinedOutput = output.join('\n'); + expect(joinedOutput, contains('The following packages had errors:')); + expect( + joinedOutput, + contains( + ' a_package:\n' + ' Main package analysis failed\n' + ' Skills analysis failed\n' + ' Metrics violations found. See the package\'s local "analysis_options.yaml" for configured thresholds.', + ), + ); + }, + ); }); test('skips if requested if "pub get" fails in the resolver', () async { @@ -1637,3 +1936,37 @@ packages/package_a/lib/foo.dart }); }); } + +void _writeFakePubspecWithLinter( + RepositoryPackage package, { + bool inDevDependencies = false, + bool inDependencies = false, + bool includeFlutter = true, +}) { + package.pubspecFile.writeAsStringSync(''' +name: ${package.directory.basename} +version: 0.0.1 +environment: + sdk: ">=2.14.0 <4.0.0" + ${includeFlutter ? 'flutter: ">=2.5.0"' : ''} +dependencies: + ${includeFlutter ? 'flutter:\n sdk: flutter' : ''} +${inDependencies ? ' dart_code_linter: 4.1.5' : ''} +${inDevDependencies ? 'dev_dependencies:\n dart_code_linter: 4.1.5' : ''} +'''); +} + +void _mockCallsForFlutterAnalyze( + RecordingProcessRunner processRunner, { + List extraFlutterCalls = const [], + List extraDartCalls = const [], +}) { + processRunner.mockProcessesForExecutable['flutter'] = [ + FakeProcessInfo(MockProcess(), const ['pub', 'get']), + ...extraFlutterCalls, + ]; + processRunner.mockProcessesForExecutable['dart'] = [ + FakeProcessInfo(MockProcess(), const ['analyze']), + ...extraDartCalls, + ]; +}