From 0a65ef84c7548f0243332d3efc8bfc7084cc84d9 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Thu, 18 Jun 2026 09:18:57 -0700 Subject: [PATCH 01/37] first pass --- script/githooks/bin/install_hooks.dart | 22 +++++ script/githooks/bin/main.dart | 15 +++ .../githooks/lib/src/pre_commit_command.dart | 75 ++++++++++++++ script/githooks/pre-commit | 6 ++ script/githooks/pubspec.yaml | 13 +++ script/githooks/test/pre_commit_test.dart | 98 +++++++++++++++++++ 6 files changed, 229 insertions(+) create mode 100644 script/githooks/bin/install_hooks.dart create mode 100644 script/githooks/bin/main.dart create mode 100644 script/githooks/lib/src/pre_commit_command.dart create mode 100755 script/githooks/pre-commit create mode 100644 script/githooks/pubspec.yaml create mode 100644 script/githooks/test/pre_commit_test.dart diff --git a/script/githooks/bin/install_hooks.dart b/script/githooks/bin/install_hooks.dart new file mode 100644 index 000000000000..87682a2779a8 --- /dev/null +++ b/script/githooks/bin/install_hooks.dart @@ -0,0 +1,22 @@ +import 'dart:io'; +import 'package:path/path.dart' as p; + +void main() async { + var repoRoot = Directory.current; + while (repoRoot.path != '/' && !Directory(p.join(repoRoot.path, '.git')).existsSync()) { + repoRoot = repoRoot.parent; + } + + if (repoRoot.path == '/') { + print('❌ Could not find .git directory.'); + exit(1); + } + + final result = await Process.run('git', ['config', 'core.hooksPath', 'script/githooks'], workingDirectory: repoRoot.path); + if (result.exitCode == 0) { + print('✅ Git hooks installed successfully!'); + } else { + print('❌ Failed to install Git hooks: ${result.stderr}'); + exit(1); + } +} diff --git a/script/githooks/bin/main.dart b/script/githooks/bin/main.dart new file mode 100644 index 000000000000..57100edb8f4d --- /dev/null +++ b/script/githooks/bin/main.dart @@ -0,0 +1,15 @@ +import 'dart:io'; +import 'package:args/command_runner.dart'; +import '../lib/src/pre_commit_command.dart'; + +void main(List args) async { + final runner = CommandRunner('githooks', 'Git hooks for flutter/packages') + ..addCommand(PreCommitCommand()); + + try { + await runner.run(args); + } catch (e) { + print(e); + exit(1); + } +} diff --git a/script/githooks/lib/src/pre_commit_command.dart b/script/githooks/lib/src/pre_commit_command.dart new file mode 100644 index 000000000000..48ba4c970ca0 --- /dev/null +++ b/script/githooks/lib/src/pre_commit_command.dart @@ -0,0 +1,75 @@ +import 'dart:io'; +import 'package:args/command_runner.dart'; + +class PreCommitCommand extends Command { + @override + final String name = 'pre-commit'; + + @override + final String description = 'Runs pre-commit checks like format and analyze.'; + + @override + Future run() async { + // 1. Get the list of staged files + final diffResult = await Process.run('git', ['diff', '--cached', '--name-only', '--diff-filter=ACM']); + if (diffResult.exitCode != 0) { + print('Failed to get staged files.'); + exit(1); + } + + // Filter for Dart files + final stagedDartFiles = (diffResult.stdout as String) + .split('\n') + .map((file) => file.trim()) + .where((file) => file.endsWith('.dart')) + .toList(); + + if (stagedDartFiles.isEmpty) { + // No Dart files are being committed; exit cleanly + return; + } + + print('🔍 Running pre-commit checks on staged Dart files...'); + bool hasError = false; + + // 2. Code Formatting Check + print('Checking formatting...'); + final formatResult = await Process.run('dart', [ + 'format', + '--output=none', + '--set-exit-if-changed', + ...stagedDartFiles, + ]); + + if (formatResult.exitCode != 0) { + print('❌ Formatting issues found in the following files:'); + print((formatResult.stdout as String).trim()); + print('👉 Please run "dart format" on these files to fix them.'); + hasError = true; + } else { + print('✅ Formatting looks good.'); + } + + // 3. Static Analysis Check + print('Running static analysis...'); + final analyzeResult = await Process.run('dart', [ + 'analyze', + '--fatal-infos', + ...stagedDartFiles, + ]); + + if (analyzeResult.exitCode != 0) { + print('❌ Static analysis errors found:'); + print((analyzeResult.stdout as String).trim()); + hasError = true; + } else { + print('✅ Static analysis looks good.'); + } + + // 4. Final Verdict + if (hasError) { + print('🚨 Pre-commit checks failed. Please fix the above errors and try committing again.'); + exit(1); + } + } +} diff --git a/script/githooks/pre-commit b/script/githooks/pre-commit new file mode 100755 index 000000000000..528775e30233 --- /dev/null +++ b/script/githooks/pre-commit @@ -0,0 +1,6 @@ +#!/usr/bin/env bash +set -e + +echo "HOOK EXECUTED" >&2 +HOOKS_DIR="$(dirname "$0")" +exec dart "$HOOKS_DIR/bin/main.dart" pre-commit "$@" diff --git a/script/githooks/pubspec.yaml b/script/githooks/pubspec.yaml new file mode 100644 index 000000000000..a191a975b763 --- /dev/null +++ b/script/githooks/pubspec.yaml @@ -0,0 +1,13 @@ +name: githooks +description: Git hooks for the flutter/packages repository. +publish_to: none + +environment: + sdk: ^3.10.0-0 + +dependencies: + args: any + +dev_dependencies: + path: any + test: any diff --git a/script/githooks/test/pre_commit_test.dart b/script/githooks/test/pre_commit_test.dart new file mode 100644 index 000000000000..f67165385095 --- /dev/null +++ b/script/githooks/test/pre_commit_test.dart @@ -0,0 +1,98 @@ +import 'dart:io'; +import 'package:test/test.dart'; +import 'package:path/path.dart' as p; + +void main() { + group('pre-commit hook', () { + late Directory tempDir; + late String githooksPath; + + setUp(() async { + tempDir = await Directory.systemTemp.createTemp('pre_commit_test_'); + + await Process.run('git', ['init'], workingDirectory: tempDir.path); + await Process.run('git', ['config', 'user.email', 'test@example.com'], workingDirectory: tempDir.path); + await Process.run('git', ['config', 'user.name', 'Test User'], workingDirectory: tempDir.path); + + var repoRoot = Directory.current; + while (repoRoot.path != '/' && !Directory(p.join(repoRoot.path, '.git')).existsSync()) { + repoRoot = repoRoot.parent; + } + + githooksPath = p.join(repoRoot.path, 'script', 'githooks'); + + // Set the hooksPath to the actual githooks directory + await Process.run('git', ['config', 'core.hooksPath', githooksPath], workingDirectory: tempDir.path); + }); + + tearDown(() async { + await tempDir.delete(recursive: true); + }); + + Future runHook() async { + // Actually run git commit to trigger the hook! + return Process.run('git', ['commit', '-m', 'test'], workingDirectory: tempDir.path); + } + + test('passes on a clean commit', () async { + final file = File(p.join(tempDir.path, 'test_file.dart')); + await file.writeAsString(''' +void main() { + print('Hello, world!'); +} +'''); + + await Process.run('git', ['add', 'test_file.dart'], workingDirectory: tempDir.path); + + final result = await runHook(); + expect(result.exitCode, 0); + final output = '${result.stdout}${result.stderr}'; + expect(output, contains('Formatting looks good.')); + expect(output, contains('Static analysis looks good.')); + }); + + test('fails on formatting error', () async { + final file = File(p.join(tempDir.path, 'test_file.dart')); + await file.writeAsString(''' +void main(){ + print('Hello, world!'); +} +'''); + + await Process.run('git', ['add', 'test_file.dart'], workingDirectory: tempDir.path); + + final result = await runHook(); + expect(result.exitCode, 1); + final output = '${result.stdout}${result.stderr}'; + expect(output, contains('Formatting issues found')); + }); + + test('fails on analysis error', () async { + final file = File(p.join(tempDir.path, 'test_file.dart')); + await file.writeAsString(''' +void main() { + print('Hello, world!') +} +'''); + + await Process.run('git', ['add', 'test_file.dart'], workingDirectory: tempDir.path); + + final result = await runHook(); + expect(result.exitCode, 1); + final output = '${result.stdout}${result.stderr}'; + expect(output, contains('Static analysis errors found')); + }); + + test('ignores non-dart files', () async { + final file = File(p.join(tempDir.path, 'README.md')); + await file.writeAsString('# Hello'); + + await Process.run('git', ['add', 'README.md'], workingDirectory: tempDir.path); + + final result = await runHook(); + expect(result.exitCode, 0); + final output = '${result.stdout}${result.stderr}'; + expect(output, isNot(contains('Checking formatting...'))); + }); + }); +} From ccda0df5c750adc5384a685fd6d5e60053cf7c58 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Thu, 18 Jun 2026 11:21:37 -0700 Subject: [PATCH 02/37] rippling revisions --- script/githooks/bin/install_hooks.dart | 14 +++++- script/githooks/bin/main.dart | 20 +++----- script/githooks/lib/githooks.dart | 16 ++++++ .../githooks/lib/src/pre_commit_command.dart | 49 +++++++++++-------- script/githooks/pubspec.yaml | 2 +- script/githooks/test/pre_commit_test.dart | 15 +++--- 6 files changed, 74 insertions(+), 42 deletions(-) create mode 100644 script/githooks/lib/githooks.dart diff --git a/script/githooks/bin/install_hooks.dart b/script/githooks/bin/install_hooks.dart index 87682a2779a8..929e03a81263 100644 --- a/script/githooks/bin/install_hooks.dart +++ b/script/githooks/bin/install_hooks.dart @@ -1,8 +1,14 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// ignore_for_file: avoid_print + import 'dart:io'; import 'package:path/path.dart' as p; void main() async { - var repoRoot = Directory.current; + Directory repoRoot = Directory.current; while (repoRoot.path != '/' && !Directory(p.join(repoRoot.path, '.git')).existsSync()) { repoRoot = repoRoot.parent; } @@ -12,7 +18,11 @@ void main() async { exit(1); } - final result = await Process.run('git', ['config', 'core.hooksPath', 'script/githooks'], workingDirectory: repoRoot.path); + final ProcessResult result = await Process.run('git', [ + 'config', + 'core.hooksPath', + 'script/githooks', + ], workingDirectory: repoRoot.path); if (result.exitCode == 0) { print('✅ Git hooks installed successfully!'); } else { diff --git a/script/githooks/bin/main.dart b/script/githooks/bin/main.dart index 57100edb8f4d..7d734dd82b52 100644 --- a/script/githooks/bin/main.dart +++ b/script/githooks/bin/main.dart @@ -1,15 +1,11 @@ -import 'dart:io'; -import 'package:args/command_runner.dart'; -import '../lib/src/pre_commit_command.dart'; +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. -void main(List args) async { - final runner = CommandRunner('githooks', 'Git hooks for flutter/packages') - ..addCommand(PreCommitCommand()); +import 'dart:io' as io; - try { - await runner.run(args); - } catch (e) { - print(e); - exit(1); - } +import 'package:githooks/githooks.dart'; + +Future main(List args) async { + io.exitCode = await run(args); } diff --git a/script/githooks/lib/githooks.dart b/script/githooks/lib/githooks.dart new file mode 100644 index 000000000000..59b3e6fe3fff --- /dev/null +++ b/script/githooks/lib/githooks.dart @@ -0,0 +1,16 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:args/command_runner.dart'; + +import 'src/pre_commit_command.dart'; + +/// Runs the githooks command line utility. +Future run(List args) async { + final runner = CommandRunner('githooks', 'Git hooks for flutter/packages') + ..addCommand(PreCommitCommand()); + + final bool success = await runner.run(args) ?? false; + return success ? 0 : 1; +} diff --git a/script/githooks/lib/src/pre_commit_command.dart b/script/githooks/lib/src/pre_commit_command.dart index 48ba4c970ca0..4f7e016a808d 100644 --- a/script/githooks/lib/src/pre_commit_command.dart +++ b/script/githooks/lib/src/pre_commit_command.dart @@ -1,40 +1,51 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// ignore_for_file: avoid_print + import 'dart:io'; import 'package:args/command_runner.dart'; -class PreCommitCommand extends Command { +/// The command that implements the pre-commit githook +class PreCommitCommand extends Command { @override final String name = 'pre-commit'; @override - final String description = 'Runs pre-commit checks like format and analyze.'; + final String description = 'Checks to run before a "git commit"'; @override - Future run() async { - // 1. Get the list of staged files - final diffResult = await Process.run('git', ['diff', '--cached', '--name-only', '--diff-filter=ACM']); + Future run() async { + // Find changed Dart files. + final ProcessResult diffResult = await Process.run('git', [ + 'diff', + '--cached', + '--name-only', + '--diff-filter=ACM', + ]); if (diffResult.exitCode != 0) { print('Failed to get staged files.'); exit(1); } - // Filter for Dart files - final stagedDartFiles = (diffResult.stdout as String) + final List stagedDartFiles = (diffResult.stdout as String) .split('\n') .map((file) => file.trim()) .where((file) => file.endsWith('.dart')) .toList(); if (stagedDartFiles.isEmpty) { - // No Dart files are being committed; exit cleanly - return; + // No Dart files are being committed. + return true; } print('🔍 Running pre-commit checks on staged Dart files...'); - bool hasError = false; + var hasError = false; - // 2. Code Formatting Check + // Check formatting. print('Checking formatting...'); - final formatResult = await Process.run('dart', [ + final ProcessResult formatResult = await Process.run('dart', [ 'format', '--output=none', '--set-exit-if-changed', @@ -44,15 +55,17 @@ class PreCommitCommand extends Command { if (formatResult.exitCode != 0) { print('❌ Formatting issues found in the following files:'); print((formatResult.stdout as String).trim()); - print('👉 Please run "dart format" on these files to fix them.'); + print( + '👉 Please run "dart format" on these files to fix them.', + ); hasError = true; } else { print('✅ Formatting looks good.'); } - // 3. Static Analysis Check + // Run static analysis. print('Running static analysis...'); - final analyzeResult = await Process.run('dart', [ + final ProcessResult analyzeResult = await Process.run('dart', [ 'analyze', '--fatal-infos', ...stagedDartFiles, @@ -66,10 +79,6 @@ class PreCommitCommand extends Command { print('✅ Static analysis looks good.'); } - // 4. Final Verdict - if (hasError) { - print('🚨 Pre-commit checks failed. Please fix the above errors and try committing again.'); - exit(1); - } + return !hasError; } } diff --git a/script/githooks/pubspec.yaml b/script/githooks/pubspec.yaml index a191a975b763..be8db13c80c0 100644 --- a/script/githooks/pubspec.yaml +++ b/script/githooks/pubspec.yaml @@ -7,7 +7,7 @@ environment: dependencies: args: any + path: any dev_dependencies: - path: any test: any diff --git a/script/githooks/test/pre_commit_test.dart b/script/githooks/test/pre_commit_test.dart index f67165385095..64de7928c025 100644 --- a/script/githooks/test/pre_commit_test.dart +++ b/script/githooks/test/pre_commit_test.dart @@ -1,6 +1,7 @@ import 'dart:io'; -import 'package:test/test.dart'; + import 'package:path/path.dart' as p; +import 'package:test/test.dart'; void main() { group('pre-commit hook', () { @@ -14,7 +15,7 @@ void main() { await Process.run('git', ['config', 'user.email', 'test@example.com'], workingDirectory: tempDir.path); await Process.run('git', ['config', 'user.name', 'Test User'], workingDirectory: tempDir.path); - var repoRoot = Directory.current; + Directory repoRoot = Directory.current; while (repoRoot.path != '/' && !Directory(p.join(repoRoot.path, '.git')).existsSync()) { repoRoot = repoRoot.parent; } @@ -44,7 +45,7 @@ void main() { await Process.run('git', ['add', 'test_file.dart'], workingDirectory: tempDir.path); - final result = await runHook(); + final ProcessResult result = await runHook(); expect(result.exitCode, 0); final output = '${result.stdout}${result.stderr}'; expect(output, contains('Formatting looks good.')); @@ -61,10 +62,10 @@ void main(){ await Process.run('git', ['add', 'test_file.dart'], workingDirectory: tempDir.path); - final result = await runHook(); + final ProcessResult result = await runHook(); expect(result.exitCode, 1); final output = '${result.stdout}${result.stderr}'; - expect(output, contains('Formatting issues found')); + expect(output, contains('Formatting issues found in the following files:')); }); test('fails on analysis error', () async { @@ -77,7 +78,7 @@ void main() { await Process.run('git', ['add', 'test_file.dart'], workingDirectory: tempDir.path); - final result = await runHook(); + final ProcessResult result = await runHook(); expect(result.exitCode, 1); final output = '${result.stdout}${result.stderr}'; expect(output, contains('Static analysis errors found')); @@ -89,7 +90,7 @@ void main() { await Process.run('git', ['add', 'README.md'], workingDirectory: tempDir.path); - final result = await runHook(); + final ProcessResult result = await runHook(); expect(result.exitCode, 0); final output = '${result.stdout}${result.stderr}'; expect(output, isNot(contains('Checking formatting...'))); From 09a89f28ca7dec2102f22b4119cb3b580e86a84b Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Thu, 18 Jun 2026 11:40:49 -0700 Subject: [PATCH 03/37] use plugin tool instead --- .../githooks/lib/src/pre_commit_command.dart | 76 ++++++----- script/githooks/test/pre_commit_test.dart | 127 ++++++------------ 2 files changed, 83 insertions(+), 120 deletions(-) diff --git a/script/githooks/lib/src/pre_commit_command.dart b/script/githooks/lib/src/pre_commit_command.dart index 4f7e016a808d..b4658c7e7c1c 100644 --- a/script/githooks/lib/src/pre_commit_command.dart +++ b/script/githooks/lib/src/pre_commit_command.dart @@ -7,8 +7,26 @@ import 'dart:io'; import 'package:args/command_runner.dart'; -/// The command that implements the pre-commit githook +/// The command that implements the pre-commit githook. class PreCommitCommand extends Command { + /// Creates a [PreCommitCommand]. + PreCommitCommand({ + Future Function( + String executable, + List arguments, { + String? workingDirectory, + })? + processRunner, + }) : processRunner = processRunner ?? Process.run; + + /// The process runner injected for testing. + final Future Function( + String executable, + List arguments, { + String? workingDirectory, + }) + processRunner; + @override final String name = 'pre-commit'; @@ -17,46 +35,35 @@ class PreCommitCommand extends Command { @override Future run() async { - // Find changed Dart files. - final ProcessResult diffResult = await Process.run('git', [ - 'diff', - '--cached', - '--name-only', - '--diff-filter=ACM', - ]); - if (diffResult.exitCode != 0) { - print('Failed to get staged files.'); - exit(1); + // Find the repo root where the plugin tool is located. + Directory repoRoot = Directory.current; + while (repoRoot.path != '/' && !Directory('${repoRoot.path}/.git').existsSync()) { + repoRoot = repoRoot.parent; } - final List stagedDartFiles = (diffResult.stdout as String) - .split('\n') - .map((file) => file.trim()) - .where((file) => file.endsWith('.dart')) - .toList(); - - if (stagedDartFiles.isEmpty) { - // No Dart files are being committed. - return true; + if (repoRoot.path == '/') { + print('❌ Could not find .git directory.'); + return false; } - print('🔍 Running pre-commit checks on staged Dart files...'); + final toolScript = '${repoRoot.path}/script/tool/bin/flutter_plugin_tools.dart'; + + print('🔍 Running pre-commit checks on changed packages using flutter_plugin_tools...'); var hasError = false; // Check formatting. print('Checking formatting...'); - final ProcessResult formatResult = await Process.run('dart', [ + final ProcessResult formatResult = await processRunner('dart', [ + 'run', + toolScript, 'format', - '--output=none', - '--set-exit-if-changed', - ...stagedDartFiles, - ]); + '--run-on-changed-packages', + '--fail-on-change', + ], workingDirectory: repoRoot.path); if (formatResult.exitCode != 0) { - print('❌ Formatting issues found in the following files:'); - print((formatResult.stdout as String).trim()); print( - '👉 Please run "dart format" on these files to fix them.', + '❌ Formatting issues found. Please run "dart run script/tool/bin/flutter_plugin_tools.dart format --run-on-changed-packages" to fix them.', ); hasError = true; } else { @@ -65,15 +72,16 @@ class PreCommitCommand extends Command { // Run static analysis. print('Running static analysis...'); - final ProcessResult analyzeResult = await Process.run('dart', [ + final ProcessResult analyzeResult = await processRunner('dart', [ + 'run', + toolScript, 'analyze', + '--run-on-changed-packages', '--fatal-infos', - ...stagedDartFiles, - ]); + ], workingDirectory: repoRoot.path); if (analyzeResult.exitCode != 0) { - print('❌ Static analysis errors found:'); - print((analyzeResult.stdout as String).trim()); + print('❌ Static analysis errors found. Please fix the errors listed above.'); hasError = true; } else { print('✅ Static analysis looks good.'); diff --git a/script/githooks/test/pre_commit_test.dart b/script/githooks/test/pre_commit_test.dart index 64de7928c025..9178632f794d 100644 --- a/script/githooks/test/pre_commit_test.dart +++ b/script/githooks/test/pre_commit_test.dart @@ -1,99 +1,54 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + import 'dart:io'; -import 'package:path/path.dart' as p; +import 'package:githooks/src/pre_commit_command.dart'; import 'package:test/test.dart'; void main() { group('pre-commit hook', () { - late Directory tempDir; - late String githooksPath; - - setUp(() async { - tempDir = await Directory.systemTemp.createTemp('pre_commit_test_'); - - await Process.run('git', ['init'], workingDirectory: tempDir.path); - await Process.run('git', ['config', 'user.email', 'test@example.com'], workingDirectory: tempDir.path); - await Process.run('git', ['config', 'user.name', 'Test User'], workingDirectory: tempDir.path); - - Directory repoRoot = Directory.current; - while (repoRoot.path != '/' && !Directory(p.join(repoRoot.path, '.git')).existsSync()) { - repoRoot = repoRoot.parent; - } - - githooksPath = p.join(repoRoot.path, 'script', 'githooks'); - - // Set the hooksPath to the actual githooks directory - await Process.run('git', ['config', 'core.hooksPath', githooksPath], workingDirectory: tempDir.path); + test('passes when both format and analyze succeed', () async { + final command = PreCommitCommand( + processRunner: + (String executable, List arguments, {String? workingDirectory}) async { + return ProcessResult(0, 0, 'Success', ''); + }, + ); + + final bool result = await command.run(); + expect(result, isTrue); }); - tearDown(() async { - await tempDir.delete(recursive: true); + test('fails when formatting fails', () async { + final command = PreCommitCommand( + processRunner: + (String executable, List arguments, {String? workingDirectory}) async { + if (arguments.contains('format')) { + return ProcessResult(0, 1, 'bad_file.dart', ''); + } + return ProcessResult(0, 0, 'Success', ''); + }, + ); + + final bool result = await command.run(); + expect(result, isFalse); }); - Future runHook() async { - // Actually run git commit to trigger the hook! - return Process.run('git', ['commit', '-m', 'test'], workingDirectory: tempDir.path); - } - - test('passes on a clean commit', () async { - final file = File(p.join(tempDir.path, 'test_file.dart')); - await file.writeAsString(''' -void main() { - print('Hello, world!'); -} -'''); - - await Process.run('git', ['add', 'test_file.dart'], workingDirectory: tempDir.path); - - final ProcessResult result = await runHook(); - expect(result.exitCode, 0); - final output = '${result.stdout}${result.stderr}'; - expect(output, contains('Formatting looks good.')); - expect(output, contains('Static analysis looks good.')); - }); - - test('fails on formatting error', () async { - final file = File(p.join(tempDir.path, 'test_file.dart')); - await file.writeAsString(''' -void main(){ - print('Hello, world!'); -} -'''); - - await Process.run('git', ['add', 'test_file.dart'], workingDirectory: tempDir.path); - - final ProcessResult result = await runHook(); - expect(result.exitCode, 1); - final output = '${result.stdout}${result.stderr}'; - expect(output, contains('Formatting issues found in the following files:')); - }); - - test('fails on analysis error', () async { - final file = File(p.join(tempDir.path, 'test_file.dart')); - await file.writeAsString(''' -void main() { - print('Hello, world!') -} -'''); - - await Process.run('git', ['add', 'test_file.dart'], workingDirectory: tempDir.path); - - final ProcessResult result = await runHook(); - expect(result.exitCode, 1); - final output = '${result.stdout}${result.stderr}'; - expect(output, contains('Static analysis errors found')); - }); - - test('ignores non-dart files', () async { - final file = File(p.join(tempDir.path, 'README.md')); - await file.writeAsString('# Hello'); - - await Process.run('git', ['add', 'README.md'], workingDirectory: tempDir.path); - - final ProcessResult result = await runHook(); - expect(result.exitCode, 0); - final output = '${result.stdout}${result.stderr}'; - expect(output, isNot(contains('Checking formatting...'))); + test('fails when analysis fails', () async { + final command = PreCommitCommand( + processRunner: + (String executable, List arguments, {String? workingDirectory}) async { + if (arguments.contains('analyze')) { + return ProcessResult(0, 1, 'error in file.dart', ''); + } + return ProcessResult(0, 0, 'Success', ''); + }, + ); + + final bool result = await command.run(); + expect(result, isFalse); }); }); } From 04253cc9f757c61042e27d6d170569f64adb6460 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Thu, 18 Jun 2026 11:50:56 -0700 Subject: [PATCH 04/37] -- debugging stuff --- script/githooks/pre-commit | 1 - 1 file changed, 1 deletion(-) diff --git a/script/githooks/pre-commit b/script/githooks/pre-commit index 528775e30233..809f62c219df 100755 --- a/script/githooks/pre-commit +++ b/script/githooks/pre-commit @@ -1,6 +1,5 @@ #!/usr/bin/env bash set -e -echo "HOOK EXECUTED" >&2 HOOKS_DIR="$(dirname "$0")" exec dart "$HOOKS_DIR/bin/main.dart" pre-commit "$@" From e078f2442322532d120173736dfc25d25a098e55 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Thu, 18 Jun 2026 12:28:19 -0700 Subject: [PATCH 05/37] fake commit --- .../githooks/lib/src/pre_commit_command.dart | 76 +++++++++++++++++-- script/githooks/test/pre_commit_test.dart | 24 ++++++ 2 files changed, 93 insertions(+), 7 deletions(-) diff --git a/script/githooks/lib/src/pre_commit_command.dart b/script/githooks/lib/src/pre_commit_command.dart index b4658c7e7c1c..7e438d927e67 100644 --- a/script/githooks/lib/src/pre_commit_command.dart +++ b/script/githooks/lib/src/pre_commit_command.dart @@ -6,6 +6,7 @@ import 'dart:io'; import 'package:args/command_runner.dart'; +import 'package:path/path.dart' as p; /// The command that implements the pre-commit githook. class PreCommitCommand extends Command { @@ -33,11 +34,26 @@ class PreCommitCommand extends Command { @override final String description = 'Checks to run before a "git commit"'; + String? _findPackageName(String filePath, String repoRoot) { + Directory currentDir = File(p.join(repoRoot, filePath)).parent; + while (p.isWithin(repoRoot, currentDir.path) || p.equals(repoRoot, currentDir.path)) { + final String dirName = p.basename(currentDir.path); + if (dirName != 'example' && File(p.join(currentDir.path, 'pubspec.yaml')).existsSync()) { + return dirName; + } + if (p.equals(repoRoot, currentDir.path)) { + break; + } + currentDir = currentDir.parent; + } + return null; + } + @override Future run() async { // Find the repo root where the plugin tool is located. Directory repoRoot = Directory.current; - while (repoRoot.path != '/' && !Directory('${repoRoot.path}/.git').existsSync()) { + while (repoRoot.path != '/' && !Directory(p.join(repoRoot.path, '.git')).existsSync()) { repoRoot = repoRoot.parent; } @@ -46,9 +62,52 @@ class PreCommitCommand extends Command { return false; } - final toolScript = '${repoRoot.path}/script/tool/bin/flutter_plugin_tools.dart'; + final ProcessResult diffResult = await processRunner('git', [ + 'diff', + '--cached', + '--name-only', + '--diff-filter=ACM', + ], workingDirectory: repoRoot.path); + + if (diffResult.exitCode != 0) { + print('❌ Failed to get staged files'); + return false; + } + + final List stagedFiles = (diffResult.stdout as String) + .split('\n') + .map((e) => e.trim()) + .where((e) => e.isNotEmpty) + .toList(); + + if (stagedFiles.isEmpty) { + return true; // No files changed. + } + + final Set targetPackages = {}; + for (final file in stagedFiles) { + final String? packageName = _findPackageName(file, repoRoot.path); + if (packageName != null) { + targetPackages.add(packageName); + } + } + + if (targetPackages.isEmpty) { + return true; // None of the changed files are part of a package we care about. + } + + final String toolScript = p.join( + repoRoot.path, + 'script', + 'tool', + 'bin', + 'flutter_plugin_tools.dart', + ); + final packageArgs = '--packages=${targetPackages.join(',')}'; - print('🔍 Running pre-commit checks on changed packages using flutter_plugin_tools...'); + print( + '🔍 Running pre-commit checks on ${targetPackages.length} packages: ${targetPackages.join(', ')}', + ); var hasError = false; // Check formatting. @@ -57,13 +116,15 @@ class PreCommitCommand extends Command { 'run', toolScript, 'format', - '--run-on-changed-packages', + packageArgs, '--fail-on-change', ], workingDirectory: repoRoot.path); if (formatResult.exitCode != 0) { + if (formatResult.stdout.toString().isNotEmpty) print(formatResult.stdout); + if (formatResult.stderr.toString().isNotEmpty) print(formatResult.stderr); print( - '❌ Formatting issues found. Please run "dart run script/tool/bin/flutter_plugin_tools.dart format --run-on-changed-packages" to fix them.', + '❌ Formatting issues found. Please run "dart run script/tool/bin/flutter_plugin_tools.dart format $packageArgs" to fix them.', ); hasError = true; } else { @@ -76,11 +137,12 @@ class PreCommitCommand extends Command { 'run', toolScript, 'analyze', - '--run-on-changed-packages', - '--fatal-infos', + packageArgs, ], workingDirectory: repoRoot.path); if (analyzeResult.exitCode != 0) { + if (analyzeResult.stdout.toString().isNotEmpty) print(analyzeResult.stdout); + if (analyzeResult.stderr.toString().isNotEmpty) print(analyzeResult.stderr); print('❌ Static analysis errors found. Please fix the errors listed above.'); hasError = true; } else { diff --git a/script/githooks/test/pre_commit_test.dart b/script/githooks/test/pre_commit_test.dart index 9178632f794d..72dbd0c8d694 100644 --- a/script/githooks/test/pre_commit_test.dart +++ b/script/githooks/test/pre_commit_test.dart @@ -13,6 +13,9 @@ void main() { final command = PreCommitCommand( processRunner: (String executable, List arguments, {String? workingDirectory}) async { + if (executable == 'git') { + return ProcessResult(0, 0, 'script/githooks/lib/githooks.dart\n', ''); + } return ProcessResult(0, 0, 'Success', ''); }, ); @@ -25,6 +28,9 @@ void main() { final command = PreCommitCommand( processRunner: (String executable, List arguments, {String? workingDirectory}) async { + if (executable == 'git') { + return ProcessResult(0, 0, 'script/githooks/lib/githooks.dart\n', ''); + } if (arguments.contains('format')) { return ProcessResult(0, 1, 'bad_file.dart', ''); } @@ -40,6 +46,9 @@ void main() { final command = PreCommitCommand( processRunner: (String executable, List arguments, {String? workingDirectory}) async { + if (executable == 'git') { + return ProcessResult(0, 0, 'script/githooks/lib/githooks.dart\n', ''); + } if (arguments.contains('analyze')) { return ProcessResult(0, 1, 'error in file.dart', ''); } @@ -50,5 +59,20 @@ void main() { final bool result = await command.run(); expect(result, isFalse); }); + + test('ignores non-dart files', () async { + final command = PreCommitCommand( + processRunner: + (String executable, List arguments, {String? workingDirectory}) async { + if (executable == 'git') { + return ProcessResult(0, 0, 'README.md\n', ''); + } + return ProcessResult(0, 0, 'Success', ''); + }, + ); + + final bool result = await command.run(); + expect(result, isTrue); + }); }); } From d61f271cc70cc5af8094adcc4472fbc8dbb21045 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Thu, 18 Jun 2026 12:30:59 -0700 Subject: [PATCH 06/37] test + command fix --- .../lib/src/android_camera_camerax.dart | 2 +- script/githooks/lib/src/pre_commit_command.dart | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart b/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart index 0c84bdf2f2e5..ecd5a4be360f 100644 --- a/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart +++ b/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart @@ -76,7 +76,7 @@ class AndroidCameraCameraX extends CameraPlatform { }, ); - /// Handles retrieving media orientation for a device. + /// Handles retrieving media orientation for a device.f late final DeviceOrientationManager deviceOrientationManager = DeviceOrientationManager( onDeviceOrientationChanged: (_, String orientation) { final DeviceOrientation deviceOrientation = _deserializeDeviceOrientation(orientation); diff --git a/script/githooks/lib/src/pre_commit_command.dart b/script/githooks/lib/src/pre_commit_command.dart index 7e438d927e67..feb6e2cc31a4 100644 --- a/script/githooks/lib/src/pre_commit_command.dart +++ b/script/githooks/lib/src/pre_commit_command.dart @@ -121,8 +121,12 @@ class PreCommitCommand extends Command { ], workingDirectory: repoRoot.path); if (formatResult.exitCode != 0) { - if (formatResult.stdout.toString().isNotEmpty) print(formatResult.stdout); - if (formatResult.stderr.toString().isNotEmpty) print(formatResult.stderr); + if (formatResult.stdout.toString().isNotEmpty) { + print(formatResult.stdout); + } + if (formatResult.stderr.toString().isNotEmpty) { + print(formatResult.stderr); + } print( '❌ Formatting issues found. Please run "dart run script/tool/bin/flutter_plugin_tools.dart format $packageArgs" to fix them.', ); From a487e2209c8d6fd5c32d904dda834aac81d672aa Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Thu, 18 Jun 2026 12:31:29 -0700 Subject: [PATCH 07/37] fake commit --- .../camera_android_camerax/lib/src/android_camera_camerax.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart b/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart index ecd5a4be360f..0c84bdf2f2e5 100644 --- a/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart +++ b/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart @@ -76,7 +76,7 @@ class AndroidCameraCameraX extends CameraPlatform { }, ); - /// Handles retrieving media orientation for a device.f + /// Handles retrieving media orientation for a device. late final DeviceOrientationManager deviceOrientationManager = DeviceOrientationManager( onDeviceOrientationChanged: (_, String orientation) { final DeviceOrientation deviceOrientation = _deserializeDeviceOrientation(orientation); From 85219e8d5af444340752f3de08d2cc7a9a163b32 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Mon, 22 Jun 2026 10:06:51 -0700 Subject: [PATCH 08/37] try optimizing by cutting out native toolchains --- .../githooks/lib/src/pre_commit_command.dart | 32 +++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/script/githooks/lib/src/pre_commit_command.dart b/script/githooks/lib/src/pre_commit_command.dart index feb6e2cc31a4..e20cc1abb683 100644 --- a/script/githooks/lib/src/pre_commit_command.dart +++ b/script/githooks/lib/src/pre_commit_command.dart @@ -105,6 +105,29 @@ class PreCommitCommand extends Command { ); final packageArgs = '--packages=${targetPackages.join(',')}'; + // Determine which toolchains are needed based on file extensions + final bool hasDart = stagedFiles.any((f) => f.endsWith('.dart')); + final bool hasClang = stagedFiles.any( + (f) => + f.endsWith('.c') || + f.endsWith('.cc') || + f.endsWith('.cpp') || + f.endsWith('.h') || + f.endsWith('.m') || + f.endsWith('.mm'), + ); + final bool hasJava = stagedFiles.any((f) => f.endsWith('.java')); + final bool hasKotlin = stagedFiles.any((f) => f.endsWith('.kt')); + final bool hasSwift = stagedFiles.any((f) => f.endsWith('.swift')); + + final formatFlags = [ + if (!hasDart) '--no-dart', + if (!hasClang) '--no-clang-format', + if (!hasJava) '--no-java', + if (!hasKotlin) '--no-kotlin', + if (!hasSwift) '--no-swift', + ]; + print( '🔍 Running pre-commit checks on ${targetPackages.length} packages: ${targetPackages.join(', ')}', ); @@ -118,6 +141,7 @@ class PreCommitCommand extends Command { 'format', packageArgs, '--fail-on-change', + ...formatFlags, ], workingDirectory: repoRoot.path); if (formatResult.exitCode != 0) { @@ -145,8 +169,12 @@ class PreCommitCommand extends Command { ], workingDirectory: repoRoot.path); if (analyzeResult.exitCode != 0) { - if (analyzeResult.stdout.toString().isNotEmpty) print(analyzeResult.stdout); - if (analyzeResult.stderr.toString().isNotEmpty) print(analyzeResult.stderr); + if (analyzeResult.stdout.toString().isNotEmpty) { + print(analyzeResult.stdout); + } + if (analyzeResult.stderr.toString().isNotEmpty) { + print(analyzeResult.stderr); + } print('❌ Static analysis errors found. Please fix the errors listed above.'); hasError = true; } else { From 021dd712bf95887cc0d8b0948aeac79b2ce5961a Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Mon, 22 Jun 2026 10:07:44 -0700 Subject: [PATCH 09/37] fake commit --- .../camera_android_camerax/lib/src/android_camera_camerax.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart b/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart index 0c84bdf2f2e5..820e4a3e0963 100644 --- a/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart +++ b/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart @@ -24,7 +24,7 @@ class AndroidCameraCameraX extends CameraPlatform { CameraPlatform.instance = AndroidCameraCameraX(); } - /// The [ProcessCameraProvider] instance used to access camera functionality. + /// The [ProcessCameraProvider] instancee used to access camera functionality. @visibleForTesting ProcessCameraProvider? processCameraProvider; From 49089828ca6b46595c7a1055d5829c4cc544d437 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Mon, 22 Jun 2026 10:11:54 -0700 Subject: [PATCH 10/37] run dart analyze directly --- .../githooks/lib/src/pre_commit_command.dart | 50 +++++++++++-------- 1 file changed, 28 insertions(+), 22 deletions(-) diff --git a/script/githooks/lib/src/pre_commit_command.dart b/script/githooks/lib/src/pre_commit_command.dart index e20cc1abb683..098ae50c2968 100644 --- a/script/githooks/lib/src/pre_commit_command.dart +++ b/script/githooks/lib/src/pre_commit_command.dart @@ -34,12 +34,12 @@ class PreCommitCommand extends Command { @override final String description = 'Checks to run before a "git commit"'; - String? _findPackageName(String filePath, String repoRoot) { + String? _findPackagePath(String filePath, String repoRoot) { Directory currentDir = File(p.join(repoRoot, filePath)).parent; while (p.isWithin(repoRoot, currentDir.path) || p.equals(repoRoot, currentDir.path)) { final String dirName = p.basename(currentDir.path); if (dirName != 'example' && File(p.join(currentDir.path, 'pubspec.yaml')).existsSync()) { - return dirName; + return currentDir.path; } if (p.equals(repoRoot, currentDir.path)) { break; @@ -84,18 +84,20 @@ class PreCommitCommand extends Command { return true; // No files changed. } - final Set targetPackages = {}; + final Set targetPackageDirs = {}; for (final file in stagedFiles) { - final String? packageName = _findPackageName(file, repoRoot.path); - if (packageName != null) { - targetPackages.add(packageName); + final String? packageDir = _findPackagePath(file, repoRoot.path); + if (packageDir != null) { + targetPackageDirs.add(packageDir); } } - if (targetPackages.isEmpty) { + if (targetPackageDirs.isEmpty) { return true; // None of the changed files are part of a package we care about. } + final Set targetPackages = targetPackageDirs.map((dir) => p.basename(dir)).toSet(); + final String toolScript = p.join( repoRoot.path, 'script', @@ -161,22 +163,26 @@ class PreCommitCommand extends Command { // Run static analysis. print('Running static analysis...'); - final ProcessResult analyzeResult = await processRunner('dart', [ - 'run', - toolScript, - 'analyze', - packageArgs, - ], workingDirectory: repoRoot.path); - - if (analyzeResult.exitCode != 0) { - if (analyzeResult.stdout.toString().isNotEmpty) { - print(analyzeResult.stdout); - } - if (analyzeResult.stderr.toString().isNotEmpty) { - print(analyzeResult.stderr); + for (final packageDir in targetPackageDirs) { + final ProcessResult analyzeResult = await processRunner('dart', [ + 'analyze', + '--fatal-infos', + ], workingDirectory: packageDir); + + if (analyzeResult.exitCode != 0) { + if (analyzeResult.stdout.toString().isNotEmpty) { + print(analyzeResult.stdout); + } + if (analyzeResult.stderr.toString().isNotEmpty) { + print(analyzeResult.stderr); + } + print('❌ Static analysis errors found in ${p.basename(packageDir)}.'); + hasError = true; } - print('❌ Static analysis errors found. Please fix the errors listed above.'); - hasError = true; + } + + if (hasError) { + print('❌ Please fix the errors listed above.'); } else { print('✅ Static analysis looks good.'); } From d05c928d8111568356401f21bc6db0aa0e6a3726 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Mon, 22 Jun 2026 10:13:08 -0700 Subject: [PATCH 11/37] undo fake commit --- .../camera_android_camerax/lib/src/android_camera_camerax.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart b/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart index 820e4a3e0963..0c84bdf2f2e5 100644 --- a/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart +++ b/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart @@ -24,7 +24,7 @@ class AndroidCameraCameraX extends CameraPlatform { CameraPlatform.instance = AndroidCameraCameraX(); } - /// The [ProcessCameraProvider] instancee used to access camera functionality. + /// The [ProcessCameraProvider] instance used to access camera functionality. @visibleForTesting ProcessCameraProvider? processCameraProvider; From 2d89dd81d7120bfa3efb9008f1df61a876e20eab Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Mon, 22 Jun 2026 10:18:03 -0700 Subject: [PATCH 12/37] run format and analyze directly on staged files --- .../githooks/lib/src/pre_commit_command.dart | 89 ++++++++++++------- 1 file changed, 59 insertions(+), 30 deletions(-) diff --git a/script/githooks/lib/src/pre_commit_command.dart b/script/githooks/lib/src/pre_commit_command.dart index 098ae50c2968..073a4f28d616 100644 --- a/script/githooks/lib/src/pre_commit_command.dart +++ b/script/githooks/lib/src/pre_commit_command.dart @@ -108,7 +108,6 @@ class PreCommitCommand extends Command { final packageArgs = '--packages=${targetPackages.join(',')}'; // Determine which toolchains are needed based on file extensions - final bool hasDart = stagedFiles.any((f) => f.endsWith('.dart')); final bool hasClang = stagedFiles.any( (f) => f.endsWith('.c') || @@ -122,13 +121,7 @@ class PreCommitCommand extends Command { final bool hasKotlin = stagedFiles.any((f) => f.endsWith('.kt')); final bool hasSwift = stagedFiles.any((f) => f.endsWith('.swift')); - final formatFlags = [ - if (!hasDart) '--no-dart', - if (!hasClang) '--no-clang-format', - if (!hasJava) '--no-java', - if (!hasKotlin) '--no-kotlin', - if (!hasSwift) '--no-swift', - ]; + final List dartFiles = stagedFiles.where((f) => f.endsWith('.dart')).toList(); print( '🔍 Running pre-commit checks on ${targetPackages.length} packages: ${targetPackages.join(', ')}', @@ -137,37 +130,73 @@ class PreCommitCommand extends Command { // Check formatting. print('Checking formatting...'); - final ProcessResult formatResult = await processRunner('dart', [ - 'run', - toolScript, - 'format', - packageArgs, - '--fail-on-change', - ...formatFlags, - ], workingDirectory: repoRoot.path); - if (formatResult.exitCode != 0) { - if (formatResult.stdout.toString().isNotEmpty) { - print(formatResult.stdout); + // Format Dart files instantly using dart format directly + if (dartFiles.isNotEmpty) { + final ProcessResult dartFormatResult = await processRunner('dart', [ + 'format', + '--set-exit-if-changed', + ...dartFiles, + ], workingDirectory: repoRoot.path); + + if (dartFormatResult.exitCode != 0) { + if (dartFormatResult.stdout.toString().isNotEmpty) { + print(dartFormatResult.stdout); + } + if (dartFormatResult.stderr.toString().isNotEmpty) { + print(dartFormatResult.stderr); + } + print('❌ Formatting issues found in Dart files. Please run "dart format" to fix them.'); + hasError = true; } - if (formatResult.stderr.toString().isNotEmpty) { - print(formatResult.stderr); + } + + // Format native files using flutter_plugin_tools + final bool needsNativeFormat = hasClang || hasJava || hasKotlin || hasSwift; + if (needsNativeFormat) { + final nativeFormatFlags = [ + '--no-dart', + if (!hasClang) '--no-clang-format', + if (!hasJava) '--no-java', + if (!hasKotlin) '--no-kotlin', + if (!hasSwift) '--no-swift', + ]; + + final ProcessResult nativeFormatResult = await processRunner('dart', [ + 'run', + toolScript, + 'format', + packageArgs, + '--fail-on-change', + ...nativeFormatFlags, + ], workingDirectory: repoRoot.path); + + if (nativeFormatResult.exitCode != 0) { + if (nativeFormatResult.stdout.toString().isNotEmpty) { + print(nativeFormatResult.stdout); + } + if (nativeFormatResult.stderr.toString().isNotEmpty) { + print(nativeFormatResult.stderr); + } + print( + '❌ Formatting issues found in native files. Please run "dart run script/tool/bin/flutter_plugin_tools.dart format $packageArgs" to fix them.', + ); + hasError = true; } - print( - '❌ Formatting issues found. Please run "dart run script/tool/bin/flutter_plugin_tools.dart format $packageArgs" to fix them.', - ); - hasError = true; - } else { + } + + if (!hasError) { print('✅ Formatting looks good.'); } - // Run static analysis. + // Run static analysis directly on staged files print('Running static analysis...'); - for (final packageDir in targetPackageDirs) { + if (dartFiles.isNotEmpty) { final ProcessResult analyzeResult = await processRunner('dart', [ 'analyze', '--fatal-infos', - ], workingDirectory: packageDir); + ...dartFiles, + ], workingDirectory: repoRoot.path); if (analyzeResult.exitCode != 0) { if (analyzeResult.stdout.toString().isNotEmpty) { @@ -176,7 +205,7 @@ class PreCommitCommand extends Command { if (analyzeResult.stderr.toString().isNotEmpty) { print(analyzeResult.stderr); } - print('❌ Static analysis errors found in ${p.basename(packageDir)}.'); + print('❌ Static analysis errors found.'); hasError = true; } } From 7e53502099dc28694f41f7bd1d0e19f75056ce7f Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Mon, 22 Jun 2026 10:19:13 -0700 Subject: [PATCH 13/37] fake commit --- .../camera_android_camerax/lib/src/android_camera_camerax.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart b/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart index 0c84bdf2f2e5..34b8c8ea3168 100644 --- a/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart +++ b/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart @@ -19,7 +19,7 @@ class AndroidCameraCameraX extends CameraPlatform { /// Constructs an [AndroidCameraCameraX]. AndroidCameraCameraX(); - /// Registers this class as the default instance of [CameraPlatform]. + /// Registers this class as the default instancee of [CameraPlatform]. static void registerWith() { CameraPlatform.instance = AndroidCameraCameraX(); } From 5b8c9a887ac5cc930b09b1a2824eec706efdf6f8 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Mon, 22 Jun 2026 10:22:29 -0700 Subject: [PATCH 14/37] use ANSI escape codes to erase status logs --- .../githooks/lib/src/pre_commit_command.dart | 23 +++++++++++++++---- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/script/githooks/lib/src/pre_commit_command.dart b/script/githooks/lib/src/pre_commit_command.dart index 073a4f28d616..30495778e627 100644 --- a/script/githooks/lib/src/pre_commit_command.dart +++ b/script/githooks/lib/src/pre_commit_command.dart @@ -129,7 +129,7 @@ class PreCommitCommand extends Command { var hasError = false; // Check formatting. - print('Checking formatting...'); + stdout.write('Checking formatting...'); // Format Dart files instantly using dart format directly if (dartFiles.isNotEmpty) { @@ -140,6 +140,9 @@ class PreCommitCommand extends Command { ], workingDirectory: repoRoot.path); if (dartFormatResult.exitCode != 0) { + if (!hasError) { + stdout.write('\x1B[2K\r'); + } if (dartFormatResult.stdout.toString().isNotEmpty) { print(dartFormatResult.stdout); } @@ -172,6 +175,9 @@ class PreCommitCommand extends Command { ], workingDirectory: repoRoot.path); if (nativeFormatResult.exitCode != 0) { + if (!hasError) { + stdout.write('\x1B[2K\r'); + } if (nativeFormatResult.stdout.toString().isNotEmpty) { print(nativeFormatResult.stdout); } @@ -186,11 +192,12 @@ class PreCommitCommand extends Command { } if (!hasError) { - print('✅ Formatting looks good.'); + stdout.write('\x1B[2K\r✅ Formatting looks good.\n'); } // Run static analysis directly on staged files - print('Running static analysis...'); + var analyzeHasError = false; + stdout.write('Running static analysis...'); if (dartFiles.isNotEmpty) { final ProcessResult analyzeResult = await processRunner('dart', [ 'analyze', @@ -199,6 +206,9 @@ class PreCommitCommand extends Command { ], workingDirectory: repoRoot.path); if (analyzeResult.exitCode != 0) { + if (!analyzeHasError) { + stdout.write('\x1B[2K\r'); + } if (analyzeResult.stdout.toString().isNotEmpty) { print(analyzeResult.stdout); } @@ -206,14 +216,17 @@ class PreCommitCommand extends Command { print(analyzeResult.stderr); } print('❌ Static analysis errors found.'); + analyzeHasError = true; hasError = true; } } + if (!analyzeHasError) { + stdout.write('\x1B[2K\r✅ Static analysis looks good.\n'); + } + if (hasError) { print('❌ Please fix the errors listed above.'); - } else { - print('✅ Static analysis looks good.'); } return !hasError; From f4eed3d59fc87103915a3c4e754f7133ba4064c0 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Mon, 22 Jun 2026 10:23:34 -0700 Subject: [PATCH 15/37] fake commit --- .../io/flutter/plugins/camerax/Camera2CameraInfoProxyApi.java | 2 +- .../camera_android_camerax/lib/src/android_camera_camerax.dart | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/camera/camera_android_camerax/android/src/main/java/io/flutter/plugins/camerax/Camera2CameraInfoProxyApi.java b/packages/camera/camera_android_camerax/android/src/main/java/io/flutter/plugins/camerax/Camera2CameraInfoProxyApi.java index 8f64ad0a50d1..5c426e8f7785 100644 --- a/packages/camera/camera_android_camerax/android/src/main/java/io/flutter/plugins/camerax/Camera2CameraInfoProxyApi.java +++ b/packages/camera/camera_android_camerax/android/src/main/java/io/flutter/plugins/camerax/Camera2CameraInfoProxyApi.java @@ -16,7 +16,7 @@ /** * ProxyApi implementation for {@link Camera2CameraInfo}. This class may handle instantiating native * object instances that are attached to a Dart instance or handle method calls on the associated - * native class or an instance of that class. + * native class or an instance of that class.f */ @OptIn(markerClass = ExperimentalCamera2Interop.class) class Camera2CameraInfoProxyApi extends PigeonApiCamera2CameraInfo { diff --git a/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart b/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart index 34b8c8ea3168..0c84bdf2f2e5 100644 --- a/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart +++ b/packages/camera/camera_android_camerax/lib/src/android_camera_camerax.dart @@ -19,7 +19,7 @@ class AndroidCameraCameraX extends CameraPlatform { /// Constructs an [AndroidCameraCameraX]. AndroidCameraCameraX(); - /// Registers this class as the default instancee of [CameraPlatform]. + /// Registers this class as the default instance of [CameraPlatform]. static void registerWith() { CameraPlatform.instance = AndroidCameraCameraX(); } From 638ce3434e8ebb32dd99d6b07b4ef3915dc28814 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Mon, 22 Jun 2026 10:24:31 -0700 Subject: [PATCH 16/37] change emoji to runner --- script/githooks/lib/src/pre_commit_command.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/githooks/lib/src/pre_commit_command.dart b/script/githooks/lib/src/pre_commit_command.dart index 30495778e627..56c9ab48e467 100644 --- a/script/githooks/lib/src/pre_commit_command.dart +++ b/script/githooks/lib/src/pre_commit_command.dart @@ -124,7 +124,7 @@ class PreCommitCommand extends Command { final List dartFiles = stagedFiles.where((f) => f.endsWith('.dart')).toList(); print( - '🔍 Running pre-commit checks on ${targetPackages.length} packages: ${targetPackages.join(', ')}', + '🏃 Running pre-commit checks on ${targetPackages.length} packages: ${targetPackages.join(', ')}', ); var hasError = false; From c19c8f3bbce4a5fe5fb622b9b1d9a5314fea6d09 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Mon, 22 Jun 2026 10:25:25 -0700 Subject: [PATCH 17/37] undo fake commit --- .../io/flutter/plugins/camerax/Camera2CameraInfoProxyApi.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/camera/camera_android_camerax/android/src/main/java/io/flutter/plugins/camerax/Camera2CameraInfoProxyApi.java b/packages/camera/camera_android_camerax/android/src/main/java/io/flutter/plugins/camerax/Camera2CameraInfoProxyApi.java index 5c426e8f7785..8f64ad0a50d1 100644 --- a/packages/camera/camera_android_camerax/android/src/main/java/io/flutter/plugins/camerax/Camera2CameraInfoProxyApi.java +++ b/packages/camera/camera_android_camerax/android/src/main/java/io/flutter/plugins/camerax/Camera2CameraInfoProxyApi.java @@ -16,7 +16,7 @@ /** * ProxyApi implementation for {@link Camera2CameraInfo}. This class may handle instantiating native * object instances that are attached to a Dart instance or handle method calls on the associated - * native class or an instance of that class.f + * native class or an instance of that class. */ @OptIn(markerClass = ExperimentalCamera2Interop.class) class Camera2CameraInfoProxyApi extends PigeonApiCamera2CameraInfo { From f8837ba6ac767691bc2c54c85944c84552a4550e Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Mon, 22 Jun 2026 10:36:27 -0700 Subject: [PATCH 18/37] make unit tests stricter --- script/githooks/test/pre_commit_test.dart | 32 +++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/script/githooks/test/pre_commit_test.dart b/script/githooks/test/pre_commit_test.dart index 72dbd0c8d694..a29f7f745eee 100644 --- a/script/githooks/test/pre_commit_test.dart +++ b/script/githooks/test/pre_commit_test.dart @@ -10,10 +10,15 @@ import 'package:test/test.dart'; void main() { group('pre-commit hook', () { test('passes when both format and analyze succeed', () async { + final List> executedArguments = []; final command = PreCommitCommand( processRunner: (String executable, List arguments, {String? workingDirectory}) async { + executedArguments.add(arguments); if (executable == 'git') { + if (arguments.contains('--show-toplevel')) { + return ProcessResult(0, 0, '/fake/repo/root\n', ''); + } return ProcessResult(0, 0, 'script/githooks/lib/githooks.dart\n', ''); } return ProcessResult(0, 0, 'Success', ''); @@ -22,6 +27,18 @@ void main() { final bool result = await command.run(); expect(result, isTrue); + + // Verify the exact arguments passed to format and analyze + expect( + executedArguments, + anyElement( + equals(['format', '--set-exit-if-changed', 'script/githooks/lib/githooks.dart']), + ), + ); + expect( + executedArguments, + anyElement(equals(['analyze', '--fatal-infos', 'script/githooks/lib/githooks.dart'])), + ); }); test('fails when formatting fails', () async { @@ -29,6 +46,9 @@ void main() { processRunner: (String executable, List arguments, {String? workingDirectory}) async { if (executable == 'git') { + if (arguments.contains('--show-toplevel')) { + return ProcessResult(0, 0, '/fake/repo/root\n', ''); + } return ProcessResult(0, 0, 'script/githooks/lib/githooks.dart\n', ''); } if (arguments.contains('format')) { @@ -47,6 +67,9 @@ void main() { processRunner: (String executable, List arguments, {String? workingDirectory}) async { if (executable == 'git') { + if (arguments.contains('--show-toplevel')) { + return ProcessResult(0, 0, '/fake/repo/root\n', ''); + } return ProcessResult(0, 0, 'script/githooks/lib/githooks.dart\n', ''); } if (arguments.contains('analyze')) { @@ -61,10 +84,15 @@ void main() { }); test('ignores non-dart files', () async { + final List> executedArguments = []; final command = PreCommitCommand( processRunner: (String executable, List arguments, {String? workingDirectory}) async { + executedArguments.add(arguments); if (executable == 'git') { + if (arguments.contains('--show-toplevel')) { + return ProcessResult(0, 0, '/fake/repo/root\n', ''); + } return ProcessResult(0, 0, 'README.md\n', ''); } return ProcessResult(0, 0, 'Success', ''); @@ -73,6 +101,10 @@ void main() { final bool result = await command.run(); expect(result, isTrue); + + // Verify that dart format and analyze were NEVER called. + expect(executedArguments.any((args) => args.contains('format')), isFalse); + expect(executedArguments.any((args) => args.contains('analyze')), isFalse); }); }); } From 352580c13e562dab7b186ff5be1b21686ffd4579 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Mon, 22 Jun 2026 10:48:25 -0700 Subject: [PATCH 19/37] self review --- script/githooks/lib/src/pre_commit_command.dart | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/script/githooks/lib/src/pre_commit_command.dart b/script/githooks/lib/src/pre_commit_command.dart index 56c9ab48e467..8fb5a70f30ca 100644 --- a/script/githooks/lib/src/pre_commit_command.dart +++ b/script/githooks/lib/src/pre_commit_command.dart @@ -62,6 +62,7 @@ class PreCommitCommand extends Command { return false; } + // Get all staged files that are added, copied, or modified. final ProcessResult diffResult = await processRunner('git', [ 'diff', '--cached', @@ -81,7 +82,8 @@ class PreCommitCommand extends Command { .toList(); if (stagedFiles.isEmpty) { - return true; // No files changed. + // No files changed. + return true; } final Set targetPackageDirs = {}; @@ -93,7 +95,8 @@ class PreCommitCommand extends Command { } if (targetPackageDirs.isEmpty) { - return true; // None of the changed files are part of a package we care about. + // None of the changed files are part of a package we care about. + return true; } final Set targetPackages = targetPackageDirs.map((dir) => p.basename(dir)).toSet(); @@ -107,7 +110,7 @@ class PreCommitCommand extends Command { ); final packageArgs = '--packages=${targetPackages.join(',')}'; - // Determine which toolchains are needed based on file extensions + // Determine which toolchains are needed based on file extensions to avoid uneccessary slowdown. final bool hasClang = stagedFiles.any( (f) => f.endsWith('.c') || @@ -131,7 +134,7 @@ class PreCommitCommand extends Command { // Check formatting. stdout.write('Checking formatting...'); - // Format Dart files instantly using dart format directly + // Format staged Dart files. if (dartFiles.isNotEmpty) { final ProcessResult dartFormatResult = await processRunner('dart', [ 'format', @@ -154,7 +157,7 @@ class PreCommitCommand extends Command { } } - // Format native files using flutter_plugin_tools + // Format staged native files. final bool needsNativeFormat = hasClang || hasJava || hasKotlin || hasSwift; if (needsNativeFormat) { final nativeFormatFlags = [ @@ -195,7 +198,7 @@ class PreCommitCommand extends Command { stdout.write('\x1B[2K\r✅ Formatting looks good.\n'); } - // Run static analysis directly on staged files + // Run static analysis on staged files. var analyzeHasError = false; stdout.write('Running static analysis...'); if (dartFiles.isNotEmpty) { From a6a2a789a79a81d3535dc4599cefc8b7b12fdfd1 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Mon, 22 Jun 2026 10:54:52 -0700 Subject: [PATCH 20/37] expect args --- script/githooks/test/pre_commit_test.dart | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/script/githooks/test/pre_commit_test.dart b/script/githooks/test/pre_commit_test.dart index a29f7f745eee..655857eb6032 100644 --- a/script/githooks/test/pre_commit_test.dart +++ b/script/githooks/test/pre_commit_test.dart @@ -42,16 +42,18 @@ void main() { }); test('fails when formatting fails', () async { + final List> executedArguments = []; final command = PreCommitCommand( processRunner: (String executable, List arguments, {String? workingDirectory}) async { + executedArguments.add(arguments); if (executable == 'git') { if (arguments.contains('--show-toplevel')) { return ProcessResult(0, 0, '/fake/repo/root\n', ''); } return ProcessResult(0, 0, 'script/githooks/lib/githooks.dart\n', ''); } - if (arguments.contains('format')) { + if (arguments.isNotEmpty && arguments[0] == 'format') { return ProcessResult(0, 1, 'bad_file.dart', ''); } return ProcessResult(0, 0, 'Success', ''); @@ -60,19 +62,28 @@ void main() { final bool result = await command.run(); expect(result, isFalse); + + expect( + executedArguments, + anyElement( + equals(['format', '--set-exit-if-changed', 'script/githooks/lib/githooks.dart']), + ), + ); }); test('fails when analysis fails', () async { + final List> executedArguments = []; final command = PreCommitCommand( processRunner: (String executable, List arguments, {String? workingDirectory}) async { + executedArguments.add(arguments); if (executable == 'git') { if (arguments.contains('--show-toplevel')) { return ProcessResult(0, 0, '/fake/repo/root\n', ''); } return ProcessResult(0, 0, 'script/githooks/lib/githooks.dart\n', ''); } - if (arguments.contains('analyze')) { + if (arguments.isNotEmpty && arguments[0] == 'analyze') { return ProcessResult(0, 1, 'error in file.dart', ''); } return ProcessResult(0, 0, 'Success', ''); @@ -81,6 +92,11 @@ void main() { final bool result = await command.run(); expect(result, isFalse); + + expect( + executedArguments, + anyElement(equals(['analyze', '--fatal-infos', 'script/githooks/lib/githooks.dart'])), + ); }); test('ignores non-dart files', () async { From 5f94b320028ed9ed60f0322ef71875eaaf2116d7 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Mon, 22 Jun 2026 12:37:27 -0700 Subject: [PATCH 21/37] gemini review --- script/githooks/bin/install_hooks.dart | 6 +++--- script/githooks/lib/src/pre_commit_command.dart | 14 +++++++++----- script/githooks/test/pre_commit_test.dart | 12 ------------ 3 files changed, 12 insertions(+), 20 deletions(-) diff --git a/script/githooks/bin/install_hooks.dart b/script/githooks/bin/install_hooks.dart index 929e03a81263..c21f20abdfc1 100644 --- a/script/githooks/bin/install_hooks.dart +++ b/script/githooks/bin/install_hooks.dart @@ -9,11 +9,11 @@ import 'package:path/path.dart' as p; void main() async { Directory repoRoot = Directory.current; - while (repoRoot.path != '/' && !Directory(p.join(repoRoot.path, '.git')).existsSync()) { + while (repoRoot.path != repoRoot.parent.path && + !Directory(p.join(repoRoot.path, '.git')).existsSync()) { repoRoot = repoRoot.parent; } - - if (repoRoot.path == '/') { + if (!Directory(p.join(repoRoot.path, '.git')).existsSync()) { print('❌ Could not find .git directory.'); exit(1); } diff --git a/script/githooks/lib/src/pre_commit_command.dart b/script/githooks/lib/src/pre_commit_command.dart index 8fb5a70f30ca..9db09a0f5032 100644 --- a/script/githooks/lib/src/pre_commit_command.dart +++ b/script/githooks/lib/src/pre_commit_command.dart @@ -53,11 +53,11 @@ class PreCommitCommand extends Command { Future run() async { // Find the repo root where the plugin tool is located. Directory repoRoot = Directory.current; - while (repoRoot.path != '/' && !Directory(p.join(repoRoot.path, '.git')).existsSync()) { + while (repoRoot.path != repoRoot.parent.path && + !Directory(p.join(repoRoot.path, '.git')).existsSync()) { repoRoot = repoRoot.parent; } - - if (repoRoot.path == '/') { + if (!Directory(p.join(repoRoot.path, '.git')).existsSync()) { print('❌ Could not find .git directory.'); return false; } @@ -144,7 +144,7 @@ class PreCommitCommand extends Command { if (dartFormatResult.exitCode != 0) { if (!hasError) { - stdout.write('\x1B[2K\r'); + stdout.write(stdout.supportsAnsiEscapes ? r'\x1B[2K\r' : r'\n'); } if (dartFormatResult.stdout.toString().isNotEmpty) { print(dartFormatResult.stdout); @@ -195,7 +195,11 @@ class PreCommitCommand extends Command { } if (!hasError) { - stdout.write('\x1B[2K\r✅ Formatting looks good.\n'); + stdout.write( + stdout.supportsAnsiEscapes + ? r'\x1B[2K\r✅ Formatting looks good.\n' + : r'✅ Formatting looks good.\n', + ); } // Run static analysis on staged files. diff --git a/script/githooks/test/pre_commit_test.dart b/script/githooks/test/pre_commit_test.dart index 655857eb6032..369b4ba378c7 100644 --- a/script/githooks/test/pre_commit_test.dart +++ b/script/githooks/test/pre_commit_test.dart @@ -16,9 +16,6 @@ void main() { (String executable, List arguments, {String? workingDirectory}) async { executedArguments.add(arguments); if (executable == 'git') { - if (arguments.contains('--show-toplevel')) { - return ProcessResult(0, 0, '/fake/repo/root\n', ''); - } return ProcessResult(0, 0, 'script/githooks/lib/githooks.dart\n', ''); } return ProcessResult(0, 0, 'Success', ''); @@ -48,9 +45,6 @@ void main() { (String executable, List arguments, {String? workingDirectory}) async { executedArguments.add(arguments); if (executable == 'git') { - if (arguments.contains('--show-toplevel')) { - return ProcessResult(0, 0, '/fake/repo/root\n', ''); - } return ProcessResult(0, 0, 'script/githooks/lib/githooks.dart\n', ''); } if (arguments.isNotEmpty && arguments[0] == 'format') { @@ -78,9 +72,6 @@ void main() { (String executable, List arguments, {String? workingDirectory}) async { executedArguments.add(arguments); if (executable == 'git') { - if (arguments.contains('--show-toplevel')) { - return ProcessResult(0, 0, '/fake/repo/root\n', ''); - } return ProcessResult(0, 0, 'script/githooks/lib/githooks.dart\n', ''); } if (arguments.isNotEmpty && arguments[0] == 'analyze') { @@ -106,9 +97,6 @@ void main() { (String executable, List arguments, {String? workingDirectory}) async { executedArguments.add(arguments); if (executable == 'git') { - if (arguments.contains('--show-toplevel')) { - return ProcessResult(0, 0, '/fake/repo/root\n', ''); - } return ProcessResult(0, 0, 'README.md\n', ''); } return ProcessResult(0, 0, 'Success', ''); From bc0ce9f401e3393739615493111241b55c7a758b Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Mon, 22 Jun 2026 12:40:25 -0700 Subject: [PATCH 22/37] fix print --- script/githooks/lib/src/pre_commit_command.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/script/githooks/lib/src/pre_commit_command.dart b/script/githooks/lib/src/pre_commit_command.dart index 9db09a0f5032..81ea620db31d 100644 --- a/script/githooks/lib/src/pre_commit_command.dart +++ b/script/githooks/lib/src/pre_commit_command.dart @@ -144,7 +144,7 @@ class PreCommitCommand extends Command { if (dartFormatResult.exitCode != 0) { if (!hasError) { - stdout.write(stdout.supportsAnsiEscapes ? r'\x1B[2K\r' : r'\n'); + stdout.write(stdout.supportsAnsiEscapes ? '\x1B[2K\r' : '\n'); } if (dartFormatResult.stdout.toString().isNotEmpty) { print(dartFormatResult.stdout); @@ -197,8 +197,8 @@ class PreCommitCommand extends Command { if (!hasError) { stdout.write( stdout.supportsAnsiEscapes - ? r'\x1B[2K\r✅ Formatting looks good.\n' - : r'✅ Formatting looks good.\n', + ? '\x1B[2K\r✅ Formatting looks good.\n' + : '✅ Formatting looks good.\n', ); } From 38e5327cfee265b87d12f76c6c63c0cbcb56de52 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Mon, 22 Jun 2026 15:44:43 -0700 Subject: [PATCH 23/37] address gemini review --- script/githooks/bin/install_hooks.dart | 6 +++-- .../githooks/lib/src/pre_commit_command.dart | 17 +++++++++---- script/githooks/test/pre_commit_test.dart | 24 +++++++++++++++++++ 3 files changed, 40 insertions(+), 7 deletions(-) diff --git a/script/githooks/bin/install_hooks.dart b/script/githooks/bin/install_hooks.dart index c21f20abdfc1..e770cd338c1d 100644 --- a/script/githooks/bin/install_hooks.dart +++ b/script/githooks/bin/install_hooks.dart @@ -10,10 +10,12 @@ import 'package:path/path.dart' as p; void main() async { Directory repoRoot = Directory.current; while (repoRoot.path != repoRoot.parent.path && - !Directory(p.join(repoRoot.path, '.git')).existsSync()) { + !(Directory(p.join(repoRoot.path, '.git')).existsSync() || + File(p.join(repoRoot.path, '.git')).existsSync())) { repoRoot = repoRoot.parent; } - if (!Directory(p.join(repoRoot.path, '.git')).existsSync()) { + if (!(Directory(p.join(repoRoot.path, '.git')).existsSync() || + File(p.join(repoRoot.path, '.git')).existsSync())) { print('❌ Could not find .git directory.'); exit(1); } diff --git a/script/githooks/lib/src/pre_commit_command.dart b/script/githooks/lib/src/pre_commit_command.dart index 81ea620db31d..30afa285f476 100644 --- a/script/githooks/lib/src/pre_commit_command.dart +++ b/script/githooks/lib/src/pre_commit_command.dart @@ -54,10 +54,12 @@ class PreCommitCommand extends Command { // Find the repo root where the plugin tool is located. Directory repoRoot = Directory.current; while (repoRoot.path != repoRoot.parent.path && - !Directory(p.join(repoRoot.path, '.git')).existsSync()) { + !(Directory(p.join(repoRoot.path, '.git')).existsSync() || + File(p.join(repoRoot.path, '.git')).existsSync())) { repoRoot = repoRoot.parent; } - if (!Directory(p.join(repoRoot.path, '.git')).existsSync()) { + if (!(Directory(p.join(repoRoot.path, '.git')).existsSync() || + File(p.join(repoRoot.path, '.git')).existsSync())) { print('❌ Could not find .git directory.'); return false; } @@ -117,6 +119,7 @@ class PreCommitCommand extends Command { f.endsWith('.cc') || f.endsWith('.cpp') || f.endsWith('.h') || + f.endsWith('.hpp') || f.endsWith('.m') || f.endsWith('.mm'), ); @@ -179,7 +182,7 @@ class PreCommitCommand extends Command { if (nativeFormatResult.exitCode != 0) { if (!hasError) { - stdout.write('\x1B[2K\r'); + stdout.write(stdout.supportsAnsiEscapes ? '\x1B[2K\r' : '\n'); } if (nativeFormatResult.stdout.toString().isNotEmpty) { print(nativeFormatResult.stdout); @@ -214,7 +217,7 @@ class PreCommitCommand extends Command { if (analyzeResult.exitCode != 0) { if (!analyzeHasError) { - stdout.write('\x1B[2K\r'); + stdout.write(stdout.supportsAnsiEscapes ? '\x1B[2K\r' : '\n'); } if (analyzeResult.stdout.toString().isNotEmpty) { print(analyzeResult.stdout); @@ -229,7 +232,11 @@ class PreCommitCommand extends Command { } if (!analyzeHasError) { - stdout.write('\x1B[2K\r✅ Static analysis looks good.\n'); + stdout.write( + stdout.supportsAnsiEscapes + ? '\x1B[2K\r✅ Static analysis looks good.\n' + : '✅ Static analysis looks good.\n', + ); } if (hasError) { diff --git a/script/githooks/test/pre_commit_test.dart b/script/githooks/test/pre_commit_test.dart index 369b4ba378c7..b0da053a1688 100644 --- a/script/githooks/test/pre_commit_test.dart +++ b/script/githooks/test/pre_commit_test.dart @@ -110,5 +110,29 @@ void main() { expect(executedArguments.any((args) => args.contains('format')), isFalse); expect(executedArguments.any((args) => args.contains('analyze')), isFalse); }); + test('runs native formatter when native files are staged', () async { + final List> executedArguments = []; + final command = PreCommitCommand( + processRunner: + (String executable, List arguments, {String? workingDirectory}) async { + executedArguments.add(arguments); + if (executable == 'git') { + return ProcessResult(0, 0, 'packages/pkg/ios/Classes/Foo.m\n', ''); + } + return ProcessResult(0, 0, 'Success', ''); + }, + ); + + final bool result = await command.run(); + expect(result, isTrue); + + expect( + executedArguments.any( + (args) => + args.contains('format') && args.contains('--no-dart') && args.contains('--no-java'), + ), + isTrue, + ); + }); }); } From d725d6868a29392707daeb21b7bc45c6cc1be61a Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Tue, 23 Jun 2026 16:57:24 -0700 Subject: [PATCH 24/37] first agentic pass at refactoring --- .../githooks/lib/src/pre_commit_command.dart | 130 +++++------------- script/githooks/test/pre_commit_test.dart | 43 ++++-- .../tool/lib/src/common/package_command.dart | 13 ++ .../src/common/package_looping_command.dart | 11 +- script/tool/lib/src/format_command.dart | 21 ++- 5 files changed, 105 insertions(+), 113 deletions(-) diff --git a/script/githooks/lib/src/pre_commit_command.dart b/script/githooks/lib/src/pre_commit_command.dart index 30afa285f476..198fa755f445 100644 --- a/script/githooks/lib/src/pre_commit_command.dart +++ b/script/githooks/lib/src/pre_commit_command.dart @@ -112,22 +112,7 @@ class PreCommitCommand extends Command { ); final packageArgs = '--packages=${targetPackages.join(',')}'; - // Determine which toolchains are needed based on file extensions to avoid uneccessary slowdown. - final bool hasClang = stagedFiles.any( - (f) => - f.endsWith('.c') || - f.endsWith('.cc') || - f.endsWith('.cpp') || - f.endsWith('.h') || - f.endsWith('.hpp') || - f.endsWith('.m') || - f.endsWith('.mm'), - ); - final bool hasJava = stagedFiles.any((f) => f.endsWith('.java')); - final bool hasKotlin = stagedFiles.any((f) => f.endsWith('.kt')); - final bool hasSwift = stagedFiles.any((f) => f.endsWith('.swift')); - - final List dartFiles = stagedFiles.where((f) => f.endsWith('.dart')).toList(); + final String customFilesArg = '--custom-files=${stagedFiles.join(',')}'; print( '🏃 Running pre-commit checks on ${targetPackages.length} packages: ${targetPackages.join(', ')}', @@ -136,65 +121,26 @@ class PreCommitCommand extends Command { // Check formatting. stdout.write('Checking formatting...'); + final ProcessResult formatResult = await processRunner('dart', [ + 'run', + toolScript, + 'format', + customFilesArg, + '--fail-on-change', + ], workingDirectory: repoRoot.path); - // Format staged Dart files. - if (dartFiles.isNotEmpty) { - final ProcessResult dartFormatResult = await processRunner('dart', [ - 'format', - '--set-exit-if-changed', - ...dartFiles, - ], workingDirectory: repoRoot.path); - - if (dartFormatResult.exitCode != 0) { - if (!hasError) { - stdout.write(stdout.supportsAnsiEscapes ? '\x1B[2K\r' : '\n'); - } - if (dartFormatResult.stdout.toString().isNotEmpty) { - print(dartFormatResult.stdout); - } - if (dartFormatResult.stderr.toString().isNotEmpty) { - print(dartFormatResult.stderr); - } - print('❌ Formatting issues found in Dart files. Please run "dart format" to fix them.'); - hasError = true; + if (formatResult.exitCode != 0) { + if (!hasError) { + stdout.write(stdout.supportsAnsiEscapes ? '\x1B[2K\r' : '\n'); } - } - - // Format staged native files. - final bool needsNativeFormat = hasClang || hasJava || hasKotlin || hasSwift; - if (needsNativeFormat) { - final nativeFormatFlags = [ - '--no-dart', - if (!hasClang) '--no-clang-format', - if (!hasJava) '--no-java', - if (!hasKotlin) '--no-kotlin', - if (!hasSwift) '--no-swift', - ]; - - final ProcessResult nativeFormatResult = await processRunner('dart', [ - 'run', - toolScript, - 'format', - packageArgs, - '--fail-on-change', - ...nativeFormatFlags, - ], workingDirectory: repoRoot.path); - - if (nativeFormatResult.exitCode != 0) { - if (!hasError) { - stdout.write(stdout.supportsAnsiEscapes ? '\x1B[2K\r' : '\n'); - } - if (nativeFormatResult.stdout.toString().isNotEmpty) { - print(nativeFormatResult.stdout); - } - if (nativeFormatResult.stderr.toString().isNotEmpty) { - print(nativeFormatResult.stderr); - } - print( - '❌ Formatting issues found in native files. Please run "dart run script/tool/bin/flutter_plugin_tools.dart format $packageArgs" to fix them.', - ); - hasError = true; + if (formatResult.stdout.toString().isNotEmpty) { + print(formatResult.stdout); } + if (formatResult.stderr.toString().isNotEmpty) { + print(formatResult.stderr); + } + print('❌ Formatting issues found. Please run "dart run script/tool/bin/flutter_plugin_tools.dart format $customFilesArg" to fix them.'); + hasError = true; } if (!hasError) { @@ -208,27 +154,27 @@ class PreCommitCommand extends Command { // Run static analysis on staged files. var analyzeHasError = false; stdout.write('Running static analysis...'); - if (dartFiles.isNotEmpty) { - final ProcessResult analyzeResult = await processRunner('dart', [ - 'analyze', - '--fatal-infos', - ...dartFiles, - ], workingDirectory: repoRoot.path); - - if (analyzeResult.exitCode != 0) { - if (!analyzeHasError) { - stdout.write(stdout.supportsAnsiEscapes ? '\x1B[2K\r' : '\n'); - } - if (analyzeResult.stdout.toString().isNotEmpty) { - print(analyzeResult.stdout); - } - if (analyzeResult.stderr.toString().isNotEmpty) { - print(analyzeResult.stderr); - } - print('❌ Static analysis errors found.'); - analyzeHasError = true; - hasError = true; + final ProcessResult analyzeResult = await processRunner('dart', [ + 'run', + toolScript, + 'analyze', + customFilesArg, + '--dart', + ], workingDirectory: repoRoot.path); + + if (analyzeResult.exitCode != 0) { + if (!analyzeHasError) { + stdout.write(stdout.supportsAnsiEscapes ? '\x1B[2K\r' : '\n'); + } + if (analyzeResult.stdout.toString().isNotEmpty) { + print(analyzeResult.stdout); + } + if (analyzeResult.stderr.toString().isNotEmpty) { + print(analyzeResult.stderr); } + print('❌ Static analysis errors found.'); + analyzeHasError = true; + hasError = true; } if (!analyzeHasError) { diff --git a/script/githooks/test/pre_commit_test.dart b/script/githooks/test/pre_commit_test.dart index b0da053a1688..cc81cc978ec3 100644 --- a/script/githooks/test/pre_commit_test.dart +++ b/script/githooks/test/pre_commit_test.dart @@ -5,6 +5,7 @@ import 'dart:io'; import 'package:githooks/src/pre_commit_command.dart'; +import 'package:path/path.dart' as path; import 'package:test/test.dart'; void main() { @@ -25,16 +26,19 @@ void main() { final bool result = await command.run(); expect(result, isTrue); + final String repoRoot = Directory.current.parent.parent.path; + final String toolScript = '$repoRoot/script/tool/bin/flutter_plugin_tools.dart'; + // Verify the exact arguments passed to format and analyze expect( executedArguments, anyElement( - equals(['format', '--set-exit-if-changed', 'script/githooks/lib/githooks.dart']), + equals(['run', toolScript, 'format', '--custom-files=script/githooks/lib/githooks.dart', '--fail-on-change']), ), ); expect( executedArguments, - anyElement(equals(['analyze', '--fatal-infos', 'script/githooks/lib/githooks.dart'])), + anyElement(equals(['run', toolScript, 'analyze', '--custom-files=script/githooks/lib/githooks.dart', '--dart'])), ); }); @@ -47,7 +51,7 @@ void main() { if (executable == 'git') { return ProcessResult(0, 0, 'script/githooks/lib/githooks.dart\n', ''); } - if (arguments.isNotEmpty && arguments[0] == 'format') { + if (arguments.isNotEmpty && arguments.length > 2 && arguments[2] == 'format') { return ProcessResult(0, 1, 'bad_file.dart', ''); } return ProcessResult(0, 0, 'Success', ''); @@ -57,10 +61,13 @@ void main() { final bool result = await command.run(); expect(result, isFalse); + final String repoRoot = Directory.current.parent.parent.path; + final String toolScript = '$repoRoot/script/tool/bin/flutter_plugin_tools.dart'; + expect( executedArguments, anyElement( - equals(['format', '--set-exit-if-changed', 'script/githooks/lib/githooks.dart']), + equals(['run', toolScript, 'format', '--custom-files=script/githooks/lib/githooks.dart', '--fail-on-change']), ), ); }); @@ -74,7 +81,7 @@ void main() { if (executable == 'git') { return ProcessResult(0, 0, 'script/githooks/lib/githooks.dart\n', ''); } - if (arguments.isNotEmpty && arguments[0] == 'analyze') { + if (arguments.isNotEmpty && arguments.length > 2 && arguments[2] == 'analyze') { return ProcessResult(0, 1, 'error in file.dart', ''); } return ProcessResult(0, 0, 'Success', ''); @@ -84,9 +91,12 @@ void main() { final bool result = await command.run(); expect(result, isFalse); + final String repoRoot = Directory.current.parent.parent.path; + final String toolScript = '$repoRoot/script/tool/bin/flutter_plugin_tools.dart'; + expect( executedArguments, - anyElement(equals(['analyze', '--fatal-infos', 'script/githooks/lib/githooks.dart'])), + anyElement(equals(['run', toolScript, 'analyze', '--custom-files=script/githooks/lib/githooks.dart', '--dart'])), ); }); @@ -110,7 +120,15 @@ void main() { expect(executedArguments.any((args) => args.contains('format')), isFalse); expect(executedArguments.any((args) => args.contains('analyze')), isFalse); }); - test('runs native formatter when native files are staged', () async { + test('runs format and analyze when files are staged in a valid package', () async { + final String repoRoot = Directory.current.parent.parent.path; + final String toolScript = '$repoRoot/script/tool/bin/flutter_plugin_tools.dart'; + + final Directory pkgDir = Directory(path.join(repoRoot, 'packages', 'pkg')); + pkgDir.createSync(recursive: true); + File(path.join(pkgDir.path, 'pubspec.yaml')).writeAsStringSync('name: pkg'); + addTearDown(() => pkgDir.deleteSync(recursive: true)); + final List> executedArguments = []; final command = PreCommitCommand( processRunner: @@ -127,11 +145,14 @@ void main() { expect(result, isTrue); expect( - executedArguments.any( - (args) => - args.contains('format') && args.contains('--no-dart') && args.contains('--no-java'), + executedArguments, + anyElement( + equals(['run', toolScript, 'format', '--custom-files=packages/pkg/ios/Classes/Foo.m', '--fail-on-change']), ), - isTrue, + ); + expect( + executedArguments, + anyElement(equals(['run', toolScript, 'analyze', '--custom-files=packages/pkg/ios/Classes/Foo.m', '--dart'])), ); }); }); diff --git a/script/tool/lib/src/common/package_command.dart b/script/tool/lib/src/common/package_command.dart index 8d0fbb591436..eea348ee0996 100644 --- a/script/tool/lib/src/common/package_command.dart +++ b/script/tool/lib/src/common/package_command.dart @@ -158,10 +158,16 @@ abstract class PackageCommand extends Command { 'Currently only logs per-package timing for multi-package commands, ' 'but more information may be added in the future.', ); + argParser.addMultiOption( + _customFilesArg, + help: 'A list of custom files to run the command on.', + hide: true, + ); } // Package selection. static const String _packagesArg = 'packages'; + static const String _customFilesArg = 'custom-files'; static const String _packagesForBranchArg = 'packages-for-branch'; static const String _currentPackageArg = 'current-package'; static const String _pluginsLegacyAliasArg = 'plugins'; @@ -387,6 +393,7 @@ abstract class PackageCommand extends Command { _runOnDirtyPackagesArg, _packagesForBranchArg, _currentPackageArg, + _customFilesArg, }; if (packageSelectionFlags.where((String flag) => argResults!.wasParsed(flag)).length > 1) { printError( @@ -482,6 +489,12 @@ abstract class PackageCommand extends Command { throw ToolExit(exitInvalidArguments); } packages = {currentPackageName}; + } else if (argResults!.wasParsed(_customFilesArg)) { + final List customFiles = getStringListArg(_customFilesArg); + packages = _getChangedPackageNames(customFiles); + if (packages.isEmpty) { + return; + } } final Set excludedPackageNames = getExcludedPackageNames(); diff --git a/script/tool/lib/src/common/package_looping_command.dart b/script/tool/lib/src/common/package_looping_command.dart index 96d71605d84c..117944f5febf 100644 --- a/script/tool/lib/src/common/package_looping_command.dart +++ b/script/tool/lib/src/common/package_looping_command.dart @@ -296,9 +296,14 @@ abstract class PackageLoopingCommand extends PackageCommand { final runStart = DateTime.now(); // Populate the list of changed files for subclasses to use. - final GitVersionFinder gitVersionFinder = await retrieveVersionFinder(); - baseSha = await gitVersionFinder.getBaseSha(); - changedFiles = await gitVersionFinder.getChangedFiles(); + if (argResults!.wasParsed('custom-files')) { + baseSha = 'HEAD'; // Since custom-files explicitly lists the files, baseSha doesn't strictly apply, but we need to set it to something. + changedFiles = getStringListArg('custom-files'); + } else { + final GitVersionFinder gitVersionFinder = await retrieveVersionFinder(); + baseSha = await gitVersionFinder.getBaseSha(); + changedFiles = await gitVersionFinder.getChangedFiles(); + } // Check whether the command needs to run. if (changedFiles.isNotEmpty && changedFiles.every(shouldIgnoreFile)) { diff --git a/script/tool/lib/src/format_command.dart b/script/tool/lib/src/format_command.dart index 2e25c93b8f39..afb30f3d64c2 100644 --- a/script/tool/lib/src/format_command.dart +++ b/script/tool/lib/src/format_command.dart @@ -94,9 +94,6 @@ class FormatCommand extends PackageLoopingCommand { @override Future initializeRun() async { - final String javaFormatterPath = await _downloadJavaFormatterIfNecessary(); - final String kotlinFormatterPath = await _downloadKotlinFormatterIfNecessary(); - // All but Dart is formatted here rather than in runForPackage because // running the formatters separately for each package is an order of // magnitude slower, due to the startup overhead of the formatters. @@ -106,10 +103,10 @@ class FormatCommand extends PackageLoopingCommand { // formatter isn't running in the context of the package. final Iterable files = await _getFilteredFilePaths(getFiles(), relativeTo: packagesDir); if (getBoolArg(_javaArg)) { - await _formatJava(files, javaFormatterPath); + await _formatJava(files); } if (getBoolArg(_kotlinArg)) { - await _formatKotlin(files, kotlinFormatterPath); + await _formatKotlin(files); } if (getBoolArg(_clangFormatArg)) { await _formatCppAndObjectiveC(files); @@ -269,9 +266,10 @@ class FormatCommand extends PackageLoopingCommand { throw ToolExit(_exitDependencyMissing); } - Future _formatJava(Iterable files, String formatterPath) async { + Future _formatJava(Iterable files) async { final Iterable javaFiles = _getPathsWithExtensions(files, {'.java'}); if (javaFiles.isNotEmpty) { + final String formatterPath = await _downloadJavaFormatterIfNecessary(); final String java = getStringArg(_javaPathArg); if (!await _hasDependency(java)) { printError( @@ -294,9 +292,10 @@ class FormatCommand extends PackageLoopingCommand { } } - Future _formatKotlin(Iterable files, String formatterPath) async { + Future _formatKotlin(Iterable files) async { final Iterable kotlinFiles = _getPathsWithExtensions(files, {'.kt'}); if (kotlinFiles.isNotEmpty) { + final String formatterPath = await _downloadKotlinFormatterIfNecessary(); final String java = getStringArg(_javaPathArg); if (!await _hasDependency(java)) { printError( @@ -394,8 +393,16 @@ class FormatCommand extends PackageLoopingCommand { const handFormattedExtension = '.dart'; const handFormattedPragma = '// This file is hand-formatted.'; + final bool useCustomFiles = argResults!.wasParsed('custom-files'); + return files .where((File file) { + if (useCustomFiles) { + final String repoRelativePath = getRelativePosixPath(file, from: rootDir); + if (!changedFiles.contains(repoRelativePath)) { + return false; + } + } // See comment above near [handFormattedPragma]. return path.extension(file.path) != handFormattedExtension || !file.readAsLinesSync().contains(handFormattedPragma); From e344ca740bd5965fe181a33e9988c9a7e72a4d3d Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Thu, 25 Jun 2026 15:35:07 -0700 Subject: [PATCH 25/37] implement --run-on-staged-packages --- .../githooks/lib/src/pre_commit_command.dart | 100 +++--------------- script/githooks/test/pre_commit_test.dart | 82 +++----------- .../lib/src/common/git_version_finder.dart | 17 +++ .../tool/lib/src/common/package_command.dart | 21 ++-- .../src/common/package_looping_command.dart | 11 +- script/tool/lib/src/format_command.dart | 7 +- .../test/common/package_command_test.dart | 49 +++++++++ script/tool/test/format_command_test.dart | 65 +++++++++++- 8 files changed, 184 insertions(+), 168 deletions(-) diff --git a/script/githooks/lib/src/pre_commit_command.dart b/script/githooks/lib/src/pre_commit_command.dart index 198fa755f445..27905fce8066 100644 --- a/script/githooks/lib/src/pre_commit_command.dart +++ b/script/githooks/lib/src/pre_commit_command.dart @@ -34,74 +34,20 @@ class PreCommitCommand extends Command { @override final String description = 'Checks to run before a "git commit"'; - String? _findPackagePath(String filePath, String repoRoot) { - Directory currentDir = File(p.join(repoRoot, filePath)).parent; - while (p.isWithin(repoRoot, currentDir.path) || p.equals(repoRoot, currentDir.path)) { - final String dirName = p.basename(currentDir.path); - if (dirName != 'example' && File(p.join(currentDir.path, 'pubspec.yaml')).existsSync()) { - return currentDir.path; - } - if (p.equals(repoRoot, currentDir.path)) { - break; - } - currentDir = currentDir.parent; - } - return null; - } @override Future run() async { - // Find the repo root where the plugin tool is located. - Directory repoRoot = Directory.current; - while (repoRoot.path != repoRoot.parent.path && - !(Directory(p.join(repoRoot.path, '.git')).existsSync() || - File(p.join(repoRoot.path, '.git')).existsSync())) { - repoRoot = repoRoot.parent; - } - if (!(Directory(p.join(repoRoot.path, '.git')).existsSync() || - File(p.join(repoRoot.path, '.git')).existsSync())) { - print('❌ Could not find .git directory.'); - return false; - } + final ProcessResult rootResult = await processRunner('git', [ + 'rev-parse', + '--show-toplevel', + ], workingDirectory: Directory.current.path); - // Get all staged files that are added, copied, or modified. - final ProcessResult diffResult = await processRunner('git', [ - 'diff', - '--cached', - '--name-only', - '--diff-filter=ACM', - ], workingDirectory: repoRoot.path); - - if (diffResult.exitCode != 0) { - print('❌ Failed to get staged files'); + if (rootResult.exitCode != 0) { + print('❌ Could not find git repository.'); return false; } - final List stagedFiles = (diffResult.stdout as String) - .split('\n') - .map((e) => e.trim()) - .where((e) => e.isNotEmpty) - .toList(); - - if (stagedFiles.isEmpty) { - // No files changed. - return true; - } - - final Set targetPackageDirs = {}; - for (final file in stagedFiles) { - final String? packageDir = _findPackagePath(file, repoRoot.path); - if (packageDir != null) { - targetPackageDirs.add(packageDir); - } - } - - if (targetPackageDirs.isEmpty) { - // None of the changed files are part of a package we care about. - return true; - } - - final Set targetPackages = targetPackageDirs.map((dir) => p.basename(dir)).toSet(); + final repoRoot = Directory((rootResult.stdout as String).trim()); final String toolScript = p.join( repoRoot.path, @@ -110,13 +56,7 @@ class PreCommitCommand extends Command { 'bin', 'flutter_plugin_tools.dart', ); - final packageArgs = '--packages=${targetPackages.join(',')}'; - final String customFilesArg = '--custom-files=${stagedFiles.join(',')}'; - - print( - '🏃 Running pre-commit checks on ${targetPackages.length} packages: ${targetPackages.join(', ')}', - ); var hasError = false; // Check formatting. @@ -125,25 +65,23 @@ class PreCommitCommand extends Command { 'run', toolScript, 'format', - customFilesArg, + '--run-on-staged-packages', '--fail-on-change', ], workingDirectory: repoRoot.path); if (formatResult.exitCode != 0) { - if (!hasError) { - stdout.write(stdout.supportsAnsiEscapes ? '\x1B[2K\r' : '\n'); - } + stdout.write(stdout.supportsAnsiEscapes ? '\x1B[2K\r' : '\n'); if (formatResult.stdout.toString().isNotEmpty) { print(formatResult.stdout); } if (formatResult.stderr.toString().isNotEmpty) { print(formatResult.stderr); } - print('❌ Formatting issues found. Please run "dart run script/tool/bin/flutter_plugin_tools.dart format $customFilesArg" to fix them.'); + print( + 'Formatting issues found. Please run "dart run script/tool/bin/flutter_plugin_tools.dart format --run-on-staged-packages" to fix them.', + ); hasError = true; - } - - if (!hasError) { + } else { stdout.write( stdout.supportsAnsiEscapes ? '\x1B[2K\r✅ Formatting looks good.\n' @@ -152,20 +90,17 @@ class PreCommitCommand extends Command { } // Run static analysis on staged files. - var analyzeHasError = false; stdout.write('Running static analysis...'); final ProcessResult analyzeResult = await processRunner('dart', [ 'run', toolScript, 'analyze', - customFilesArg, + '--run-on-staged-packages', '--dart', ], workingDirectory: repoRoot.path); if (analyzeResult.exitCode != 0) { - if (!analyzeHasError) { - stdout.write(stdout.supportsAnsiEscapes ? '\x1B[2K\r' : '\n'); - } + stdout.write(stdout.supportsAnsiEscapes ? '\x1B[2K\r' : '\n'); if (analyzeResult.stdout.toString().isNotEmpty) { print(analyzeResult.stdout); } @@ -173,11 +108,8 @@ class PreCommitCommand extends Command { print(analyzeResult.stderr); } print('❌ Static analysis errors found.'); - analyzeHasError = true; hasError = true; - } - - if (!analyzeHasError) { + } else { stdout.write( stdout.supportsAnsiEscapes ? '\x1B[2K\r✅ Static analysis looks good.\n' diff --git a/script/githooks/test/pre_commit_test.dart b/script/githooks/test/pre_commit_test.dart index cc81cc978ec3..e85d5fd4e463 100644 --- a/script/githooks/test/pre_commit_test.dart +++ b/script/githooks/test/pre_commit_test.dart @@ -16,8 +16,8 @@ void main() { processRunner: (String executable, List arguments, {String? workingDirectory}) async { executedArguments.add(arguments); - if (executable == 'git') { - return ProcessResult(0, 0, 'script/githooks/lib/githooks.dart\n', ''); + if (executable == 'git' && arguments.contains('rev-parse')) { + return ProcessResult(0, 0, '/mock/repo/root\n', ''); } return ProcessResult(0, 0, 'Success', ''); }, @@ -26,19 +26,19 @@ void main() { final bool result = await command.run(); expect(result, isTrue); - final String repoRoot = Directory.current.parent.parent.path; + const String repoRoot = '/mock/repo/root'; final String toolScript = '$repoRoot/script/tool/bin/flutter_plugin_tools.dart'; // Verify the exact arguments passed to format and analyze expect( executedArguments, anyElement( - equals(['run', toolScript, 'format', '--custom-files=script/githooks/lib/githooks.dart', '--fail-on-change']), + equals(['run', toolScript, 'format', '--run-on-staged-packages', '--fail-on-change']), ), ); expect( executedArguments, - anyElement(equals(['run', toolScript, 'analyze', '--custom-files=script/githooks/lib/githooks.dart', '--dart'])), + anyElement(equals(['run', toolScript, 'analyze', '--run-on-staged-packages', '--dart'])), ); }); @@ -48,8 +48,8 @@ void main() { processRunner: (String executable, List arguments, {String? workingDirectory}) async { executedArguments.add(arguments); - if (executable == 'git') { - return ProcessResult(0, 0, 'script/githooks/lib/githooks.dart\n', ''); + if (executable == 'git' && arguments.contains('rev-parse')) { + return ProcessResult(0, 0, '/mock/repo/root\n', ''); } if (arguments.isNotEmpty && arguments.length > 2 && arguments[2] == 'format') { return ProcessResult(0, 1, 'bad_file.dart', ''); @@ -61,13 +61,13 @@ void main() { final bool result = await command.run(); expect(result, isFalse); - final String repoRoot = Directory.current.parent.parent.path; + const String repoRoot = '/mock/repo/root'; final String toolScript = '$repoRoot/script/tool/bin/flutter_plugin_tools.dart'; expect( executedArguments, anyElement( - equals(['run', toolScript, 'format', '--custom-files=script/githooks/lib/githooks.dart', '--fail-on-change']), + equals(['run', toolScript, 'format', '--run-on-staged-packages', '--fail-on-change']), ), ); }); @@ -78,8 +78,8 @@ void main() { processRunner: (String executable, List arguments, {String? workingDirectory}) async { executedArguments.add(arguments); - if (executable == 'git') { - return ProcessResult(0, 0, 'script/githooks/lib/githooks.dart\n', ''); + if (executable == 'git' && arguments.contains('rev-parse')) { + return ProcessResult(0, 0, '/mock/repo/root\n', ''); } if (arguments.isNotEmpty && arguments.length > 2 && arguments[2] == 'analyze') { return ProcessResult(0, 1, 'error in file.dart', ''); @@ -91,68 +91,12 @@ void main() { final bool result = await command.run(); expect(result, isFalse); - final String repoRoot = Directory.current.parent.parent.path; + const String repoRoot = '/mock/repo/root'; final String toolScript = '$repoRoot/script/tool/bin/flutter_plugin_tools.dart'; expect( executedArguments, - anyElement(equals(['run', toolScript, 'analyze', '--custom-files=script/githooks/lib/githooks.dart', '--dart'])), - ); - }); - - test('ignores non-dart files', () async { - final List> executedArguments = []; - final command = PreCommitCommand( - processRunner: - (String executable, List arguments, {String? workingDirectory}) async { - executedArguments.add(arguments); - if (executable == 'git') { - return ProcessResult(0, 0, 'README.md\n', ''); - } - return ProcessResult(0, 0, 'Success', ''); - }, - ); - - final bool result = await command.run(); - expect(result, isTrue); - - // Verify that dart format and analyze were NEVER called. - expect(executedArguments.any((args) => args.contains('format')), isFalse); - expect(executedArguments.any((args) => args.contains('analyze')), isFalse); - }); - test('runs format and analyze when files are staged in a valid package', () async { - final String repoRoot = Directory.current.parent.parent.path; - final String toolScript = '$repoRoot/script/tool/bin/flutter_plugin_tools.dart'; - - final Directory pkgDir = Directory(path.join(repoRoot, 'packages', 'pkg')); - pkgDir.createSync(recursive: true); - File(path.join(pkgDir.path, 'pubspec.yaml')).writeAsStringSync('name: pkg'); - addTearDown(() => pkgDir.deleteSync(recursive: true)); - - final List> executedArguments = []; - final command = PreCommitCommand( - processRunner: - (String executable, List arguments, {String? workingDirectory}) async { - executedArguments.add(arguments); - if (executable == 'git') { - return ProcessResult(0, 0, 'packages/pkg/ios/Classes/Foo.m\n', ''); - } - return ProcessResult(0, 0, 'Success', ''); - }, - ); - - final bool result = await command.run(); - expect(result, isTrue); - - expect( - executedArguments, - anyElement( - equals(['run', toolScript, 'format', '--custom-files=packages/pkg/ios/Classes/Foo.m', '--fail-on-change']), - ), - ); - expect( - executedArguments, - anyElement(equals(['run', toolScript, 'analyze', '--custom-files=packages/pkg/ios/Classes/Foo.m', '--dart'])), + anyElement(equals(['run', toolScript, 'analyze', '--run-on-staged-packages', '--dart'])), ); }); }); diff --git a/script/tool/lib/src/common/git_version_finder.dart b/script/tool/lib/src/common/git_version_finder.dart index c033e255729d..7479bcb4a91d 100644 --- a/script/tool/lib/src/common/git_version_finder.dart +++ b/script/tool/lib/src/common/git_version_finder.dart @@ -48,6 +48,23 @@ class GitVersionFinder { return changedFiles.toList(); } + /// Get a list of all the staged files. + Future> getStagedFiles() async { + final io.ProcessResult changedFilesCommand = await baseGitDir.runCommand([ + 'diff', + '--cached', + '--name-only', + '--diff-filter=ACM', + ]); + final changedFilesStdout = changedFilesCommand.stdout.toString(); + if (changedFilesStdout.isEmpty) { + return []; + } + final List changedFiles = changedFilesStdout.split('\n') + ..removeWhere((String element) => element.isEmpty); + return changedFiles.toList(); + } + /// Get a list of all the changed files. Future> getDiffContents({ String? targetPath, diff --git a/script/tool/lib/src/common/package_command.dart b/script/tool/lib/src/common/package_command.dart index eea348ee0996..410227ee457e 100644 --- a/script/tool/lib/src/common/package_command.dart +++ b/script/tool/lib/src/common/package_command.dart @@ -158,16 +158,15 @@ abstract class PackageCommand extends Command { 'Currently only logs per-package timing for multi-package commands, ' 'but more information may be added in the future.', ); - argParser.addMultiOption( - _customFilesArg, - help: 'A list of custom files to run the command on.', - hide: true, + argParser.addFlag( + _runOnStagedPackagesArg, + help: 'Run the command on all packages with staged changes.', ); } // Package selection. static const String _packagesArg = 'packages'; - static const String _customFilesArg = 'custom-files'; + static const String _runOnStagedPackagesArg = 'run-on-staged-packages'; static const String _packagesForBranchArg = 'packages-for-branch'; static const String _currentPackageArg = 'current-package'; static const String _pluginsLegacyAliasArg = 'plugins'; @@ -393,7 +392,7 @@ abstract class PackageCommand extends Command { _runOnDirtyPackagesArg, _packagesForBranchArg, _currentPackageArg, - _customFilesArg, + _runOnStagedPackagesArg, }; if (packageSelectionFlags.where((String flag) => argResults!.wasParsed(flag)).length > 1) { printError( @@ -412,6 +411,7 @@ abstract class PackageCommand extends Command { !(getBoolArg(_exactMatchOnlyArg) || argResults!.wasParsed(_runOnChangedPackagesArg) || argResults!.wasParsed(_runOnDirtyPackagesArg) || + argResults!.wasParsed(_runOnStagedPackagesArg) || argResults!.wasParsed(_packagesForBranchArg)); var packages = Set.from(getStringListArg(_packagesArg)); @@ -489,9 +489,12 @@ abstract class PackageCommand extends Command { throw ToolExit(exitInvalidArguments); } packages = {currentPackageName}; - } else if (argResults!.wasParsed(_customFilesArg)) { - final List customFiles = getStringListArg(_customFilesArg); - packages = _getChangedPackageNames(customFiles); + } else if (getBoolArg(_runOnStagedPackagesArg)) { + final gitVersionFinder = GitVersionFinder(await gitDir, baseSha: 'HEAD'); + print('Running for all packages that have staged changes\n'); + packages = _getChangedPackageNames( + await gitVersionFinder.getStagedFiles(), + ); if (packages.isEmpty) { return; } diff --git a/script/tool/lib/src/common/package_looping_command.dart b/script/tool/lib/src/common/package_looping_command.dart index 117944f5febf..89bc3729c028 100644 --- a/script/tool/lib/src/common/package_looping_command.dart +++ b/script/tool/lib/src/common/package_looping_command.dart @@ -296,9 +296,14 @@ abstract class PackageLoopingCommand extends PackageCommand { final runStart = DateTime.now(); // Populate the list of changed files for subclasses to use. - if (argResults!.wasParsed('custom-files')) { - baseSha = 'HEAD'; // Since custom-files explicitly lists the files, baseSha doesn't strictly apply, but we need to set it to something. - changedFiles = getStringListArg('custom-files'); + if (getBoolArg('run-on-staged-packages')) { + baseSha = 'HEAD'; + final GitVersionFinder gitVersionFinder = GitVersionFinder(await gitDir, baseSha: baseSha); + changedFiles = await gitVersionFinder.getStagedFiles(); + } else if (getBoolArg('run-on-dirty-packages')) { + baseSha = 'HEAD'; + final GitVersionFinder gitVersionFinder = GitVersionFinder(await gitDir, baseSha: baseSha); + changedFiles = await gitVersionFinder.getChangedFiles(includeUncommitted: true); } else { final GitVersionFinder gitVersionFinder = await retrieveVersionFinder(); baseSha = await gitVersionFinder.getBaseSha(); diff --git a/script/tool/lib/src/format_command.dart b/script/tool/lib/src/format_command.dart index afb30f3d64c2..5c5f9c932b71 100644 --- a/script/tool/lib/src/format_command.dart +++ b/script/tool/lib/src/format_command.dart @@ -393,11 +393,14 @@ class FormatCommand extends PackageLoopingCommand { const handFormattedExtension = '.dart'; const handFormattedPragma = '// This file is hand-formatted.'; - final bool useCustomFiles = argResults!.wasParsed('custom-files'); + final bool useDiff = getBoolArg('run-on-staged-packages') || + getBoolArg('run-on-dirty-packages') || + getBoolArg('run-on-changed-packages') || + getBoolArg('packages-for-branch'); return files .where((File file) { - if (useCustomFiles) { + if (useDiff) { final String repoRelativePath = getRelativePosixPath(file, from: rootDir); if (!changedFiles.contains(repoRelativePath)) { return false; diff --git a/script/tool/test/common/package_command_test.dart b/script/tool/test/common/package_command_test.dart index abe197a9be31..d1bf459b5cde 100644 --- a/script/tool/test/common/package_command_test.dart +++ b/script/tool/test/common/package_command_test.dart @@ -1018,6 +1018,55 @@ packages/b_package/lib/src/foo.dart expect(command.plugins, unorderedEquals([packageA.path])); }); }); + + group('test run-on-staged-packages', () { + test('no packages should be tested if there are no changes.', () async { + createFakePackage('a_package', packagesDir); + await runCapturingPrint(runner, ['sample', '--run-on-staged-packages']); + + expect(command.plugins, unorderedEquals([])); + }); + + test('Only changed packages should be tested.', () async { + gitProcessRunner.mockProcessesForExecutable['git-diff'] = [ + FakeProcessInfo(MockProcess(stdout: 'packages/a_package/lib/a_package.dart')), + ]; + final RepositoryPackage packageA = createFakePackage('a_package', packagesDir); + createFakePlugin('b_package', packagesDir); + final List output = await runCapturingPrint(runner, [ + 'sample', + '--run-on-staged-packages', + ]); + + expect( + output, + containsAllInOrder([ + contains('Running for all packages that have staged changes'), + ]), + ); + + expect(command.plugins, unorderedEquals([packageA.path])); + }); + + test('multiple packages changed should test all the changed packages', () async { + gitProcessRunner.mockProcessesForExecutable['git-diff'] = [ + FakeProcessInfo( + MockProcess( + stdout: ''' +packages/a_package/lib/a_package.dart +packages/b_package/lib/src/foo.dart +''', + ), + ), + ]; + final RepositoryPackage packageA = createFakePackage('a_package', packagesDir); + final RepositoryPackage packageB = createFakePackage('b_package', packagesDir); + createFakePackage('c_package', packagesDir); + await runCapturingPrint(runner, ['sample', '--run-on-staged-packages']); + + expect(command.plugins, unorderedEquals([packageA.path, packageB.path])); + }); + }); }); group('--packages-for-branch', () { diff --git a/script/tool/test/format_command_test.dart b/script/tool/test/format_command_test.dart index 18647d28d783..ba01b840329f 100644 --- a/script/tool/test/format_command_test.dart +++ b/script/tool/test/format_command_test.dart @@ -21,6 +21,7 @@ void main() { late MockPlatform mockPlatform; late Directory packagesDir; late RecordingProcessRunner processRunner; + late RecordingProcessRunner gitProcessRunner; late FormatCommand analyzeCommand; late CommandRunner runner; late String javaFormatPath; @@ -29,7 +30,7 @@ void main() { setUp(() { mockPlatform = MockPlatform(); final GitDir gitDir; - (:packagesDir, :processRunner, gitProcessRunner: _, :gitDir) = configureBaseCommandMocks( + (:packagesDir, :processRunner, :gitProcessRunner, :gitDir) = configureBaseCommandMocks( platform: mockPlatform, ); analyzeCommand = FormatCommand( @@ -179,6 +180,68 @@ void main() { ); }); + test('formats only uncommitted files when --run-on-dirty-packages is used', () async { + const files = ['lib/a.dart', 'lib/src/b.dart', 'lib/src/c.dart']; + final RepositoryPackage plugin = createFakePlugin( + 'a_plugin', + packagesDir, + extraFiles: files, + dartConstraint: _dartConstraint, + ); + fakePubGet(plugin); + + // Mock git diff to return only lib/a.dart (called three times) + const changedFilePath = 'packages/a_plugin/lib/a.dart'; + gitProcessRunner.mockProcessesForExecutable['git-diff'] = [ + FakeProcessInfo(MockProcess(stdout: changedFilePath)), + FakeProcessInfo(MockProcess(stdout: changedFilePath)), + FakeProcessInfo(MockProcess(stdout: changedFilePath)), + ]; + + await runCapturingPrint(runner, [ + 'format', + '--run-on-dirty-packages', + ]); + + expect( + processRunner.recordedCalls, + orderedEquals([ + ProcessCall('dart', const ['format', 'lib/a.dart'], plugin.path), + ]), + ); + }); + + test('formats only staged files when --run-on-staged-packages is used', () async { + const files = ['lib/a.dart', 'lib/src/b.dart', 'lib/src/c.dart']; + final RepositoryPackage plugin = createFakePlugin( + 'a_plugin', + packagesDir, + extraFiles: files, + dartConstraint: _dartConstraint, + ); + fakePubGet(plugin); + + // Mock git diff --cached to return only lib/src/b.dart (called three times) + const stagedFilePath = 'packages/a_plugin/lib/src/b.dart'; + gitProcessRunner.mockProcessesForExecutable['git-diff'] = [ + FakeProcessInfo(MockProcess(stdout: stagedFilePath)), + FakeProcessInfo(MockProcess(stdout: stagedFilePath)), + FakeProcessInfo(MockProcess(stdout: stagedFilePath)), + ]; + + await runCapturingPrint(runner, [ + 'format', + '--run-on-staged-packages', + ]); + + expect( + processRunner.recordedCalls, + orderedEquals([ + ProcessCall('dart', const ['format', 'lib/src/b.dart'], plugin.path), + ]), + ); + }); + test('skips dart if --no-dart flag is provided', () async { const files = ['lib/a.dart']; final RepositoryPackage plugin = createFakePlugin( From 814caf1e4ba6147a2af8a6ebe9c1201c57fc679c Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Thu, 25 Jun 2026 15:43:51 -0700 Subject: [PATCH 26/37] small optimizations --- .../githooks/lib/src/pre_commit_command.dart | 76 ++++++++++--------- script/githooks/test/pre_commit_test.dart | 48 ++++++++++++ 2 files changed, 89 insertions(+), 35 deletions(-) diff --git a/script/githooks/lib/src/pre_commit_command.dart b/script/githooks/lib/src/pre_commit_command.dart index 27905fce8066..696b08e06f67 100644 --- a/script/githooks/lib/src/pre_commit_command.dart +++ b/script/githooks/lib/src/pre_commit_command.dart @@ -49,6 +49,18 @@ class PreCommitCommand extends Command { final repoRoot = Directory((rootResult.stdout as String).trim()); + // Check if there are any staged changes first to exit early. + final ProcessResult diffResult = await processRunner('git', [ + 'diff', + '--cached', + '--name-only', + ], workingDirectory: repoRoot.path); + + if (diffResult.exitCode == 0 && (diffResult.stdout as String).trim().isEmpty) { + print('✅ No staged changes to check.'); + return true; + } + final String toolScript = p.join( repoRoot.path, 'script', @@ -57,10 +69,9 @@ class PreCommitCommand extends Command { 'flutter_plugin_tools.dart', ); - var hasError = false; + print('Checking staged changes...'); - // Check formatting. - stdout.write('Checking formatting...'); + // Run format first so analyze runs on the final formatted code. final ProcessResult formatResult = await processRunner('dart', [ 'run', toolScript, @@ -69,8 +80,10 @@ class PreCommitCommand extends Command { '--fail-on-change', ], workingDirectory: repoRoot.path); + var hasError = false; + + // Report formatting results. if (formatResult.exitCode != 0) { - stdout.write(stdout.supportsAnsiEscapes ? '\x1B[2K\r' : '\n'); if (formatResult.stdout.toString().isNotEmpty) { print(formatResult.stdout); } @@ -82,43 +95,36 @@ class PreCommitCommand extends Command { ); hasError = true; } else { - stdout.write( - stdout.supportsAnsiEscapes - ? '\x1B[2K\r✅ Formatting looks good.\n' - : '✅ Formatting looks good.\n', - ); + print('Formatting looks good!'); } - // Run static analysis on staged files. - stdout.write('Running static analysis...'); - final ProcessResult analyzeResult = await processRunner('dart', [ - 'run', - toolScript, - 'analyze', - '--run-on-staged-packages', - '--dart', - ], workingDirectory: repoRoot.path); - - if (analyzeResult.exitCode != 0) { - stdout.write(stdout.supportsAnsiEscapes ? '\x1B[2K\r' : '\n'); - if (analyzeResult.stdout.toString().isNotEmpty) { - print(analyzeResult.stdout); + // Only run static analysis if formatting passed, to ensure we analyze the final formatted code. + if (!hasError) { + final ProcessResult analyzeResult = await processRunner('dart', [ + 'run', + toolScript, + 'analyze', + '--run-on-staged-packages', + '--dart', + ], workingDirectory: repoRoot.path); + + // Report static analysis results. + if (analyzeResult.exitCode != 0) { + if (analyzeResult.stdout.toString().isNotEmpty) { + print(analyzeResult.stdout); + } + if (analyzeResult.stderr.toString().isNotEmpty) { + print(analyzeResult.stderr); + } + print('Static analysis errors found.'); + hasError = true; + } else { + print('Static analysis looks good!'); } - if (analyzeResult.stderr.toString().isNotEmpty) { - print(analyzeResult.stderr); - } - print('❌ Static analysis errors found.'); - hasError = true; - } else { - stdout.write( - stdout.supportsAnsiEscapes - ? '\x1B[2K\r✅ Static analysis looks good.\n' - : '✅ Static analysis looks good.\n', - ); } if (hasError) { - print('❌ Please fix the errors listed above.'); + print('Static analysis failed. Please fix the errors listed above.'); } return !hasError; diff --git a/script/githooks/test/pre_commit_test.dart b/script/githooks/test/pre_commit_test.dart index e85d5fd4e463..6f14302ef856 100644 --- a/script/githooks/test/pre_commit_test.dart +++ b/script/githooks/test/pre_commit_test.dart @@ -70,6 +70,11 @@ void main() { equals(['run', toolScript, 'format', '--run-on-staged-packages', '--fail-on-change']), ), ); + // Verify that static analysis was skipped because formatting failed + expect( + executedArguments.any((args) => args.contains('analyze')), + isFalse, + ); }); test('fails when analysis fails', () async { @@ -99,5 +104,48 @@ void main() { anyElement(equals(['run', toolScript, 'analyze', '--run-on-staged-packages', '--dart'])), ); }); + + test('exits early with success if there are no staged changes', () async { + final List> executedArguments = []; + final command = PreCommitCommand( + processRunner: + (String executable, List arguments, {String? workingDirectory}) async { + executedArguments.add(arguments); + if (executable == 'git') { + if (arguments.contains('rev-parse')) { + return ProcessResult(0, 0, '/mock/repo/root\n', ''); + } + if (arguments.contains('diff')) { + return ProcessResult(0, 0, '', ''); + } + } + return ProcessResult(0, 0, 'Success', ''); + }, + ); + + final bool result = await command.run(); + expect(result, isTrue); + + // Verify that no format or analyze commands were run + expect( + executedArguments.any((args) => args.contains('format') || args.contains('analyze')), + isFalse, + ); + }); + + test('fails when git root cannot be found', () async { + final command = PreCommitCommand( + processRunner: + (String executable, List arguments, {String? workingDirectory}) async { + if (executable == 'git' && arguments.contains('rev-parse')) { + return ProcessResult(0, 1, '', 'Not a git repository'); + } + return ProcessResult(0, 0, 'Success', ''); + }, + ); + + final bool result = await command.run(); + expect(result, isFalse); + }); }); } From 32faa2466af42499e97f6d2bddea8d8740306f1c Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Thu, 25 Jun 2026 15:44:41 -0700 Subject: [PATCH 27/37] msg --- script/githooks/lib/src/pre_commit_command.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/githooks/lib/src/pre_commit_command.dart b/script/githooks/lib/src/pre_commit_command.dart index 696b08e06f67..ede451609f54 100644 --- a/script/githooks/lib/src/pre_commit_command.dart +++ b/script/githooks/lib/src/pre_commit_command.dart @@ -69,7 +69,7 @@ class PreCommitCommand extends Command { 'flutter_plugin_tools.dart', ); - print('Checking staged changes...'); + print('Checking staged changes (format and static analysis)...'); // Run format first so analyze runs on the final formatted code. final ProcessResult formatResult = await processRunner('dart', [ From 9dc4b9d81a351f73dde9c27d81ce3a4da2bced76 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Thu, 25 Jun 2026 15:58:08 -0700 Subject: [PATCH 28/37] address reid review that is still relevant --- .../githooks/lib/src/pre_commit_command.dart | 134 +++++++++++------- 1 file changed, 83 insertions(+), 51 deletions(-) diff --git a/script/githooks/lib/src/pre_commit_command.dart b/script/githooks/lib/src/pre_commit_command.dart index ede451609f54..634035437487 100644 --- a/script/githooks/lib/src/pre_commit_command.dart +++ b/script/githooks/lib/src/pre_commit_command.dart @@ -34,44 +34,82 @@ class PreCommitCommand extends Command { @override final String description = 'Checks to run before a "git commit"'; - + /// Runs a pre-commit check for correct formatting and static analysis. + /// + /// It will check for any staged changes and run the plugin tool format and analyze commands on them. + /// If any of the commands fail, it will return false; otherwise, it will return true. @override Future run() async { + final Directory? repoRoot = await _findRepoRoot(); + if (repoRoot == null) { + print('Could not find git repository.'); + return false; + } + + if (!await _hasStagedChanges(repoRoot)) { + print('No staged changes to check.'); + return true; + } + + final String toolScript = p.join( + repoRoot.path, + 'script', + 'tool', + 'bin', + 'flutter_plugin_tools.dart', + ); + + print('Running pre-commit format and static analysis checks for staged changes...'); + + // Check format. + final bool formatPassed = await _checkFormatting(repoRoot, toolScript); + if (!formatPassed) { + return false; + } + + // Check static analysis if format passed. + return _checkStaticAnalysis(repoRoot, toolScript); + } + + /// Finds the repository root directory using git. + /// + /// Returns null if git fails or if the directory is not a git repository. + Future _findRepoRoot() async { final ProcessResult rootResult = await processRunner('git', [ 'rev-parse', '--show-toplevel', ], workingDirectory: Directory.current.path); if (rootResult.exitCode != 0) { - print('❌ Could not find git repository.'); - return false; + return null; } + return Directory((rootResult.stdout as String).trim()); + } - final repoRoot = Directory((rootResult.stdout as String).trim()); - - // Check if there are any staged changes first to exit early. + /// Checks if there are any staged changes in the repository. + Future _hasStagedChanges(Directory repoRoot) async { final ProcessResult diffResult = await processRunner('git', [ 'diff', '--cached', '--name-only', ], workingDirectory: repoRoot.path); - if (diffResult.exitCode == 0 && (diffResult.stdout as String).trim().isEmpty) { - print('✅ No staged changes to check.'); - return true; + if (diffResult.exitCode != 0) { + print('Failed to check staged changes.'); + if (diffResult.stderr.toString().isNotEmpty) { + print(diffResult.stderr); + } + // If we cannot determine the diff, abort pre-commit check. + return false; } - final String toolScript = p.join( - repoRoot.path, - 'script', - 'tool', - 'bin', - 'flutter_plugin_tools.dart', - ); - - print('Checking staged changes (format and static analysis)...'); + return (diffResult.stdout as String).trim().isNotEmpty; + } - // Run format first so analyze runs on the final formatted code. + /// Runs the formatting check on staged files. + /// + /// Returns true if all staged files are correctly formatted. + Future _checkFormatting(Directory repoRoot, String toolScript) async { final ProcessResult formatResult = await processRunner('dart', [ 'run', toolScript, @@ -80,9 +118,6 @@ class PreCommitCommand extends Command { '--fail-on-change', ], workingDirectory: repoRoot.path); - var hasError = false; - - // Report formatting results. if (formatResult.exitCode != 0) { if (formatResult.stdout.toString().isNotEmpty) { print(formatResult.stdout); @@ -93,40 +128,37 @@ class PreCommitCommand extends Command { print( 'Formatting issues found. Please run "dart run script/tool/bin/flutter_plugin_tools.dart format --run-on-staged-packages" to fix them.', ); - hasError = true; - } else { - print('Formatting looks good!'); + return false; } - // Only run static analysis if formatting passed, to ensure we analyze the final formatted code. - if (!hasError) { - final ProcessResult analyzeResult = await processRunner('dart', [ - 'run', - toolScript, - 'analyze', - '--run-on-staged-packages', - '--dart', - ], workingDirectory: repoRoot.path); - - // Report static analysis results. - if (analyzeResult.exitCode != 0) { - if (analyzeResult.stdout.toString().isNotEmpty) { - print(analyzeResult.stdout); - } - if (analyzeResult.stderr.toString().isNotEmpty) { - print(analyzeResult.stderr); - } - print('Static analysis errors found.'); - hasError = true; - } else { - print('Static analysis looks good!'); - } - } + print('Formatting looks good!'); + return true; + } + + /// Runs the static analysis check on staged files. + /// + /// Returns true if all staged files pass analysis. + Future _checkStaticAnalysis(Directory repoRoot, String toolScript) async { + final ProcessResult analyzeResult = await processRunner('dart', [ + 'run', + toolScript, + 'analyze', + '--run-on-staged-packages', + '--dart', + ], workingDirectory: repoRoot.path); - if (hasError) { + if (analyzeResult.exitCode != 0) { + if (analyzeResult.stdout.toString().isNotEmpty) { + print(analyzeResult.stdout); + } + if (analyzeResult.stderr.toString().isNotEmpty) { + print(analyzeResult.stderr); + } print('Static analysis failed. Please fix the errors listed above.'); + return false; } - return !hasError; + print('Static analysis looks good!'); + return true; } } From 1ccc3076ee5c898a7e1b2a98b1d6a3e57347e23f Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Fri, 26 Jun 2026 09:53:47 -0700 Subject: [PATCH 29/37] fix analysis --- script/githooks/test/pre_commit_test.dart | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/script/githooks/test/pre_commit_test.dart b/script/githooks/test/pre_commit_test.dart index 6f14302ef856..de023768e01e 100644 --- a/script/githooks/test/pre_commit_test.dart +++ b/script/githooks/test/pre_commit_test.dart @@ -5,7 +5,6 @@ import 'dart:io'; import 'package:githooks/src/pre_commit_command.dart'; -import 'package:path/path.dart' as path; import 'package:test/test.dart'; void main() { @@ -26,8 +25,8 @@ void main() { final bool result = await command.run(); expect(result, isTrue); - const String repoRoot = '/mock/repo/root'; - final String toolScript = '$repoRoot/script/tool/bin/flutter_plugin_tools.dart'; + const repoRoot = '/mock/repo/root'; + const toolScript = '$repoRoot/script/tool/bin/flutter_plugin_tools.dart'; // Verify the exact arguments passed to format and analyze expect( @@ -61,8 +60,8 @@ void main() { final bool result = await command.run(); expect(result, isFalse); - const String repoRoot = '/mock/repo/root'; - final String toolScript = '$repoRoot/script/tool/bin/flutter_plugin_tools.dart'; + const repoRoot = '/mock/repo/root'; + const toolScript = '$repoRoot/script/tool/bin/flutter_plugin_tools.dart'; expect( executedArguments, @@ -96,8 +95,8 @@ void main() { final bool result = await command.run(); expect(result, isFalse); - const String repoRoot = '/mock/repo/root'; - final String toolScript = '$repoRoot/script/tool/bin/flutter_plugin_tools.dart'; + const repoRoot = '/mock/repo/root'; + const toolScript = '$repoRoot/script/tool/bin/flutter_plugin_tools.dart'; expect( executedArguments, From 3858e8dc4a05be7c40eeeb229de27d5e3002fd49 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Fri, 26 Jun 2026 09:55:56 -0700 Subject: [PATCH 30/37] clarify if test skipped --- script/githooks/lib/src/pre_commit_command.dart | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/script/githooks/lib/src/pre_commit_command.dart b/script/githooks/lib/src/pre_commit_command.dart index 634035437487..d9c96be3a732 100644 --- a/script/githooks/lib/src/pre_commit_command.dart +++ b/script/githooks/lib/src/pre_commit_command.dart @@ -131,7 +131,12 @@ class PreCommitCommand extends Command { return false; } - print('Formatting looks good!'); + final stdoutStr = formatResult.stdout.toString(); + if (stdoutStr.contains('Ran for 0 package(s)')) { + print('Formatting skipped (no staged packages).'); + } else { + print('Formatting looks good!'); + } return true; } @@ -158,7 +163,12 @@ class PreCommitCommand extends Command { return false; } - print('Static analysis looks good!'); + final String stdoutStr = analyzeResult.stdout.toString(); + if (stdoutStr.contains('Ran for 0 package(s)')) { + print('Static analysis skipped (no staged packages).'); + } else { + print('Static analysis looks good!'); + } return true; } } From dc86fa7aaf2fe629434497e18c337f9281686985 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Fri, 26 Jun 2026 10:01:38 -0700 Subject: [PATCH 31/37] little tweaks --- .../githooks/lib/src/pre_commit_command.dart | 36 ++++++------ script/githooks/test/pre_commit_test.dart | 57 ++++++++++++++++--- 2 files changed, 69 insertions(+), 24 deletions(-) diff --git a/script/githooks/lib/src/pre_commit_command.dart b/script/githooks/lib/src/pre_commit_command.dart index d9c96be3a732..fff24478d8a6 100644 --- a/script/githooks/lib/src/pre_commit_command.dart +++ b/script/githooks/lib/src/pre_commit_command.dart @@ -4,6 +4,7 @@ // ignore_for_file: avoid_print +import 'dart:convert'; import 'dart:io'; import 'package:args/command_runner.dart'; import 'package:path/path.dart' as p; @@ -46,8 +47,8 @@ class PreCommitCommand extends Command { return false; } - if (!await _hasStagedChanges(repoRoot)) { - print('No staged changes to check.'); + if (!await _hasStagedPackages(repoRoot)) { + print('No staged package changes to check.'); return true; } @@ -86,8 +87,11 @@ class PreCommitCommand extends Command { return Directory((rootResult.stdout as String).trim()); } - /// Checks if there are any staged changes in the repository. - Future _hasStagedChanges(Directory repoRoot) async { + /// Checks if there are any staged package changes in the repository. + /// + /// Returns true if at least one staged file is located within a package directory + /// (under packages/ or third_party/packages/). + Future _hasStagedPackages(Directory repoRoot) async { final ProcessResult diffResult = await processRunner('git', [ 'diff', '--cached', @@ -103,7 +107,15 @@ class PreCommitCommand extends Command { return false; } - return (diffResult.stdout as String).trim().isNotEmpty; + final stdoutStr = diffResult.stdout as String; + if (stdoutStr.trim().isEmpty) { + return false; + } + + final List lines = LineSplitter.split(stdoutStr).toList(); + return lines.any((String path) => + path.startsWith('packages/') || + path.startsWith('third_party/packages/')); } /// Runs the formatting check on staged files. @@ -131,12 +143,7 @@ class PreCommitCommand extends Command { return false; } - final stdoutStr = formatResult.stdout.toString(); - if (stdoutStr.contains('Ran for 0 package(s)')) { - print('Formatting skipped (no staged packages).'); - } else { - print('Formatting looks good!'); - } + print('Formatting looks good!'); return true; } @@ -163,12 +170,7 @@ class PreCommitCommand extends Command { return false; } - final String stdoutStr = analyzeResult.stdout.toString(); - if (stdoutStr.contains('Ran for 0 package(s)')) { - print('Static analysis skipped (no staged packages).'); - } else { - print('Static analysis looks good!'); - } + print('Static analysis looks good!'); return true; } } diff --git a/script/githooks/test/pre_commit_test.dart b/script/githooks/test/pre_commit_test.dart index de023768e01e..035aaf6877cd 100644 --- a/script/githooks/test/pre_commit_test.dart +++ b/script/githooks/test/pre_commit_test.dart @@ -15,8 +15,13 @@ void main() { processRunner: (String executable, List arguments, {String? workingDirectory}) async { executedArguments.add(arguments); - if (executable == 'git' && arguments.contains('rev-parse')) { - return ProcessResult(0, 0, '/mock/repo/root\n', ''); + if (executable == 'git') { + if (arguments.contains('rev-parse')) { + return ProcessResult(0, 0, '/mock/repo/root\n', ''); + } + if (arguments.contains('diff')) { + return ProcessResult(0, 0, 'packages/a_plugin/lib/a.dart\n', ''); + } } return ProcessResult(0, 0, 'Success', ''); }, @@ -47,8 +52,13 @@ void main() { processRunner: (String executable, List arguments, {String? workingDirectory}) async { executedArguments.add(arguments); - if (executable == 'git' && arguments.contains('rev-parse')) { - return ProcessResult(0, 0, '/mock/repo/root\n', ''); + if (executable == 'git') { + if (arguments.contains('rev-parse')) { + return ProcessResult(0, 0, '/mock/repo/root\n', ''); + } + if (arguments.contains('diff')) { + return ProcessResult(0, 0, 'packages/a_plugin/lib/a.dart\n', ''); + } } if (arguments.isNotEmpty && arguments.length > 2 && arguments[2] == 'format') { return ProcessResult(0, 1, 'bad_file.dart', ''); @@ -82,8 +92,13 @@ void main() { processRunner: (String executable, List arguments, {String? workingDirectory}) async { executedArguments.add(arguments); - if (executable == 'git' && arguments.contains('rev-parse')) { - return ProcessResult(0, 0, '/mock/repo/root\n', ''); + if (executable == 'git') { + if (arguments.contains('rev-parse')) { + return ProcessResult(0, 0, '/mock/repo/root\n', ''); + } + if (arguments.contains('diff')) { + return ProcessResult(0, 0, 'packages/a_plugin/lib/a.dart\n', ''); + } } if (arguments.isNotEmpty && arguments.length > 2 && arguments[2] == 'analyze') { return ProcessResult(0, 1, 'error in file.dart', ''); @@ -104,7 +119,35 @@ void main() { ); }); - test('exits early with success if there are no staged changes', () async { + test('exits early with success if there are no staged packages', () async { + final List> executedArguments = []; + final command = PreCommitCommand( + processRunner: + (String executable, List arguments, {String? workingDirectory}) async { + executedArguments.add(arguments); + if (executable == 'git') { + if (arguments.contains('rev-parse')) { + return ProcessResult(0, 0, '/mock/repo/root\n', ''); + } + if (arguments.contains('diff')) { + return ProcessResult(0, 0, 'script/githooks/test/pre_commit_test.dart\n', ''); + } + } + return ProcessResult(0, 0, 'Success', ''); + }, + ); + + final bool result = await command.run(); + expect(result, isTrue); + + // Verify that no format or analyze commands were run + expect( + executedArguments.any((args) => args.contains('format') || args.contains('analyze')), + isFalse, + ); + }); + + test('exits early with success if there are no staged changes at all', () async { final List> executedArguments = []; final command = PreCommitCommand( processRunner: From 7a17ad84cc28f186cd2b37a0072c16d778eb71c6 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Fri, 26 Jun 2026 12:09:44 -0700 Subject: [PATCH 32/37] self review --- script/githooks/bin/install_hooks.dart | 6 +- .../githooks/lib/src/pre_commit_command.dart | 21 +++--- script/githooks/test/pre_commit_test.dart | 42 ++++++++---- script/tool/lib/src/format_command.dart | 5 +- script/tool/test/format_command_test.dart | 68 +++++++++++++------ 5 files changed, 91 insertions(+), 51 deletions(-) diff --git a/script/githooks/bin/install_hooks.dart b/script/githooks/bin/install_hooks.dart index e770cd338c1d..385c926a170a 100644 --- a/script/githooks/bin/install_hooks.dart +++ b/script/githooks/bin/install_hooks.dart @@ -16,7 +16,7 @@ void main() async { } if (!(Directory(p.join(repoRoot.path, '.git')).existsSync() || File(p.join(repoRoot.path, '.git')).existsSync())) { - print('❌ Could not find .git directory.'); + print('Installation failed because .git directory could not be found.'); exit(1); } @@ -26,9 +26,9 @@ void main() async { 'script/githooks', ], workingDirectory: repoRoot.path); if (result.exitCode == 0) { - print('✅ Git hooks installed successfully!'); + print('Git hooks installed successfully!'); } else { - print('❌ Failed to install Git hooks: ${result.stderr}'); + print('Failed to install Git hooks: ${result.stderr}'); exit(1); } } diff --git a/script/githooks/lib/src/pre_commit_command.dart b/script/githooks/lib/src/pre_commit_command.dart index fff24478d8a6..042ee6e6d9a8 100644 --- a/script/githooks/lib/src/pre_commit_command.dart +++ b/script/githooks/lib/src/pre_commit_command.dart @@ -37,7 +37,7 @@ class PreCommitCommand extends Command { /// Runs a pre-commit check for correct formatting and static analysis. /// - /// It will check for any staged changes and run the plugin tool format and analyze commands on them. + /// It runs the plugin tool format and analyze commands on all packages that have staged changes. /// If any of the commands fail, it will return false; otherwise, it will return true. @override Future run() async { @@ -47,7 +47,11 @@ class PreCommitCommand extends Command { return false; } - if (!await _hasStagedPackages(repoRoot)) { + final bool? hasStaged = await _hasStagedPackages(repoRoot); + if (hasStaged == null) { + return false; + } + if (!hasStaged) { print('No staged package changes to check.'); return true; } @@ -90,8 +94,9 @@ class PreCommitCommand extends Command { /// Checks if there are any staged package changes in the repository. /// /// Returns true if at least one staged file is located within a package directory - /// (under packages/ or third_party/packages/). - Future _hasStagedPackages(Directory repoRoot) async { + /// (under packages/ or third_party/packages/), false if there are none, or null + /// if the git command fails. + Future _hasStagedPackages(Directory repoRoot) async { final ProcessResult diffResult = await processRunner('git', [ 'diff', '--cached', @@ -103,8 +108,8 @@ class PreCommitCommand extends Command { if (diffResult.stderr.toString().isNotEmpty) { print(diffResult.stderr); } - // If we cannot determine the diff, abort pre-commit check. - return false; + // If we cannot determine the diff, abort pre-commit check by returning null. + return null; } final stdoutStr = diffResult.stdout as String; @@ -120,7 +125,7 @@ class PreCommitCommand extends Command { /// Runs the formatting check on staged files. /// - /// Returns true if all staged files are correctly formatted. + /// Returns true if all staged files are correctly formatted or false otherwise. Future _checkFormatting(Directory repoRoot, String toolScript) async { final ProcessResult formatResult = await processRunner('dart', [ 'run', @@ -149,7 +154,7 @@ class PreCommitCommand extends Command { /// Runs the static analysis check on staged files. /// - /// Returns true if all staged files pass analysis. + /// Returns true if all staged files pass analysis or false otherwise. Future _checkStaticAnalysis(Directory repoRoot, String toolScript) async { final ProcessResult analyzeResult = await processRunner('dart', [ 'run', diff --git a/script/githooks/test/pre_commit_test.dart b/script/githooks/test/pre_commit_test.dart index 035aaf6877cd..f3c772b811fc 100644 --- a/script/githooks/test/pre_commit_test.dart +++ b/script/githooks/test/pre_commit_test.dart @@ -8,6 +8,9 @@ import 'package:githooks/src/pre_commit_command.dart'; import 'package:test/test.dart'; void main() { + const repoRoot = '/mock/repo/root'; + const toolScript = '$repoRoot/script/tool/bin/flutter_plugin_tools.dart'; + group('pre-commit hook', () { test('passes when both format and analyze succeed', () async { final List> executedArguments = []; @@ -17,7 +20,7 @@ void main() { executedArguments.add(arguments); if (executable == 'git') { if (arguments.contains('rev-parse')) { - return ProcessResult(0, 0, '/mock/repo/root\n', ''); + return ProcessResult(0, 0, '$repoRoot\n', ''); } if (arguments.contains('diff')) { return ProcessResult(0, 0, 'packages/a_plugin/lib/a.dart\n', ''); @@ -30,9 +33,6 @@ void main() { final bool result = await command.run(); expect(result, isTrue); - const repoRoot = '/mock/repo/root'; - const toolScript = '$repoRoot/script/tool/bin/flutter_plugin_tools.dart'; - // Verify the exact arguments passed to format and analyze expect( executedArguments, @@ -54,7 +54,7 @@ void main() { executedArguments.add(arguments); if (executable == 'git') { if (arguments.contains('rev-parse')) { - return ProcessResult(0, 0, '/mock/repo/root\n', ''); + return ProcessResult(0, 0, '$repoRoot\n', ''); } if (arguments.contains('diff')) { return ProcessResult(0, 0, 'packages/a_plugin/lib/a.dart\n', ''); @@ -70,9 +70,6 @@ void main() { final bool result = await command.run(); expect(result, isFalse); - const repoRoot = '/mock/repo/root'; - const toolScript = '$repoRoot/script/tool/bin/flutter_plugin_tools.dart'; - expect( executedArguments, anyElement( @@ -94,7 +91,7 @@ void main() { executedArguments.add(arguments); if (executable == 'git') { if (arguments.contains('rev-parse')) { - return ProcessResult(0, 0, '/mock/repo/root\n', ''); + return ProcessResult(0, 0, '$repoRoot\n', ''); } if (arguments.contains('diff')) { return ProcessResult(0, 0, 'packages/a_plugin/lib/a.dart\n', ''); @@ -110,9 +107,6 @@ void main() { final bool result = await command.run(); expect(result, isFalse); - const repoRoot = '/mock/repo/root'; - const toolScript = '$repoRoot/script/tool/bin/flutter_plugin_tools.dart'; - expect( executedArguments, anyElement(equals(['run', toolScript, 'analyze', '--run-on-staged-packages', '--dart'])), @@ -127,7 +121,7 @@ void main() { executedArguments.add(arguments); if (executable == 'git') { if (arguments.contains('rev-parse')) { - return ProcessResult(0, 0, '/mock/repo/root\n', ''); + return ProcessResult(0, 0, '$repoRoot\n', ''); } if (arguments.contains('diff')) { return ProcessResult(0, 0, 'script/githooks/test/pre_commit_test.dart\n', ''); @@ -155,7 +149,7 @@ void main() { executedArguments.add(arguments); if (executable == 'git') { if (arguments.contains('rev-parse')) { - return ProcessResult(0, 0, '/mock/repo/root\n', ''); + return ProcessResult(0, 0, '$repoRoot\n', ''); } if (arguments.contains('diff')) { return ProcessResult(0, 0, '', ''); @@ -175,6 +169,26 @@ void main() { ); }); + test('fails when checking staged packages fails', () async { + final command = PreCommitCommand( + processRunner: + (String executable, List arguments, {String? workingDirectory}) async { + if (executable == 'git') { + if (arguments.contains('rev-parse')) { + return ProcessResult(0, 0, '$repoRoot\n', ''); + } + if (arguments.contains('diff')) { + return ProcessResult(0, 1, '', 'Git diff failed'); + } + } + return ProcessResult(0, 0, 'Success', ''); + }, + ); + + final bool result = await command.run(); + expect(result, isFalse); + }); + test('fails when git root cannot be found', () async { final command = PreCommitCommand( processRunner: diff --git a/script/tool/lib/src/format_command.dart b/script/tool/lib/src/format_command.dart index 5c5f9c932b71..a2ebba0f8205 100644 --- a/script/tool/lib/src/format_command.dart +++ b/script/tool/lib/src/format_command.dart @@ -393,10 +393,7 @@ class FormatCommand extends PackageLoopingCommand { const handFormattedExtension = '.dart'; const handFormattedPragma = '// This file is hand-formatted.'; - final bool useDiff = getBoolArg('run-on-staged-packages') || - getBoolArg('run-on-dirty-packages') || - getBoolArg('run-on-changed-packages') || - getBoolArg('packages-for-branch'); + final bool useDiff = getBoolArg('run-on-staged-packages'); return files .where((File file) { diff --git a/script/tool/test/format_command_test.dart b/script/tool/test/format_command_test.dart index ba01b840329f..db3459890db2 100644 --- a/script/tool/test/format_command_test.dart +++ b/script/tool/test/format_command_test.dart @@ -180,7 +180,7 @@ void main() { ); }); - test('formats only uncommitted files when --run-on-dirty-packages is used', () async { + test('formats only staged files when --run-on-staged-packages is used', () async { const files = ['lib/a.dart', 'lib/src/b.dart', 'lib/src/c.dart']; final RepositoryPackage plugin = createFakePlugin( 'a_plugin', @@ -190,44 +190,67 @@ void main() { ); fakePubGet(plugin); - // Mock git diff to return only lib/a.dart (called three times) - const changedFilePath = 'packages/a_plugin/lib/a.dart'; - gitProcessRunner.mockProcessesForExecutable['git-diff'] = [ - FakeProcessInfo(MockProcess(stdout: changedFilePath)), - FakeProcessInfo(MockProcess(stdout: changedFilePath)), - FakeProcessInfo(MockProcess(stdout: changedFilePath)), - ]; + // Mock git diff --cached to return only lib/src/b.dart (called three times) + const stagedFilePath = 'packages/a_plugin/lib/src/b.dart'; + gitProcessRunner.mockProcessesForExecutable['git-diff'] = List.generate( + 3, + (_) => FakeProcessInfo(MockProcess(stdout: stagedFilePath)), + ); await runCapturingPrint(runner, [ 'format', - '--run-on-dirty-packages', + '--run-on-staged-packages', ]); expect( processRunner.recordedCalls, orderedEquals([ - ProcessCall('dart', const ['format', 'lib/a.dart'], plugin.path), + ProcessCall('dart', const ['format', 'lib/src/b.dart'], plugin.path), ]), ); }); - test('formats only staged files when --run-on-staged-packages is used', () async { - const files = ['lib/a.dart', 'lib/src/b.dart', 'lib/src/c.dart']; + test('skips formatting when there are no staged packages', () async { final RepositoryPackage plugin = createFakePlugin( 'a_plugin', packagesDir, - extraFiles: files, dartConstraint: _dartConstraint, ); fakePubGet(plugin); - // Mock git diff --cached to return only lib/src/b.dart (called three times) - const stagedFilePath = 'packages/a_plugin/lib/src/b.dart'; - gitProcessRunner.mockProcessesForExecutable['git-diff'] = [ - FakeProcessInfo(MockProcess(stdout: stagedFilePath)), - FakeProcessInfo(MockProcess(stdout: stagedFilePath)), - FakeProcessInfo(MockProcess(stdout: stagedFilePath)), - ]; + gitProcessRunner.mockProcessesForExecutable['git-diff'] = List.generate( + 3, + (_) => FakeProcessInfo(MockProcess(stdout: '')), + ); + + await runCapturingPrint(runner, ['format', '--run-on-staged-packages']); + + expect(processRunner.recordedCalls, isEmpty); + }); + + test('formats only staged files across multiple packages', () async { + final RepositoryPackage pluginA = createFakePlugin( + 'plugin_a', + packagesDir, + extraFiles: ['lib/a.dart', 'lib/src/unused_a.dart'], + dartConstraint: _dartConstraint, + ); + fakePubGet(pluginA); + + final RepositoryPackage pluginB = createFakePlugin( + 'plugin_b', + packagesDir, + extraFiles: ['lib/b.dart', 'lib/src/unused_b.dart'], + dartConstraint: _dartConstraint, + ); + fakePubGet(pluginB); + + // Mock git diff to return both staged files (called three times) + const stagedFiles = 'packages/plugin_a/lib/a.dart\npackages/plugin_b/lib/b.dart\n'; + gitProcessRunner.mockProcessesForExecutable['git-diff'] = List.generate( + 3, + (_) => FakeProcessInfo(MockProcess(stdout: stagedFiles)), + ); await runCapturingPrint(runner, [ 'format', @@ -236,8 +259,9 @@ void main() { expect( processRunner.recordedCalls, - orderedEquals([ - ProcessCall('dart', const ['format', 'lib/src/b.dart'], plugin.path), + unorderedEquals([ + ProcessCall('dart', const ['format', 'lib/a.dart'], pluginA.path), + ProcessCall('dart', const ['format', 'lib/b.dart'], pluginB.path), ]), ); }); From 55d6235a2e404f3b0facee95dc8935b689a89be4 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Fri, 26 Jun 2026 12:26:53 -0700 Subject: [PATCH 33/37] test nits --- script/githooks/test/pre_commit_test.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/script/githooks/test/pre_commit_test.dart b/script/githooks/test/pre_commit_test.dart index f3c772b811fc..d1576fab2636 100644 --- a/script/githooks/test/pre_commit_test.dart +++ b/script/githooks/test/pre_commit_test.dart @@ -60,7 +60,7 @@ void main() { return ProcessResult(0, 0, 'packages/a_plugin/lib/a.dart\n', ''); } } - if (arguments.isNotEmpty && arguments.length > 2 && arguments[2] == 'format') { + if (arguments.contains('format')) { return ProcessResult(0, 1, 'bad_file.dart', ''); } return ProcessResult(0, 0, 'Success', ''); @@ -97,7 +97,7 @@ void main() { return ProcessResult(0, 0, 'packages/a_plugin/lib/a.dart\n', ''); } } - if (arguments.isNotEmpty && arguments.length > 2 && arguments[2] == 'analyze') { + if (arguments.contains('analyze')) { return ProcessResult(0, 1, 'error in file.dart', ''); } return ProcessResult(0, 0, 'Success', ''); From db09b0604d7137ef6d62881b9fc84f740286d280 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Mon, 29 Jun 2026 15:03:03 -0700 Subject: [PATCH 34/37] address review --- script/githooks/README.md | 30 +++++ .../githooks/lib/src/pre_commit_command.dart | 51 ++++---- script/githooks/pubspec.yaml | 2 +- ...test.dart => pre_commit_command_test.dart} | 18 +-- .../lib/src/common/git_version_finder.dart | 42 ++++-- .../tool/lib/src/common/package_command.dart | 25 ++-- .../src/common/package_looping_command.dart | 12 +- script/tool/lib/src/format_command.dart | 3 +- .../test/common/git_version_finder_test.dart | 58 +++++---- .../test/common/package_command_test.dart | 39 +++++- script/tool/test/format_command_test.dart | 120 ++++++++++++++++++ 11 files changed, 303 insertions(+), 97 deletions(-) create mode 100644 script/githooks/README.md rename script/githooks/test/{pre_commit_test.dart => pre_commit_command_test.dart} (95%) diff --git a/script/githooks/README.md b/script/githooks/README.md new file mode 100644 index 000000000000..bdf5563a8ddd --- /dev/null +++ b/script/githooks/README.md @@ -0,0 +1,30 @@ +# Git Hooks + +This directory contains Git hooks for the `flutter/packages` repository. + +## Installation + +To install the Git hooks, run the following commands from the root of the repository: + +```bash +# Fetch dependencies for the githooks package +dart pub get -C script/githooks + +# Run the installation script +dart script/githooks/bin/install_hooks.dart +``` + +## Available Hooks + +### pre-commit + +The `pre-commit` hook runs automatically when you run `git commit` and performs the following on any staged changes: + +1. **Formatting**: It runs `flutter_plugin_tools format --run-on-staged-packages` to verify that all staged files in the targeted packages are correctly formatted. +2. **Static Analysis**: If formatting passes, it runs `flutter_plugin_tools analyze --run-on-staged-packages --dart` to run static analysis on the staged packages. + +If either check fails, it aborts the commit. To bypass the hook (for a WIP commit, for example), you can use the `--no-verify` flag: + +```bash +git commit -m "WIP" --no-verify +``` diff --git a/script/githooks/lib/src/pre_commit_command.dart b/script/githooks/lib/src/pre_commit_command.dart index 042ee6e6d9a8..694e3984881c 100644 --- a/script/githooks/lib/src/pre_commit_command.dart +++ b/script/githooks/lib/src/pre_commit_command.dart @@ -4,7 +4,6 @@ // ignore_for_file: avoid_print -import 'dart:convert'; import 'dart:io'; import 'package:args/command_runner.dart'; import 'package:path/path.dart' as p; @@ -33,7 +32,8 @@ class PreCommitCommand extends Command { final String name = 'pre-commit'; @override - final String description = 'Checks to run before a "git commit"'; + final String description = + 'Runs formatting and static analysis checks on staged changes before a "git commit"'; /// Runs a pre-commit check for correct formatting and static analysis. /// @@ -67,13 +67,12 @@ class PreCommitCommand extends Command { print('Running pre-commit format and static analysis checks for staged changes...'); // Check format. - final bool formatPassed = await _checkFormatting(repoRoot, toolScript); + final bool formatPassed = await _executeCheckFormatting(repoRoot, toolScript); if (!formatPassed) { return false; } - // Check static analysis if format passed. - return _checkStaticAnalysis(repoRoot, toolScript); + return _executeCheckStaticAnalysis(repoRoot, toolScript); } /// Finds the repository root directory using git. @@ -100,6 +99,7 @@ class PreCommitCommand extends Command { final ProcessResult diffResult = await processRunner('git', [ 'diff', '--cached', + '-z', '--name-only', ], workingDirectory: repoRoot.path); @@ -113,20 +113,21 @@ class PreCommitCommand extends Command { } final stdoutStr = diffResult.stdout as String; - if (stdoutStr.trim().isEmpty) { + if (stdoutStr.isEmpty) { return false; } - final List lines = LineSplitter.split(stdoutStr).toList(); - return lines.any((String path) => - path.startsWith('packages/') || - path.startsWith('third_party/packages/')); + final List lines = stdoutStr.split('\u0000') + ..removeWhere((String element) => element.isEmpty); + return lines.any( + (String path) => path.startsWith('packages/') || path.startsWith('third_party/packages/'), + ); } /// Runs the formatting check on staged files. /// /// Returns true if all staged files are correctly formatted or false otherwise. - Future _checkFormatting(Directory repoRoot, String toolScript) async { + Future _executeCheckFormatting(Directory repoRoot, String toolScript) async { final ProcessResult formatResult = await processRunner('dart', [ 'run', toolScript, @@ -136,15 +137,11 @@ class PreCommitCommand extends Command { ], workingDirectory: repoRoot.path); if (formatResult.exitCode != 0) { - if (formatResult.stdout.toString().isNotEmpty) { - print(formatResult.stdout); - } - if (formatResult.stderr.toString().isNotEmpty) { - print(formatResult.stderr); - } - print( - 'Formatting issues found. Please run "dart run script/tool/bin/flutter_plugin_tools.dart format --run-on-staged-packages" to fix them.', - ); + print(''' +Formatting check failed. +To fix formatting automatically, run: + dart run script/tool/bin/flutter_plugin_tools.dart format --run-on-staged-packages +To bypass this check, commit with --no-verify.'''); return false; } @@ -155,7 +152,7 @@ class PreCommitCommand extends Command { /// Runs the static analysis check on staged files. /// /// Returns true if all staged files pass analysis or false otherwise. - Future _checkStaticAnalysis(Directory repoRoot, String toolScript) async { + Future _executeCheckStaticAnalysis(Directory repoRoot, String toolScript) async { final ProcessResult analyzeResult = await processRunner('dart', [ 'run', toolScript, @@ -165,13 +162,11 @@ class PreCommitCommand extends Command { ], workingDirectory: repoRoot.path); if (analyzeResult.exitCode != 0) { - if (analyzeResult.stdout.toString().isNotEmpty) { - print(analyzeResult.stdout); - } - if (analyzeResult.stderr.toString().isNotEmpty) { - print(analyzeResult.stderr); - } - print('Static analysis failed. Please fix the errors listed above.'); + print(''' +Static analysis check failed. +To view and fix analysis errors, run: + dart run script/tool/bin/flutter_plugin_tools.dart analyze --run-on-staged-packages --dart +To bypass this check, commit with --no-verify.'''); return false; } diff --git a/script/githooks/pubspec.yaml b/script/githooks/pubspec.yaml index be8db13c80c0..45f95e4db240 100644 --- a/script/githooks/pubspec.yaml +++ b/script/githooks/pubspec.yaml @@ -1,5 +1,5 @@ name: githooks -description: Git hooks for the flutter/packages repository. +description: Git hooks for the flutter/packages repository. For more information and instructions to install the hooks, see README.md publish_to: none environment: diff --git a/script/githooks/test/pre_commit_test.dart b/script/githooks/test/pre_commit_command_test.dart similarity index 95% rename from script/githooks/test/pre_commit_test.dart rename to script/githooks/test/pre_commit_command_test.dart index d1576fab2636..5da81df28b3b 100644 --- a/script/githooks/test/pre_commit_test.dart +++ b/script/githooks/test/pre_commit_command_test.dart @@ -23,7 +23,7 @@ void main() { return ProcessResult(0, 0, '$repoRoot\n', ''); } if (arguments.contains('diff')) { - return ProcessResult(0, 0, 'packages/a_plugin/lib/a.dart\n', ''); + return ProcessResult(0, 0, 'packages/a_plugin/lib/a.dart\u0000', ''); } } return ProcessResult(0, 0, 'Success', ''); @@ -57,7 +57,7 @@ void main() { return ProcessResult(0, 0, '$repoRoot\n', ''); } if (arguments.contains('diff')) { - return ProcessResult(0, 0, 'packages/a_plugin/lib/a.dart\n', ''); + return ProcessResult(0, 0, 'packages/a_plugin/lib/a.dart\u0000', ''); } } if (arguments.contains('format')) { @@ -77,10 +77,7 @@ void main() { ), ); // Verify that static analysis was skipped because formatting failed - expect( - executedArguments.any((args) => args.contains('analyze')), - isFalse, - ); + expect(executedArguments.any((args) => args.contains('analyze')), isFalse); }); test('fails when analysis fails', () async { @@ -94,7 +91,7 @@ void main() { return ProcessResult(0, 0, '$repoRoot\n', ''); } if (arguments.contains('diff')) { - return ProcessResult(0, 0, 'packages/a_plugin/lib/a.dart\n', ''); + return ProcessResult(0, 0, 'packages/a_plugin/lib/a.dart\u0000', ''); } } if (arguments.contains('analyze')) { @@ -124,7 +121,12 @@ void main() { return ProcessResult(0, 0, '$repoRoot\n', ''); } if (arguments.contains('diff')) { - return ProcessResult(0, 0, 'script/githooks/test/pre_commit_test.dart\n', ''); + return ProcessResult( + 0, + 0, + 'script/githooks/test/pre_commit_command_test.dart\u0000', + '', + ); } } return ProcessResult(0, 0, 'Success', ''); diff --git a/script/tool/lib/src/common/git_version_finder.dart b/script/tool/lib/src/common/git_version_finder.dart index 7479bcb4a91d..4b34ceae7eaf 100644 --- a/script/tool/lib/src/common/git_version_finder.dart +++ b/script/tool/lib/src/common/git_version_finder.dart @@ -35,34 +35,50 @@ class GitVersionFinder { final String baseSha = await getBaseSha(); final io.ProcessResult changedFilesCommand = await baseGitDir.runCommand([ 'diff', + '-z', '--name-only', baseSha, if (!includeUncommitted) 'HEAD', ]); - final changedFilesStdout = changedFilesCommand.stdout.toString(); - if (changedFilesStdout.isEmpty) { - return []; - } - final List changedFiles = changedFilesStdout.split('\n') - ..removeWhere((String element) => element.isEmpty); - return changedFiles.toList(); + return _splitDiffOutputs(changedFilesCommand.stdout.toString()); } /// Get a list of all the staged files. Future> getStagedFiles() async { - final io.ProcessResult changedFilesCommand = await baseGitDir.runCommand([ + final io.ProcessResult changedFilesCommand = await baseGitDir.runCommand(const [ 'diff', '--cached', + '-z', '--name-only', '--diff-filter=ACM', ]); - final changedFilesStdout = changedFilesCommand.stdout.toString(); - if (changedFilesStdout.isEmpty) { + return _splitDiffOutputs(changedFilesCommand.stdout.toString()); + } + + /// Splits the stdout of a `git diff` command into a list of file paths. + /// + /// When `git diff` is run with the `-z` flag, it outputs file paths separated + /// by a null byte (`\u0000`, ASCII 0). This is the safest way to parse filenames + /// because: + /// 1. Filenames can contain spaces, quotes, and newlines, but they can never + /// contain null bytes. + /// 2. Git does not quote or escape "unusual" characters in filenames when + /// using `-z`, giving us the raw path. + /// + /// For backward compatibility with legacy unit tests that mock `git diff` + /// using newline-terminated strings, this helper falls back to splitting + /// by `\n` if no null bytes are detected in the output. + List _splitDiffOutputs(String stdout) { + if (stdout.isEmpty) { return []; } - final List changedFiles = changedFilesStdout.split('\n') - ..removeWhere((String element) => element.isEmpty); - return changedFiles.toList(); + final List files; + if (stdout.contains('\u0000')) { + files = stdout.split('\u0000'); + } else { + files = stdout.split('\n'); + } + return files.where((String file) => file.isNotEmpty).toList(); } /// Get a list of all the changed files. diff --git a/script/tool/lib/src/common/package_command.dart b/script/tool/lib/src/common/package_command.dart index 410227ee457e..636c80f62dd9 100644 --- a/script/tool/lib/src/common/package_command.dart +++ b/script/tool/lib/src/common/package_command.dart @@ -96,7 +96,7 @@ abstract class PackageCommand extends Command { 'Cannot be combined with $_packagesArg.\n', ); argParser.addFlag( - _runOnDirtyPackagesArg, + runOnDirtyPackagesArg, negatable: false, help: 'Run the command on packages with changes that have not been committed.\n' @@ -159,22 +159,27 @@ abstract class PackageCommand extends Command { 'but more information may be added in the future.', ); argParser.addFlag( - _runOnStagedPackagesArg, + runOnStagedPackagesArg, help: 'Run the command on all packages with staged changes.', ); } // Package selection. static const String _packagesArg = 'packages'; - static const String _runOnStagedPackagesArg = 'run-on-staged-packages'; static const String _packagesForBranchArg = 'packages-for-branch'; static const String _currentPackageArg = 'current-package'; static const String _pluginsLegacyAliasArg = 'plugins'; static const String _runOnChangedPackagesArg = 'run-on-changed-packages'; - static const String _runOnDirtyPackagesArg = 'run-on-dirty-packages'; static const String _exactMatchOnlyArg = 'exact-match-only'; static const String _excludeArg = 'exclude'; static const String _filterPackagesArg = 'filter-packages-to'; + + /// Package selection flag for packages with staged changes. + static const String runOnStagedPackagesArg = 'run-on-staged-packages'; + + /// Package selection flag for packages with any changes (staged or not). + static const String runOnDirtyPackagesArg = 'run-on-dirty-packages'; + // Diff base selection. static const String _baseBranchArg = 'base-branch'; static const String _baseShaArg = 'base-sha'; @@ -389,10 +394,10 @@ abstract class PackageCommand extends Command { final packageSelectionFlags = { _packagesArg, _runOnChangedPackagesArg, - _runOnDirtyPackagesArg, + runOnDirtyPackagesArg, _packagesForBranchArg, _currentPackageArg, - _runOnStagedPackagesArg, + runOnStagedPackagesArg, }; if (packageSelectionFlags.where((String flag) => argResults!.wasParsed(flag)).length > 1) { printError( @@ -410,8 +415,8 @@ abstract class PackageCommand extends Command { final bool allowGroupMatching = !(getBoolArg(_exactMatchOnlyArg) || argResults!.wasParsed(_runOnChangedPackagesArg) || - argResults!.wasParsed(_runOnDirtyPackagesArg) || - argResults!.wasParsed(_runOnStagedPackagesArg) || + argResults!.wasParsed(runOnDirtyPackagesArg) || + argResults!.wasParsed(runOnStagedPackagesArg) || argResults!.wasParsed(_packagesForBranchArg)); var packages = Set.from(getStringListArg(_packagesArg)); @@ -465,7 +470,7 @@ abstract class PackageCommand extends Command { print('Running for all packages that have diffs relative to "$baseSha"\n'); packages = _getChangedPackageNames(changedFiles); } - } else if (getBoolArg(_runOnDirtyPackagesArg)) { + } else if (getBoolArg(runOnDirtyPackagesArg)) { final gitVersionFinder = GitVersionFinder(await gitDir, baseSha: 'HEAD'); print('Running for all packages that have uncommitted changes\n'); // _changesRequireFullTest is deliberately not used here, as this flag is @@ -489,7 +494,7 @@ abstract class PackageCommand extends Command { throw ToolExit(exitInvalidArguments); } packages = {currentPackageName}; - } else if (getBoolArg(_runOnStagedPackagesArg)) { + } else if (getBoolArg(runOnStagedPackagesArg)) { final gitVersionFinder = GitVersionFinder(await gitDir, baseSha: 'HEAD'); print('Running for all packages that have staged changes\n'); packages = _getChangedPackageNames( diff --git a/script/tool/lib/src/common/package_looping_command.dart b/script/tool/lib/src/common/package_looping_command.dart index 89bc3729c028..0d7f528da23f 100644 --- a/script/tool/lib/src/common/package_looping_command.dart +++ b/script/tool/lib/src/common/package_looping_command.dart @@ -296,14 +296,14 @@ abstract class PackageLoopingCommand extends PackageCommand { final runStart = DateTime.now(); // Populate the list of changed files for subclasses to use. - if (getBoolArg('run-on-staged-packages')) { + if (getBoolArg(PackageCommand.runOnDirtyPackagesArg)) { baseSha = 'HEAD'; - final GitVersionFinder gitVersionFinder = GitVersionFinder(await gitDir, baseSha: baseSha); - changedFiles = await gitVersionFinder.getStagedFiles(); - } else if (getBoolArg('run-on-dirty-packages')) { - baseSha = 'HEAD'; - final GitVersionFinder gitVersionFinder = GitVersionFinder(await gitDir, baseSha: baseSha); + final gitVersionFinder = GitVersionFinder(await gitDir, baseSha: baseSha); changedFiles = await gitVersionFinder.getChangedFiles(includeUncommitted: true); + } else if (getBoolArg(PackageCommand.runOnStagedPackagesArg)) { + baseSha = 'HEAD'; + final gitVersionFinder = GitVersionFinder(await gitDir, baseSha: baseSha); + changedFiles = await gitVersionFinder.getStagedFiles(); } else { final GitVersionFinder gitVersionFinder = await retrieveVersionFinder(); baseSha = await gitVersionFinder.getBaseSha(); diff --git a/script/tool/lib/src/format_command.dart b/script/tool/lib/src/format_command.dart index a2ebba0f8205..d0d32649a618 100644 --- a/script/tool/lib/src/format_command.dart +++ b/script/tool/lib/src/format_command.dart @@ -12,6 +12,7 @@ import 'package:pub_semver/pub_semver.dart'; import 'common/core.dart'; import 'common/output_utils.dart'; +import 'common/package_command.dart' show PackageCommand; import 'common/package_looping_command.dart'; import 'common/pub_utils.dart'; import 'common/repository_package.dart'; @@ -393,7 +394,7 @@ class FormatCommand extends PackageLoopingCommand { const handFormattedExtension = '.dart'; const handFormattedPragma = '// This file is hand-formatted.'; - final bool useDiff = getBoolArg('run-on-staged-packages'); + final bool useDiff = getBoolArg(PackageCommand.runOnStagedPackagesArg); return files .where((File file) { diff --git a/script/tool/test/common/git_version_finder_test.dart b/script/tool/test/common/git_version_finder_test.dart index 9ac9dd2f4759..8ea776ceae40 100644 --- a/script/tool/test/common/git_version_finder_test.dart +++ b/script/tool/test/common/git_version_finder_test.dart @@ -43,10 +43,7 @@ void main() { }); test('get correct files changed based on git diff', () async { - gitDiffResponse = ''' -file1/file1.cc -file2/file2.cc -'''; + gitDiffResponse = 'file1/file1.cc\u0000file2/file2.cc\u0000'; final finder = GitVersionFinder(gitDir, baseSha: 'some base sha'); final List changedFiles = await finder.getChangedFiles(); @@ -55,10 +52,7 @@ file2/file2.cc test('use correct base sha if not specified', () async { mergeBaseResponse = 'shaqwiueroaaidf12312jnadf123nd'; - gitDiffResponse = ''' -file1/pubspec.yaml -file2/file2.cc -'''; + gitDiffResponse = 'file1/pubspec.yaml\u0000file2/file2.cc\u0000'; final finder = GitVersionFinder(gitDir); await finder.getChangedFiles(); @@ -70,15 +64,12 @@ file2/file2.cc 'HEAD', ], throwOnError: false), ); - verify(gitDir.runCommand(['diff', '--name-only', mergeBaseResponse, 'HEAD'])); + verify(gitDir.runCommand(['diff', '-z', '--name-only', mergeBaseResponse, 'HEAD'])); }); test('uses correct base branch to find base sha if specified', () async { mergeBaseResponse = 'shaqwiueroaaidf12312jnadf123nd'; - gitDiffResponse = ''' -file1/pubspec.yaml -file2/file2.cc -'''; + gitDiffResponse = 'file1/pubspec.yaml\u0000file2/file2.cc\u0000'; final finder = GitVersionFinder(gitDir, baseBranch: 'upstream/main'); await finder.getChangedFiles(); @@ -90,29 +81,48 @@ file2/file2.cc 'HEAD', ], throwOnError: false), ); - verify(gitDir.runCommand(['diff', '--name-only', mergeBaseResponse, 'HEAD'])); + verify(gitDir.runCommand(['diff', '-z', '--name-only', mergeBaseResponse, 'HEAD'])); }); test('use correct base sha if specified', () async { const customBaseSha = 'aklsjdcaskf12312'; - gitDiffResponse = ''' -file1/pubspec.yaml -file2/file2.cc -'''; + gitDiffResponse = 'file1/pubspec.yaml\u0000file2/file2.cc\u0000'; final finder = GitVersionFinder(gitDir, baseSha: customBaseSha); await finder.getChangedFiles(); - verify(gitDir.runCommand(['diff', '--name-only', customBaseSha, 'HEAD'])); + verify(gitDir.runCommand(['diff', '-z', '--name-only', customBaseSha, 'HEAD'])); }); test('include uncommitted files if requested', () async { const customBaseSha = 'aklsjdcaskf12312'; - gitDiffResponse = ''' -file1/pubspec.yaml -file2/file2.cc -'''; + gitDiffResponse = 'file1/pubspec.yaml\u0000file2/file2.cc\u0000'; final finder = GitVersionFinder(gitDir, baseSha: customBaseSha); await finder.getChangedFiles(includeUncommitted: true); // The call should not have HEAD as a final argument like the default diff. - verify(gitDir.runCommand(['diff', '--name-only', customBaseSha])); + verify(gitDir.runCommand(['diff', '-z', '--name-only', customBaseSha])); + }); + + test('handles filenames with spaces and quotes', () async { + gitDiffResponse = 'file name with space.cc\u0000file"with"quote.cc\u0000'; + final finder = GitVersionFinder(gitDir, baseSha: 'some base sha'); + final List changedFiles = await finder.getChangedFiles(); + + expect(changedFiles, equals(['file name with space.cc', 'file"with"quote.cc'])); + }); + + test('handles legacy newline-terminated output', () async { + gitDiffResponse = 'file1.cc\nfile2.cc\n'; + final finder = GitVersionFinder(gitDir, baseSha: 'some base sha'); + final List changedFiles = await finder.getChangedFiles(); + + expect(changedFiles, equals(['file1.cc', 'file2.cc'])); + }); + + test('throws ProcessException when git diff fails', () async { + when( + gitDir.runCommand(argThat(contains('diff')), throwOnError: anyNamed('throwOnError')), + ).thenThrow(const ProcessException('git', ['diff'], 'Git diff failed')); + + final finder = GitVersionFinder(gitDir, baseSha: 'some base sha'); + expect(() => finder.getChangedFiles(), throwsA(isA())); }); } diff --git a/script/tool/test/common/package_command_test.dart b/script/tool/test/common/package_command_test.dart index d1bf459b5cde..2aa109f050aa 100644 --- a/script/tool/test/common/package_command_test.dart +++ b/script/tool/test/common/package_command_test.dart @@ -349,7 +349,24 @@ packages/plugin1/plugin1/plugin1.dart Error? commandError; final List output = await runCapturingPrint( runner, - ['sample', '--packages-for-branch', '--packages=plugin1'], + ['sample', '--packages-for-branch', '--run-on-changed-packages'], + errorHandler: (Error e) { + commandError = e; + }, + ); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([contains('Only one of the package selection arguments')]), + ); + }); + + test('does not allow --run-on-dirty-packages with --run-on-staged-packages', () async { + Error? commandError; + final List output = await runCapturingPrint( + runner, + ['sample', '--run-on-dirty-packages', '--run-on-staged-packages'], errorHandler: (Error e) { commandError = e; }, @@ -1102,7 +1119,9 @@ packages/b_package/lib/src/foo.dart // Ensure that it's diffing against the merge-base. expect( gitProcessRunner.recordedCalls, - contains(const ProcessCall('git-diff', ['--name-only', 'abc123', 'HEAD'], null)), + contains( + const ProcessCall('git-diff', ['-z', '--name-only', 'abc123', 'HEAD'], null), + ), ); }); @@ -1133,7 +1152,9 @@ packages/b_package/lib/src/foo.dart // Ensure that it's diffing against the prior commit. expect( gitProcessRunner.recordedCalls, - contains(const ProcessCall('git-diff', ['--name-only', 'HEAD~', 'HEAD'], null)), + contains( + const ProcessCall('git-diff', ['-z', '--name-only', 'HEAD~', 'HEAD'], null), + ), ); }); @@ -1165,7 +1186,9 @@ packages/b_package/lib/src/foo.dart // Ensure that it's diffing against the prior commit. expect( gitProcessRunner.recordedCalls, - contains(const ProcessCall('git-diff', ['--name-only', 'HEAD~', 'HEAD'], null)), + contains( + const ProcessCall('git-diff', ['-z', '--name-only', 'HEAD~', 'HEAD'], null), + ), ); }); @@ -1209,7 +1232,9 @@ packages/b_package/lib/src/foo.dart // Ensure that it's diffing against the prior commit. expect( gitProcessRunner.recordedCalls, - contains(const ProcessCall('git-diff', ['--name-only', 'HEAD~', 'HEAD'], null)), + contains( + const ProcessCall('git-diff', ['-z', '--name-only', 'HEAD~', 'HEAD'], null), + ), ); }); @@ -1240,7 +1265,9 @@ packages/b_package/lib/src/foo.dart // Ensure that it's diffing against the prior commit. expect( gitProcessRunner.recordedCalls, - contains(const ProcessCall('git-diff', ['--name-only', 'HEAD~', 'HEAD'], null)), + contains( + const ProcessCall('git-diff', ['-z', '--name-only', 'HEAD~', 'HEAD'], null), + ), ); }); diff --git a/script/tool/test/format_command_test.dart b/script/tool/test/format_command_test.dart index db3459890db2..53529c2b4a93 100644 --- a/script/tool/test/format_command_test.dart +++ b/script/tool/test/format_command_test.dart @@ -266,6 +266,126 @@ void main() { ); }); + test( + 'does not format staged files that are hand-formatted or in excluded directories when --run-on-staged-packages is used', + () async { + const files = [ + 'lib/a.dart', + 'lib/src/b.dart', + 'example/build/a.dart', + '.dart_tool/a.dart', + ]; + final RepositoryPackage plugin = createFakePlugin( + 'a_plugin', + packagesDir, + extraFiles: files, + dartConstraint: _dartConstraint, + ); + fakePubGet(plugin); + + // Write pragma to lib/src/b.dart + final p.Context posixContext = p.posix; + childFileWithSubcomponents( + plugin.directory, + posixContext.split('lib/src/b.dart'), + ).writeAsStringSync('// This file is hand-formatted.\ncode...'); + + // Mock git diff to return all of them + final String stagedFiles = files.map((String f) => 'packages/a_plugin/$f').join('\n'); + gitProcessRunner.mockProcessesForExecutable['git-diff'] = List.generate( + 3, + (_) => FakeProcessInfo(MockProcess(stdout: stagedFiles)), + ); + + await runCapturingPrint(runner, ['format', '--run-on-staged-packages']); + + // Only lib/a.dart should be formatted. + // lib/src/b.dart is hand-formatted. + // example/build/a.dart is in excluded dir. + // .dart_tool/a.dart is in excluded dir. + expect( + processRunner.recordedCalls, + orderedEquals([ + ProcessCall('dart', const ['format', 'lib/a.dart'], plugin.path), + ]), + ); + }, + ); + + test( + 'skips Java and Kotlin formatting when no Java or Kotlin files are staged', + () async { + final RepositoryPackage plugin = createFakePlugin( + 'a_plugin', + packagesDir, + extraFiles: [ + 'lib/a.dart', + 'android/src/main/java/io/flutter/plugins/a_plugin/a.java', + 'android/src/main/kotlin/io/flutter/plugins/a_plugin/b.kt', + ], + dartConstraint: _dartConstraint, + ); + fakePubGet(plugin); + + // Mock git diff to return only the Dart file + const stagedFilePath = 'packages/a_plugin/lib/a.dart'; + gitProcessRunner.mockProcessesForExecutable['git-diff'] = List.generate( + 3, + (_) => FakeProcessInfo(MockProcess(stdout: stagedFilePath)), + ); + + await runCapturingPrint(runner, ['format', '--run-on-staged-packages']); + + // Should only run dart format, no java format + expect( + processRunner.recordedCalls, + orderedEquals([ + ProcessCall('dart', const ['format', 'lib/a.dart'], plugin.path), + ]), + ); + }, + ); + + test( + 'formats only staged Java files and skips Dart when only Java is staged', + () async { + const javaFile = 'android/src/main/java/io/flutter/plugins/a_plugin/a.java'; + final RepositoryPackage plugin = createFakePlugin( + 'a_plugin', + packagesDir, + extraFiles: [ + 'lib/a.dart', + javaFile, + ], + dartConstraint: _dartConstraint, + ); + fakePubGet(plugin); + + // Mock git diff to return only the Java file + const stagedFilePath = 'packages/a_plugin/$javaFile'; + gitProcessRunner.mockProcessesForExecutable['git-diff'] = List.generate( + 3, + (_) => FakeProcessInfo(MockProcess(stdout: stagedFilePath)), + ); + + await runCapturingPrint(runner, ['format', '--run-on-staged-packages']); + + // Should only run java format, no dart format + expect( + processRunner.recordedCalls, + orderedEquals([ + const ProcessCall('java', ['-version'], null), + ProcessCall('java', [ + '-jar', + javaFormatPath, + '--replace', + ...getPackagesDirRelativePaths(plugin, [javaFile]), + ], packagesDir.path), + ]), + ); + }, + ); + test('skips dart if --no-dart flag is provided', () async { const files = ['lib/a.dart']; final RepositoryPackage plugin = createFakePlugin( From 220baabc584d701136abc12c50d2c07f9fbfe7db Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Mon, 29 Jun 2026 15:19:57 -0700 Subject: [PATCH 35/37] self review --- script/githooks/README.md | 6 +++--- script/githooks/pubspec.yaml | 2 +- script/tool/lib/src/common/git_version_finder.dart | 13 +++---------- 3 files changed, 7 insertions(+), 14 deletions(-) diff --git a/script/githooks/README.md b/script/githooks/README.md index bdf5563a8ddd..8323cbfaa17e 100644 --- a/script/githooks/README.md +++ b/script/githooks/README.md @@ -18,10 +18,10 @@ dart script/githooks/bin/install_hooks.dart ### pre-commit -The `pre-commit` hook runs automatically when you run `git commit` and performs the following on any staged changes: +The `pre-commit` hook runs automatically when you run `git commit` and performs the following checks on any staged changes: -1. **Formatting**: It runs `flutter_plugin_tools format --run-on-staged-packages` to verify that all staged files in the targeted packages are correctly formatted. -2. **Static Analysis**: If formatting passes, it runs `flutter_plugin_tools analyze --run-on-staged-packages --dart` to run static analysis on the staged packages. +1. **Formatting**: It runs `dart run script/tool/bin/flutter_plugin_tools.dart format --run-on-staged-packages` to verify that all staged files in the targeted packages are correctly formatted. +2. **Static Analysis**: If formatting passes, it runs `dart run script/tool/bin/flutter_plugin_tools.dart analyze --run-on-staged-packages --dart` to run static analysis on the staged packages. If either check fails, it aborts the commit. To bypass the hook (for a WIP commit, for example), you can use the `--no-verify` flag: diff --git a/script/githooks/pubspec.yaml b/script/githooks/pubspec.yaml index 45f95e4db240..71a0e69c1d42 100644 --- a/script/githooks/pubspec.yaml +++ b/script/githooks/pubspec.yaml @@ -1,5 +1,5 @@ name: githooks -description: Git hooks for the flutter/packages repository. For more information and instructions to install the hooks, see README.md +description: Git hooks for the flutter/packages repository. For more information on the hooks and how to install them, see README.md publish_to: none environment: diff --git a/script/tool/lib/src/common/git_version_finder.dart b/script/tool/lib/src/common/git_version_finder.dart index 4b34ceae7eaf..07a91c1eb806 100644 --- a/script/tool/lib/src/common/git_version_finder.dart +++ b/script/tool/lib/src/common/git_version_finder.dart @@ -58,16 +58,9 @@ class GitVersionFinder { /// Splits the stdout of a `git diff` command into a list of file paths. /// /// When `git diff` is run with the `-z` flag, it outputs file paths separated - /// by a null byte (`\u0000`, ASCII 0). This is the safest way to parse filenames - /// because: - /// 1. Filenames can contain spaces, quotes, and newlines, but they can never - /// contain null bytes. - /// 2. Git does not quote or escape "unusual" characters in filenames when - /// using `-z`, giving us the raw path. - /// - /// For backward compatibility with legacy unit tests that mock `git diff` - /// using newline-terminated strings, this helper falls back to splitting - /// by `\n` if no null bytes are detected in the output. + /// by a null byte (`\u0000`, ASCII 0). Otherwise, it outputs file paths + /// separated by newlines. This method accounts for that by checking for + /// null bytes and falling back to splitting by newlines if none are found. List _splitDiffOutputs(String stdout) { if (stdout.isEmpty) { return []; From 4432af2d60b29ec7b957d87e58cb5b8c7fd184b4 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Tue, 30 Jun 2026 15:50:50 -0700 Subject: [PATCH 36/37] ignore --- script/githooks/lib/src/pre_commit_command.dart | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/script/githooks/lib/src/pre_commit_command.dart b/script/githooks/lib/src/pre_commit_command.dart index 694e3984881c..9fcea0b38a57 100644 --- a/script/githooks/lib/src/pre_commit_command.dart +++ b/script/githooks/lib/src/pre_commit_command.dart @@ -47,14 +47,14 @@ class PreCommitCommand extends Command { return false; } - final bool? hasStaged = await _hasStagedPackages(repoRoot); - if (hasStaged == null) { - return false; - } - if (!hasStaged) { - print('No staged package changes to check.'); - return true; - } + // final bool? hasStaged = await _hasStagedPackages(repoRoot); + // if (hasStaged == null) { + // return false; + // } + // if (!hasStaged) { + // print('No staged package changes to check.'); + // return true; + // } final String toolScript = p.join( repoRoot.path, From befac5a098b31fa0ce89c68df87381e050232344 Mon Sep 17 00:00:00 2001 From: Camille Simon Date: Wed, 1 Jul 2026 09:43:01 -0700 Subject: [PATCH 37/37] add docs + todo for plugin tool overhead --- .../githooks/lib/src/pre_commit_command.dart | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/script/githooks/lib/src/pre_commit_command.dart b/script/githooks/lib/src/pre_commit_command.dart index 9fcea0b38a57..cd606f1f1c48 100644 --- a/script/githooks/lib/src/pre_commit_command.dart +++ b/script/githooks/lib/src/pre_commit_command.dart @@ -47,14 +47,18 @@ class PreCommitCommand extends Command { return false; } - // final bool? hasStaged = await _hasStagedPackages(repoRoot); - // if (hasStaged == null) { - // return false; - // } - // if (!hasStaged) { - // print('No staged package changes to check.'); - // return true; - // } + // Skips check if there are no staged package changes because this + // makes a ~5s difference in runtime. + // TODO(camsim99): Investigate why the plugins tool adds overhead: + // https://github.com/flutter/flutter/issues/188870. + final bool? hasStaged = await _hasStagedPackages(repoRoot); + if (hasStaged == null) { + return false; + } + if (!hasStaged) { + print('No staged package changes to check.'); + return true; + } final String toolScript = p.join( repoRoot.path,