Skip to content
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

Jump to bottom

addons/namingng.py: Improve output and unit test. #5820

Merged
merged 2 commits into from
Jan 3, 2024
Merged

addons/namingng.py: Improve output and unit test. #5820

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8aeacbf
addons/namingng.py: Report column positions, expand unit test.
mvds00 Jan 2, 2024
4a6ca25
addons/namingng.py: Fix labeling of global variable naming error.
mvds00 Jan 2, 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
4 changes: 2 additions & 2 deletions addons/cppcheckdata.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1619,11 +1619,11 @@ def is_suppressed(location, message, errorId):
return True
return False

def reportError(location, severity, message, addon, errorId, extra=''):
def reportError(location, severity, message, addon, errorId, extra='', columnOverride=None):
if '--cli' in sys.argv:
msg = { 'file': location.file,
'linenr': location.linenr,
'column': location.column,
'column': location.column if columnOverride is None else columnOverride,
'severity': severity,
'message': message,
'addon': addon,
Expand Down
52 changes: 27 additions & 25 deletions addons/namingng.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,14 @@

# Auxiliary class
class DataStruct:
def __init__(self, file, linenr, string):
def __init__(self, file, linenr, string, column=0):
self.file = file
self.linenr = linenr
self.str = string
self.column = 0
self.column = column

def reportNamingError(location,message,errorId='namingConvention',severity='style',extra=''):
cppcheckdata.reportError(location,severity,message,'namingng',errorId,extra)
def reportNamingError(location,message,errorId='namingConvention',severity='style',extra='',column=None):
cppcheckdata.reportError(location,severity,message,'namingng',errorId,extra,columnOverride=column)

def configError(error,fatal=True):
print('config error: %s'%error)
Expand Down Expand Up @@ -167,8 +167,9 @@ def check_include_guard_name(conf_include_guard,cfg,directive):
if len(parts) != 2:
msg = 'syntax error'
reportNamingError(directive,msg,'syntax')
return None
return None,None
guard_name = parts[1]
guard_column = 1+directive.str.find(guard_name)
danmar marked this conversation as resolved.
Show resolved Hide resolved

filename = directive.file
if conf_include_guard.get('input','path') == 'basename':
Expand All @@ -188,9 +189,9 @@ def check_include_guard_name(conf_include_guard,cfg,directive):
expect_guard_name = conf_include_guard.get('prefix','') + barename + conf_include_guard.get('suffix','')
if expect_guard_name != guard_name:
msg = 'include guard naming violation; %s != %s'%(guard_name,expect_guard_name)
reportNamingError(directive,msg,'includeGuardName')
reportNamingError(directive,msg,'includeGuardName',column=guard_column)

return guard_name
return guard_name,guard_column

def check_include_guard(conf_include_guard,cfg,unguarded_include_files):
# Scan for '#ifndef FILE_H' as the first directive, in the first N lines.
Expand All @@ -200,19 +201,19 @@ def check_include_guard(conf_include_guard,cfg,unguarded_include_files):
# - test whether include guards are in place
max_linenr = conf_include_guard.get('max_linenr', 5)

def report(directive,msg,errorId):
reportNamingError(directive,msg,errorId)
def report(directive,msg,errorId,column=0):
reportNamingError(directive,msg,errorId,column=column)

def report_pending_ifndef(directive):
report(directive,'include guard #ifndef is not followed by #define','includeGuardIncomplete')
def report_pending_ifndef(directive,column):
report(directive,'include guard #ifndef is not followed by #define','includeGuardIncomplete',column=column)

last_fn = None
pending_ifndef = None
phase = 0
for directive in cfg.directives:
if last_fn != directive.file:
if pending_ifndef:
report_pending_ifndef(pending_ifndef)
report_pending_ifndef(pending_ifndef,guard_column)
pending_ifndef = None
last_fn = directive.file
phase = 0
Expand All @@ -237,7 +238,7 @@ def report_pending_ifndef(directive):
report(directive,'first preprocessor directive should be include guard #ifndef','includeGuardMissing')
phase = -1
continue
guard_name = check_include_guard_name(conf_include_guard,cfg,directive)
guard_name,guard_column = check_include_guard_name(conf_include_guard,cfg,directive)
if guard_name == None:
phase = -1
continue
Expand All @@ -256,13 +257,13 @@ def report_pending_ifndef(directive):
phase = -1
continue
if guard_name != parts[1]:
report(directive,'include guard does not guard; %s != %s'%(guard_name,parts[1]),'includeGuardAwayFromDuty',severity='warning')
report(directive,'include guard does not guard; %s != %s'%(guard_name,parts[1]),'includeGuardAwayFromDuty',severity='warning',column=guard_column)

unguarded_include_files.remove(directive.file)

phase = -1
if pending_ifndef:
report_pending_ifndef(pending_ifndef)
report_pending_ifndef(pending_ifndef,guard_column)

def process(dumpfiles, configfile, debugprint=False):
conf = loadConfig(configfile)
Expand Down Expand Up @@ -294,7 +295,7 @@ def process(dumpfiles, configfile, debugprint=False):
if conf.namespace:
for tk in data.rawTokens:
if tk.str == 'namespace':
mockToken = DataStruct(tk.next.file, tk.next.linenr, tk.next.str)
mockToken = DataStruct(tk.next.file, tk.next.linenr, tk.next.str, tk.next.column)
msgType = 'Namespace'
for exp in conf.namespace:
evalExpr(conf.namespace, exp, mockToken, msgType)
Expand Down Expand Up @@ -333,9 +334,10 @@ def process(dumpfiles, configfile, debugprint=False):
reportNamingError(var.typeStartToken,
'Variable ' +
var.nameToken.str +
' violates naming convention')
' violates naming convention',
column=var.nameToken.column)

