addons/namingng.py: Reinstate dict-with-regexps, cosmetic overhaul. (#5824)

`namingng.py` somewhat supported specifying a dict instead of a list for
regular expressions, until the feature was broken by a patch of mine
recently. This PR contains a patch rewriting the feature and expanding
relevant unit tests.

To improve maintainability, a second patch is added that refactors the
code for better readability and structure.
This commit is contained in:
Maarten van der Schrieck 2024-01-04 16:26:54 +01:00 committed by GitHub
parent 481d4578ab
commit 21a9de7d42
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 215 additions and 207 deletions

View File

@ -1,5 +1,5 @@
{
"RE_FILE": {".*[A-Z]": [true, "under_scored"]},
"RE_FILE": [".*[A-Z]"],
"RE_NAMESPACE": {".*[A-Z]": [true, "under_scored"],
".*\\_$": [true, "under_scored"]},
"RE_FUNCTIONNAME": {".*\\_": [true, "camelCase"],

View File

@ -55,13 +55,32 @@ def configError(error,fatal=True):
if fatal:
sys.exit(1)
def validateConfigREs(list_or_dict,json_key):
have_error = False
for item in list_or_dict:
try:
re.compile(item)
except re.error as err:
configError("item '%s' of '%s' is not a valid regular expression: %s"%(item,json_key,err),fatal=False)
have_error = True
continue
if not isinstance(list_or_dict,dict):
continue
# item is actually a dict key; check value
value = list_or_dict[item]
if (not isinstance(value,list) or len(value) != 2
or not isinstance(value[0],bool) or not isinstance(value[1],str)):
configError("item '%s' of '%s' must be an array [bool,string]"%(item,json_key),fatal=False)
have_error = True
return have_error
def loadConfig(configfile):
if not os.path.exists(configfile):
configError("cannot find config file '%s'"%configfile)
try:
with open(configfile) as fh:
try:
data = json.load(fh)
except json.JSONDecodeError as e:
configError("error parsing config file as JSON at line %d: %s"%(e.lineno,e.msg))
@ -82,18 +101,18 @@ def loadConfig(configfile):
config = Config()
mapping = {
'file': ('RE_FILE', list),
'namespace': ('RE_NAMESPACE', list),
'include_guard': ('include_guard', dict),
'variable': ('RE_VARNAME', list),
'variable_prefixes': ('var_prefixes', dict, {}),
'private_member': ('RE_PRIVATE_MEMBER_VARIABLE', list),
'public_member': ('RE_PUBLIC_MEMBER_VARIABLE', list),
'global_variable': ('RE_GLOBAL_VARNAME', list),
'function_name': ('RE_FUNCTIONNAME', list),
'function_prefixes': ('function_prefixes', dict, {}),
'class_name': ('RE_CLASS_NAME', list),
'skip_one_char_variables': ('skip_one_char_variables', bool),
'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,)),
}
# parse defined keys and store as members of config object
@ -103,23 +122,18 @@ def loadConfig(configfile):
default = None if len(opts)<3 else opts[2]
value = data.pop(json_key,default)
if value is not None and not isinstance(value, req_type):
req_typename = req_type.__name__
if value is not None and type(value) not in req_type:
req_typename = ' or '.join([tp.__name__ for tp in req_type])
got_typename = type(value).__name__
configError('%s must be %s (not %s), or not set'%(json_key,req_typename,got_typename),fatal=False)
have_error = True
continue
if req_type == list and value is not None:
# type 'list' implies a list of regular expressions
for item in value:
try:
re.compile(item)
except re.error as err:
configError("item '%s' of '%s' is not a valid regular expression: %s"%(item,json_key,err),fatal=False)
# type list implies that this is either a list of REs or a dict with RE keys
if list in req_type and value is not None:
re_error = validateConfigREs(value,json_key)
if re_error:
have_error = True
if have_error:
continue
setattr(config,key,value)
@ -135,34 +149,19 @@ def loadConfig(configfile):
return config
def checkTrueRegex(data, expr, msg):
res = re.match(expr, data.str)
if res:
reportNamingError(data,msg)
def checkFalseRegex(data, expr, msg):
res = re.match(expr, data.str)
if not res:
reportNamingError(data,msg)
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)
elif ~conf[exp][0]:
msg = msgType + ' ' + mockToken.str + ' violates naming convention : ' + conf[exp][1]
checkFalseRegex(mockToken, exp, msg)
else:
msg = msgType + ' ' + mockToken.str + ' violates naming convention : ' + conf[exp][0]
checkFalseRegex(mockToken, exp, msg)
else:
report_as_error = False
msg = msgType + ' ' + mockToken.str + ' violates naming convention'
checkFalseRegex(mockToken, exp, msg)
def check_include_guard_name(conf_include_guard,cfg,directive):
if isinstance(conf, dict):
report_as_error = conf[exp][0]
msg += ': ' + conf[exp][1]
res = re.match(exp,mockToken.str)
if bool(res) == report_as_error:
reportNamingError(mockToken,msg)
def check_include_guard_name(conf,directive):
parts = directive.str.split()
if len(parts) != 2:
msg = 'syntax error'
@ -172,9 +171,9 @@ def check_include_guard_name(conf_include_guard,cfg,directive):
guard_column = 1+directive.str.find(guard_name)
filename = directive.file
if conf_include_guard.get('input','path') == 'basename':
if conf.include_guard.get('input','path') == 'basename':
filename = os.path.basename(filename)
use_case = conf_include_guard.get('case','upper')
use_case = conf.include_guard.get('case','upper')
if use_case == 'upper':
filename = filename.upper()
elif use_case == 'lower':
@ -186,20 +185,20 @@ def check_include_guard_name(conf_include_guard,cfg,directive):
sys.exit(1)
barename = re.sub('[^A-Za-z0-9]','_',filename).strip('_')
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:
msg = 'include guard naming violation; %s != %s'%(guard_name,expect_guard_name)
reportNamingError(directive,msg,'includeGuardName',column=guard_column)
return guard_name,guard_column
def check_include_guard(conf_include_guard,cfg,unguarded_include_files):
def check_include_guards(conf,cfg,unguarded_include_files):
# Scan for '#ifndef FILE_H' as the first directive, in the first N lines.
# Then test whether the next directive #defines the found name.
# Various tests are done:
# - check include guards for their naming and consistency
# - 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,column=0):
reportNamingError(directive,msg,errorId,column=column)
@ -225,20 +224,19 @@ def check_include_guard(conf_include_guard,cfg,unguarded_include_files):
continue
if directive.linenr > max_linenr:
if phase == 0 and conf_include_guard.get('required',1):
if phase == 0 and conf.include_guard.get('required',1):
report(directive,'include guard not found before line %d'%max_linenr,'includeGuardMissing')
pass
phase = -1
continue
if phase == 0:
# looking for '#ifndef FILE_H'
if not directive.str.startswith('#ifndef'):
if conf_include_guard.get('required',1):
if conf.include_guard.get('required',1):
report(directive,'first preprocessor directive should be include guard #ifndef','includeGuardMissing')
phase = -1
continue
guard_name,guard_column = check_include_guard_name(conf_include_guard,cfg,directive)
guard_name,guard_column = check_include_guard_name(conf,directive)
if guard_name == None:
phase = -1
continue
@ -265,7 +263,7 @@ def check_include_guard(conf_include_guard,cfg,unguarded_include_files):
if pending_ifndef:
report_pending_ifndef(pending_ifndef,guard_column)
def process(dumpfiles, configfile, debugprint=False):
def process(dumpfiles, configfile):
conf = loadConfig(configfile)
if conf.include_guard:
@ -278,9 +276,9 @@ def process(dumpfiles, configfile, debugprint=False):
if not args.cli:
print('Checking ' + afile + '...')
data = cppcheckdata.CppcheckData(afile)
process_data(conf,data)
# Check File naming
if conf.file:
def check_file_naming(conf,data):
for source_file in data.files:
basename = os.path.basename(source_file)
good = False
@ -291,33 +289,27 @@ def process(dumpfiles, configfile, debugprint=False):
mockToken = DataStruct(source_file, 0, basename)
reportNamingError(mockToken, 'File name ' + basename + ' violates naming convention')
# Check Namespace naming
if conf.namespace:
def check_namespace_naming(conf,data):
for tk in data.rawTokens:
if tk.str == 'namespace':
if tk.str != 'namespace':
continue
mockToken = DataStruct(tk.next.file, tk.next.linenr, tk.next.str, tk.next.column)
msgType = 'Namespace'
for exp in conf.namespace:
evalExpr(conf.namespace, exp, mockToken, msgType)
evalExpr(conf.namespace, exp, mockToken, 'Namespace')
unguarded_include_files = []
if conf.include_guard:
if conf.include_guard.get('required',1):
unguarded_include_files = [fn for fn in data.files if re.match(include_guard_header_re,fn)]
for cfg in data.configurations:
if not args.cli:
print('Checking %s, config %s...' % (afile, cfg.name))
if conf.variable:
def check_variable_naming(conf,cfg):
for var in cfg.variables:
if var.nameToken and var.access != 'Global' and var.access != 'Public' and var.access != 'Private':
if not var.nameToken:
continue
if var.access in ('Global','Public','Private'):
continue
prev = var.nameToken.previous
varType = prev.str
while "*" in varType and len(varType.replace("*", "")) == 0:
prev = prev.previous
varType = prev.str + varType
if debugprint:
if args.debugprint:
print("Variable Name: " + str(var.nameToken.str))
print("original Type Name: " + str(var.nameToken.valueType.originalTypeName))
print("Type Name: " + var.nameToken.valueType.type)
@ -338,45 +330,22 @@ def process(dumpfiles, configfile, debugprint=False):
column=var.nameToken.column)
mockToken = DataStruct(var.typeStartToken.file, var.typeStartToken.linenr, var.nameToken.str, var.nameToken.column)
msgType = 'Variable'
for exp in conf.variable:
evalExpr(conf.variable, exp, mockToken, msgType)
evalExpr(conf.variable, exp, mockToken, 'Variable')
# Check Private Variable naming
if conf.private_member:
# TODO: Not converted yet
# Naming check for Global, Private and Public member variables
def check_gpp_naming(conf_list,cfg,access,message):
for var in cfg.variables:
if (var.access is None) or var.access != 'Private':
if var.access != access:
continue
mockToken = DataStruct(var.typeStartToken.file, var.typeStartToken.linenr, var.nameToken.str, var.nameToken.column)
msgType = 'Private member variable'
for exp in conf.private_member:
evalExpr(conf.private_member, exp, mockToken, msgType)
for exp in conf_list:
evalExpr(conf_list, exp, mockToken, message)
# Check Public Member Variable naming
if conf.public_member:
for var in cfg.variables:
if (var.access is None) or var.access != 'Public':
continue
mockToken = DataStruct(var.typeStartToken.file, var.typeStartToken.linenr, var.nameToken.str, var.nameToken.column)
msgType = 'Public member variable'
for exp in conf.public_member:
evalExpr(conf.public_member, exp, mockToken, msgType)
# Check Global Variable naming
if conf.global_variable:
for var in cfg.variables:
if (var.access is None) or var.access != 'Global':
continue
mockToken = DataStruct(var.typeStartToken.file, var.typeStartToken.linenr, var.nameToken.str, var.nameToken.column)
msgType = 'Global variable'
for exp in conf.global_variable:
evalExpr(conf.global_variable, exp, mockToken, msgType)
# Check Functions naming
if conf.function_name:
def check_function_naming(conf,cfg):
for token in cfg.tokenlist:
if token.function:
if not token.function:
continue
if token.function.type in ('Constructor', 'Destructor', 'CopyConstructor', 'MoveConstructor'):
continue
retval = token.previous.str
@ -384,7 +353,7 @@ def process(dumpfiles, configfile, debugprint=False):
while "*" in retval and len(retval.replace("*", "")) == 0:
prev = prev.previous
retval = prev.str + retval
if debugprint:
if args.debugprint:
print("\t:: {} {}".format(retval, token.function.name))
if retval and retval in conf.function_prefixes:
@ -395,19 +364,43 @@ def process(dumpfiles, configfile, debugprint=False):
for exp in conf.function_name:
evalExpr(conf.function_name, exp, mockToken, msgType)
# Check Class naming
if conf.class_name:
def check_class_naming(conf,cfg):
for fnc in cfg.functions:
# Check if it is Constructor/Destructor
if fnc.type == 'Constructor' or fnc.type == 'Destructor':
if fnc.type not in ('Constructor','Destructor'):
continue
mockToken = DataStruct(fnc.tokenDef.file, fnc.tokenDef.linenr, fnc.name, fnc.tokenDef.column)
msgType = 'Class ' + fnc.type
for exp in conf.class_name:
evalExpr(conf.class_name, exp, mockToken, msgType)
# Check include guard naming
def process_data(conf,data):
if conf.file:
check_file_naming(conf,data)
if conf.namespace:
check_namespace_naming(conf,data)
unguarded_include_files = []
if conf.include_guard and conf.include_guard.get('required',1):
unguarded_include_files = [fn for fn in data.files if re.match(include_guard_header_re,fn)]
for cfg in data.configurations:
if not args.cli:
print('Checking config %s...' % cfg.name)
if conf.variable:
check_variable_naming(conf,cfg)
if conf.private_member:
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.global_variable:
check_gpp_naming(conf.global_variable,cfg,'Global','Global variable')
if conf.function_name:
check_function_naming(conf,cfg)
if conf.class_name:
check_class_naming(conf,cfg)
if conf.include_guard:
check_include_guard(conf.include_guard,cfg,unguarded_include_files)
check_include_guards(conf,cfg,unguarded_include_files)
for fn in unguarded_include_files:
mockToken = DataStruct(fn,0,os.path.basename(fn))
@ -421,6 +414,6 @@ if __name__ == "__main__":
help="Naming check config file")
args = parser.parse_args()
process(args.dumpfile, args.configfile, args.debugprint)
process(args.dumpfile, args.configfile)
sys.exit(0)

