Date: Mon, 22 Oct 2012 20:02:36 +0000 (UTC) From: Chris Rees <crees@FreeBSD.org> To: src-committers@freebsd.org, svn-src-user@freebsd.org Subject: svn commit: r241910 - user/crees/rclint Message-ID: <201210222002.q9MK2bYx092877@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: crees (ports committer) Date: Mon Oct 22 20:02:36 2012 New Revision: 241910 URL: http://svn.freebsd.org/changeset/base/241910 Log: Almost complete rewrite-- now revolves around objects. Less stupid and easier to extend. Modified: user/crees/rclint/errors.en user/crees/rclint/rclint.py Modified: user/crees/rclint/errors.en ============================================================================== --- user/crees/rclint/errors.en Mon Oct 22 19:44:17 2012 (r241909) +++ user/crees/rclint/errors.en Mon Oct 22 20:02:36 2012 (r241910) @@ -5,20 +5,31 @@ # format as follows: error_id message explanation -defaults_invalid Invalid lines in defaults block. Allowed are variable assignments (including eval statements) and comments. A blank line ends the block. -defaults_mandatory_colon Override blanks in mandatory values (:=/:- vs =/-) Values that must not be blank (such as _enable) should be set by default as ${var:=value}; thus disallowing blank values (man sh) -defaults_non_mandatory_colon Do not clobber blank values for non-mandatory variables Syntax for variables that are not mandatory is ${var=value}; including := will override var="" set in rc.conf (man sh) -defaults_old_style Prefer condensed version for setting default values of variables When setting the default value for a variable, it is much less verbose and clearer to use the : ${variable=var} notation, as well as it being obvious that the source and destination variable are the same -defaults_value_quoted Do not quote values set by default With the syntax ${variable=value}, value can even contain spaces and does not need quoting - -definitions_missing Definitions block missing Following the default definitions must be other definitions (such as command, command_args) -definitions_quoted Do not quote values of variables Unless the value contains spaces/tabs, do not quote variable assignments +file_order Order of rc file incorrect Order of the rc file should be shebang/header/$FreeBSD$/sourcing rc_subr/name/rcvar/load_rc_config/setting defaults/setting other definitions/defining functions. Do not include unassociated shell commands, and blocks must be separated by single blank lines. Single blank lines may appear inside the defaults, definitions and functions blocks -functions_brace_inline Braces should have their own lines Do not put a brace on the same line as the function definition; it should have its own line -functions_brace_missing Function definition does not start with a brace The function definition must begin with an opening brace on its own line, and end with a closing brace on its own line functions_neverending Unclosed function block Functions must end with a closing brace on its own line +functions_problem Function incorrectly defined Functions should not have spaces in the definition functions_short One-line functions discouraged; put command directly in variable It is wasteful to write a function just for one command. It is possible to put commands directly inside declarations; name_prestart="install -d -o $name_user /var/run/$name" for example -functions_spaces No spaces allowed in function definitions Do not separate the function name from the parentheses with spaces; func() not func () + +orphaned_line Orphaned line Do not put unassociated shell commands inside rc scripts; put them inside functions + +rcorder_order rcorder block in the wrong order; should be PROVIDE/REQUIRE/BEFORE/KEYWORD See the article on RC scripting + +shebang Incorrect shebang used All rc scripts must start with the correct shebang; #!/bin/sh + +variables_defaults_mandatory_colon Override blanks in mandatory values (:=/:- vs =/-) Values that must not be blank (such as _enable) should be set by default as ${var:=value}; thus disallowing blank values (man sh) +variables_defaults_non_mandatory_colon Do not clobber blank values for non-mandatory variables Syntax for variables that are not mandatory is ${var=value}; including := will override var="" set in rc.conf (man sh) +variables_defaults_old_style Prefer condensed version for setting default values of variables When setting the default value for a variable, it is much less verbose and clearer to use the : ${variable=var} notation, as well as it being obvious that the source and destination variable are the same +variables_order Order of variables incorrect Order of the variables should be setting defaults then setting other variables + +value_quoted Do not quote values unless necessary Unless there are spaces in the value, quotes are unnecessary. With the syntax ${variable=value}, value can even contain spaces and does not need quoting + + + + +defaults_invalid Invalid lines in defaults block. Allowed are variable assignments (including eval statements) and comments. A blank line ends the block. + +definitions_missing Definitions block missing Following the default definitions must be other definitions (such as command, command_args) load_rc_config_extra load_rc_config not followed by blank line The load_rc_config line must form its own block load_rc_config_missing load_rc_config is late, incorrect or missing Directly following the name/rcvar block must be load_rc_config $name, unquoted @@ -30,7 +41,6 @@ rc_subr_late rc.subr sourcing late or no rcorder_keyword_freebsd Do not include FreeBSD in the KEYWORD rcorder line Historically FreeBSD scripts were marked in the KEYWORD section. This is no longer necessary rcorder_missing Missing rcorder block Following the FreeBSD RCSId keyword must be nothing but empty comment lines and an rcorder block -rcorder_order rcorder block in the wrong order; should be PROVIDE/REQUIRE/BEFORE/KEYWORD See the article on RC scripting rcvar_extra extra lines in name/rcvar block Order should be [blank line]/name=/rcvar=/[blank line] rcvar_incorrect rcvar is not set correctly rcvar must be directly set to name_enable. Do not quote, and do not use indirection; ${name}_enable is slower than example_enable @@ -40,8 +50,6 @@ rcvar_quoted rcvar is quoted Do not quot rcsid Missing FreeBSD RCSId keyword All rc scripts must contain a line beginning # $FreeBSD$. Do not include blank lines without comment markers at the beginning (#) until the script begins run_rc_argument Incorrect argument to run_rc_command The last line of the file should be run_rc_command $1 -run_rc_cruft Order of rc file incorrect Order of the rc file should be shebang/header/$FreeBSD$/setting defaults/setting other definitions/defining functions. Do not include unassociated shell commands, and blocks must be separated by single blank lines. Single blank lines may appear inside the defaults, definitions and functions blocks. run_rc_followed run_rc_command line is not the last line in the file Do not write anything after the run_rc_command line run_rc_quoted Quoted argument to run_rc_command No need to quote the argument to run_rc_command -shebang Incorrect shebang used All rc scripts must start with the correct shebang; #!/bin/sh Modified: user/crees/rclint/rclint.py ============================================================================== --- user/crees/rclint/rclint.py Mon Oct 22 19:44:17 2012 (r241909) +++ user/crees/rclint/rclint.py Mon Oct 22 20:02:36 2012 (r241910) @@ -22,331 +22,339 @@ # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF # THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. # -# $FreeBSD$ # -major = 0 -minor = 0 -micro = 0 +__version__ = '$FreeBSD$' + +MAJOR = 0 +MINOR = 0 +MICRO = 0 import argparse import logging import re import textwrap -def read_db(dbname, language): - try: +class Db: + def __init__(self, dbname, language): + self._contents = [] with open('%s.%s' % (dbname, language)) as f: logging.debug('Sucking in %s database' % dbname) - contents = { } for e in f.readlines(): + e = e.rstrip('\n') if not e or e[0] == '#': continue - e = e.split(' ') - contents[e[0]] = e[1:] - except: - logging.error('Cannot open %s database for language %s' % (dbname, language)) - exit() - return contents - -def explain(error): - if verbosity > 0: - print textwrap.fill(error[1], - initial_indent='==> ', subsequent_indent=' ') - -def error(type, line_number, filename): - logging.error("[%d]%s: %s " % (line_number + 1, filename, errors[type][0])) - explain(errors[type]) - -def check_quoted(string): - return True if string[0] == '"' or string[0] == "'" else False - -def mandatory(var): - mand = ['enable'] - return True if var.split('_')[-1] in mand else False - -def get_value(var, line): - try: - return re.match('%s=(\S+)$' % var, line).group(1) - except: - return False + self._contents.append(e.split('\t')) -def get_assignment(line): - try: - return re.match('(\S+)=(.+)$', line).groups() - except: + def _get(self, key, index): + for c in self._contents: + if c[0] == key: + return c[index] return False -def check_header(lines, num, filename): - # Basic order; shebang, copyright, RCSId, gap, rcorder - - logging.debug('Check shebang') - if lines[num] != '#!/bin/sh': - error('shebang', num, filename) - - logging.debug('Skipping license') - num += 1 - while (not re.match('# \$FreeBSD[:$]', lines[num]) and - (lines[num] == '' or lines[num][0] == '#')): - num += 1 - - logging.debug('Checking for RCSId') - if not re.match('# \$FreeBSD[:$]', lines[num]): - error('rcsid', num, filename) - - num += 1 - while lines[num] == '#' or lines[num] == '': num += 1 - - logging.debug('Checking rcorder order and sucking in names') - if not re.match('# [PRBK]', lines[num]): - error('rcorder_missing', num, filename) - orders = ['provide', 'require', 'before', 'keyword'] - index = 0 - rcorder = {o: [] for o in orders} - while index < 4: - order = orders[index] - try: - for result in re.match('# %s: (.*)' % order.upper(), - lines[num]).group(1).split(' '): - rcorder[order].append(result) - num += 1 - except: - index += 1 - - if 'FreeBSD' in rcorder['keyword']: - error('rcorder_keyword_freebsd', num, filename) - - if re.match('# [PRBK]', lines[num]): - error('rcorder_order', num, filename) - - return num - -def check_intro(lines, num, filename): - logging.debug('Checking sourcing lines') - while lines[num] == '' or lines[num][0] == '#': - num += 1 - - if lines[num] != '. /etc/rc.subr': - error('rc_subr_late', num, filename) - - logging.debug('Checking name assignment') - while lines[num] == '' or lines[num][0] == '#' or lines[num][0] == '.': - num += 1 - - name = get_value('name', lines[num]) - if not name: - error('name_missing', num, filename) - elif check_quoted(name): - error('name_quoted', num, filename) - else: - logging.debug('name discovered as %s' % name) - num += 1 - - logging.debug('Checking rcvar') - rcvar = get_value('rcvar', lines[num]) - logging.debug('rcvar discovered as %s' % rcvar if rcvar else 'rcvar not discovered') - - if not rcvar: - error('rcvar_missing', num, filename) - elif check_quoted(rcvar): - error('rcvar_quoted', num, filename) - elif rcvar != '%s_enable' % name: - error('rcvar_incorrect', num, filename) - else: - num += 1 - - logging.debug('Checking load_rc_config') - if lines[num] == '': - num += 1 - else: - error('rcvar_extra', num, filename) - - if re.match('load_rc_config (?:\$name|%s)' % name, lines[num]): - num += 1 - else: - error('load_rc_config_missing', num, filename) - - if lines[num] == '': - num += 1 - else: - error('load_rc_config_extra', num, filename) - - return num - -def check_defaults(lines, num, filename): - logging.debug('Checking defaults set') + def error(self, key): + return self._get(key, 1) - default = { } - try: - while lines[num]: - while lines[num][0] == '#' or lines[num][:4] == 'eval': - num += 1 - if lines[num][0] == ':': - # Shorthand set self-default assignment - (target, operator, value) = re.match(': \${([^:=]+)(:?=)([^}]+)}', lines[num]).groups() - if operator == ':=' and not mandatory(target): - error('defaults_non_mandatory_colon', num, filename) - elif operator == '=' and mandatory(target): - error('defaults_mandatory_colon', num, filename) - - else: - # Longhand set default assignment - (target, source, operator, value) = re.match('([^=]+)=\${([^:-]+)(:?-)([^}]+)}', lines[num]).groups() - if target == source: - error('defaults_old_style', num, filename) - - if operator == ':-' and not mandatory(target): - error('defaults_non_mandatory_colon', num, filename) - elif operator == '-' and mandatory(target): - error('defaults_mandatory_colon', num, filename) + def explanation(self, key): + return self._get(key, 2) - if check_quoted(value): - error('defaults_value_quoted', num, filename) - - num += 1 - except: - error('defaults_invalid', num, filename) + def give(self, key, num): + err = self.error(key) + if err: + logging.error('[%d]: %s ' % (num+1, err)) + if verbosity > 0: + print textwrap.fill(self.explanation(key), + initial_indent='==> ', + subsequent_indent=' ') + else: + logging.error('No such error: %s' % key) + +class Statement: + def __init__(self, line, number): + types = {'.': 'source', 'load_rc_config': 'load_rc_config', + 'run_rc_command': 'run_rc_command'} + spl = line.split(' ') + if spl[0] in types: + self.name = spl[0] + self.type = types[spl[0]] + self.value = ' '.join(spl[1:]) + self.line = number + else: + self.value = False + + def quoted(self): + if self.value and (self.value[0] == '"' or self.value[0] == '\''): + return True + else: + return False + + def pointless_quoted(self): + if self.quoted(): + if self.type == 'shorthand': + return True + elif ' ' not in self.value and '\t' not in self.value: + return True + return False - # Allow line breaks in the middle; if we put - # gaps in that's usually good style. Lookahead! - if lines[num+1] and (':' in lines[num+1] or '-' in lines[num+1]): - return check_defaults(lines, num, filename) - else: - return num - -def check_definitions(lines, num, filename): - logging.debug('Checking the basic definitions') - - num += 1 - - while lines[num]: - while lines[num][0] == '#' or lines[num][:4] == 'eval': num += 1 - try: - (var, value) = get_assignment(lines[num]) - except: - error('definitions_missing', num, filename) - return num - - if check_quoted(value): - if ' ' not in lines[num] and ' ' not in lines[num]: - error('definitions_quoted', num, filename) - num += 1 - - # As in check_defaults, allow line breaks in the middle; if we put - # gaps in that's usually good style. Lookahead! - if lines[num+1] and '=' in lines[num+1]: - return check_definitions(lines, num, filename) - else: - return num - -def check_functions(lines, num, filename): - logging.debug('Now checking functions') - - num += 1 - func = { } - - while lines[num]: - while not lines[num] or lines[num][0] == '#': - num += 1 - - if lines[num][-2:] != '()': - if lines[num][-1] == '{': - error('functions_brace_inline', num, filename) +class Variable(Statement): + def __init__(self, line, number): + basic = re.compile(r'([^\s=]+)=(.*)') + result = basic.match(line) + if result: + is_longhand = self.is_longhand_default(line) + if is_longhand: + (self.name, self.source, colon, self.value) = is_longhand + self.clobber = True if colon[0] == ':' else False + self.type = 'longhand' else: - logging.debug('No functions left!') - return num - if ' ' in lines[num]: - error('functions_spaces', num, filename) - func['name'] = lines[num][:-2] - func['length'] = 0 - - num += 1 - if lines[num] != '{': - error('functions_brace_missing', num, filename) - - num += 1 - - try: - while lines[num] != '}': - print lines[num] - num += 1 - if lines[num] and lines[num][0] != '#': - func['length'] += 1 - except: - error('functions_neverending', num, filename) - return num - - if func['length'] == 1: - error('functions_short', num, filename) - - print lines[num] - print lines[num+1] - print lines[num+2] - - if lines[num+2] and '{' in lines[num+2]: - return check_functions(lines, num, filename) - else: - return num + (self.name, self.value) = result.groups() + self.type = ( + 'init' if self.name in ('name', 'rcvar') else 'basic') + + elif line[:4] == 'eval': + self.value = line + self.name = line + self.type = 'eval' + else: + is_shorthand = self.is_shorthand_default(line) + if is_shorthand: + (self.name, assignment, self.value) = is_shorthand + self.clobber = True if assignment[0] == ':' else False + self.type = 'shorthand' + + if not hasattr(self, 'value'): + self.value = False + self.line = number + + def is_longhand_default(self, line): + match = re.match(r'([^=\s]+)=\${(^[:-]+)(:?-)([^}]+)', line) + return match.groups() if match else False + + def is_shorthand_default(self, line): + match = re.match(r': \${([^\s:=]+)(:?=)([^}]+)}', line) + return match.groups() if match else False + +class Comment: + def __init__(self, line, number): + self.value = line if line and line[0] == '#' else False + self.line = number + + def match(self, regex): + result = re.match(regex, self.value) + if result: + return result.groups() + else: + return False + +class Shebang: + def __init__(self, comment): + self.line = comment.line + result = comment.match(r'#!(\S+)\s*(.*)') + if result: + self.value = result[0] + self.args = result[1] + else: + self.value = False + +class Rcorder: + def __init__(self, comment): + self.line = comment.line + result = comment.match('# ([A-Z]+): (.+)') + if result: + (self.type, self.value) = (result[0], result[1:]) + else: + self.value = False + +class RcsId: + def __init__(self, comment): + self.line = comment.line + result = comment.match(r'# \$Free' + r'BSD([:$].*)') + if result: + self.value = result[0] + else: + self.value = False + +class Function: + def __init__(self, lines, num): + if lines[1] and lines[1][0] == '{': + try: + self.name = re.match(r'([\S_]+)\(\)$', lines[0]).group(1) + except: + error.give('functions_problem', num) + self.length = 0 + self.line = num + self.value = [] + while lines[self.length] != '}': + self.length += 1 + if self.length >= len(lines): + error.give('functions_neverending', num) + break + self.value.append(lines[self.length]) + # Remove { and } lines from length + self.length -= 2 + logging.debug('Found function %s' % self.name) + else: + self.value = False -def check_run_rc(lines, num, filename): - logging.debug('Checking the last line of the file contains run_rc_command') + def short(self): + return True if self.length <= 1 else False - while not lines[num] or lines[num][0] == '#': - num += 1 + def linenumbers(self): + return range(self.line, self.line+self.length+3) - try: - arg = re.match('run_rc_command (.*)$', lines[num]).group(1) - if check_quoted(arg): - error('run_rc_quoted', num, filename) - elif arg != r'$1': - error('run_rc_argument', num, filename) - - if num < len(lines) - 1: - error('run_rc_followed', num, filename) - except: - error('run_rc_cruft', num, filename) - -def general_checks(lines, filename): - logging.debug('Checking for unrecommended sequences and orphan commands') - # Don't use regex on last line, it must contain run_rc_command, which fails - # unassociated shell command test. We already hacked for load_rc_config - for num in range(0, len(lines) - 1): - for regex in problems.keys(): - if lines[num] and re.search(regex, lines[num]): - logging.warn("[%d]%s: %s " % (num + 1, filename, - problems[regex][0])) - explain(problems[regex]) + def contains_line(self, line): + return True if line in self.linenumbers() else False def do_rclint(filename): logging.debug('Suck in file %s' % filename) try: - lines=[line.rstrip('\n') for line in open(filename)] - except: + lines = [line.rstrip('\n') for line in open(filename)] + except: logging.error('Cannot open %s for testing' % filename) return - lineno = {'begin': 0} - - lineno['header'] = check_header(lines, lineno['begin'], filename) - lineno['intro'] = check_intro(lines, lineno['header'], filename) - lineno['defaults'] = check_defaults(lines, lineno['intro'], filename) - lineno['definitions'] = check_definitions(lines, lineno['defaults'], filename) - lineno['functions'] = check_functions(lines, lineno['definitions'], filename) - check_run_rc(lines, lineno['functions'], filename) - general_checks(lines, filename) + lineobj = {'Variable': [], + 'Comment': [], + 'Statement': []} + + for num in range(0, len(lines)): + for obj in lineobj.keys(): + tmp = eval(obj)(lines[num], num) + if tmp.value != False: + lineobj[obj].append(tmp) + break + + lineobj['Shebang'] = [Shebang(lineobj['Comment'][0])] + lineobj['Function'] = [] + + for num in range(0, len(lines)-1): + tmp = Function(lines[num:], num) + if tmp.value: + lineobj['Function'].append(tmp) + + lineobj.update({'Rcorder': [], 'RcsId': []}) + + for comment in lineobj['Comment']: + tmp = Rcorder(comment) + if tmp.value: + lineobj['Rcorder'].append(tmp) + tmp = RcsId(comment) + if tmp.value: + lineobj['RcsId'] = [tmp] + + logging.debug('OK, done collecting variables. Time to check!') + + logging.debug('Checking shebang') + if len(lineobj['Shebang']) < 1: + error.give('shebang', num) + + logging.debug('Checking order of file') + linenumbers = [] + for obj in ('Shebang', 'RcsId', 'Rcorder'): + for o in lineobj[obj]: + linenumbers.append(o.line) + + for s in lineobj['Statement']: + if s.type == 'source': + linenumbers.append(s.line) + + for v in lineobj['Variable']: + if v.type == 'init': + linenumbers.append(v.line) + + for s in lineobj['Statement']: + if s.type == 'load_rc_config': + linenumbers.append(s.line) + + for v in lineobj['Variable']: + if v.type != 'init': + linenumbers.append(v.line) + + [linenumbers.append(f.line) for f in lineobj['Function']] + + for s in lineobj['Statement']: + if s.type == 'run_rc_command': + linenumbers.append(s.line) + + if sorted(linenumbers) != linenumbers: + error.give('file_order', 0) + + logging.debug('Checking all lines are accounted for') + for obj in lineobj.keys(): + for o in lineobj[obj]: + linenumbers.append(o.line) + for r in range(0, len(lines)): + if r not in linenumbers and lines[r] != '': + if True not in [f.contains_line(r) for f in lineobj['Function']]: + error.give('orphaned_line', r) + + logging.debug('Checking rcorder') + linenumbers = [] + for typ in ('PROVIDE', 'REQUIRE', 'BEFORE', 'KEYWORD'): + for o in lineobj['Rcorder']: + if o.type == typ: + linenumbers.append(o.line) + if sorted(linenumbers) != linenumbers: + error.give('rcorder_order') + + for o in lineobj['Rcorder']: + if o.type == 'KEYWORD' and 'freebsd' in [v.lower() for v in o.value]: + error.give('rcorder_keyword_freebsd', o.line) + + logging.debug('Checking order of variables') + linenumbers = [] + for typ in (('init'), ('longhand', 'shorthand'), ('basic')): + for var in lineobj['Variable']: + if var.type in typ: + linenumbers.append(var.line) + if sorted(linenumbers) != linenumbers: + error.give('variables_order', 0) + + logging.debug('Checking for pointless quoting') + for obj in lineobj['Variable']+lineobj['Statement']: + if obj.pointless_quoted(): + error.give('value_quoted', obj.line) + + logging.debug('Checking for rcvar set correctly') + for var in lineobj['Variable']: + if var.name == 'name': + progname = var.value + elif var.name == 'rcvar': + try: + if progname + '_enable' not in var.value: + error.give('rcvar_incorrect', var.line) + except: + error.give('file_order', var.line) + + logging.debug('Checking for short functions') + for function in lineobj['Function']: + if function.short(): + error.give('functions_short', function.line) + + logging.debug('Checking for defaults clobbering blank values') + for var in lineobj['Variable']: + if var.type in ('longhand', 'shorthand'): + if var.name.split('_')[-1] not in ('enable') and var.clobber: + error.give('variables_defaults_non_mandatory_colon', var.line) + elif not var.clobber and var.name.split('_')[-1] in ('enable'): + error.give('variables_defaults_mandatory_colon', var.line) + if var.type == 'longhand' and var.name == var.source: + error.give('variables_defaults_old_style', var.line) parser = argparse.ArgumentParser() parser.add_argument('filenames', nargs = '+') parser.add_argument('--language', nargs = 1, type=str, default = ['en'], help = 'sets the language that errors are reported in') parser.add_argument('-v', action='count', help='raises debug level; provides detailed explanations of errors') +parser.add_argument('--version', action='version', version='%s.%s.%s-%s'%(MAJOR, MINOR, MICRO, __version__)) args = parser.parse_args() verbosity = args.v logging.basicConfig(level=logging.DEBUG if verbosity > 1 else logging.WARN) -errors = read_db('errors', args.language[0]) -problems = read_db('problems', args.language[0]) +error = Db('errors', args.language[0]) +# problem = Db('problems', args.language[0]) -for file in args.filenames: - do_rclint(file) +for f in args.filenames: + print('Checking %s' % f) + do_rclint(f)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201210222002.q9MK2bYx092877>