mockToken = DataStruct(var.typeStartToken.file, var.typeStartToken.linenr, var.nameToken.str)
mockToken = DataStruct(var.typeStartToken.file, var.typeStartToken.linenr, var.nameToken.str, var.nameToken.column)
msgType = 'Variable'
for exp in conf.variable:
evalExpr(conf.variable, exp, mockToken, msgType)
Expand All @@ -346,7 +348,7 @@ def process(dumpfiles, configfile, debugprint=False):
for var in cfg.variables:
if (var.access is None) or var.access != 'Private':
continue
mockToken = DataStruct(var.typeStartToken.file, var.typeStartToken.linenr, var.nameToken.str)
mockToken = DataStruct(var.typeStartToken.file, var.typeStartToken.linenr, var.nameToken.str, var.nameToken.column)
msgType = 'Private member variable'
for exp in conf.private_member:
evalExpr(conf.private_member, exp, mockToken, msgType)
Expand All @@ -356,7 +358,7 @@ def process(dumpfiles, configfile, debugprint=False):
for var in cfg.variables:
if (var.access is None) or var.access != 'Public':
continue
mockToken = DataStruct(var.typeStartToken.file, var.typeStartToken.linenr, var.nameToken.str)
mockToken = DataStruct(var.typeStartToken.file, var.typeStartToken.linenr, var.nameToken.str, var.nameToken.column)
msgType = 'Public member variable'
for exp in conf.public_member:
evalExpr(conf.public_member, exp, mockToken, msgType)
Expand All @@ -366,8 +368,8 @@ def process(dumpfiles, configfile, debugprint=False):
for var in cfg.variables:
if (var.access is None) or var.access != 'Global':
continue
mockToken = DataStruct(var.typeStartToken.file, var.typeStartToken.linenr, var.nameToken.str)
msgType = 'Public member variable'
mockToken = DataStruct(var.typeStartToken.file, var.typeStartToken.linenr, var.nameToken.str, var.nameToken.column)
msgType = 'Global variable'
for exp in conf.global_variable:
evalExpr(conf.global_variable, exp, mockToken, msgType)

Expand All @@ -387,8 +389,8 @@ def process(dumpfiles, configfile, debugprint=False):

