Improve the diagnostics for include order presubmit checks
The information given by the presubmit checks when an include error
violation is found is not always the clearest, especially to a new
Chrome developer. The rules for determining which group a header file
goes into are not immediately obvious and the warnings don't say whether
the problem is a sort order or a group order. This change adds that
information. It also puts the URL on a separate line for easier
selection, especially important on Windows.
The old output looks like this:
** Presubmit Messages **
Your #include order seems to be broken. Remember to use the right collation (LC_COLLATE=C) and check https://google-styleguide.googlecode.com
m/svn/trunk/cppguide.html#Names_and_Order_of_Includes
chrome\browser\task_manager\task_manager.cc:10 \
chrome\browser\task_manager\task_manager.cc:11
The new output looks like this:
** Presubmit Messages **
Your #include order seems to be broken. Remember to use the right collation (LC_COLLATE=C) and check
https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Names_and_Order_of_Includes
chrome\browser\task_manager\task_manager.cc:10: - c++ system include file in wrong block \
chrome\browser\task_manager\task_manager.cc:11: - line belongs before previous line
Notice the explanations for why a line is highlighted, and the
non-wordwrapped URL.
[email protected]
Review URL: https://codereview.chromium.org/1213113004
Cr-Commit-Position: refs/heads/master@{#336813}
diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index 885055b..2effa50 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -63,7 +63,7 @@
_INCLUDE_ORDER_WARNING = (
'Your #include order seems to be broken. Remember to use the right '
- 'collation (LC_COLLATE=C) and check https://google-styleguide.googlecode'
+ 'collation (LC_COLLATE=C) and check\nhttps://google-styleguide.googlecode'
'.com/svn/trunk/cppguide.html#Names_and_Order_of_Includes')
_BANNED_OBJC_FUNCTIONS = (
@@ -702,33 +702,38 @@
previous_line = ''
previous_line_num = 0
problem_linenums = []
+ out_of_order = " - line belongs before previous line"
for line_num, line in scope:
if c_system_include_pattern.match(line):
if state != C_SYSTEM_INCLUDES:
- problem_linenums.append((line_num, previous_line_num))
+ problem_linenums.append((line_num, previous_line_num,
+ " - C system include file in wrong block"))
elif previous_line and previous_line > line:
- problem_linenums.append((line_num, previous_line_num))
+ problem_linenums.append((line_num, previous_line_num,
+ out_of_order))
elif cpp_system_include_pattern.match(line):
if state == C_SYSTEM_INCLUDES:
state = CPP_SYSTEM_INCLUDES
elif state == CUSTOM_INCLUDES:
- problem_linenums.append((line_num, previous_line_num))
+ problem_linenums.append((line_num, previous_line_num,
+ " - c++ system include file in wrong block"))
elif previous_line and previous_line > line:
- problem_linenums.append((line_num, previous_line_num))
+ problem_linenums.append((line_num, previous_line_num, out_of_order))
elif custom_include_pattern.match(line):
if state != CUSTOM_INCLUDES:
state = CUSTOM_INCLUDES
elif previous_line and previous_line > line:
- problem_linenums.append((line_num, previous_line_num))
+ problem_linenums.append((line_num, previous_line_num, out_of_order))
else:
- problem_linenums.append(line_num)
+ problem_linenums.append((line_num, previous_line_num,
+ "Unknown include type"))
previous_line = line
previous_line_num = line_num
warnings = []
- for (line_num, previous_line_num) in problem_linenums:
+ for (line_num, previous_line_num, failure_type) in problem_linenums:
if line_num in changed_linenums or previous_line_num in changed_linenums:
- warnings.append(' %s:%d' % (file_path, line_num))
+ warnings.append(' %s:%d:%s' % (file_path, line_num, failure_type))
return warnings
