As of 2023-11-10, this effort needs additional contributions to get over the line.

How to help

Patch (but do not merge into main) this change to .clang_tidy:

- HeaderFilterRegex: "(!third_party/)(!gen/).*/flutter/.*"
+ HeaderFilterRegex: "[..\/]+\/flutter\/.*"

Next, check for existing (hidden due to the Regex) failures:

# Warning: This will take 30m-60m and lock down your machine/slow it down.
./prebuilts/macos-arm64/dart-sdk/bin/dart ./tools/clang_tidy/bin/main.dart --lint-all

# I like these sub-commands for filtering the output
./prebuilts/macos-arm64/dart-sdk/bin/dart ./tools/clang_tidy/bin/main.dart --lint-all |  grep flutter\.\*error: | sort -u

# You can also use --lint-regex to lint a subset of the repo:
./prebuilts/macos-arm64/dart-sdk/bin/dart ./tools/clang_tidy/bin/main.dart ./tools/clang_tidy/bin/main.dart --lint-regex ".*\/impeller\/.*"

In most cases, how to fix the lint is trivial, but some (like making constructors explicit have downstream effects, and you'll need to also patch up call sites). My recommendation is to to take it slow - it's better to send fixes for a few files at a time (and get those fixes merged) versus have a giant PR with hundreds of files changed.

If you need help, or want a review, feel free to add @matanlurey.

Status

(Feel free to add more items as we migrate the rest of the repo, see above)

• Migrate LOG_X to kLogX
  • Deprecate fml::LOG_X in favor of kLogX. engine#46052
  • Migrate from LOG_X to kLogX. engine#46107
• Comply-ify display_list and friends
  • [Unblock Lint Headers] Make DlColor(uint32_t argb) explicit #135057
  • [Unblock Lint Headers] Make DlPaint Clang Tidy Approved #135058
  • [Unblock Lint Headers] Make DlBlendColorFilter Clang Tidy Approved #135060
  • [Unblock Lint Headers] Make DlColorColorSource Clang Tidy Approved #135061
  • [Unblock Lint Headers] Make Dl*ImageFilter Clang Tidy Approved #135062
  • [Unblock Lint Headers] Make DlBlurMaskFilter Clang Tidy Approved #135063
  • [Unblock Lint Headers] Make DlPathEffect Clang Tidy Approved #135064
• Misc
  • Rename layoutGoals to kLayoutGoals to enforce lints on headers. engine#46054
  • FlutterMouse.* -> kFlutterMouse.*, so we can lint header files. engine#46056
  • Add TODO(name) to comply with Clang Tidy. engine#46057
  • Conform to clang_tidy in client_wrapper headers. engine#46058
  • Make a variety of low-impact Clang tidy fixes. engine#46114
  • Make a variety of low-impact Clang tidy fixes in Impeller. engine#46116

Simple reproduction case:

1. Open flutter/impeller/base/allocation.h, break a lint, e.g. readability-identifier-naming:

namespace impeller {

+ // Invalid, should start with "k"
+ const size_t xAllocationMaxSize = std::numeric_limits<size_t>::max();

2. Open flutter/impeller/base/allocation.cc, and make a similar change:

namespace impeller {

+ // Invalid, should start with "k"
+ const size_t zAllocationMaxSize = std::numeric_limits<size_t>::max();

3. Run clang_tidy:

# This will vary based on your host, but you get the gist.
$ dart tools/clang_tidy/bin/main.dart --target-variant host_debug_unopt_arm64 --lint-head

🔶 linting flutter/impeller/base/allocation.cc
🔶 linting flutter/impeller/base/allocation.cc
[0:04] Jobs:  50% done,   0/2   completed,  1 in progress,   0 pending,   1 failed.    
❌ Failures for clang-tidy on /Users/matanl/Developer/engine/src/flutter/impeller/base/allocation.cc:
/Users/matanl/Developer/engine/src/flutter/impeller/base/allocation.cc:15:14: error: unused variable 'zAllocationMaxSize' [clang-diagnostic-unused-const-variable,-warnings-as-errors]
 15 | const size_t zAllocationMaxSize = std::numeric_limits<size_t>::max();
    |              ^~~~~~~~~~~~~~~~~~
/Users/matanl/Developer/engine/src/flutter/impeller/base/allocation.cc:15:14: error: invalid case style for global constant 'zAllocationMaxSize' [readability-identifier-naming,-warnings-as-errors]
 15 | const size_t zAllocationMaxSize = std::numeric_limits<size_t>::max();
    |              ^~~~~~~~~~~~~~~~~~
    |              kZAllocationMaxSize

Notice that only allocation.cc is reported as a violation. Try just step 1 to see no violations at all.

From @zanderso on chat:

IIRC clang-tidy works in a slightly funky way. It's okay that the header files are not listed in compile_commands.json. clang-tidy runs after preprocessing, so header files are inlined

However, it's likely that we never did the work to figure out what to set HeaderFilterRegex to, which specifies to clang-tidy the header files that it should report issues from.

When there is an issue in a header file, it will get reported for each TU that includes it, which is annoying.

There's also a bug in clang-tidy (I can dig up the link if needed) which causes clang-tidy to analyze the entire preprocessed TU, and then throw away the results for headers that don't match the HeaderFilterRegex, so we are probably already paying the cost of doing the analysis