if retval and retval in conf.function_prefixes:
if not token.function.name.startswith(conf.function_prefixes[retval]):
reportNamingError(token, 'Function ' + token.function.name + ' violates naming convention')
mockToken = DataStruct(token.file, token.linenr, token.function.name)
reportNamingError(token, 'Function ' + token.function.name + ' violates naming convention', column=token.column)
mockToken = DataStruct(token.file, token.linenr, token.function.name, token.column)
msgType = 'Function'
for exp in conf.function_name:
evalExpr(conf.function_name, exp, mockToken, msgType)
Expand All @@ -398,7 +400,7 @@ def process(dumpfiles, configfile, debugprint=False):
for fnc in cfg.functions:
# Check if it is Constructor/Destructor
if fnc.type == 'Constructor' or fnc.type == 'Destructor':
mockToken = DataStruct(fnc.tokenDef.file, fnc.tokenDef.linenr, fnc.name)
mockToken = DataStruct(fnc.tokenDef.file, fnc.tokenDef.linenr, fnc.name, fnc.tokenDef.column)
msgType = 'Class ' + fnc.type
for exp in conf.class_name:
evalExpr(conf.class_name, exp, mockToken, msgType)
Expand Down
72 changes: 57 additions & 15 deletions test/cli/test-other.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,11 @@ def test_addon_namingng(tmpdir):
"RE_FILE": [
"[^/]*[a-z][a-z0-9_]*[a-z0-9]\\.c\\Z"
],
"RE_CLASS_NAME": ["[a-z][a-z0-9_]*[a-z0-9]\\Z"],
"RE_NAMESPACE": ["[a-z][a-z0-9_]*[a-z0-9]\\Z"],
"RE_VARNAME": ["[a-z][a-z0-9_]*[a-z0-9]\\Z"],
"RE_PUBLIC_MEMBER_VARIABLE": ["[a-z][a-z0-9_]*[a-z0-9]\\Z"],
"RE_PRIVATE_MEMBER_VARIABLE": ["[a-z][a-z0-9_]*[a-z0-9]\\Z"],
"RE_GLOBAL_VARNAME": ["[a-z][a-z0-9_]*[a-z0-9]\\Z"],
"RE_FUNCTIONNAME": ["[a-z][a-z0-9_]*[a-z0-9]\\Z"],
"include_guard": {
Expand Down Expand Up @@ -384,11 +388,11 @@ def test_addon_namingng(tmpdir):
#endif
""".format(test_unguarded_include_file))

test_file_basename = 'test_.c'
test_file_basename = 'test_.cpp'
test_file = os.path.join(tmpdir, test_file_basename)
with open(test_file, 'wt') as f:
f.write("""
#include "{}"
#include "%s"

void invalid_function_();
void _invalid_function();
Expand All @@ -403,7 +407,17 @@ def test_addon_namingng(tmpdir):

int _invalid_global;
static int _invalid_static_global;
""".format(test_include_file_basename))

class _clz {
public:
_clz() : _invalid_public(0), _invalid_private(0) { }
int _invalid_public;
private:
char _invalid_private;
};

namespace _invalid_namespace { }
"""%(test_include_file_basename))

args = ['--addon='+addon_file, '--verbose', '--enable=all', test_file]

