Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(223)

Side by Side Diff: PRESUBMIT.py

Issue 1293273005: Add presubmit check for header inclusion violation. (Closed) Base URL: https://chromium.googlesource.com/v8/v8.git@master
Patch Set: Addressed comments. Created 5 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 # Copyright 2012 the V8 project authors. All rights reserved. 1 # Copyright 2012 the V8 project authors. All rights reserved.
2 # Redistribution and use in source and binary forms, with or without 2 # Redistribution and use in source and binary forms, with or without
3 # modification, are permitted provided that the following conditions are 3 # modification, are permitted provided that the following conditions are
4 # met: 4 # met:
5 # 5 #
6 # * Redistributions of source code must retain the above copyright 6 # * Redistributions of source code must retain the above copyright
7 # notice, this list of conditions and the following disclaimer. 7 # notice, this list of conditions and the following disclaimer.
8 # * Redistributions in binary form must reproduce the above 8 # * Redistributions in binary form must reproduce the above
9 # copyright notice, this list of conditions and the following 9 # copyright notice, this list of conditions and the following
10 # disclaimer in the documentation and/or other materials provided 10 # disclaimer in the documentation and/or other materials provided
(...skipping 123 matching lines...) Expand 10 before | Expand all | Expand 10 after
134 error_descriptions)) 134 error_descriptions))
135 if warning_descriptions: 135 if warning_descriptions:
136 results.append(output_api.PresubmitPromptOrNotify( 136 results.append(output_api.PresubmitPromptOrNotify(
137 'You added one or more #includes of files that are temporarily\n' 137 'You added one or more #includes of files that are temporarily\n'
138 'allowed but being removed. Can you avoid introducing the\n' 138 'allowed but being removed. Can you avoid introducing the\n'
139 '#include? See relevant DEPS file(s) for details and contacts.', 139 '#include? See relevant DEPS file(s) for details and contacts.',
140 warning_descriptions)) 140 warning_descriptions))
141 return results 141 return results
142 142
143 143
144 def _CheckNoInlineHeaderIncludesInNormalHeaders(input_api, output_api):
145 """Attempts to prevent inclusion of inline headers into normal header
146 files. This tries to establish a layering where inline headers can be
147 included by other inline headers or compilation units only."""
148 file_inclusion_pattern = r'(?!.+-inl\.h).+\.h'
149 include_directive_pattern = input_api.re.compile(r'#include ".+-inl.h"')
150 include_warning = (
151 'You might be including an inline header (e.g. foo-inl.h) within a\n'
152 'normal header (e.g. bar.h) file. Can you avoid introducing the\n'
153 '#include? The commit queue will not block on this warning.')
154
155 def FilterFile(affected_file):
156 black_list = (_EXCLUDED_PATHS +
157 input_api.DEFAULT_BLACK_LIST)
158 return input_api.FilterSourceFile(
159 affected_file,
160 white_list=(file_inclusion_pattern, ),
161 black_list=black_list)
162
163 problems = []
164 for f in input_api.AffectedSourceFiles(FilterFile):
165 local_path = f.LocalPath()
166 for line_number, line in f.ChangedContents():
167 if (include_directive_pattern.search(line)):
168 problems.append(
169 '%s:%d\n %s' % (local_path, line_number, line.strip()))
170
171 if problems:
172 return [output_api.PresubmitPromptOrNotify(include_warning, problems)]
173 else:
174 return []
175
176
144 def _CheckNoProductionCodeUsingTestOnlyFunctions(input_api, output_api): 177 def _CheckNoProductionCodeUsingTestOnlyFunctions(input_api, output_api):
145 """Attempts to prevent use of functions intended only for testing in 178 """Attempts to prevent use of functions intended only for testing in
146 non-testing code. For now this is just a best-effort implementation 179 non-testing code. For now this is just a best-effort implementation
147 that ignores header files and may have some false positives. A 180 that ignores header files and may have some false positives. A
148 better implementation would probably need a proper C++ parser. 181 better implementation would probably need a proper C++ parser.
149 """ 182 """
150 # We only scan .cc files, as the declaration of for-testing functions in 183 # We only scan .cc files, as the declaration of for-testing functions in
151 # header files are hard to distinguish from calls to such functions without a 184 # header files are hard to distinguish from calls to such functions without a
152 # proper C++ parser. 185 # proper C++ parser.
153 file_inclusion_pattern = r'.+\.cc' 186 file_inclusion_pattern = r'.+\.cc'
(...skipping 34 matching lines...) Expand 10 before | Expand all | Expand 10 after
188 """Checks common to both upload and commit.""" 221 """Checks common to both upload and commit."""
189 results = [] 222 results = []
190 results.extend(input_api.canned_checks.CheckOwners( 223 results.extend(input_api.canned_checks.CheckOwners(
191 input_api, output_api, source_file_filter=None)) 224 input_api, output_api, source_file_filter=None))
192 results.extend(input_api.canned_checks.CheckPatchFormatted( 225 results.extend(input_api.canned_checks.CheckPatchFormatted(
193 input_api, output_api)) 226 input_api, output_api))
194 results.extend(_V8PresubmitChecks(input_api, output_api)) 227 results.extend(_V8PresubmitChecks(input_api, output_api))
195 results.extend(_CheckUnwantedDependencies(input_api, output_api)) 228 results.extend(_CheckUnwantedDependencies(input_api, output_api))
196 results.extend( 229 results.extend(
197 _CheckNoProductionCodeUsingTestOnlyFunctions(input_api, output_api)) 230 _CheckNoProductionCodeUsingTestOnlyFunctions(input_api, output_api))
231 results.extend(
232 _CheckNoInlineHeaderIncludesInNormalHeaders(input_api, output_api))
198 return results 233 return results
199 234
200 235
201 def _SkipTreeCheck(input_api, output_api): 236 def _SkipTreeCheck(input_api, output_api):
202 """Check the env var whether we want to skip tree check. 237 """Check the env var whether we want to skip tree check.
203 Only skip if include/v8-version.h has been updated.""" 238 Only skip if include/v8-version.h has been updated."""
204 src_version = 'include/v8-version.h' 239 src_version = 'include/v8-version.h'
205 FilterFile = lambda file: file.LocalPath() == src_version 240 FilterFile = lambda file: file.LocalPath() == src_version
206 if not input_api.AffectedSourceFiles( 241 if not input_api.AffectedSourceFiles(
207 lambda file: file.LocalPath() == src_version): 242 lambda file: file.LocalPath() == src_version):
(...skipping 52 matching lines...) Expand 10 before | Expand all | Expand 10 after
260 'v8_win64_rel': set(['defaulttests']), 295 'v8_win64_rel': set(['defaulttests']),
261 'v8_mac_rel': set(['defaulttests']), 296 'v8_mac_rel': set(['defaulttests']),
262 'v8_linux_arm_rel': set(['defaulttests']), 297 'v8_linux_arm_rel': set(['defaulttests']),
263 'v8_linux_arm64_rel': set(['defaulttests']), 298 'v8_linux_arm64_rel': set(['defaulttests']),
264 'v8_linux_mipsel_compile_rel': set(['defaulttests']), 299 'v8_linux_mipsel_compile_rel': set(['defaulttests']),
265 'v8_linux_mips64el_compile_rel': set(['defaulttests']), 300 'v8_linux_mips64el_compile_rel': set(['defaulttests']),
266 'v8_android_arm_compile_rel': set(['defaulttests']), 301 'v8_android_arm_compile_rel': set(['defaulttests']),
267 'v8_linux_chromium_gn_rel': set(['defaulttests']), 302 'v8_linux_chromium_gn_rel': set(['defaulttests']),
268 }, 303 },
269 } 304 }
OLDNEW
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698