Skip to content

Commit

Permalink
addons/namingng.py: Add config sanity checks and unit test.
Browse files Browse the repository at this point in the history 
The config file is parsed and (superficially) validated, before starting
validation. Various errors are checked and reported, along with a non-zero exit
status.

After parsing and type validation, the config values are stored in an object,
so they can be referenced as object members instead of dict keys
(`conf.variable` instead of `conf["RE_VARNAME"]`). This separates config syntax
from application code.

A unit test is added to test config file validation.

The example config JSON in namingng.py is fixed.
  • Loading branch information
@mvds00
mvds00 committed Jan 1, 2024
1 parent fd21729 commit 3487a50
Show file tree
Hide file tree
Showing 2 changed files with 210 additions and 48 deletions.
177 changes: 129 additions & 48 deletions addons/namingng.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@
# JSON format:
#
# {
# "RE_VARNAME": "[a-z]*[a-zA-Z0-9_]*\\Z",
# "RE_VARNAME": ["[a-z]*[a-zA-Z0-9_]*\\Z"],
# "RE_PRIVATE_MEMBER_VARIABLE": null,
# "RE_FUNCTIONNAME": "[a-z0-9A-Z]*\\Z",
# "RE_FUNCTIONNAME": ["[a-z0-9A-Z]*\\Z"],
# "_comment": "comments can be added to the config with underscore-prefixed keys",
# "include_guard": {
# "input": "path",
# "prefix": "GUARD_",
Expand Down Expand Up @@ -49,10 +50,89 @@ def __init__(self, file, linenr, string):
def reportNamingError(location,message,errorId='namingConvention',severity='style',extra=''):
cppcheckdata.reportError(location,severity,message,'namingng',errorId,extra)

def configError(error,fatal=True):
print('config error: %s'%error)
if fatal:
sys.exit(1)

def loadConfig(configfile):
with open(configfile) as fh:
data = json.load(fh)
return data
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))
except Exception as e:
configError("error opening config file '%s': %s"%(configfile,e))

if not isinstance(data, dict):
configError('config file must contain a JSON object at the top level')

# All errors are emitted before bailing out, to make the unit test more
# effective.
have_error = False

# Put config items in a class, so that settings can be accessed using
# config.feature
class Config:
pass
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),
}

# parse defined keys and store as members of config object
for key,opts in mapping.items():
json_key = opts[0]
req_type = opts[1]
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__
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)
have_error = True
if have_error:
continue

setattr(config,key,value)

# check remaining keys, only accept underscore-prefixed comments
for key,value in data.items():
if key == '' or key[0] != '_':
configError("unknown config key '%s'"%key,fatal=False)
have_error = True

if have_error:
sys.exit(1)

return config


def checkTrueRegex(data, expr, msg):
Expand Down Expand Up @@ -82,7 +162,7 @@ def evalExpr(conf, exp, mockToken, msgType):
msg = msgType + ' ' + mockToken.str + ' violates naming convention'
checkFalseRegex(mockToken, exp, msg)

def check_include_guard_name(conf_ig,cfg,directive):
def check_include_guard_name(conf_include_guard,cfg,directive):
parts = directive.str.split()
if len(parts) != 2:
msg = 'syntax error'
Expand All @@ -91,9 +171,9 @@ def check_include_guard_name(conf_ig,cfg,directive):
guard_name = parts[1]

filename = directive.file
if conf_ig.get('input','path') == 'basename':
if conf_include_guard.get('input','path') == 'basename':
filename = os.path.basename(filename)
use_case = conf_ig.get('case','upper')
use_case = conf_include_guard.get('case','upper')
if use_case == 'upper':
filename = filename.upper()
elif use_case == 'lower':
Expand All @@ -105,20 +185,20 @@ def check_include_guard_name(conf_ig,cfg,directive):
sys.exit(1)

barename = re.sub('[^A-Za-z0-9]','_',filename).strip('_')
expect_guard_name = conf_ig.get('prefix','') + barename + conf_ig.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')

return guard_name

def check_include_guard(conf_ig,cfg,unguarded_include_files):
def check_include_guard(conf_include_guard,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_ig.get('max_linenr', 5)
max_linenr = conf_include_guard.get('max_linenr', 5)

def report(directive,msg,errorId):
reportNamingError(directive,msg,errorId)
Expand All @@ -144,7 +224,7 @@ def report_pending_ifndef(directive):
continue

if directive.linenr > max_linenr:
if phase == 0 and conf_ig.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
Expand All @@ -153,11 +233,11 @@ def report_pending_ifndef(directive):
if phase == 0:
# looking for '#ifndef FILE_H'
if not directive.str.startswith('#ifndef'):
if conf_ig.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 = check_include_guard_name(conf_ig,cfg,directive)
guard_name = check_include_guard_name(conf_include_guard,cfg,directive)
if guard_name == None:
phase = -1
continue
Expand Down Expand Up @@ -187,9 +267,9 @@ def report_pending_ifndef(directive):
def process(dumpfiles, configfile, debugprint=False):
conf = loadConfig(configfile)

if "include_guard" in conf and conf["include_guard"]:
if conf.include_guard:
global include_guard_header_re
include_guard_header_re = conf["include_guard"].get('RE_HEADERFILE',"[^/].*\\.h\\Z")
include_guard_header_re = conf.include_guard.get('RE_HEADERFILE',"[^/].*\\.h\\Z")

for afile in dumpfiles:
if not afile[-5:] == '.dump':
Expand All @@ -199,35 +279,35 @@ def process(dumpfiles, configfile, debugprint=False):
data = cppcheckdata.CppcheckData(afile)

# Check File naming
if "RE_FILE" in conf and conf["RE_FILE"]:
if conf.file:
for source_file in data.files:
basename = os.path.basename(source_file)
good = False
for exp in conf["RE_FILE"]:
for exp in conf.file:
good |= bool(re.match(exp, source_file))
good |= bool(re.match(exp, basename))
if not good:
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"]:
if conf.namespace:
for tk in data.rawTokens:
if tk.str == 'namespace':
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)
for exp in conf.namespace:
evalExpr(conf.namespace, exp, mockToken, msgType)

