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

@@ -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'

.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!
 

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

addons/namingng.json

@@ -0,0 +1,6 @@
+{
+    "script":"namingng.py",
+    "args":[
+        "--configfile=namingng.config.json"
+    ]
+}

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 reportNamingError(location,message,errorId='namingConvention',severity='style',extra=''):
-def reportError(filename, linenr, severity, msg):
+    cppcheckdata.reportError(location,severity,message,'namingng',errorId,extra)
-    message = "[{filename}:{linenr}] ( {severity} ) naming.py: {msg}\n".format(
-        filename=filename,
-        linenr=linenr,
-        severity=severity,
-        msg=msg
-    )
-    sys.stderr.write(message)
-    return message
-
 
 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(
+                                reportNamingError(var.typeStartToken,
-                                                    var.typeStartToken.file,
-                                                    var.typeStartToken.linenr,
-                                                    'style',
                                                     '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(
+                                reportNamingError(token, 'Function ' + token.function.name + ' violates naming convention')
-                                    token.file, token.linenr, 'style', '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)
+                            evalExpr(conf["RE_CLASS_NAME"], exp, mockToken, msgType)
-    return errors
-
 
 if __name__ == "__main__":
-    parser = argparse.ArgumentParser(description='Naming verification')
+    parser = cppcheckdata.ArgumentParser()
-    parser.add_argument('dumpfiles', type=str, nargs='+',
-                        help='A set of dumpfiles to process')
     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)
+    process(args.dumpfile, 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)
 
     sys.exit(0)

addons/test/namingng_test.c

@@ -1,50 +0,0 @@
-#include <stddef.h>
-#include <stdint.h>
-
-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;
-}

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')
+    addon_file = os.path.join(tmpdir, 'namingng.json')
-    # TODO: trigger warning
+    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):

win_installer/cppcheck.wxs

@@ -156,7 +156,8 @@
               <File Id='misc.py' Name='misc.py' Source='$(var.AddonsDir)\misc.py' />
               <File Id='misra.py' Name='misra.py' Source='$(var.AddonsDir)\misra.py' />
               <File Id='misra_9.py' Name='misra_9.py' Source='$(var.AddonsDir)\misra_9.py' />
-              <File Id='naming.json' Name='naming.json' Source='$(var.AddonsDir)\naming.json' />
+              <File Id='namingng.json' Name='namingng.json' Source='$(var.AddonsDir)\namingng.json' />
+              <File Id='namingng.config.json' Name='namingng.config.json' Source='$(var.AddonsDir)\namingng.config.json' />
               <File Id='naming.py' Name='naming.py' Source='$(var.AddonsDir)\naming.py' />
               <File Id='namingng.py' Name='namingng.py' Source='$(var.AddonsDir)\namingng.py' />
               <File Id='ROS_naming.json' Name='ROS_naming.json' Source='$(var.AddonsDir)\ROS_naming.json' />