From 441293b175656c052d7f1c8c94cf0cae7a854e2f Mon Sep 17 00:00:00 2001 From: Maarten van der Schrieck Date: Fri, 22 Dec 2023 23:16:32 +0100 Subject: [PATCH 1/2] addons/namingng.py: Enable use with --cli, to enable --addon=namingng. 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, which now also covers (include) file naming. --- .github/workflows/CI-unixish.yml | 2 - .github/workflows/CI-windows.yml | 2 - addons/namingng.py | 102 +++++++++--------------------- addons/test/namingng_test.c | 50 --------------- test/cli/test-other.py | 105 ++++++++++++++++++++++++++++--- 5 files changed, 126 insertions(+), 135 deletions(-) delete mode 100644 addons/test/namingng_test.c diff --git a/.github/workflows/CI-unixish.yml b/.github/workflows/CI-unixish.yml index af056b56058..e28f435fd85 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 39d769f1a91..1de7af8d99d 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/namingng.py b/addons/namingng.py index 6ce9aa2b36f..3f1d72d0ca2 100755 --- a/addons/namingng.py +++ b/addons/namingng.py @@ -29,25 +29,16 @@ 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", 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 2eaf3899666..00000000000 --- 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 a183161fc9a..e68e5ffd869 100644 --- a/test/cli/test-other.py +++ b/test/cli/test-other.py @@ -324,25 +324,112 @@ def test_addon_naming(tmpdir): 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 "{}" + +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); - args = ['--addon=namingng', '--enable=all', test_file] +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): From 7f273cc0c87e5ab4322e8e727681e9aacbc29aa8 Mon Sep 17 00:00:00 2001 From: Maarten van der Schrieck Date: Fri, 22 Dec 2023 23:53:27 +0100 Subject: [PATCH 2/2] addons/namingng.py: Add namingng.json and namingng.config.json. Confusingly, the example config file for namingng.py was named naming.json, while another addon naming.py can be used using a *different* file that is also named naming.json, residing in a different directory. The feature where an addon can be named addon.json, referring with its "script" key to the actual addon script, further adds to possible confusion. By renaming the default namingng config file from naming.json to namingng.config.json, it is made explicit that this file is a config file for the namingng.py addon. A file namingng.json is added as an example file to be used using --addon=namingng.json. This matches the setup of namingng as created and tested in test-other.py. --- addons/README.md | 4 +++- addons/{naming.json => namingng.config.json} | 0 addons/namingng.json | 6 ++++++ addons/namingng.py | 2 +- win_installer/cppcheck.wxs | 3 ++- 5 files changed, 12 insertions(+), 3 deletions(-) rename addons/{naming.json => namingng.config.json} (100%) create mode 100644 addons/namingng.json diff --git a/addons/README.md b/addons/README.md index 34b1f7b2600..563c0c793f6 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 00000000000..83f9a5a55d8 --- /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 3f1d72d0ca2..d6732494cf2 100755 --- a/addons/namingng.py +++ b/addons/namingng.py @@ -207,7 +207,7 @@ def process(dumpfiles, configfile, debugprint=False): 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") args = parser.parse_args() diff --git a/win_installer/cppcheck.wxs b/win_installer/cppcheck.wxs index d3929061820..56fb0d4bd05 100644 --- a/win_installer/cppcheck.wxs +++ b/win_installer/cppcheck.wxs @@ -156,7 +156,8 @@ - + +