View File

@ -347,7 +347,10 @@ def test_addon_namingng(tmpdir):
"RE_NAMESPACE": ["[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_PRIVATE_MEMBER_VARIABLE": {
".*_tmp\\Z":[true,"illegal suffix _tmp"],
"priv_.*\\Z":[false,"required prefix priv_ missing"]
},
"RE_GLOBAL_VARNAME": ["[a-z][a-z0-9_]*[a-z0-9]\\Z"],
"RE_FUNCTIONNAME": ["[a-z][a-z0-9_]*[a-z0-9]\\Z"],
"include_guard": {
@ -410,10 +413,12 @@ static int _invalid_static_global;
class _clz {
public:
_clz() : _invalid_public(0), _invalid_private(0) { }
_clz() : _invalid_public(0), _invalid_private(0), priv_good(0), priv_bad_tmp(0) { }
int _invalid_public;
private:
char _invalid_private;
int priv_good;
int priv_bad_tmp;
};
namespace _invalid_namespace { }
@ -480,15 +485,18 @@ namespace _invalid_namespace { }
'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) { }',
' _clz() : _invalid_public(0), _invalid_private(0), priv_good(0), priv_bad_tmp(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),
'{}:23:10: style: Private member variable _invalid_private violates naming convention: required prefix priv_ missing [namingng-namingConvention]'.format(test_file),
' char _invalid_private;',
' ^',
'{}:26:11: style: Namespace _invalid_namespace violates naming convention [namingng-namingConvention]'.format(test_file),
'{}:25: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),
'namespace _invalid_namespace { }',
' ^',
]
@ -518,7 +526,12 @@ def test_addon_namingng_config(tmpdir):
"RE_NAMESPACE": false,
"RE_VARNAME": ["+bad pattern","[a-z]_good_pattern\\Z","(parentheses?"],
"RE_PRIVATE_MEMBER_VARIABLE": "[a-z][a-z0-9_]*[a-z0-9]\\Z",
"RE_PUBLIC_MEMBER_VARIABLE": "[a-z][a-z0-9_]*[a-z0-9]\\Z",
"RE_PUBLIC_MEMBER_VARIABLE": {
"tmp_.*\\Z":[true,"illegal prefix tmp_"],
"bad_.*\\Z":true,
"public_.*\\Z":[false],
"pub_.*\\Z":[0,"required prefix pub_ missing"]
},
"RE_GLOBAL_VARNAME": "[a-z][a-z0-9_]*[a-z0-9]\\Z",
"RE_FUNCTIONNAME": "[a-z][a-z0-9_]*[a-z0-9]\\Z",
"RE_CLASS_NAME": "[a-z][a-z0-9_]*[a-z0-9]\\Z",
@ -561,17 +574,19 @@ def test_addon_namingng_config(tmpdir):
assert lines == [
"Output:",
"config error: RE_FILE must be list (not str), or not set",
"config error: RE_NAMESPACE must be list (not bool), or not set",
"config error: RE_NAMESPACE must be list or dict (not bool), or not set",
"config error: include_guard must be dict (not bool), or not set",
"config error: item '+bad pattern' of 'RE_VARNAME' is not a valid regular expression: nothing to repeat at position 0",
"config error: item '(parentheses?' of 'RE_VARNAME' is not a valid regular expression: missing ), unterminated subpattern at position 0",
"config error: var_prefixes must be dict (not list), or not set",
"config error: RE_PRIVATE_MEMBER_VARIABLE must be list (not str), or not set",
"config error: RE_PUBLIC_MEMBER_VARIABLE must be list (not str), or not set",
"config error: RE_GLOBAL_VARNAME must be list (not str), or not set",
"config error: RE_FUNCTIONNAME must be list (not str), or not set",
"config error: RE_PRIVATE_MEMBER_VARIABLE must be list or dict (not str), or not set",
"config error: item 'bad_.*\\Z' of 'RE_PUBLIC_MEMBER_VARIABLE' must be an array [bool,string]",
"config error: item 'public_.*\\Z' of 'RE_PUBLIC_MEMBER_VARIABLE' must be an array [bool,string]",
"config error: item 'pub_.*\\Z' of 'RE_PUBLIC_MEMBER_VARIABLE' must be an array [bool,string]",
"config error: RE_GLOBAL_VARNAME must be list or dict (not str), or not set",
"config error: RE_FUNCTIONNAME must be list or dict (not str), or not set",
"config error: function_prefixes must be dict (not bool), or not set",
"config error: RE_CLASS_NAME must be list (not str), or not set",
"config error: RE_CLASS_NAME must be list or dict (not str), or not set",
"config error: skip_one_char_variables must be bool (not str), or not set",
"config error: unknown config key 'RE_VAR_NAME' [internalError]",
"",