Introduce a prototype of a "header guard enforcement" tool #48903

matanlurey · 2023-12-12T00:13:49Z

This is a prototype I threw together in about 1-2 hours. It enforces and automatically fixes header guards that don't match the the Google C++ style guide. For example, here is (trimmed) output at HEAD:

line 5, column 1 of impeller/aiks/picture.h: Unexpected #pragma once
  ╷
5 │ #pragma once
  │ ^^^^^^^^^^^^
  ╵
line 5, column 1 of flow/stopwatch.h: Expected #ifndef FLUTTER_FLOW_STOPWATCH_H_
  ╷
5 │ #ifndef FLUTTER_FLOW_INSTRUMENTATION_H_
  │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  ╵
The following 731 files have invalid header guards:

tools/header_guard_check/lib/header_guard_check.dart

tools/header_guard_check/pubspec.yaml

…tool

Automatically generated by #48903. Progress towards flutter/flutter#133415.

…tool

Hixie · 2023-12-14T01:07:33Z

test-exempt: tested by #48903

…tool

matanlurey · 2024-01-26T01:54:06Z

Ok, I think I'm ready to land the tool (without enforcing it on CI - that will be another PR). PTAL.

gaaclarke

LGTM, just a couple of suggestions. There were a few places where there may be false positives, but we'll get those sorted out easy if this is running on ci.

gaaclarke · 2024-01-26T17:29:25Z

tools/header_guard_check/lib/header_guard_check.dart

