[Webkit-unassigned] [Bug 27461] Add checks for namespace indentation to cpplint

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=27461


David Levin <levin at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #33181|review?                     |review+
               Flag|                            |




--- Comment #11 from David Levin <levin at chromium.org>  2009-07-21 09:45:40 PDT ---
(From update of attachment 33181)
It could be landed as is.  If the lander would fix these issues, it would be
even better.


> diff --git a/WebKitTools/Scripts/modules/cpplint.py b/WebKitTools/Scripts/modules/cpplint.py
> +def check_namespace_indentation(filename, clean_lines, line_number, file_extension, error):
> +    """Looks for indentation errors inside of namespaces.
> +
> +    Args:
> +      filename: The name of the current file.
> +      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.
> +      error: The function to call with any errors found.
> +    """
> +
> +    line = clean_lines.elided[line_number] # get rid of comments and strings

It is nice to make comment look like sentences.  Start with a capital. End with
a period.  (I know this was copied from elsewhere in the file.)


> +    if is_implementation_file:
> +        for current_line in clean_lines.raw_lines[line_number + 1:]:
> +            line_offset += 1
> +
> +            # Skip not only empty lines but also those with (goto) labels.

You should be able to hit a goto label before you hit other code and break out
of this, but no big deal.  Maybe it is useful to leave this in case we decide
to extend this function to not have the break.


> diff --git a/WebKitTools/Scripts/modules/cpplint_unittest.py b/WebKitTools/Scripts/modules/cpplint_unittest.py

> +        self.assert_multi_line_lint(
> +            'namespace WebCore {\n\n'
> +            'bool Document::Foo() {}\n'
> +            '    return true;\n'

It would be nice to have valid code.  Something like this:

            'bool Document::Foo()\n'
            '{\n'
            '    return true;\n'
            '}',

> +            '}',
> +            '',
> +            'foo.cpp')
> +        self.assert_multi_line_lint(
> +            'namespace OuterNamespace {\n'
> +            'namespace InnerNamespace {\n'
> +            'Document::Foo() { }\n'
> +            '}',
> +            '',
> +            'foo.cpp')
> +        self.assert_multi_line_lint(
> +            '    namespace WebCore {\n\n'
> +            '    void Document::Foo() {}\n'
> +            'start:  // infinite loops are fun!\n'
> +            '        goto start;\n'
Invalid code.

> +            '    }',
> +            '',
> +            'foo.cpp')

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.