Expand All @@ -417,38 +431,66 @@ def test_addon_namingng(tmpdir):
'Includes:',
'Platform:native'
]
lines = [line for line in stderr.splitlines() if line.strip() != '^' and line != '']
lines = [line for line in stderr.splitlines() if line != '']
expect = [
'{}:0:0: style: File name {} violates naming convention [namingng-namingConvention]'.format(test_include_file,test_include_file_basename),
'{}:2:0: style: include guard naming violation; TEST_H != _TEST_H [namingng-includeGuardName]'.format(test_include_file),
'^',
'{}:2:9: style: include guard naming violation; TEST_H != _TEST_H [namingng-includeGuardName]'.format(test_include_file),
'#ifndef TEST_H',
'{}:5:0: style: Function InvalidFunction violates naming convention [namingng-namingConvention]'.format(test_include_file),
' ^',
'{}:5:6: style: Function InvalidFunction violates naming convention [namingng-namingConvention]'.format(test_include_file),
'void InvalidFunction();',
'{}:6:0: style: Public member variable _invalid_extern_global violates naming convention [namingng-namingConvention]'.format(test_include_file),
' ^',
'{}:6:12: style: Global variable _invalid_extern_global violates naming convention [namingng-namingConvention]'.format(test_include_file),
'extern int _invalid_extern_global;',
' ^',

'{}:0:0: style: File name {} violates naming convention [namingng-namingConvention]'.format(test_unguarded_include_file,test_unguarded_include_file_basename),
'^',
'{}:0:0: style: Missing include guard [namingng-includeGuardMissing]'.format(test_unguarded_include_file),
'{}:2:0: style: Function InvalidFunctionUnguarded violates naming convention [namingng-namingConvention]'.format(test_unguarded_include_file),
'^',
'{}:2:6: style: Function InvalidFunctionUnguarded violates naming convention [namingng-namingConvention]'.format(test_unguarded_include_file),
'void InvalidFunctionUnguarded();',
' ^',

'{}:0:0: style: File name {} violates naming convention [namingng-namingConvention]'.format(test_file,test_file_basename),
'{}:7:0: style: Variable _invalid_arg violates naming convention [namingng-namingConvention]'.format(test_file),
'^',
'{}:7:26: style: Variable _invalid_arg violates naming convention [namingng-namingConvention]'.format(test_file),
'void valid_function2(int _invalid_arg);',
'{}:8:0: style: Variable invalid_arg_ violates naming convention [namingng-namingConvention]'.format(test_file),
' ^',
'{}:8:26: style: Variable invalid_arg_ violates naming convention [namingng-namingConvention]'.format(test_file),
'void valid_function3(int invalid_arg_);',
'{}:10:22: style: Variable invalid_arg32 violates naming convention [namingng-namingConvention]'.format(test_file),
' ^',
'{}:10:31: style: Variable invalid_arg32 violates naming convention [namingng-namingConvention]'.format(test_file),
'void valid_function5(uint32_t invalid_arg32);',
'{}:4:0: style: Function invalid_function_ violates naming convention [namingng-namingConvention]'.format(test_file),
' ^',
'{}:4:6: style: Function invalid_function_ violates naming convention [namingng-namingConvention]'.format(test_file),
'void invalid_function_();',
'{}:5:0: style: Function _invalid_function violates naming convention [namingng-namingConvention]'.format(test_file),
' ^',
'{}:5:6: style: Function _invalid_function violates naming convention [namingng-namingConvention]'.format(test_file),
'void _invalid_function();',
' ^',
'{}:12:10: style: Function invalid_function7 violates naming convention [namingng-namingConvention]'.format(test_file),
'uint16_t invalid_function7(int valid_arg);',
'{}:15:0: style: Public member variable _invalid_global violates naming convention [namingng-namingConvention]'.format(test_file),
' ^',
'{}:15:5: style: Global variable _invalid_global violates naming convention [namingng-namingConvention]'.format(test_file),
'int _invalid_global;',
'{}:16:0: style: Public member variable _invalid_static_global violates naming convention [namingng-namingConvention]'.format(test_file),
' ^',
'{}:16:12: style: Global variable _invalid_static_global violates naming convention [namingng-namingConvention]'.format(test_file),
'static int _invalid_static_global;',
' ^',
'{}:20:5: style: Class Constructor _clz violates naming convention [namingng-namingConvention]'.format(test_file),
' _clz() : _invalid_public(0), _invalid_private(0) { }',
' ^',
'{}:21:9: style: Public member variable _invalid_public violates naming convention [namingng-namingConvention]'.format(test_file),
' int _invalid_public;',
' ^',
'{}:23:10: style: Private member variable _invalid_private violates naming convention [namingng-namingConvention]'.format(test_file),
' char _invalid_private;',
' ^',
'{}:26:11: style: Namespace _invalid_namespace violates naming convention [namingng-namingConvention]'.format(test_file),
'namespace _invalid_namespace { }',
' ^',
]
# test sorted lines; the order of messages may vary and is not of importance
lines.sort()
Expand Down
Loading