Skip to content

Commit

Permalink
Add presubmit rule banning relative header includes
Browse files Browse the repository at this point in the history
Relative header includes (#include path containing "../") can be used to cheat
the dependency system because they're not checked properly. This CL adds a
presubmit rule to catch these.

This rules applies to third_party/WebKit, but not anywhere else in third_party/.
There's one currently existing file I know of that would fail this rule:
ppapi/lib/gl/include/GLES2/gl2.h

I did not change this file when cleaning up the other headers since it appears
to be code imported from a third-party repo. I guess whoever updates this file
will have to bypass the rule.

BUG=724264

Review-Url: https://codereview.chromium.org/2900173003
Cr-Commit-Position: refs/heads/master@{#474396}
  • Loading branch information
rlanday authored and Commit bot committed May 24, 2017
1 parent 5d947f5 commit 90089e1
Showing 1 changed file with 38 additions and 0 deletions.
38 changes: 38 additions & 0 deletions PRESUBMIT.py
Original file line number Diff line number Diff line change
Expand Up @@ -2150,6 +2150,43 @@ def _CheckForRiskyJsFeatures(input_api, output_api):
""" % "\n".join(" %s:%d\n" % line for line in arrow_lines))]


def _CheckForRelativeIncludes(input_api, output_api):
from cpp_checker import CppChecker

bad_files = {}
for f in input_api.AffectedFiles(include_deletes=False):
if (f.LocalPath().startswith('third_party') and
not f.LocalPath().startswith('third_party/WebKit') and
not f.LocalPath().startswith('third_party\\WebKit')):
continue

if not CppChecker.IsCppFile(f.LocalPath()):
continue

relative_includes = [line for line_num, line in f.ChangedContents()
if "#include" in line and "../" in line]
bad_files[f.LocalPath()] = relative_includes

if not bad_files:
return []

error_descriptions = []
for file_path, bad_lines in bad_files.iteritems():
error_description = file_path
for line in bad_lines:
error_description += '\n ' + line
error_descriptions.append(error_description)

results = []
results.append(output_api.PresubmitError(
'You added one or more relative #include paths (including "../").\n'
'These shouldn\'t be used because they can be used to include headers\n'
'from code that\'s not correctly specified as a dependency in the\n'
'relevant BUILD.gn file(s).',
error_descriptions))

return results

def _AndroidSpecificOnUploadChecks(input_api, output_api):
"""Groups checks that target android code."""
results = []
Expand Down Expand Up @@ -2211,6 +2248,7 @@ def _CommonChecks(input_api, output_api):
results.extend(_CheckIpcOwners(input_api, output_api))
results.extend(_CheckUselessForwardDeclarations(input_api, output_api))
results.extend(_CheckForRiskyJsFeatures(input_api, output_api))
results.extend(_CheckForRelativeIncludes(input_api, output_api))

if any('PRESUBMIT.py' == f.LocalPath() for f in input_api.AffectedFiles()):
results.extend(input_api.canned_checks.RunUnitTestsInDirectory(
Expand Down

0 comments on commit 90089e1

Please sign in to comment.