walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

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

For naming issues reported, column was always set to `0`, which is now
fixed.

Global variable naming errors were reported as "Public member" issues,
which is also fixed.

The unit test now covers namespaces, class names, public and private
member variables.
This commit is contained in:
thingsconnected 2024-01-03 14:00:47 +01:00 committed by GitHub
parent 5e59652fd3
commit 8261ded475
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 86 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
addons/cppcheckdata.py
View File

@ -1619,11 +1619,11 @@ def is_suppressed(location, message, errorId):
return True return True
return False 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: if '--cli' in sys.argv:
msg = { 'file': location.file, msg = { 'file': location.file,
'linenr': location.linenr, 'linenr': location.linenr,
'column': location.column, 'column': location.column if columnOverride is None else columnOverride,
'severity': severity, 'severity': severity,
'message': message, 'message': message,
'addon': addon, 'addon': addon,

52
addons/namingng.py
View File

@ -41,14 +41,14 @@ import json
# Auxiliary class # Auxiliary class
class DataStruct: class DataStruct:
def __init__(self, file, linenr, string): def __init__(self, file, linenr, string, column=0):
self.file = file self.file = file
self.linenr = linenr self.linenr = linenr
self.str = string self.str = string
self.column = 0 self.column = column
def reportNamingError(location,message,errorId='namingConvention',severity='style',extra=''): def reportNamingError(location,message,errorId='namingConvention',severity='style',extra='',column=None):
cppcheckdata.reportError(location,severity,message,'namingng',errorId,extra) cppcheckdata.reportError(location,severity,message,'namingng',errorId,extra,columnOverride=column)
def configError(error,fatal=True): def configError(error,fatal=True):
print('config error: %s'%error) print('config error: %s'%error)
@ -167,8 +167,9 @@ def check_include_guard_name(conf_include_guard,cfg,directive):
if len(parts) != 2: if len(parts) != 2:
msg = 'syntax error' msg = 'syntax error'
reportNamingError(directive,msg,'syntax') reportNamingError(directive,msg,'syntax')
return None return None,None
guard_name = parts[1] guard_name = parts[1]
guard_column = 1+directive.str.find(guard_name)
filename = directive.file filename = directive.file
if conf_include_guard.get('input','path') == 'basename': if conf_include_guard.get('input','path') == 'basename':
@ -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','') expect_guard_name = conf_include_guard.get('prefix','') + barename + conf_include_guard.get('suffix','')
if expect_guard_name != guard_name: if expect_guard_name != guard_name:
msg = 'include guard naming violation; %s != %s'%(guard_name,expect_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): 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. # Scan for '#ifndef FILE_H' as the first directive, in the first N lines.
@ -200,11 +201,11 @@ def check_include_guard(conf_include_guard,cfg,unguarded_include_files):
# - test whether include guards are in place # - test whether include guards are in place
max_linenr = conf_include_guard.get('max_linenr', 5) max_linenr = conf_include_guard.get('max_linenr', 5)
def report(directive,msg,errorId): def report(directive,msg,errorId,column=0):
reportNamingError(directive,msg,errorId) reportNamingError(directive,msg,errorId,column=column)
def report_pending_ifndef(directive): def report_pending_ifndef(directive,column):
report(directive,'include guard #ifndef is not followed by #define','includeGuardIncomplete') report(directive,'include guard #ifndef is not followed by #define','includeGuardIncomplete',column=column)
last_fn = None last_fn = None
pending_ifndef = None pending_ifndef = None
@ -212,7 +213,7 @@ def check_include_guard(conf_include_guard,cfg,unguarded_include_files):
for directive in cfg.directives: for directive in cfg.directives:
if last_fn != directive.file: if last_fn != directive.file:
if pending_ifndef: if pending_ifndef:
report_pending_ifndef(pending_ifndef) report_pending_ifndef(pending_ifndef,guard_column)
pending_ifndef = None pending_ifndef = None
last_fn = directive.file last_fn = directive.file
phase = 0 phase = 0
@ -237,7 +238,7 @@ def check_include_guard(conf_include_guard,cfg,unguarded_include_files):
report(directive,'first preprocessor directive should be include guard #ifndef','includeGuardMissing') report(directive,'first preprocessor directive should be include guard #ifndef','includeGuardMissing')
phase = -1 phase = -1
continue 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: if guard_name == None:
phase = -1 phase = -1
continue continue
@ -256,13 +257,13 @@ def check_include_guard(conf_include_guard,cfg,unguarded_include_files):
phase = -1 phase = -1
continue continue
if guard_name != parts[1]: 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) unguarded_include_files.remove(directive.file)
phase = -1 phase = -1
if pending_ifndef: if pending_ifndef:
report_pending_ifndef(pending_ifndef) report_pending_ifndef(pending_ifndef,guard_column)
def process(dumpfiles, configfile, debugprint=False): def process(dumpfiles, configfile, debugprint=False):
conf = loadConfig(configfile) conf = loadConfig(configfile)
@ -294,7 +295,7 @@ def process(dumpfiles, configfile, debugprint=False):
if conf.namespace: if conf.namespace:
for tk in data.rawTokens: for tk in data.rawTokens:
if tk.str == 'namespace': 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' msgType = 'Namespace'
for exp in conf.namespace: for exp in conf.namespace:
evalExpr(conf.namespace, exp, mockToken, msgType) evalExpr(conf.namespace, exp, mockToken, msgType)
@ -333,9 +334,10 @@ def process(dumpfiles, configfile, debugprint=False):
reportNamingError(var.typeStartToken, reportNamingError(var.typeStartToken,
'Variable ' + 'Variable ' +
var.nameToken.str + 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' msgType = 'Variable'
for exp in conf.variable: for exp in conf.variable:
evalExpr(conf.variable, exp, mockToken, msgType) evalExpr(conf.variable, exp, mockToken, msgType)
@ -346,7 +348,7 @@ def process(dumpfiles, configfile, debugprint=False):
for var in cfg.variables: for var in cfg.variables:
if (var.access is None) or var.access != 'Private': if (var.access is None) or var.access != 'Private':
continue 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' msgType = 'Private member variable'
for exp in conf.private_member: for exp in conf.private_member:
evalExpr(conf.private_member, exp, mockToken, msgType) evalExpr(conf.private_member, exp, mockToken, msgType)
@ -356,7 +358,7 @@ def process(dumpfiles, configfile, debugprint=False):
for var in cfg.variables: for var in cfg.variables:
if (var.access is None) or var.access != 'Public': if (var.access is None) or var.access != 'Public':
continue 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' msgType = 'Public member variable'
for exp in conf.public_member: for exp in conf.public_member:
evalExpr(conf.public_member, exp, mockToken, msgType) evalExpr(conf.public_member, exp, mockToken, msgType)
@ -366,8 +368,8 @@ def process(dumpfiles, configfile, debugprint=False):
for var in cfg.variables: for var in cfg.variables:
if (var.access is None) or var.access != 'Global': if (var.access is None) or var.access != 'Global':
continue 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' msgType = 'Global variable'
for exp in conf.global_variable: for exp in conf.global_variable:
evalExpr(conf.global_variable, exp, mockToken, msgType) evalExpr(conf.global_variable, exp, mockToken, msgType)
@ -387,8 +389,8 @@ def process(dumpfiles, configfile, debugprint=False):
if retval and retval in conf.function_prefixes: if retval and retval in conf.function_prefixes:
if not token.function.name.startswith(conf.function_prefixes[retval]): if not token.function.name.startswith(conf.function_prefixes[retval]):
reportNamingError(token, 'Function ' + token.function.name + ' violates naming convention') reportNamingError(token, 'Function ' + token.function.name + ' violates naming convention', column=token.column)
mockToken = DataStruct(token.file, token.linenr, token.function.name) mockToken = DataStruct(token.file, token.linenr, token.function.name, token.column)
msgType = 'Function' msgType = 'Function'
for exp in conf.function_name: for exp in conf.function_name:
evalExpr(conf.function_name, exp, mockToken, msgType) evalExpr(conf.function_name, exp, mockToken, msgType)
@ -398,7 +400,7 @@ def process(dumpfiles, configfile, debugprint=False):
for fnc in cfg.functions: for fnc in cfg.functions:
# Check if it is Constructor/Destructor # Check if it is Constructor/Destructor
if fnc.type == 'Constructor' or fnc.type == '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 msgType = 'Class ' + fnc.type
for exp in conf.class_name: for exp in conf.class_name:
evalExpr(conf.class_name, exp, mockToken, msgType) evalExpr(conf.class_name, exp, mockToken, msgType)

72
test/cli/test-other.py
View File

@ -343,7 +343,11 @@ def test_addon_namingng(tmpdir):
"RE_FILE": [ "RE_FILE": [
"[^/]*[a-z][a-z0-9_]*[a-z0-9]\\.c\\Z" "[^/]*[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_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_GLOBAL_VARNAME": ["[a-z][a-z0-9_]*[a-z0-9]\\Z"],
"RE_FUNCTIONNAME": ["[a-z][a-z0-9_]*[a-z0-9]\\Z"], "RE_FUNCTIONNAME": ["[a-z][a-z0-9_]*[a-z0-9]\\Z"],
"include_guard": { "include_guard": {
@ -384,11 +388,11 @@ extern int _invalid_extern_global;
#endif #endif
""".format(test_unguarded_include_file)) """.format(test_unguarded_include_file))
test_file_basename = 'test_.c' test_file_basename = 'test_.cpp'
test_file = os.path.join(tmpdir, test_file_basename) test_file = os.path.join(tmpdir, test_file_basename)
with open(test_file, 'wt') as f: with open(test_file, 'wt') as f:
f.write(""" f.write("""
#include "{}" #include "%s"
void invalid_function_(); void invalid_function_();
void _invalid_function(); void _invalid_function();
@ -403,7 +407,17 @@ uint16_t ui16_valid_function8(int valid_arg);
int _invalid_global; int _invalid_global;
static int _invalid_static_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] args = ['--addon='+addon_file, '--verbose', '--enable=all', test_file]
@ -417,38 +431,66 @@ static int _invalid_static_global;
'Includes:', 'Includes:',
'Platform:native' 'Platform:native'
] ]
lines = [line for line in stderr.splitlines() if line.strip() != '^' and line != ''] lines = [line for line in stderr.splitlines() if line != '']
expect = [ expect = [
'{}:0:0: style: File name {} violates naming convention [namingng-namingConvention]'.format(test_include_file,test_include_file_basename), '{}: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', '#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();', '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;', '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: 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), '{}: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();', 'void InvalidFunctionUnguarded();',
' ^',
'{}:0:0: style: File name {} violates naming convention [namingng-namingConvention]'.format(test_file,test_file_basename), '{}: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);', '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_);', '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);', '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_();', '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();', 'void _invalid_function();',
' ^',
'{}:12:10: style: Function invalid_function7 violates naming convention [namingng-namingConvention]'.format(test_file), '{}:12:10: style: Function invalid_function7 violates naming convention [namingng-namingConvention]'.format(test_file),
'uint16_t invalid_function7(int valid_arg);', '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;', '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;', '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 # test sorted lines; the order of messages may vary and is not of importance
lines.sort() lines.sort()