From 1f670733f6c8269ee3809baf7a39d86e2142041a Mon Sep 17 00:00:00 2001 From: Maarten van der Schrieck Date: Fri, 5 Jan 2024 22:29:39 +0100 Subject: [PATCH] addons/namingng.py: Implement backward compatibility with addons/naming.py. Merging naming.py into namingng.py reduces redundant code and leverages argument validation and the unit tests for namingng.py, with the aim to improve maintainability. naming.py supported RE_CONST to check constants, which is now a feature of namingng.py as well; the unit test is expanded to cover the feature. naming.py has been updated to call through to namingng.py. Suppressions are updated to suppres the new namingng error id [naming-namingConvention]. The unit tests for naming.py are kept as-is. --- .selfcheck_suppressions | 9 ++-- addons/naming.py | 87 ++------------------------------- addons/namingng.py | 103 +++++++++++++++++++++++++++++++--------- gui/codeeditorstyle.cpp | 18 +++---- gui/codeeditorstyle.h | 18 +++---- test/cli/test-other.py | 16 +++++-- 6 files changed, 118 insertions(+), 133 deletions(-) diff --git a/.selfcheck_suppressions b/.selfcheck_suppressions index 03e54874475..67ccb0b846b 100644 --- a/.selfcheck_suppressions +++ b/.selfcheck_suppressions @@ -3,13 +3,13 @@ missingIncludeSystem # temporary suppressions - fix the warnings! simplifyUsing:lib/valueptr.h varid0:gui/projectfile.cpp -naming-privateMemberVariable:gui/test/cppchecklibrarydata/testcppchecklibrarydata.h +naming-namingConvention:gui/test/cppchecklibrarydata/testcppchecklibrarydata.h symbolDatabaseWarning:*/moc_*.cpp simplifyUsing:*/moc_*.cpp # warnings in Qt generated code we cannot fix funcArgNamesDifferent:*/moc_*.cpp -naming-varname:*/ui_*.h +naming-namingConvention:*/ui_*.h functionStatic:*/ui_fileview.h # --debug-warnings suppressions @@ -17,12 +17,11 @@ valueFlowBailout valueFlowBailoutIncompleteVar autoNoType -naming-varname:externals/simplecpp/simplecpp.h -naming-privateMemberVariable:externals/simplecpp/simplecpp.h +naming-namingConvention:externals/simplecpp/simplecpp.h # these warnings need to be addressed upstream uninitMemberVar:externals/tinyxml2/tinyxml2.h noExplicitConstructor:externals/tinyxml2/tinyxml2.h missingOverride:externals/tinyxml2/tinyxml2.h invalidPrintfArgType_sint:externals/tinyxml2/tinyxml2.h -naming-privateMemberVariable:externals/tinyxml2/tinyxml2.h \ No newline at end of file +naming-namingConvention:externals/tinyxml2/tinyxml2.h \ No newline at end of file diff --git a/addons/naming.py b/addons/naming.py index 948f158b6f4..f087e4d8276 100755 --- a/addons/naming.py +++ b/addons/naming.py @@ -2,90 +2,11 @@ # # cppcheck addon for naming conventions # -# Example usage (variable name must start with lowercase, function name must start with uppercase): -# $ cppcheck --dump path-to-src/ -# $ python addons/naming.py --var='[a-z].*' --function='[A-Z].*' path-to-src/*.dump -# +# namingng.py was made backward compatible with naming.py; call through. -import cppcheckdata import sys -import re - - -def validate_regex(expr): - try: - re.compile(expr) - except re.error: - print('Error: "{}" is not a valid regular expression.'.format(expr)) - exit(1) - - -RE_VARNAME = None -RE_CONSTNAME = None -RE_PRIVATE_MEMBER_VARIABLE = None -RE_FUNCTIONNAME = None -for arg in sys.argv[1:]: - if arg[:6] == '--var=': - RE_VARNAME = arg[6:] - validate_regex(RE_VARNAME) - elif arg.startswith('--const='): - RE_CONSTNAME = arg[arg.find('=')+1:] - validate_regex(RE_CONSTNAME) - elif arg.startswith('--private-member-variable='): - RE_PRIVATE_MEMBER_VARIABLE = arg[arg.find('=')+1:] - validate_regex(RE_PRIVATE_MEMBER_VARIABLE) - elif arg[:11] == '--function=': - RE_FUNCTIONNAME = arg[11:] - validate_regex(RE_FUNCTIONNAME) - - -def reportError(token, severity, msg, errorId): - cppcheckdata.reportError(token, severity, msg, 'naming', errorId) - - -for arg in sys.argv[1:]: - if not arg.endswith('.dump'): - continue - print('Checking ' + arg + '...') - data = cppcheckdata.CppcheckData(arg) - - for cfg in data.iterconfigurations(): - print('Checking %s, config %s...' % (arg, cfg.name)) - if RE_VARNAME: - for var in cfg.variables: - if var.access == 'Private': - continue - if var.nameToken and not var.isConst: - res = re.match(RE_VARNAME, var.nameToken.str) - if not res: - reportError(var.typeStartToken, 'style', 'Variable ' + - var.nameToken.str + ' violates naming convention', 'varname') - if RE_CONSTNAME: - for var in cfg.variables: - if var.access == 'Private': - continue - if var.nameToken and var.isConst: - res = re.match(RE_CONSTNAME, var.nameToken.str) - if not res: - reportError(var.typeStartToken, 'style', 'Constant ' + - var.nameToken.str + ' violates naming convention', 'constname') - if RE_PRIVATE_MEMBER_VARIABLE: - for var in cfg.variables: - if (var.access is None) or var.access != 'Private': - continue - res = re.match(RE_PRIVATE_MEMBER_VARIABLE, var.nameToken.str) - if not res: - reportError(var.typeStartToken, 'style', 'Private member variable ' + - var.nameToken.str + ' violates naming convention', 'privateMemberVariable') - if RE_FUNCTIONNAME: - for scope in cfg.scopes: - if scope.type == 'Function': - function = scope.function - if function is not None and function.type in ('Constructor', 'Destructor', 'CopyConstructor', 'MoveConstructor'): - continue - res = re.match(RE_FUNCTIONNAME, scope.className) - if not res: - reportError( - scope.bodyStart, 'style', 'Function ' + scope.className + ' violates naming convention', 'functionName') +import cppcheckdata +import namingng +namingng.main() sys.exit(cppcheckdata.EXIT_CODE) diff --git a/addons/namingng.py b/addons/namingng.py index 8cc2b8480d3..a011809eb94 100755 --- a/addons/namingng.py +++ b/addons/namingng.py @@ -39,6 +39,8 @@ import argparse import json +addon_name = os.path.basename(sys.argv[0]).replace('.py','') + # Auxiliary class class DataStruct: def __init__(self, file, linenr, string, column=0): @@ -48,7 +50,7 @@ def __init__(self, file, linenr, string, column=0): self.column = column def reportNamingError(location,message,errorId='namingConvention',severity='style',extra='',column=None): - cppcheckdata.reportError(location,severity,message,'namingng',errorId,extra,columnOverride=column) + cppcheckdata.reportError(location,severity,message,addon_name,errorId,extra,columnOverride=column) def configError(error,fatal=True): print('config error: %s'%error) @@ -75,6 +77,56 @@ def validateConfigREs(list_or_dict,json_key): return have_error +def configMapping(): + return { + 'file': ('RE_FILE', (list,)), + 'namespace': ('RE_NAMESPACE', (list,dict)), + 'include_guard': ('include_guard', (dict,)), + 'variable': ('RE_VARNAME', (list,dict)), + 'variable_prefixes': ('var_prefixes', (dict,), {}), + 'private_member': ('RE_PRIVATE_MEMBER_VARIABLE', (list,dict)), + 'const': ('RE_CONST', (list,dict)), + 'public_member': ('RE_PUBLIC_MEMBER_VARIABLE', (list,dict)), + 'global_variable': ('RE_GLOBAL_VARNAME', (list,dict)), + 'function_name': ('RE_FUNCTIONNAME', (list,dict)), + 'function_prefixes': ('function_prefixes', (dict,), {}), + 'class_name': ('RE_CLASS_NAME', (list,dict)), + 'skip_one_char_variables': ('skip_one_char_variables', (bool,)), + } + +def genDeprecatedConfig(): + class Config: + pass + config = Config() + + mapping = configMapping() + for key,opts in mapping.items(): + default = None if len(opts)<3 else opts[2] + setattr(config,key,default) + + have_error = False + def validate(re): + have_error |= validateConfigRE([re]) + + if args.var: + setattr(config,'variable',[args.var]) + setattr(config,'global_variable',[args.var]) + + if args.const: + setattr(config,'const',[args.const]) + + if args.private_member_variable: + setattr(config,'private_member',[args.private_member_variable]) + + if args.function: + setattr(config,'function',[args.function]) + + if have_error: + sys.exit(0) + + return config + + def loadConfig(configfile): if not os.path.exists(configfile): configError("cannot find config file '%s'"%configfile) @@ -100,20 +152,7 @@ class Config: pass config = Config() - mapping = { - 'file': ('RE_FILE', (list,)), - 'namespace': ('RE_NAMESPACE', (list,dict)), - 'include_guard': ('include_guard', (dict,)), - 'variable': ('RE_VARNAME', (list,dict)), - 'variable_prefixes': ('var_prefixes', (dict,), {}), - 'private_member': ('RE_PRIVATE_MEMBER_VARIABLE', (list,dict)), - 'public_member': ('RE_PUBLIC_MEMBER_VARIABLE', (list,dict)), - 'global_variable': ('RE_GLOBAL_VARNAME', (list,dict)), - 'function_name': ('RE_FUNCTIONNAME', (list,dict)), - 'function_prefixes': ('function_prefixes', (dict,), {}), - 'class_name': ('RE_CLASS_NAME', (list,dict)), - 'skip_one_char_variables': ('skip_one_char_variables', (bool,)), - } + mapping = configMapping() # parse defined keys and store as members of config object for key,opts in mapping.items(): @@ -263,9 +302,7 @@ def report_pending_ifndef(directive,column): if pending_ifndef: report_pending_ifndef(pending_ifndef,guard_column) -def process(dumpfiles, configfile): - conf = loadConfig(configfile) - +def process(dumpfiles, conf): if conf.include_guard: global include_guard_header_re include_guard_header_re = conf.include_guard.get('RE_HEADERFILE',"[^/].*\\.h\\Z") @@ -334,10 +371,12 @@ def check_variable_naming(conf,cfg): evalExpr(conf.variable, exp, mockToken, 'Variable') # Naming check for Global, Private and Public member variables -def check_gpp_naming(conf_list,cfg,access,message): +def check_gpp_naming(conf_list,cfg,access,message,require_const=False): for var in cfg.variables: if var.access != access: continue + if require_const and not var.isConst: + continue mockToken = DataStruct(var.typeStartToken.file, var.typeStartToken.linenr, var.nameToken.str, var.nameToken.column) for exp in conf_list: evalExpr(conf_list, exp, mockToken, message) @@ -393,6 +432,8 @@ def process_data(conf,data): check_gpp_naming(conf.private_member,cfg,'Private','Private member variable') if conf.public_member: check_gpp_naming(conf.public_member,cfg,'Public','Public member variable') + if conf.const: + check_gpp_naming(conf.const,cfg,'Public','Constant',require_const=True) if conf.global_variable: check_gpp_naming(conf.global_variable,cfg,'Global','Global variable') if conf.function_name: @@ -406,14 +447,32 @@ def process_data(conf,data): mockToken = DataStruct(fn,0,os.path.basename(fn)) reportNamingError(mockToken,'Missing include guard','includeGuardMissing') -if __name__ == "__main__": +def main(): parser = cppcheckdata.ArgumentParser() parser.add_argument("--debugprint", action="store_true", default=False, help="Add debug prints") parser.add_argument("--configfile", type=str, default="namingng.config.json", help="Naming check config file") - + # The following flags are implemented to provide backward compatibility with the former naming.py addon. + parser.add_argument("--var", type=str, + help="[deprecated] specify RE_VARNAME and RE_GLOBAL_VARIABLE") + parser.add_argument("--const", type=str, + help="[deprecated] specify RE_CONSTNAME") + parser.add_argument("--private-member-variable", type=str, + help="[deprecated] specify RE_PRIVATE_MEMBER_VARIABLE") + parser.add_argument("--function", type=str, + help="[deprecated] specify RE_FUNCTIONNAME") + + global args args = parser.parse_args() - process(args.dumpfile, args.configfile) + if args.var or args.const or args.private_member_variable or args.function: + conf = genDeprecatedConfig() + else: + conf = loadConfig(args.configfile) + + process(args.dumpfile, conf) + +if __name__ == "__main__": + main() sys.exit(0) diff --git a/gui/codeeditorstyle.cpp b/gui/codeeditorstyle.cpp index e99ce2f65fa..d224102caf7 100644 --- a/gui/codeeditorstyle.cpp +++ b/gui/codeeditorstyle.cpp @@ -23,23 +23,23 @@ #include CodeEditorStyle::CodeEditorStyle( - // cppcheck-suppress naming-varname - TODO: fix this + // cppcheck-suppress naming-namingConvention - TODO: fix this QColor CtrlFGColor, QColor CtrlBGColor, - // cppcheck-suppress naming-varname - TODO: fix this + // cppcheck-suppress naming-namingConvention - TODO: fix this QColor HiLiBGColor, - // cppcheck-suppress naming-varname - TODO: fix this + // cppcheck-suppress naming-namingConvention - TODO: fix this QColor LnNumFGColor, QColor LnNumBGColor, - // cppcheck-suppress naming-varname - TODO: fix this + // cppcheck-suppress naming-namingConvention - TODO: fix this QColor KeyWdFGColor, QFont::Weight KeyWdWeight, - // cppcheck-suppress naming-varname - TODO: fix this + // cppcheck-suppress naming-namingConvention - TODO: fix this QColor ClsFGColor, QFont::Weight ClsWeight, - // cppcheck-suppress naming-varname - TODO: fix this + // cppcheck-suppress naming-namingConvention - TODO: fix this QColor QteFGColor, QFont::Weight QteWeight, - // cppcheck-suppress naming-varname - TODO: fix this + // cppcheck-suppress naming-namingConvention - TODO: fix this QColor CmtFGColor, QFont::Weight CmtWeight, - // cppcheck-suppress naming-varname - TODO: fix this + // cppcheck-suppress naming-namingConvention - TODO: fix this QColor SymbFGColor, QColor SymbBGColor, - // cppcheck-suppress naming-varname - TODO: fix this + // cppcheck-suppress naming-namingConvention - TODO: fix this QFont::Weight SymbWeight) : widgetFGColor(CtrlFGColor), widgetBGColor(CtrlBGColor), diff --git a/gui/codeeditorstyle.h b/gui/codeeditorstyle.h index a700aa810ee..771f5df739e 100644 --- a/gui/codeeditorstyle.h +++ b/gui/codeeditorstyle.h @@ -51,23 +51,23 @@ class QSettings; class CodeEditorStyle { public: explicit CodeEditorStyle( - // cppcheck-suppress naming-varname - TODO: fix this + // cppcheck-suppress naming-namingConvention - TODO: fix this QColor CtrlFGColor, QColor CtrlBGColor, - // cppcheck-suppress naming-varname - TODO: fix this + // cppcheck-suppress naming-namingConvention - TODO: fix this QColor HiLiBGColor, - // cppcheck-suppress naming-varname - TODO: fix this + // cppcheck-suppress naming-namingConvention - TODO: fix this QColor LnNumFGColor, QColor LnNumBGColor, - // cppcheck-suppress naming-varname - TODO: fix this + // cppcheck-suppress naming-namingConvention - TODO: fix this QColor KeyWdFGColor, QFont::Weight KeyWdWeight, - // cppcheck-suppress naming-varname - TODO: fix this + // cppcheck-suppress naming-namingConvention - TODO: fix this QColor ClsFGColor, QFont::Weight ClsWeight, - // cppcheck-suppress naming-varname - TODO: fix this + // cppcheck-suppress naming-namingConvention - TODO: fix this QColor QteFGColor, QFont::Weight QteWeight, - // cppcheck-suppress naming-varname - TODO: fix this + // cppcheck-suppress naming-namingConvention - TODO: fix this QColor CmtFGColor, QFont::Weight CmtWeight, - // cppcheck-suppress naming-varname - TODO: fix this + // cppcheck-suppress naming-namingConvention - TODO: fix this QColor SymbFGColor, QColor SymbBGColor, - // cppcheck-suppress naming-varname - TODO: fix this + // cppcheck-suppress naming-namingConvention - TODO: fix this QFont::Weight SymbWeight); bool operator==(const CodeEditorStyle& rhs) const; diff --git a/test/cli/test-other.py b/test/cli/test-other.py index dc8c0e818fe..7a365c3d279 100644 --- a/test/cli/test-other.py +++ b/test/cli/test-other.py @@ -306,7 +306,7 @@ def test_addon_naming(tmpdir): assert lines == [ 'Checking {} ...'.format(test_file) ] - assert stderr == '{}:2:1: style: Variable Var violates naming convention [naming-varname]\n'.format(test_file) + assert stderr == '{}:2:5: style: Global variable Var violates naming convention [naming-namingConvention]\n'.format(test_file) def test_addon_namingng(tmpdir): @@ -331,6 +331,7 @@ def test_addon_namingng(tmpdir): "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_CONST": ["const_[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": { ".*_tmp\\Z":[true,"illegal suffix _tmp"], @@ -400,9 +401,11 @@ class _clz { public: _clz() : _invalid_public(0), _invalid_private(0), priv_good(0), priv_bad_tmp(0) { } int _invalid_public; + const int random_number = 4; + const int const_zero = 0; private: char _invalid_private; - int priv_good; + int priv_good = random_number*const_zero; int priv_bad_tmp; }; @@ -475,13 +478,16 @@ class _clz { '{}: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: required prefix priv_ missing [namingng-namingConvention]'.format(test_file), + '{}:22:15: style: Constant random_number violates naming convention [namingng-namingConvention]'.format(test_file), + ' const int random_number = 4;', + ' ^', + '{}:25:10: style: Private member variable _invalid_private violates naming convention: required prefix priv_ missing [namingng-namingConvention]'.format(test_file), ' char _invalid_private;', ' ^', - '{}:25:9: style: Private member variable priv_bad_tmp violates naming convention: illegal suffix _tmp [namingng-namingConvention]'.format(test_file), + '{}:27:9: style: Private member variable priv_bad_tmp violates naming convention: illegal suffix _tmp [namingng-namingConvention]'.format(test_file), ' int priv_bad_tmp;', ' ^', - '{}:28:11: style: Namespace _invalid_namespace violates naming convention [namingng-namingConvention]'.format(test_file), + '{}:30:11: style: Namespace _invalid_namespace violates naming convention [namingng-namingConvention]'.format(test_file), 'namespace _invalid_namespace { }', ' ^', ]