Add pre-commit hook to verify static analysis passes & format is correct by camsim99 · Pull Request #11941 · flutter/packages

camsim99 · 2026-06-18T19:33:51Z

Adds a pre-commit hook that you can install to verify that dart analyze --fatal-infos and the package format are correct when you make a call to git commit. Intended to ensure agents commit code that meets are quality standards.

Miscellaneous notes:

script/githooks/bin/install_hooks.dart is the script to install the hook that devs would need to run to use it.
I modeled the new githooks directory like https://github.com/flutter/flutter/tree/master/engine/src/flutter/tools/githooks to make it easier for folks to add new hooks
The hook separates formatting of Dart and native files as an optimization to avoid all of the configuration that the plugin tool uses to format native files that are not required if no native files have been changed

Pre-Review Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩²

gemini-code-assist

Code Review

This pull request introduces a new githooks Dart package to automate pre-commit checks, including formatting and static analysis, for staged files. The package includes an installation script, a pre-commit command runner, and associated unit tests. Review feedback highlights a potential infinite loop on Windows when locating the .git directory, raw ANSI escape sequences being printed without verifying terminal support, the use of loose any version constraints in pubspec.yaml, and redundant dead code in the test mocks.

camsim99 · 2026-06-22T19:42:52Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces a Dart-based pre-commit git hook framework to automate formatting and static analysis checks on staged files. The feedback suggests enhancing compatibility with Git submodules and worktrees by checking for .git as a file, adding support for .hpp extensions in Clang formatting, conditionally writing ANSI escape sequences based on terminal support, and adding test coverage for the native formatting execution path.

reidbaker

I left a bunch of comments on the code but before addressing any of those 2 high level questions.

Why did you rewrite analyze and format logic instead of using the logic that is already built into the flutter packages tooling?
If we were not going to use the existing flutter packages tooling for some good technical reason why not use something like https://github.com/pre-commit/pre-commit

reidbaker · 2026-06-23T14:32:06Z

