Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

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

Merged
merged 32 commits into from
Jan 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
340a691
Initial commit: directory for header_guard_check.
matanlurey Dec 11, 2023
0058f60
WIP.
matanlurey Dec 11, 2023
3aed665
Add minimal library with some validation.
matanlurey Dec 11, 2023
1784e68
Working basic tool.
matanlurey Dec 12, 2023
90892fa
WS.
matanlurey Dec 12, 2023
5b21cc1
Tweak.
matanlurey Dec 12, 2023
934d838
Merge remote-tracking branch 'upstream/main' into engine-pragma-once-…
matanlurey Dec 13, 2023
98e3307
Tweak.
matanlurey Dec 13, 2023
3afe63c
++
matanlurey Dec 13, 2023
59e98e6
++
matanlurey Dec 13, 2023
acc939a
++
matanlurey Dec 13, 2023
8d3b0f9
++
matanlurey Dec 13, 2023
1c8fa0e
++
matanlurey Dec 13, 2023
7d1a621
Change WS.
matanlurey Dec 13, 2023
68ce106
++
matanlurey Dec 13, 2023
4a55b15
Merge remote-tracking branch 'upstream/main' into engine-pragma-once-…
matanlurey Dec 13, 2023
6df1543
Merge branch 'main' into engine-pragma-once-tool
matanlurey Dec 14, 2023
5212ade
Tweak WS.
matanlurey Dec 14, 2023
d6f86fb
Merge remote-tracking branch 'upstream/main' into engine-pragma-once-…
matanlurey Dec 14, 2023
f9bfce1
Tweak expected name.
matanlurey Dec 14, 2023
1b6ea20
++
matanlurey Dec 14, 2023
de04172
++
matanlurey Dec 14, 2023
dd2b5b7
++
matanlurey Dec 14, 2023
713416d
++
matanlurey Dec 14, 2023
42cae8b
Merge branch 'main' into engine-pragma-once-tool
matanlurey Jan 23, 2024
534618b
Merge remote-tracking branch 'upstream/main' into engine-pragma-once-…
matanlurey Jan 26, 2024
03b9099
Tweak rules.
matanlurey Jan 26, 2024
6593be6
++
matanlurey Jan 26, 2024
26fa528
Address feedback.
matanlurey Jan 26, 2024
1d1627a
++
matanlurey Jan 26, 2024
f91b3fd
++
matanlurey Jan 26, 2024
7107ef1
++
matanlurey Jan 26, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions testing/run_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -1001,6 +1001,7 @@ def build_dart_host_test_list(build_dir):
],
),
(os.path.join('flutter', 'tools', 'githooks'), []),
(os.path.join('flutter', 'tools', 'header_guard_check'), []),
(os.path.join('flutter', 'tools', 'pkg', 'engine_build_configs'), []),
(os.path.join('flutter', 'tools', 'pkg', 'engine_repo_tools'), []),
(os.path.join('flutter', 'tools', 'pkg', 'git_repo_tools'), []),
Expand Down
46 changes: 46 additions & 0 deletions tools/header_guard_check/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# header_guard_check

A tool to check that C++ header guards are used consistently in the engine.

```shell
# Assuming you are in the `flutter` root of the engine repo.
dart ./tools/header_guard_check/bin/main.dart
```

The tool checks _all_ header files for the following pattern:

```h
// path/to/file.h

#ifndef PATH_TO_FILE_H_
#define PATH_TO_FILE_H_
...
#endif // PATH_TO_FILE_H_
```

