Skip to content

Add build/namespaces_names error #286

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
scrouthtv wants to merge 3 commits into from
Closed

Add build/namespaces_names error #286

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5c3ca51
[build/namespaces_names] check namespace names
scrouthtv Sep 3, 2024
7bd458c
[build/namespaces_names] added unit test
scrouthtv Sep 3, 2024
2fc7bca
[build/namespaces_names] The v8 sample fails the check
scrouthtv Sep 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions cpplint.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,7 @@
'build/include_what_you_use',
'build/namespaces_headers',
'build/namespaces_literals',
'build/namespaces_names',
'build/namespaces',
'build/printf_format',
'build/storage_class',
Expand Down Expand Up @@ -5484,6 +5485,19 @@ def CheckLanguage(filename, clean_lines, linenum, file_extension,
'https://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Namespaces'
' for more information.')

# Check namespace names for correct naming, according to
# https://google.github.io/styleguide/cppguide.html#Namespace_Names
# namespace names are "all lower-case, with words separated by underscores"
match = re.match(r'.*namespace\s+([^{=;]+)\s+[{;=]', line)
if match:
name = match.group(1)
if name != "" and not re.match(r'^((inline )?[a-z_]+(::)?)+$', name):
Comment on lines +5491 to +5494
Copy link
Member

@aaronliu0130 aaronliu0130 Sep 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be much better if the detection was based on nesting_state. The could changed by #235 should provide an example.