+      '--diff-filter=ACM',
+    ], workingDirectory: repoRoot.path);
+
+    if (diffResult.exitCode != 0) {


~~With your ACM filter there could be a commit that was just a deletion but I dont think it would give any files. What error code is returned in that situation or is that a success?~~ Looks like empty sets are handled below.

reidbaker · 2026-06-23T14:32:59Z

+    ], workingDirectory: repoRoot.path);
+
+    if (diffResult.exitCode != 0) {
+      print('❌ Failed to get staged files');


Do you think we should include the command that failed and the error code?

reidbaker · 2026-06-23T14:33:11Z

+  }
+
+  @override
+  Future<bool> run() async {


Add dart doc

reidbaker · 2026-06-23T14:35:27Z

+    }
+
+    if (targetPackageDirs.isEmpty) {
+      // None of the changed files are part of a package we care about.


I am a bit confused as to what this does. It looks like it is building a set of packages from staged files. But if that is the case then where does the filtering happen.

reidbaker · 2026-06-23T14:35:57Z

+    final packageArgs = '--packages=${targetPackages.join(',')}';
+
+    // Determine which toolchains are needed based on file extensions to avoid uneccessary slowdown.
+    final bool hasClang = stagedFiles.any(


What code determines if this list needs to change?

reidbaker · 2026-06-23T14:37:35Z

+          f.endsWith('.m') ||
+          f.endsWith('.mm'),
+    );
+    final bool hasJava = stagedFiles.any((f) => f.endsWith('.java'));


Recommend structuring this a bit more formally. What information is needed per language? How do you execute per language. Lets make the blast radius clear and the file manipulations happen outside of any language logic.

reidbaker · 2026-06-23T14:39:15Z

+    // Format staged native files.
+    final bool needsNativeFormat = hasClang || hasJava || hasKotlin || hasSwift;
+    if (needsNativeFormat) {
+      final nativeFormatFlags = [


Lets use the args package. https://pub.dev/packages/args

reidbaker · 2026-06-23T14:40:16Z

+          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.',


I am guessing ai wrote this script because it tends to use these unicode chars for success and failure. Lets remove those unicode chars.

reidbaker · 2026-06-23T14:41:04Z

+
+    if (!hasError) {
+      stdout.write(
+        stdout.supportsAnsiEscapes


is this complexity needed? What value do we get from having ansiExcapes here.

reidbaker · 2026-06-23T14:42:30Z

+
+    // Run static analysis on staged files.
+    var analyzeHasError = false;
+    stdout.write('Running static analysis...');


Structurally my gut is organizing code by job type instead of by when in the hooks process the code runs is more likely to be a durable standard.

camsim99 · 2026-06-23T23:55:10Z

@reidbaker To answer your high level question, I initially just called the plugin tool but it was extremely slow to the point where I wouldn't have felt comfortable landing this, so that's why I made optimizations around avoiding setup of non-required native tool chains and ignoring files that are not staged. However, your point is taken because the right way for me to fix things is to add the optimizations to the plugin tool itself. Working on that now! That should make the pre-commit hook itself much simpler, but I'll still refactor as needed.

I was unaware of https://pre-commit.com/#usage and modeled this PR after the engine. If you prefer we use the pre-commit tool, I can look into that, but either way I need to make some optimizations to the plugin tool to make this hook usable.

reidbaker · 2026-06-24T01:43:03Z

Fwiw I don't have a strong preference but I wanted to point out the duplication especially since there was a tool that was aware in the repo. Adding optimizations to the tool seems like a good plan.

camsim99 added 7 commits June 18, 2026 09:18

first pass

0a65ef8

rippling revisions

ccda0df

use plugin tool instead

09a89f2

-- debugging stuff

04253cc

fake commit

e078f24

test + command fix

d61f271

fake commit

a487e22

flutter-dashboard Bot added the CICD Run CI/CD label Jun 18, 2026

camsim99 added 11 commits June 22, 2026 10:06

try optimizing by cutting out native toolchains

85219e8

fake commit

021dd71

run dart analyze directly

4908982

undo fake commit

d05c928

run format and analyze directly on staged files

2d89dd8

fake commit

7e53502

use ANSI escape codes to erase status logs

5b8c9a8

fake commit

f4eed3d

change emoji to runner

638ce34

undo fake commit

c19c8f3

make unit tests stricter

f8837ba

github-actions Bot removed the CICD Run CI/CD label Jun 22, 2026

camsim99 added 2 commits June 22, 2026 10:48

self review

352580c

expect args

a6a2a78

flutter-dashboard Bot added the CICD Run CI/CD label Jun 22, 2026

camsim99 added CICD Run CI/CD and removed CICD Run CI/CD labels Jun 22, 2026

camsim99 marked this pull request as ready for review June 22, 2026 17:58

gemini-code-assist Bot reviewed Jun 22, 2026

View reviewed changes

camsim99 added 2 commits June 22, 2026 12:37

gemini review

5f94b32

fix print

bc0ce9f

camsim99 removed the CICD Run CI/CD label Jun 22, 2026

camsim99 added the CICD Run CI/CD label Jun 22, 2026

gemini-code-assist Bot reviewed Jun 22, 2026

View reviewed changes

address gemini review

38e5327

github-actions Bot removed the CICD Run CI/CD label Jun 22, 2026

camsim99 added the CICD Run CI/CD label Jun 22, 2026

camsim99 requested a review from reidbaker June 22, 2026 22:45

reidbaker requested changes Jun 23, 2026

View reviewed changes

Conversation

camsim99 commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-Review Checklist

Footnotes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

camsim99 commented Jun 22, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

reidbaker left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

camsim99 commented Jun 23, 2026

Uh oh!

reidbaker commented Jun 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

camsim99 commented Jun 18, 2026 •

edited

Loading