If the header file does not follow this pattern, the tool will print an error
message and exit with a non-zero exit code. For more information about why we
use this pattern, see [the Google C++ style guide](https://google.github.io/styleguide/cppguide.html#The__define_Guard).

> [!IMPORTANT]
> This is a prototype tool and is not yet integrated into the engine's CI.

## Automatic fixes

The tool can automatically fix header files that do not follow the pattern:

```shell
dart ./tools/header_guard_check/bin/main.dart --fix
```

## Advanced usage

### Restricting the files to check

By default, the tool checks all header files in the engine. You can restrict the
files to check by passing a relative (to the `engine/src/flutter` root) paths to
include:

```shell
dart ./tools/header_guard_check/bin/main.dart --include impeller
```
15 changes: 15 additions & 0 deletions tools/header_guard_check/bin/main.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// 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' as io;

import 'package:header_guard_check/header_guard_check.dart';

Future<int> main(List<String> arguments) async {
final int result = await HeaderGuardCheck.fromCommandLine(arguments).run();
if (result != 0) {
io.exit(result);
}
return result;
}
175 changes: 175 additions & 0 deletions tools/header_guard_check/lib/header_guard_check.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
// 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' as io;

import 'package:args/args.dart';
import 'package:engine_repo_tools/engine_repo_tools.dart';
import 'package:meta/meta.dart';
import 'package:path/path.dart' as p;

import 'src/header_file.dart';

/// Checks C++ header files for header guards.
@immutable
final class HeaderGuardCheck {
/// Creates a new header guard checker.
const HeaderGuardCheck({
required this.source,
required this.exclude,
this.include = const <String>[],
this.fix = false,
});

/// Parses the command line arguments and creates a new header guard checker.
factory HeaderGuardCheck.fromCommandLine(List<String> arguments) {
final ArgResults argResults = _parser.parse(arguments);
return HeaderGuardCheck(
source: Engine.fromSrcPath(argResults['root'] as String),
include: argResults['include'] as List<String>,
exclude: argResults['exclude'] as List<String>,
fix: argResults['fix'] as bool,
);
}

/// Engine source root.
final Engine source;

/// Whether to automatically fix most header guards.
final bool fix;

/// Path directories to include in the check.
final List<String> include;

/// Path directories to exclude from the check.
final List<String> exclude;

/// Runs the header guard check.
Future<int> run() async {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

final List<HeaderFile> badFiles = _checkFiles(_findIncludedHeaderFiles()).toList();

if (badFiles.isNotEmpty) {
io.stdout.writeln('The following ${badFiles.length} files have invalid header guards:');
for (final HeaderFile headerFile in badFiles) {
io.stdout.writeln(' ${headerFile.path}');
}

// If we're fixing, fix the files.
if (fix) {
for (final HeaderFile headerFile in badFiles) {
headerFile.fix(engineRoot: source.flutterDir.path);
}

io.stdout.writeln('Fixed ${badFiles.length} files.');
return 0;
}

return 1;
}

return 0;
}

Iterable<io.File> _findIncludedHeaderFiles() sync* {
final io.Directory dir = source.flutterDir;
for (final io.FileSystemEntity entity in dir.listSync(recursive: true)) {
if (entity is! io.File) {
continue;
}

if (!entity.path.endsWith('.h')) {
continue;
}

if (!_isIncluded(entity.path) || _isExcluded(entity.path)) {
continue;
}

yield entity;
}
}

bool _isIncluded(String path) {
for (final String includePath in include) {
final String relativePath = p.relative(includePath, from: source.flutterDir.path);
if (p.isWithin(relativePath, path) || p.equals(relativePath, path)) {
return true;
}
}
return include.isEmpty;
}

bool _isExcluded(String path) {
for (final String excludePath in exclude) {
final String relativePath = p.relative(excludePath, from: source.flutterDir.path);
if (p.isWithin(relativePath, path) || p.equals(relativePath, path)) {
return true;
}
}
return false;
}

Iterable<HeaderFile> _checkFiles(Iterable<io.File> headers) sync* {
for (final io.File header in headers) {
final HeaderFile headerFile = HeaderFile.parse(header.path);
if (headerFile.pragmaOnce != null) {
io.stderr.writeln(headerFile.pragmaOnce!.message('Unexpected #pragma once'));
yield headerFile;
}

if (headerFile.guard == null) {
io.stderr.writeln('Missing header guard in ${headerFile.path}');
yield headerFile;
}

final String expectedGuard = headerFile.computeExpectedName(engineRoot: source.flutterDir.path);
if (headerFile.guard!.ifndefValue != expectedGuard) {
io.stderr.writeln(headerFile.guard!.ifndefSpan!.message('Expected #ifndef $expectedGuard'));
yield headerFile;
}
if (headerFile.guard!.defineValue != expectedGuard) {
io.stderr.writeln(headerFile.guard!.defineSpan!.message('Expected #define $expectedGuard'));
yield headerFile;
}
if (headerFile.guard!.endifValue != expectedGuard) {
io.stderr.writeln(headerFile.guard!.endifSpan!.message('Expected #endif // $expectedGuard'));
yield headerFile;
}
}
}
}

final Engine? _engine = Engine.tryFindWithin(p.dirname(p.fromUri(io.Platform.script)));

final ArgParser _parser = ArgParser()
..addFlag(
'fix',
help: 'Automatically fixes most header guards.',
)
..addOption(
'root',
abbr: 'r',
help: 'Path to the engine source root.',
valueHelp: 'path/to/engine/src',
defaultsTo: _engine?.srcDir.path,
)
..addMultiOption(
'include',
abbr: 'i',
help: 'Paths to include in the check.',
valueHelp: 'path/to/dir/or/file (relative to the engine root)',
defaultsTo: <String>[],
)
..addMultiOption(
'exclude',
abbr: 'e',
help: 'Paths to exclude from the check.',
valueHelp: 'path/to/dir/or/file (relative to the engine root)',
defaultsTo: _engine != null ? <String>[
'build',
'impeller/compiler/code_gen_template.h',
'prebuilts',
'third_party',
] : null,
);
Loading