error(filename, linenum, 'build/namespaces_names', 4,
Copy link
Member

@aaronliu0130 aaronliu0130 Sep 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
error(filename, linenum, 'build/namespaces_names', 4,
error(filename, linenum, 'naming/namespaces', 4,

This is decidedly not a building issue. This entire check is also not suited for the language rules−checking function. (I recommend you move this to a new function, which should also solve the lint errors.

'Namespace names must be all lower-case, with words separated '
'by underscores. See '
'https://google.github.io/styleguide/cppguide.html#Namespace_Names'
' for more information.')
Comment on lines +5496 to +5499
Copy link
Member

@aaronliu0130 aaronliu0130 Sep 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'Namespace names must be all lower-case, with words separated '
'by underscores. See '
'https://google.github.io/styleguide/cppguide.html#Namespace_Names'
' for more information.')
'Namespace names must be all lower-case with words separated '
'by underscores.')

We never link to the style guide if it's pretty obvious, and there's a comma splice here.



def CheckGlobalStatic(filename, clean_lines, linenum, error):
"""Check for unsafe global or static objects.
Expand Down
28 changes: 28 additions & 0 deletions cpplint_unittest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4578,6 +4578,34 @@ def testUnnamedNamespacesInNonHeaders(self):
self.TestLanguageRulesCheck('foo.' + extension, 'namespace {', '')
self.TestLanguageRulesCheck('foo.' + extension, 'namespace foo {', '')

def testNamespaceNames(self):
for extension in ['h', 'hpp', 'hxx', 'h++', 'cuh']:
self.TestLanguageRulesCheck(
'foo.' + extension, 'namespace CamelCase {}',
'Namespace names must be all lower-case, with words separated by underscores. '
'See https://google.github.io/styleguide/cppguide.html#Namespace_Names '
'for more information. [build/namespaces_names] [4]')
self.TestLanguageRulesCheck(
'foo.' + extension, 'inline namespace Uppercase {',
'Namespace names must be all lower-case, with words separated by underscores. '
'See https://google.github.io/styleguide/cppguide.html#Namespace_Names '
'for more information. [build/namespaces_names] [4]')
self.TestLanguageRulesCheck(
'foo.' + extension, 'namespace Uppercase = std',
'Namespace names must be all lower-case, with words separated by underscores. '
'See https://google.github.io/styleguide/cppguide.html#Namespace_Names '
'for more information. [build/namespaces_names] [4]')
self.TestLanguageRulesCheck(
'foo.' + extension, 'namespace a::inline b::inline C {',
'Namespace names must be all lower-case, with words separated by underscores. '
'See https://google.github.io/styleguide/cppguide.html#Namespace_Names '
'for more information. [build/namespaces_names] [4]')
Comment on lines +4584 to +4602
Copy link
Member

@aaronliu0130 aaronliu0130 Sep 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'foo.' + extension, 'namespace CamelCase {}',
'Namespace names must be all lower-case, with words separated by underscores. '
'See https://google.github.io/styleguide/cppguide.html#Namespace_Names '
'for more information. [build/namespaces_names] [4]')
self.TestLanguageRulesCheck(
'foo.' + extension, 'inline namespace Uppercase {',
'Namespace names must be all lower-case, with words separated by underscores. '
'See https://google.github.io/styleguide/cppguide.html#Namespace_Names '
'for more information. [build/namespaces_names] [4]')
self.TestLanguageRulesCheck(
'foo.' + extension, 'namespace Uppercase = std',
'Namespace names must be all lower-case, with words separated by underscores. '
'See https://google.github.io/styleguide/cppguide.html#Namespace_Names '
'for more information. [build/namespaces_names] [4]')
self.TestLanguageRulesCheck(
'foo.' + extension, 'namespace a::inline b::inline C {',
'Namespace names must be all lower-case, with words separated by underscores. '
'See https://google.github.io/styleguide/cppguide.html#Namespace_Names '
'for more information. [build/namespaces_names] [4]')
'foo.' + extension, 'namespace CamelCase {}',
'Namespace names must be all lower-case with words separated by underscores. '
'[naming/namespaces] [4]')
self.TestLanguageRulesCheck(
'foo.' + extension, 'inline namespace Uppercase {',
'Namespace names must be all lower-case with words separated by underscores. '
'[naming/namespaces] [4]')
self.TestLanguageRulesCheck(
'foo.' + extension, 'namespace Uppercase = std',
'Namespace names must be all lower-case with words separated by underscores. '
'[naming/namespaces] [4]')
self.TestLanguageRulesCheck(
'foo.' + extension, 'namespace a::inline b::inline C {',
'Namespace names must be all lower-case with words separated by underscores. '
'[naming/namespaces] [4]')

Ditto

self.TestLanguageRulesCheck('foo.' + extension, 'namespace lower_case {}', '')
self.TestLanguageRulesCheck('foo.' + extension, 'namespace a::b::c {}', '')
self.TestLanguageRulesCheck('foo.' + extension, 'namespace a::b::inline c {}', '')
self.TestLanguageRulesCheck('foo.' + extension, 'namespace lower_case = Uppercase', '')
self.TestLanguageRulesCheck('foo.' + extension, 'inline namespace a::inline b {}}', '')

def testBuildClass(self):
# Test that the linter can parse to the end of class definitions,
# and that it will report when it can't.
Expand Down
3 changes: 2 additions & 1 deletion samples/v8-sample/simple.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ src/*
1
3
Done processing src/interface-descriptors.h
Total errors found: 2
Total errors found: 3

src/interface-descriptors.h:5: #ifndef header guard has wrong style, please use: SAMPLES_V8_SAMPLE_SRC_INTERFACE_DESCRIPTORS_H_ [build/header_guard] [5]
src/interface-descriptors.h:1255: #endif line should be "#endif // SAMPLES_V8_SAMPLE_SRC_INTERFACE_DESCRIPTORS_H_" [build/header_guard] [5]
src/interface-descriptors.h:15: Namespace names must be all lower-case, with words separated by underscores. See https://google.github.io/styleguide/cppguide.html#Namespace_Names for more information. [build/namespaces_names] [4]
Copy link
Member

@aaronliu0130 aaronliu0130 Sep 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
src/interface-descriptors.h:15: Namespace names must be all lower-case, with words separated by underscores. See https://google.github.io/styleguide/cppguide.html#Namespace_Names for more information. [build/namespaces_names] [4]
src/interface-descriptors.h:15: Namespace names must be all lower-case with words separated by underscores. [naming/namespaces] [4]