unguarded_include_files = []
if "include_guard" in conf and conf["include_guard"]:
if conf["include_guard"].get('required',1):
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 "RE_VARNAME" in conf and conf["RE_VARNAME"]:
if conf.variable:
for var in cfg.variables:
if var.nameToken and var.access != 'Global' and var.access != 'Public' and var.access != 'Private':
prev = var.nameToken.previous
Expand All @@ -245,53 +325,54 @@ def process(dumpfiles, configfile, debugprint=False):
print("\n")
print("\t-- {} {}".format(varType, str(var.nameToken.str)))

if conf["skip_one_char_variables"] and len(var.nameToken.str) == 1:
if conf.skip_one_char_variables and len(var.nameToken.str) == 1:
continue
if varType in conf.get("var_prefixes",{}):
if not var.nameToken.str.startswith(conf["var_prefixes"][varType]):
if varType in conf.variable_prefixes:
prefix = conf.variable_prefixes[varType]
if not var.nameToken.str.startswith(prefix):
reportNamingError(var.typeStartToken,
'Variable ' +
var.nameToken.str +
' 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)
for exp in conf.variable:
evalExpr(conf.variable, exp, mockToken, msgType)

# Check Private Variable naming
if "RE_PRIVATE_MEMBER_VARIABLE" in conf and conf["RE_PRIVATE_MEMBER_VARIABLE"]:
if conf.private_member:
# TODO: Not converted yet
for var in cfg.variables:
if (var.access is None) or var.access != 'Private':
continue
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)
for exp in conf.private_member:
evalExpr(conf.private_member, exp, mockToken, msgType)

# Check Public Member Variable naming
if "RE_PUBLIC_MEMBER_VARIABLE" in conf and conf["RE_PUBLIC_MEMBER_VARIABLE"]:
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)
msgType = 'Public member variable'
for exp in conf["RE_PUBLIC_MEMBER_VARIABLE"]:
evalExpr(conf["RE_PUBLIC_MEMBER_VARIABLE"], exp, mockToken, msgType)
for exp in conf.public_member:
evalExpr(conf.public_member, exp, mockToken, msgType)

# Check Global Variable naming
if "RE_GLOBAL_VARNAME" in conf and conf["RE_GLOBAL_VARNAME"]:
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)
msgType = 'Public member variable'
for exp in conf["RE_GLOBAL_VARNAME"]:
evalExpr(conf["RE_GLOBAL_VARNAME"], exp, mockToken, msgType)
for exp in conf.global_variable:
evalExpr(conf.global_variable, exp, mockToken, msgType)

# Check Functions naming
if "RE_FUNCTIONNAME" in conf and conf["RE_FUNCTIONNAME"]:
if conf.function_name:
for token in cfg.tokenlist:
if token.function:
if token.function.type in ('Constructor', 'Destructor', 'CopyConstructor', 'MoveConstructor'):
Expand All @@ -304,27 +385,27 @@ def process(dumpfiles, configfile, debugprint=False):
if debugprint:
print("\t:: {} {}".format(retval, token.function.name))

if retval and retval in conf.get("function_prefixes",{}):
if not token.function.name.startswith(conf["function_prefixes"][retval]):
if retval and retval in conf.function_prefixes:
if not token.function.name.startswith(conf.function_prefixes[retval]):
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)
for exp in conf.function_name:
evalExpr(conf.function_name, exp, mockToken, msgType)

# Check Class naming
if "RE_CLASS_NAME" in conf and conf["RE_CLASS_NAME"]:
if conf.class_name:
for fnc in cfg.functions:
# Check if it is Constructor/Destructor
if fnc.type == 'Constructor' or fnc.type == 'Destructor':
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)
for exp in conf.class_name:
evalExpr(conf.class_name, exp, mockToken, msgType)

# Check include guard naming
if "include_guard" in conf and conf["include_guard"]:
check_include_guard(conf["include_guard"],cfg,unguarded_include_files)
if conf.include_guard:
check_include_guard(conf.include_guard,cfg,unguarded_include_files)

for fn in unguarded_include_files:
mockToken = DataStruct(fn,0,os.path.basename(fn))
Expand Down
Loading

0 comments on commit 3487a50

Please sign in to comment.