From owner-svn-src-user@FreeBSD.ORG Tue Oct 16 20:24:55 2012 Return-Path: Delivered-To: svn-src-user@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 8E91F75C; Tue, 16 Oct 2012 20:24:55 +0000 (UTC) (envelope-from crees@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 6D4C88FC0A; Tue, 16 Oct 2012 20:24:55 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.4/8.14.4) with ESMTP id q9GKOtTw014005; Tue, 16 Oct 2012 20:24:55 GMT (envelope-from crees@svn.freebsd.org) Received: (from crees@localhost) by svn.freebsd.org (8.14.4/8.14.4/Submit) id q9GKOt8k014001; Tue, 16 Oct 2012 20:24:55 GMT (envelope-from crees@svn.freebsd.org) Message-Id: <201210162024.q9GKOt8k014001@svn.freebsd.org> From: Chris Rees Date: Tue, 16 Oct 2012 20:24:55 +0000 (UTC) To: src-committers@freebsd.org, svn-src-user@freebsd.org Subject: svn commit: r241617 - user/crees/rclint X-SVN-Group: user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-user@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the experimental " user" src tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Oct 2012 20:24:55 -0000 Author: crees (ports committer) Date: Tue Oct 16 20:24:54 2012 New Revision: 241617 URL: http://svn.freebsd.org/changeset/base/241617 Log: Allow line breaks in large blocks Do basic checking of definitions Use tab as delimiter; regexes need colons and backslashes Check style and position of functions Handle missing rc files more gracefully Make regex checks for the whole file Added: user/crees/rclint/problems.en Modified: user/crees/rclint/errors.en user/crees/rclint/rclint.py Modified: user/crees/rclint/errors.en ============================================================================== --- user/crees/rclint/errors.en Tue Oct 16 20:18:15 2012 (r241616) +++ user/crees/rclint/errors.en Tue Oct 16 20:24:54 2012 (r241617) @@ -1,31 +1,42 @@ -# Rigid format of this file; backslash-separated tuples to define +# Rigid format of this file; tab-separated tuples to define # error messages and explanations. +# Yes, tab isn't a great separator; alternatives that aren't +# regex chars would be gratefully heard # format as follows: -error_id\message\explanation +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 +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 -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 +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 -name_missing\name is set late or not at all\Directly following the sourcing of scripts must follow setting of the variable name -name_quoted\name is quoted\Do not quote the value of name. If it has spaces, use underscores +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_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 () -rc_subr_late\rc.subr sourcing late or nonexistent\The first non-comment non-blank line in any rc file must be sourcing /etc/rc.subr +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 -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 +name_missing name is set late or not at all Directly following the sourcing of scripts must follow setting of the variable name +name_quoted name is quoted Do not quote the value of name. If it has spaces, use underscores -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 -rcvar_missing\rcvar is set late or not at all\Setting rcvar must be done straight after setting name -rcvar_quoted\rcvar is quoted\Do not quote the value of rcvar +rc_subr_late rc.subr sourcing late or nonexistent The first non-comment non-blank line in any rc file must be sourcing /etc/rc.subr -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 +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 -shebang\Incorrect shebang used\All rc scripts must start with the correct shebang; #!/bin/sh +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 +rcvar_missing rcvar is set late or not at all Setting rcvar must be done straight after setting name +rcvar_quoted rcvar is quoted Do not quote the value of rcvar + +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 + +shebang Incorrect shebang used All rc scripts must start with the correct shebang; #!/bin/sh Added: user/crees/rclint/problems.en ============================================================================== --- /dev/null 00:00:00 1970 (empty, because file is newly added) +++ user/crees/rclint/problems.en Tue Oct 16 20:24:54 2012 (r241617) @@ -0,0 +1,2 @@ +# Format of this file: regexshort_explanationexplanation +regex explain explanation Modified: user/crees/rclint/rclint.py ============================================================================== --- user/crees/rclint/rclint.py Tue Oct 16 20:18:15 2012 (r241616) +++ user/crees/rclint/rclint.py Tue Oct 16 20:24:54 2012 (r241617) @@ -34,12 +34,27 @@ import logging import re import textwrap -def error_explain(error): - if verbosity > 0: print textwrap.fill(error['explanation'], initial_indent='==> ', subsequent_indent=' ') +def read_db(dbname, language): + try: + with open('%s.%s' % (dbname, language)) as f: + logging.debug('Sucking in %s database' % dbname) + contents = { } + for e in f.readlines(): + 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, filename, errors[type]['message'])) - error_explain(errors[type]) + logging.error("[%d]%s: %s " % (line_number, filename, errors[type][0])) + explain(errors[type]) def check_quoted(string): return True if string[0] == '"' or string[0] == "'" else False @@ -49,12 +64,12 @@ def mandatory(var): return True if var.split('_')[-1] in mand else False def get_value(var, line): - n = re.match('%s=(\S*)$' % var, line) - if n: - value=n.group(1) - return value - else: - return False + try: return re.match('%s=(\S+)$' % var, line).group(1) + except: return False + +def get_assignment(line): + try: return re.match('(\S+)=(\S+)$', line).groups() + except: return False def check_header(lines, num, filename): # Basic order; shebang, copyright, RCSId, gap, rcorder @@ -156,7 +171,7 @@ def check_defaults(lines, num, filename) default = { } try: - while lines[num] != '': + while lines[num]: while lines[num][0] == '#' or lines[num][:4] == 'eval': num += 1 if lines[num][0] == ':': # Shorthand set self-default assignment @@ -184,15 +199,99 @@ def check_defaults(lines, num, filename) except: error('defaults_invalid', num, filename) + # 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) + else: + logging.debug('No functions left!') + return num + if ' ' in lines[num]: + error('functions_spaces', num, filename) + func['name'] = lines[num][:-2] + + num += 1 + if lines[num] != '{': + error('functions_brace_missing', num, filename) + + num += 1 + tmp = num + try: + while lines[num] != '}': + tmp += 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) + +def general_checks(lines, filename): + logging.debug('Checking for unrecommended sequences') + for num in range(0, len(lines)): + for regex in problems.keys(): + if lines[num] and re.search(regex, lines[num]): + logging.warn("[%d]%s: %s " % (num, filename, problems[key][1])) + explain(problem) + def do_rclint(filename): logging.debug('Suck in file %s' % filename) - lines=[line.rstrip('\n') for line in open(filename)] - begin_num = 0 - - endofheader_num = check_header(lines, begin_num, filename) - endofintro_num = check_intro(lines, endofheader_num, filename) - endofdefaults_num = check_defaults(lines, endofintro_num, filename) + try: + 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) + general_checks(lines, filename) parser = argparse.ArgumentParser() parser.add_argument('filenames', nargs = '+') @@ -202,22 +301,10 @@ parser.add_argument('-v', action='count' args = parser.parse_args() verbosity = args.v -errordb = 'errors.%s' % args.language[0] +logging.basicConfig(level=logging.DEBUG if verbosity > 1 else logging.ERROR) -logging.basicConfig(level=(logging.DEBUG if verbosity > 1 else logging.ERROR)) - -try: - with open(errordb) as f: - logging.debug('Sucking in error database') - errors = {} - for e in f.readlines(): - if e[0] == '#' or len(e) == 1: - continue - e = e.split('\\') - errors[e[0]] = { 'message': e[1], 'explanation': e[2] } -except: - logging.error('Cannot open database for language %s' % errordb) - exit() +errors = read_db('errors', args.language[0]) +problems = read_db('problems', args.language[0]) for file in args.filenames: do_rclint(file)