From 24133d4a592a3b90f4d516864a91b6a41b719f77 Mon Sep 17 00:00:00 2001 From: thingsconnected <45596685+thingsconnected@users.noreply.github.com> Date: Sat, 30 Dec 2023 20:54:03 +0100 Subject: [PATCH] addons/namingng.py: Fix commandline use. (#5793) namingng.py was only usable in standalone mode, but now supports CLI mode, i.e. with cppcheck --addon=namingng. It uses the generic reporting provided by cppcheckdata.reportError(). All output other than reported errors is suppressed. A local function reportNamingError() is implemented to call through to cppcheckdata.reportError(), filling in common defaults. The collection of errors and the --verify feature are removed, including related workflow and a test file. These are replaced by a unit test. --- .github/workflows/CI-unixish.yml | 2 - .github/workflows/CI-windows.yml | 2 - addons/README.md | 4 +- addons/{naming.json => namingng.config.json} | 0 addons/namingng.json | 6 ++ addons/namingng.py | 104 ++++++------------ addons/test/namingng_test.c | 50 --------- test/cli/test-other.py | 105 +++++++++++++++++-- win_installer/cppcheck.wxs | 3 +- 9 files changed, 138 insertions(+), 138 deletions(-) rename addons/{naming.json => namingng.config.json} (100%) create mode 100644 addons/namingng.json delete mode 100644 addons/test/namingng_test.c diff --git a/.github/workflows/CI-unixish.yml b/.github/workflows/CI-unixish.yml index af056b560..e28f435fd 100644 --- a/.github/workflows/CI-unixish.yml +++ b/.github/workflows/CI-unixish.yml @@ -446,8 +446,6 @@ jobs: python3 ../naming.py --var='[a-z].*' --function='[a-z].*' naming_test.c.dump ../../cppcheck --dump naming_test.cpp python3 ../naming.py --var='[a-z].*' --function='[a-z].*' naming_test.cpp.dump - ../../cppcheck --dump namingng_test.c - python3 ../namingng.py --configfile ../naming.json --verify namingng_test.c.dump - name: Build democlient if: matrix.os == 'ubuntu-22.04' diff --git a/.github/workflows/CI-windows.yml b/.github/workflows/CI-windows.yml index 39d769f1a..1de7af8d9 100644 --- a/.github/workflows/CI-windows.yml +++ b/.github/workflows/CI-windows.yml @@ -202,6 +202,4 @@ jobs: rem python3 ..\naming.py --var='[a-z].*' --function='[a-z].*' naming_test.c.dump || exit /b !errorlevel! ..\..\cppcheck --dump naming_test.cpp || exit /b !errorlevel! python3 ..\naming.py --var='[a-z].*' --function='[a-z].*' naming_test.cpp.dump || exit /b !errorlevel! - ..\..\cppcheck --dump namingng_test.c || exit /b !errorlevel! - python3 ..\namingng.py --configfile ..\naming.json --verify namingng_test.c.dump || exit /b !errorlevel! diff --git a/addons/README.md b/addons/README.md index 34b1f7b26..563c0c793 100644 --- a/addons/README.md +++ b/addons/README.md @@ -33,8 +33,10 @@ Addons are scripts that analyses Cppcheck dump files to check compatibility with Helper class for reading Cppcheck dump files within an addon. - misra_9.py Implementation of the MISRA 9.x rules used by `misra` addon. -- naming.json +- namingng.config.json Example configuration for `namingng` addon. +- namingng.json + Example JSON file that can be used using --addon=namingng.json, referring to namingng.py and namingng.config.json - ROS_naming.json Example configuration for the `namingng` addon enforcing the [ROS naming convention for C++ ](http://wiki.ros.org/CppStyleGuide#Files). - runaddon.py diff --git a/addons/naming.json b/addons/namingng.config.json similarity index 100% rename from addons/naming.json rename to addons/namingng.config.json diff --git a/addons/namingng.json b/addons/namingng.json new file mode 100644 index 000000000..83f9a5a55 --- /dev/null +++ b/addons/namingng.json @@ -0,0 +1,6 @@ +{ + "script":"namingng.py", + "args":[ + "--configfile=namingng.config.json" + ] +} diff --git a/addons/namingng.py b/addons/namingng.py index 6ce9aa2b3..d6732494c 100755 --- a/addons/namingng.py +++ b/addons/namingng.py @@ -29,25 +29,16 @@ import re import argparse import json - # Auxiliary class class DataStruct: def __init__(self, file, linenr, string): self.file = file self.linenr = linenr self.str = string + self.column = 0 - -def reportError(filename, linenr, severity, msg): - message = "[{filename}:{linenr}] ( {severity} ) naming.py: {msg}\n".format( - filename=filename, - linenr=linenr, - severity=severity, - msg=msg - ) - sys.stderr.write(message) - return message - +def reportNamingError(location,message,errorId='namingConvention',severity='style',extra=''): + cppcheckdata.reportError(location,severity,message,'namingng',errorId,extra) def loadConfig(configfile): with open(configfile) as fh: @@ -55,44 +46,42 @@ def loadConfig(configfile): return data -def checkTrueRegex(data, expr, msg, errors): +def checkTrueRegex(data, expr, msg): res = re.match(expr, data.str) if res: - errors.append(reportError(data.file, data.linenr, 'style', msg)) + reportNamingError(data,msg) -def checkFalseRegex(data, expr, msg, errors): +def checkFalseRegex(data, expr, msg): res = re.match(expr, data.str) if not res: - errors.append(reportError(data.file, data.linenr, 'style', msg)) + reportNamingError(data,msg) -def evalExpr(conf, exp, mockToken, msgType, errors): +def evalExpr(conf, exp, mockToken, msgType): if isinstance(conf, dict): if conf[exp][0]: msg = msgType + ' ' + mockToken.str + ' violates naming convention : ' + conf[exp][1] - checkTrueRegex(mockToken, exp, msg, errors) + checkTrueRegex(mockToken, exp, msg) elif ~conf[exp][0]: msg = msgType + ' ' + mockToken.str + ' violates naming convention : ' + conf[exp][1] - checkFalseRegex(mockToken, exp, msg, errors) + checkFalseRegex(mockToken, exp, msg) else: msg = msgType + ' ' + mockToken.str + ' violates naming convention : ' + conf[exp][0] - checkFalseRegex(mockToken, exp, msg, errors) + checkFalseRegex(mockToken, exp, msg) else: msg = msgType + ' ' + mockToken.str + ' violates naming convention' - checkFalseRegex(mockToken, exp, msg, errors) + checkFalseRegex(mockToken, exp, msg) def process(dumpfiles, configfile, debugprint=False): - - errors = [] - conf = loadConfig(configfile) for afile in dumpfiles: if not afile[-5:] == '.dump': continue - print('Checking ' + afile + '...') + if not args.cli: + print('Checking ' + afile + '...') data = cppcheckdata.CppcheckData(afile) # Check File naming @@ -104,7 +93,8 @@ def process(dumpfiles, configfile, debugprint=False): good |= bool(re.match(exp, source_file)) good |= bool(re.match(exp, basename)) if not good: - errors.append(reportError(source_file, 0, 'style', 'File name ' + source_file + ' violates naming convention')) + mockToken = DataStruct(source_file, 0, basename) + reportNamingError(mockToken, 'File name ' + basename + ' violates naming convention') # Check Namespace naming if "RE_NAMESPACE" in conf and conf["RE_NAMESPACE"]: @@ -113,10 +103,11 @@ def process(dumpfiles, configfile, debugprint=False): mockToken = DataStruct(tk.next.file, tk.next.linenr, tk.next.str) msgType = 'Namespace' for exp in conf["RE_NAMESPACE"]: - evalExpr(conf["RE_NAMESPACE"], exp, mockToken, msgType, errors) + evalExpr(conf["RE_NAMESPACE"], exp, mockToken, msgType) for cfg in data.configurations: - print('Checking %s, config %s...' % (afile, cfg.name)) + if not args.cli: + print('Checking %s, config %s...' % (afile, cfg.name)) if "RE_VARNAME" in conf and conf["RE_VARNAME"]: for var in cfg.variables: if var.nameToken and var.access != 'Global' and var.access != 'Public' and var.access != 'Private': @@ -139,18 +130,15 @@ def process(dumpfiles, configfile, debugprint=False): continue if varType in conf.get("var_prefixes",{}): if not var.nameToken.str.startswith(conf["var_prefixes"][varType]): - errors.append(reportError( - var.typeStartToken.file, - var.typeStartToken.linenr, - 'style', + reportNamingError(var.typeStartToken, 'Variable ' + var.nameToken.str + - ' violates naming convention')) + ' violates naming convention') mockToken = DataStruct(var.typeStartToken.file, var.typeStartToken.linenr, var.nameToken.str) msgType = 'Variable' for exp in conf["RE_VARNAME"]: - evalExpr(conf["RE_VARNAME"], exp, mockToken, msgType, errors) + evalExpr(conf["RE_VARNAME"], exp, mockToken, msgType) # Check Private Variable naming if "RE_PRIVATE_MEMBER_VARIABLE" in conf and conf["RE_PRIVATE_MEMBER_VARIABLE"]: @@ -161,7 +149,7 @@ def process(dumpfiles, configfile, debugprint=False): mockToken = DataStruct(var.typeStartToken.file, var.typeStartToken.linenr, var.nameToken.str) msgType = 'Private member variable' for exp in conf["RE_PRIVATE_MEMBER_VARIABLE"]: - evalExpr(conf["RE_PRIVATE_MEMBER_VARIABLE"], exp, mockToken, msgType, errors) + evalExpr(conf["RE_PRIVATE_MEMBER_VARIABLE"], exp, mockToken, msgType) # Check Public Member Variable naming if "RE_PUBLIC_MEMBER_VARIABLE" in conf and conf["RE_PUBLIC_MEMBER_VARIABLE"]: @@ -171,7 +159,7 @@ def process(dumpfiles, configfile, debugprint=False): mockToken = DataStruct(var.typeStartToken.file, var.typeStartToken.linenr, var.nameToken.str) msgType = 'Public member variable' for exp in conf["RE_PUBLIC_MEMBER_VARIABLE"]: - evalExpr(conf["RE_PUBLIC_MEMBER_VARIABLE"], exp, mockToken, msgType, errors) + evalExpr(conf["RE_PUBLIC_MEMBER_VARIABLE"], exp, mockToken, msgType) # Check Global Variable naming if "RE_GLOBAL_VARNAME" in conf and conf["RE_GLOBAL_VARNAME"]: @@ -181,7 +169,7 @@ def process(dumpfiles, configfile, debugprint=False): mockToken = DataStruct(var.typeStartToken.file, var.typeStartToken.linenr, var.nameToken.str) msgType = 'Public member variable' for exp in conf["RE_GLOBAL_VARNAME"]: - evalExpr(conf["RE_GLOBAL_VARNAME"], exp, mockToken, msgType, errors) + evalExpr(conf["RE_GLOBAL_VARNAME"], exp, mockToken, msgType) # Check Functions naming if "RE_FUNCTIONNAME" in conf and conf["RE_FUNCTIONNAME"]: @@ -199,12 +187,11 @@ def process(dumpfiles, configfile, debugprint=False): if retval and retval in conf.get("function_prefixes",{}): if not token.function.name.startswith(conf["function_prefixes"][retval]): - errors.append(reportError( - token.file, token.linenr, 'style', 'Function ' + token.function.name + ' violates naming convention')) + reportNamingError(token, 'Function ' + token.function.name + ' violates naming convention') mockToken = DataStruct(token.file, token.linenr, token.function.name) msgType = 'Function' for exp in conf["RE_FUNCTIONNAME"]: - evalExpr(conf["RE_FUNCTIONNAME"], exp, mockToken, msgType, errors) + evalExpr(conf["RE_FUNCTIONNAME"], exp, mockToken, msgType) # Check Class naming if "RE_CLASS_NAME" in conf and conf["RE_CLASS_NAME"]: @@ -214,45 +201,16 @@ def process(dumpfiles, configfile, debugprint=False): mockToken = DataStruct(fnc.tokenDef.file, fnc.tokenDef.linenr, fnc.name) msgType = 'Class ' + fnc.type for exp in conf["RE_CLASS_NAME"]: - evalExpr(conf["RE_CLASS_NAME"], exp, mockToken, msgType, errors) - return errors - + evalExpr(conf["RE_CLASS_NAME"], exp, mockToken, msgType) if __name__ == "__main__": - parser = argparse.ArgumentParser(description='Naming verification') - parser.add_argument('dumpfiles', type=str, nargs='+', - help='A set of dumpfiles to process') + parser = cppcheckdata.ArgumentParser() parser.add_argument("--debugprint", action="store_true", default=False, help="Add debug prints") - parser.add_argument("--configfile", type=str, default="naming.json", + parser.add_argument("--configfile", type=str, default="namingng.config.json", help="Naming check config file") - parser.add_argument("--verify", action="store_true", default=False, - help="verify this script. Must be executed in test folder !") args = parser.parse_args() - errors = process(args.dumpfiles, args.configfile, args.debugprint) - - if args.verify: - print(errors) - if len(errors) < 6: - print("Not enough errors found") - sys.exit(1) - target = [ - '[namingng_test.c:8] ( style ) naming.py: Variable badui32 violates naming convention\n', - '[namingng_test.c:11] ( style ) naming.py: Variable a violates naming convention\n', - '[namingng_test.c:29] ( style ) naming.py: Variable badui32 violates naming convention\n', - '[namingng_test.c:20] ( style ) naming.py: Function ui16bad_underscore violates naming convention\n', - '[namingng_test.c:25] ( style ) naming.py: Function u32Bad violates naming convention\n', - '[namingng_test.c:37] ( style ) naming.py: Function Badui16 violates naming convention\n'] - diff = set(errors) - set(target) - if len(diff): - print("Not the right errors found {}".format(str(diff))) - sys.exit(1) - print("Verification done\n") - sys.exit(0) - - if len(errors): - print('Found errors: {}'.format(len(errors))) - sys.exit(1) + process(args.dumpfile, args.configfile, args.debugprint) sys.exit(0) diff --git a/addons/test/namingng_test.c b/addons/test/namingng_test.c deleted file mode 100644 index 2eaf38996..000000000 --- a/addons/test/namingng_test.c +++ /dev/null @@ -1,50 +0,0 @@ -#include -#include - -uint32_t ui32Good (int abc) -{ - uint32_t ui32good; - int32_t i32good; - uint32_t badui32; - int32_t badi32; - - uint32_t a; // Short - return 5; -} - -uint16_t ui16Good (int a) -{ - return 5; -} - -uint16_t ui16bad_underscore (int a) -{ - return 5; -} - -uint32_t u32Bad (int a) -{ - uint32_t ui32good; - int32_t i32good; - uint32_t badui32; - int32_t badi32; - int * intpointer=NULL; - int ** intppointer=NULL; - int *** intpppointer=NULL; - return 5; -} - -uint16_t Badui16 (int a) -{ - return 5; -} - -void * Pointer() -{ - return NULL; -} - -void ** PPointer() -{ - return NULL; -} diff --git a/test/cli/test-other.py b/test/cli/test-other.py index a183161fc..e68e5ffd8 100644 --- a/test/cli/test-other.py +++ b/test/cli/test-other.py @@ -324,25 +324,112 @@ int Var; assert stderr == '{}:2:1: style: Variable Var violates naming convention [naming-varname]\n'.format(test_file) -# the namingng addon only works standalone and not in CLI mode - see #12005 -@pytest.mark.skip def test_addon_namingng(tmpdir): - test_file = os.path.join(tmpdir, 'test.cpp') - # TODO: trigger warning + addon_file = os.path.join(tmpdir, 'namingng.json') + addon_config_file = os.path.join(tmpdir, 'namingng.config.json') + with open(addon_file, 'wt') as f: + f.write(""" +{ + "script": "addons/namingng.py", + "args": [ + "--configfile=%s" + ] +} + """%(addon_config_file).replace('\\','\\\\')) + + with open(addon_config_file, 'wt') as f: + f.write(""" +{ + "RE_FILE": [ + "[^/]*[a-z][a-z0-9_]*[a-z0-9]\\.c\\Z" + ], + "RE_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"], + "var_prefixes": {"uint32_t": "ui32"}, + "function_prefixes": {"uint16_t": "ui16", + "uint32_t": "ui32"}, + "skip_one_char_variables": false +} + """.replace('\\','\\\\')) + + + test_include_file_basename = '_test.h' + test_include_file = os.path.join(tmpdir, test_include_file_basename) + with open(test_include_file, 'wt') as f: + f.write(""" +#ifndef TEST_H +#define TEST_H + +void InvalidFunction(); +extern int _invalid_extern_global; + +#endif +""") + + test_file_basename = 'test_.c' + test_file = os.path.join(tmpdir, test_file_basename) with open(test_file, 'wt') as f: f.write(""" -typedef int MISRA_5_6_VIOLATION; - """) +#include "{}" - args = ['--addon=namingng', '--enable=all', test_file] +void invalid_function_(); +void _invalid_function(); +void valid_function1(); +void valid_function2(int _invalid_arg); +void valid_function3(int invalid_arg_); +void valid_function4(int valid_arg32); +void valid_function5(uint32_t invalid_arg32); +void valid_function6(uint32_t ui32_valid_arg); +uint16_t invalid_function7(int valid_arg); +uint16_t ui16_valid_function8(int valid_arg); + +int _invalid_global; +static int _invalid_static_global; + """.format(test_include_file_basename)) + + args = ['--addon='+addon_file, '--verbose', '--enable=all', test_file] exitcode, stdout, stderr = cppcheck(args) assert exitcode == 0 lines = stdout.splitlines() assert lines == [ - 'Checking {} ...'.format(test_file) + 'Checking {} ...'.format(test_file), + 'Defines:', + 'Undefines:', + 'Includes:', + 'Platform:native' ] - assert stderr == '' + lines = [line for line in stderr.splitlines() if line.strip() != '^' and line != ''] + expect = [ + '{}:0:0: style: File name {} violates naming convention [namingng-namingConvention]'.format(test_include_file,test_include_file_basename), + '{}:5:0: 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), + 'extern int _invalid_extern_global;', + + '{}: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), + 'void valid_function2(int _invalid_arg);', + '{}:8:0: 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), + 'void valid_function5(uint32_t invalid_arg32);', + '{}:4:0: 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), + '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), + 'int _invalid_global;', + '{}:16:0: style: Public member variable _invalid_static_global violates naming convention [namingng-namingConvention]'.format(test_file), + 'static int _invalid_static_global;', + ] + # test sorted lines; the order of messages may vary and is not of importance + lines.sort() + expect.sort() + assert lines == expect def test_addon_findcasts(tmpdir): diff --git a/win_installer/cppcheck.wxs b/win_installer/cppcheck.wxs index d39290618..56fb0d4bd 100644 --- a/win_installer/cppcheck.wxs +++ b/win_installer/cppcheck.wxs @@ -156,7 +156,8 @@ - + +