+
+    // Recursive search for header files.
+    final io.Directory dir = source.flutterDir;
+    await for (final io.FileSystemEntity entity in dir.list(recursive: true)) {


I recommend pulling this out to a generator, this method is long and probably looks really familiar to something you've written dozens of times before =)

Done! Or I misunderstood your comment, so please feel free to add more.

gaaclarke · 2024-01-26T17:33:09Z

tools/header_guard_check/lib/header_guard_check.dart

+
+    // Check each file.
+    final List<HeaderFile> badFiles = <HeaderFile>[];
+    for (final io.File file in files) {


This can be split into a generator too, then you don't have all of these continues.

gaaclarke · 2024-01-26T18:56:13Z

tools/header_guard_check/lib/src/header_file.dart

+  /// Returns the expected header guard for this file, relative to [engineRoot].
+  ///
+  /// For example, if the file is `foo/bar/baz.h`, this will return `FLUTTER_FOO_BAR_BAZ_H_`.
+  String expectedName({required String engineRoot}) {


nit: a method name should have a verb

gaaclarke · 2024-01-26T18:58:36Z

tools/header_guard_check/lib/src/header_file.dart

+    return 'FLUTTER_${underscoredRelativePath.toUpperCase()}_H_';
+  }
+
+  /// Updates the file at [path] to have the expected header guard.


What is the return value bool?

Added a comment.

zanderso

Needs an entry in https://github.com/flutter/engine/blob/main/testing/run_tests.py#L975 to run the tests on CI.

matanlurey · 2024-01-26T19:38:36Z

Needs an entry in https://github.com/flutter/engine/blob/main/testing/run_tests.py#L975 to run the tests on CI.

Done.

@cbracken

#50069) Generated by #48903 (`dart ./tools/header_guard_check/bin/main.dart --fix`). As discussed with @cbracken and @jmagman, the guards are not technically needed on the Mac/iOS code, but they (a) do not hurt and (b) still provide value if for some reason `#include` is used instead of `#import` (though I suspect we could try to add that to the tool in the future as well).

auto-submit · 2024-01-26T19:53:17Z

auto label is removed for flutter/engine/48903, due to - The status or check suite Mac mac_host_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.

gaaclarke · 2024-01-26T20:09:14Z

tools/header_guard_check/lib/header_guard_check.dart

+  final List<String> exclude;
+
+  /// Runs the header guard check.
+  Future<int> run() async {


auto-submit · 2024-01-26T20:28:00Z

auto label is removed for flutter/engine/48903, due to - The status or check suite Linux linux_fuchsia has failed. Please fix the issues identified (or deflake) before re-applying this label.

…lutter/engine#48903)

…142347) flutter/engine@525bd7d...a65a1b5 2024-01-26 [email protected] Roll Skia from c32aa37effcc to 6279c88b9e29 (1 revision) (flutter/engine#50098) 2024-01-26 [email protected] [Impeller] add compute pass API for memory barriers, re-enable for Vulkan. (flutter/engine#49946) 2024-01-26 [email protected] Introduce a prototype of a "header guard enforcement" tool (flutter/engine#48903) 2024-01-26 [email protected] Roll Skia from e24124912cc3 to c32aa37effcc (1 revision) (flutter/engine#50094) 2024-01-26 [email protected] [web] Do not wipe the PlatformViewManager when disposing of a view. (flutter/engine#49991) 2024-01-26 [email protected] Finish landing missing/incorrect header guards across `flutter/engine` (flutter/engine#50069) 2024-01-26 [email protected] Roll Skia from 32f6bff0f193 to e24124912cc3 (2 revisions) (flutter/engine#50093) 2024-01-26 [email protected] Roll Skia from cbdf09d69efc to 32f6bff0f193 (3 revisions) (flutter/engine#50092) 2024-01-26 [email protected] Roll Dart SDK from 5636e338e0b9 to 7337995bc851 (1 revision) (flutter/engine#50087) 2024-01-26 [email protected] Fix Shell::Screenshot for Impeller (flutter/engine#50072) 2024-01-26 [email protected] Roll Skia from ae73baacb793 to cbdf09d69efc (1 revision) (flutter/engine#50085) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

matanlurey added 6 commits December 11, 2023 13:46

Initial commit: directory for header_guard_check.

340a691

WIP.

0058f60

Add minimal library with some validation.

3aed665

Working basic tool.

1784e68

WS.

90892fa

Tweak.

5b21cc1

gaaclarke reviewed Dec 12, 2023

View reviewed changes

tools/header_guard_check/lib/header_guard_check.dart Outdated Show resolved Hide resolved

zanderso reviewed Dec 12, 2023

View reviewed changes

tools/header_guard_check/lib/header_guard_check.dart Outdated Show resolved Hide resolved

tools/header_guard_check/pubspec.yaml Show resolved Hide resolved

loic-sharma mentioned this pull request Dec 12, 2023

[Windows] Use #pragma once #48955

Closed

matanlurey added 8 commits December 12, 2023 17:00

Merge remote-tracking branch 'upstream/main' into engine-pragma-once-…

934d838

…tool

Tweak.

98e3307

++

3afe63c

++

59e98e6

++

acc939a

++

8d3b0f9

++

1c8fa0e

Change WS.

7d1a621

matanlurey mentioned this pull request Dec 13, 2023

Migrate impeller/**.h to header guards. #48962

Merged

++

68ce106

matanlurey added a commit that referenced this pull request Dec 13, 2023

Migrate impeller/**.h to header guards. (#48962)

29a85a7

Automatically generated by #48903. Progress towards flutter/flutter#133415.

matanlurey added 3 commits December 12, 2023 18:04

Merge remote-tracking branch 'upstream/main' into engine-pragma-once-…

4a55b15

…tool

Merge branch 'main' into engine-pragma-once-tool

6df1543

Tweak WS.

5212ade

matanlurey mentioned this pull request Dec 14, 2023

Fix header-guard naming convention in shell/. #49006

Merged

matanlurey added 5 commits December 14, 2023 13:51

Merge remote-tracking branch 'upstream/main' into engine-pragma-once-…

d6f86fb

…tool

Tweak expected name.

f9bfce1

++

1b6ea20

++

de04172

++

dd2b5b7

Tweak rules.

03b9099

matanlurey mentioned this pull request Jan 26, 2024

Finish landing missing/incorrect header guards across flutter/engine #50069

Merged

++

6593be6

matanlurey requested review from gaaclarke, jonahwilliams and zanderso January 26, 2024 01:54

gaaclarke approved these changes Jan 26, 2024

View reviewed changes

zanderso reviewed Jan 26, 2024

View reviewed changes

matanlurey added 2 commits January 26, 2024 11:37

Address feedback.

26fa528

++

1d1627a

matanlurey requested a review from zanderso January 26, 2024 19:38

matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 26, 2024

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 26, 2024

++

f91b3fd

matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 26, 2024

gaaclarke reviewed Jan 26, 2024

View reviewed changes

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 26, 2024

++

7107ef1

matanlurey merged commit d33b2dd into flutter:main Jan 26, 2024

matanlurey deleted the engine-pragma-once-tool branch January 26, 2024 21:16

engine-flutter-autoroll mentioned this pull request Jan 26, 2024

Roll Flutter Engine from 525bd7dcf7f3 to d27b18d912d7 (10 revisions) flutter/flutter#142346

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 26, 2024

d33b2dd9d Introduce a prototype of a "header guard enforcement" tool (f…

7f735b1

…lutter/engine#48903)

engine-flutter-autoroll mentioned this pull request Jan 26, 2024

Roll Flutter Engine from 525bd7dcf7f3 to a65a1b55e06a (11 revisions) flutter/flutter#142347

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 26, 2024

d33b2dd9d Introduce a prototype of a "header guard enforcement" tool (f…

c71d9da

…lutter/engine#48903)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce a prototype of a "header guard enforcement" tool #48903

Introduce a prototype of a "header guard enforcement" tool #48903

matanlurey commented Dec 12, 2023 •

edited

Loading

Hixie commented Dec 14, 2023

matanlurey commented Jan 26, 2024

gaaclarke left a comment

gaaclarke Jan 26, 2024

matanlurey Jan 26, 2024

gaaclarke Jan 26, 2024

matanlurey Jan 26, 2024

gaaclarke Jan 26, 2024

matanlurey Jan 26, 2024

gaaclarke Jan 26, 2024

matanlurey Jan 26, 2024

zanderso left a comment

matanlurey commented Jan 26, 2024

auto-submit bot commented Jan 26, 2024

gaaclarke Jan 26, 2024

auto-submit bot commented Jan 26, 2024

Introduce a prototype of a "header guard enforcement" tool #48903

Introduce a prototype of a "header guard enforcement" tool #48903

Conversation

matanlurey commented Dec 12, 2023 • edited Loading

Hixie commented Dec 14, 2023

matanlurey commented Jan 26, 2024

gaaclarke left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zanderso left a comment

Choose a reason for hiding this comment

matanlurey commented Jan 26, 2024

auto-submit bot commented Jan 26, 2024

Choose a reason for hiding this comment

auto-submit bot commented Jan 26, 2024

matanlurey commented Dec 12, 2023 •

edited

Loading