Skip to content

Commit 41c26dc

Browse files
committed
feat: initial support for header mappings
1 parent 1c23d58 commit 41c26dc

File tree

4 files changed

+61
-6
lines changed

4 files changed

+61
-6
lines changed

common.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,4 @@ class Configuration(BaseModel):
3131
dependencies: Dict[str, Union[str, "Configuration"]] = {}
3232
includeDirs: List[str] = []
3333
ignores: IgnoresConfiguration = IgnoresConfiguration()
34+
headerMappings: Dict[str, str] = {}

config.schema.json

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,16 @@
107107
"additionalProperties": false
108108
}
109109
}
110+
},
111+
"headerMappings": {
112+
"description": "Mappings between header names from clangd and the canonical header, used when suggesting includes to add",
113+
"type": "object",
114+
"patternProperties": {
115+
".*": {
116+
"description": "Mapping from header filename to canonical filename",
117+
"type": "string"
118+
}
119+
}
110120
}
111121
},
112122
"additionalProperties": false

configs/chromium.json

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,7 @@
4343
"ignores": {
4444
"add": {
4545
"headers": [
46-
"absl/base/internal/inline_variable.h",
47-
"absl/types/internal/optional.h",
48-
"absl/types/optional.h"
46+
"absl/base/internal/inline_variable.h"
4947
]
5048
},
5149
"remove": {
@@ -270,5 +268,9 @@
270268
]
271269
]
272270
}
271+
},
272+
"headerMappings": {
273+
"absl/types/optional.h": "third_party/abseil-cpp/absl/types/optional.h",
274+
"third_party/abseil-cpp/absl/types/internal/optional.h": "third_party/abseil-cpp/absl/types/optional.h"
273275
}
274276
}

filter_include_changes.py

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@
77
import pathlib
88
import re
99
import sys
10-
from typing import List, Tuple
10+
from collections import defaultdict
11+
from typing import DefaultDict, Dict, List, Tuple
1112

1213
from common import IgnoresConfiguration, IncludeChange
1314
from utils import load_config
@@ -27,9 +28,15 @@ def filter_changes(
2728
change_type_filter: IncludeChange = None,
2829
filter_generated_files=True,
2930
filter_mojom_headers=True,
31+
header_mappings: Dict[str, str] = None,
3032
):
3133
"""Filter changes"""
3234

35+
# When header mappings are provided, we can cancel out suggestions from clangd where
36+
# it suggests removing one include and adding another, when the pair is found in the
37+
# mapping, since we know that means clangd is confused on which header to include
38+
pending_changes: DefaultDict[str, Dict[str, Tuple[IncludeChange, int, int]]] = defaultdict(dict)
39+
3340
for change_type_value, line, filename, header, *_ in changes:
3441
change_type = IncludeChange.from_value(change_type_value)
3542

@@ -83,8 +90,38 @@ def filter_changes(
8390
if ignore_edge or ignore_include:
8491
continue
8592

93+
# If the header is in a provided header mapping, wait until the end to yield it
94+
if header_mappings and header in header_mappings:
95+
assert header not in pending_changes[filename]
96+
97+
# TODO - Includes inside of dependencies shouldn't be mapped since they can
98+
# access internal headers, and the mapped canonical header is from
99+
# the perspective of the project's root source directory
100+
pending_changes[filename][header] = (change_type, line, *_)
101+
continue
102+
86103
yield (change_type_value, line, filename, header, *_)
87104

105+
if header_mappings:
106+
inverse_header_mappings = {v: k for k, v in header_mappings.items()}
107+
108+
for filename in pending_changes:
109+
for header in pending_changes[filename]:
110+
change_type, line, *_ = pending_changes[filename][header]
111+
112+
if change_type is IncludeChange.ADD:
113+
# Look for a corresponding remove which would cancel out
114+
if header_mappings[header] in pending_changes[filename]:
115+
if pending_changes[filename][header][0] is IncludeChange.REMOVE:
116+
continue
117+
elif change_type is IncludeChange.REMOVE and header in inverse_header_mappings:
118+
# Look for a corresponding add which would cancel out
119+
if inverse_header_mappings[header] in pending_changes[filename]:
120+
if pending_changes[filename][header][0] is IncludeChange.ADD:
121+
continue
122+
123+
yield (change_type.value, line, filename, header_mappings, *_)
124+
88125

89126
def main():
90127
parser = argparse.ArgumentParser(description="Filter include changes output")
@@ -127,10 +164,14 @@ def main():
127164
else:
128165
change_type_filter = None
129166

167+
config = None
130168
ignores = None
131169

132-
if args.config and not args.no_filter_ignores:
133-
ignores = load_config(args.config).ignores
170+
if args.config:
171+
config = load_config(args.config)
172+
173+
if config and not args.no_filter_ignores:
174+
ignores = config.ignores
134175

135176
csv_writer = csv.writer(sys.stdout)
136177

@@ -143,6 +184,7 @@ def main():
143184
change_type_filter=change_type_filter,
144185
filter_generated_files=not args.no_filter_generated_files,
145186
filter_mojom_headers=not args.no_filter_mojom_headers,
187+
header_mappings=config.headerMappings if config else None,
146188
):
147189
csv_writer.writerow(change)
148190

0 commit comments

Comments
 (0)