Commit 7b505abe authored by eric@webkit.org's avatar eric@webkit.org
Browse files

2009-12-02 David Levin <levin@chromium.org>

        Reviewed by Adam Barth.

        check-webkit-style is too noisy about namespace indenting issues.
        https://bugs.webkit.org/show_bug.cgi?id=32096

        * Scripts/modules/cpp_style.py:
        Added a _FileState object to be able to track file level information. In this
        case, it simply tracks whether the error has already been given, so that it isn't
        done again.
        * Scripts/modules/cpp_style_unittest.py:
        Modified test cases to pass in the _FileState object and fix a test that expected
        to see the namespace error twice (now it only occurs once). No new tests because
        existing tests cover the change in functionality.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@51619 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 81183fd1
2009-12-02 David Levin <levin@chromium.org>
Reviewed by Adam Barth.
check-webkit-style is too noisy about namespace indenting issues.
https://bugs.webkit.org/show_bug.cgi?id=32096
* Scripts/modules/cpp_style.py:
Added a _FileState object to be able to track file level information. In this
case, it simply tracks whether the error has already been given, so that it isn't
done again.
* Scripts/modules/cpp_style_unittest.py:
Modified test cases to pass in the _FileState object and fix a test that expected
to see the namespace error twice (now it only occurs once). No new tests because
existing tests cover the change in functionality.
2009-12-01 Kevin Ollivier <kevino@theolliviers.com>
Reviewed by Eric Seidel.
......
......@@ -1119,6 +1119,16 @@ class _ClassState(object):
self.classinfo_stack[0].name)
class _FileState(object):
def __init__(self):
self._did_inside_namespace_indent_warning = False
def set_did_inside_namespace_indent_warning(self):
self._did_inside_namespace_indent_warning = True
def did_inside_namespace_indent_warning(self):
return self._did_inside_namespace_indent_warning
def check_for_non_standard_constructs(filename, clean_lines, line_number,
class_state, error):
"""Logs an error if we see certain non-ANSI constructs ignored by gcc-2.
......@@ -1675,7 +1685,7 @@ def get_previous_non_blank_line(clean_lines, line_number):
return ('', -1)
def check_namespace_indentation(filename, clean_lines, line_number, file_extension, error):
def check_namespace_indentation(filename, clean_lines, line_number, file_extension, file_state, error):
"""Looks for indentation errors inside of namespaces.
Args:
......@@ -1683,6 +1693,8 @@ def check_namespace_indentation(filename, clean_lines, line_number, file_extensi
clean_lines: A CleansedLines instance containing the file.
line_number: The number of the line to check.
file_extension: The extension (dot not included) of the file.
file_state: A _FileState instance which maintains information about
the state of things in the file.
error: The function to call with any errors found.
"""
......@@ -1694,8 +1706,10 @@ def check_namespace_indentation(filename, clean_lines, line_number, file_extensi
current_indentation_level = len(namespace_match.group('namespace_indentation'))
if current_indentation_level > 0:
error(filename, line_number, 'whitespace/indent', 4,
'namespace should never be indented.')
# Don't warn about an indented namespace if we already warned about indented code.
if not file_state.did_inside_namespace_indent_warning():
error(filename, line_number, 'whitespace/indent', 4,
'namespace should never be indented.')
return
looking_for_semicolon = False;
line_offset = 0
......@@ -1706,7 +1720,8 @@ def check_namespace_indentation(filename, clean_lines, line_number, file_extensi
continue
if not current_indentation_level:
if not (in_preprocessor_directive or looking_for_semicolon):
if not match(r'\S', current_line):
if not match(r'\S', current_line) and not file_state.did_inside_namespace_indent_warning():
file_state.set_did_inside_namespace_indent_warning()
error(filename, line_number + line_offset, 'whitespace/indent', 4,
'Code inside a namespace should not be indented.')
if in_preprocessor_directive or (current_line.strip()[0] == '#'): # This takes care of preprocessor directive syntax.
......@@ -2124,7 +2139,7 @@ def get_line_width(line):
return len(line)
def check_style(filename, clean_lines, line_number, file_extension, error):
def check_style(filename, clean_lines, line_number, file_extension, file_state, error):
"""Checks rules from the 'C++ style rules' section of cppguide.html.
Most of these rules are hard to test (naming, comment style), but we
......@@ -2136,6 +2151,8 @@ def check_style(filename, clean_lines, line_number, file_extension, error):
clean_lines: A CleansedLines instance containing the file.
line_number: The number of the line to check.
file_extension: The extension (without the dot) of the filename.
file_state: A _FileState instance which maintains information about
the state of things in the file.
error: The function to call with any errors found.
"""
......@@ -2203,7 +2220,7 @@ def check_style(filename, clean_lines, line_number, file_extension, error):
'operators on the left side of the line instead of the right side.')
# Some more style checks
check_namespace_indentation(filename, clean_lines, line_number, file_extension, error)
check_namespace_indentation(filename, clean_lines, line_number, file_extension, file_state, error)
check_using_std(filename, clean_lines, line_number, error)
check_max_min_macros(filename, clean_lines, line_number, error)
check_switch_indentation(filename, clean_lines, line_number, error)
......@@ -2914,7 +2931,7 @@ def check_for_include_what_you_use(filename, clean_lines, include_state, error,
def process_line(filename, file_extension,
clean_lines, line, include_state, function_state,
class_state, error):
class_state, file_state, error):
"""Processes a single line in the file.
Args:
......@@ -2927,6 +2944,8 @@ def process_line(filename, file_extension,
function_state: A _FunctionState instance which counts function lines, etc.
class_state: A _ClassState instance which maintains information about
the current stack of nested class declarations being parsed.
file_state: A _FileState instance which maintains information about
the state of things in the file.
error: A callable to which errors are reported, which takes 4 arguments:
filename, line number, error level, and message
......@@ -2936,7 +2955,7 @@ def process_line(filename, file_extension,
if search(r'\bNOLINT\b', raw_lines[line]): # ignore nolint lines
return
check_for_multiline_comments_and_strings(filename, clean_lines, line, error)
check_style(filename, clean_lines, line, file_extension, error)
check_style(filename, clean_lines, line, file_extension, file_state, error)
check_language(filename, clean_lines, line, file_extension, include_state,
error)
check_for_non_standard_constructs(filename, clean_lines, line,
......@@ -2961,6 +2980,7 @@ def process_file_data(filename, file_extension, lines, error):
include_state = _IncludeState()
function_state = _FunctionState()
class_state = _ClassState()
file_state = _FileState()
check_for_copyright(filename, lines, error)
......@@ -2971,7 +2991,7 @@ def process_file_data(filename, file_extension, lines, error):
clean_lines = CleansedLines(lines)
for line in xrange(clean_lines.num_lines()):
process_line(filename, file_extension, clean_lines, line,
include_state, function_state, class_state, error)
include_state, function_state, class_state, file_state, error)
class_state.check_finished(filename, error)
check_for_include_what_you_use(filename, clean_lines, include_state, error)
......
......@@ -120,9 +120,10 @@ class CppStyleTestBase(unittest.TestCase):
function_state = cpp_style._FunctionState()
ext = file_name[file_name.rfind('.') + 1:]
class_state = cpp_style._ClassState()
file_state = cpp_style._FileState()
cpp_style.process_line(file_name, ext, clean_lines, 0,
include_state, function_state,
class_state, error_collector)
class_state, file_state, error_collector)
# Single-line lint tests are allowed to fail the 'unlintable function'
# check.
error_collector.remove_if_present(
......@@ -137,8 +138,9 @@ class CppStyleTestBase(unittest.TestCase):
lines = cpp_style.CleansedLines(lines)
ext = file_name[file_name.rfind('.') + 1:]
class_state = cpp_style._ClassState()
file_state = cpp_style._FileState()
for i in xrange(lines.num_lines()):
cpp_style.check_style(file_name, lines, i, ext, error_collector)
cpp_style.check_style(file_name, lines, i, ext, file_state, error_collector)
cpp_style.check_for_non_standard_constructs(file_name, lines, i, class_state,
error_collector)
class_state.check_finished(file_name, error_collector)
......@@ -2834,7 +2836,16 @@ class WebKitStyleTest(CppStyleTestBase):
'};\n'
'};\n'
'}',
['Code inside a namespace should not be indented. [whitespace/indent] [4]', 'namespace should never be indented. [whitespace/indent] [4]'],
'Code inside a namespace should not be indented. [whitespace/indent] [4]',
'foo.h')
self.assert_multi_line_lint(
'namespace OuterNamespace {\n'
' class Document {\n'
' namespace InnerNamespace {\n'
'};\n'
'};\n'
'}',
'Code inside a namespace should not be indented. [whitespace/indent] [4]',
'foo.h')
self.assert_multi_line_lint(
'namespace WebCore {\n'
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment