diff --git a/panc/src/main/scripts/panlint/panlint.py b/panc/src/main/scripts/panlint/panlint.py index 1610fa83..87efd9b9 100755 --- a/panc/src/main/scripts/panlint/panlint.py +++ b/panc/src/main/scripts/panlint/panlint.py @@ -49,11 +49,11 @@ def __init__(self, start, end, message): Parameters: start (int): Position of the first character of the problem end (int): Position of the last character of the problem - message (str): A description of the problem + message (Message): A Message object describing the problem """ self.start = int(start) self.end = int(end) - self.message = str(message) + self.message = message def diagnose(self): """Format a line of diagnosis markers from a range of character positions @@ -63,6 +63,23 @@ def diagnose(self): return (' ' * self.start) + ('^' * (self.end - self.start)) +class Message: + """A class representing a message describing one issue with a line of code.""" + def __init__(self, message_id, severity, text): + """ + Parameters: + id (str): A unique five character identifier for the specific issue + severity (int): How serious the issue is from 1-3 (least serious - most serious) + text (str): A description of what the issue is + """ + self.id = str(message_id) + self.severity = int(severity) + self.text = str(text) + + def __str__(self): + return '%s: %s' % (SEVERITY_TEXT[self.severity], self.text) + + RS_COMMENT = r'(?:#|@{.*?})' RE_STRING = re.compile(r'''('.*?'|".*?"(?\s*)'), - "Spaces should be used instead of tabs": re.compile(r'(?P\t+)'), - "Trailing whitespace": re.compile(r'(?P\s+$)'), - "Use dicts instead of nlists": re.compile(r'\b(?P(?:is_)?nlist)\s*\('), - "Include statements no longer need curly braces": re.compile(r'''include\s+(?P{[^;]+})'''), - "Line is longer than %s characters" % LINE_LENGTH_LIMIT: - re.compile(r'''^(?!bind ).{0,%s}(?P.*?)$''' % LINE_LENGTH_LIMIT), - "Commas should be followed by exactly one space": re.compile(r'(?P,(?:\S|\s{2,}))'), - "Whitespace before semicolon": re.compile(r'(?P\s+;)'), - "Semicolons should be followed exactly one space or end-of-line": re.compile(r';(?P(?:\S|\s{2,}))'), - "Global variables should be uppercase": re.compile(r'variable\s+(?P[\w]+)(?<=\S[a-z]\S)'), - "Global variables should be five or more characters": re.compile(r'variable\s+(?P\w{1,4})\b'), - "Redundant use of format within error or debug call": re.compile(r'(?:error|debug)\s*\(\s*(?Pformat)\s*\('), + Message("LP001", SEV_ADVICE, "Indentation should be a multiple of four spaces"): + re.compile(r'^(?!( {4})*(\S|$))(?P\s*)'), + + Message("LP002", SEV_ADVICE, "Spaces should be used instead of tabs"): + re.compile(r'(?P\t+)'), + + Message("LP003", SEV_ADVICE, "Trailing whitespace"): + re.compile(r'(?P\s+$)'), + + Message("LP004", SEV_WARNING, "Use dicts instead of nlists"): + re.compile(r'\b(?P(?:is_)?nlist)\s*\('), + + Message("LP005", SEV_ADVICE, "Include statements no longer need curly braces"): + re.compile(r'''include\s+(?P{[^;]+})'''), + + Message("LP006", SEV_ADVICE, f"Line is longer than {LINE_LENGTH_LIMIT} characters"): + re.compile(r'''^(?!bind ).{0,%s}(?P.*?)$''' % LINE_LENGTH_LIMIT), + + Message("LP007", SEV_ADVICE, "Commas should be followed by exactly one space"): + re.compile(r'(?P,(?:\S|\s{2,}))'), + + Message("LP008", SEV_ADVICE, "Whitespace before semicolon"): + re.compile(r'(?P\s+;)'), + + Message("LP009", SEV_ADVICE, "Semicolons should be followed exactly one space or end-of-line"): + re.compile(r';(?P(?:\S|\s{2,}))'), + + Message("LP010", SEV_ADVICE, "Global variables should be uppercase"): + re.compile(r'variable\s+(?P[\w]+)(?<=\S[a-z]\S)'), + + Message("LP011", SEV_ADVICE, "Global variables should be five or more characters"): + re.compile(r'variable\s+(?P\w{1,4})\b'), + + Message("LP012", SEV_WARNING, "Redundant use of format within error or debug call"): + re.compile(r'(?:error|debug)\s*\(\s*(?Pformat)\s*\('), } # Simple regular-expression based checks that will be performed against # all strings on non-ignored lines that appear to be profile paths # Every pattern must provide a single capturing group named "error" PATH_PATTERNS = { - "Unnecessary trailing slash at end of profile path": re.compile(r'''.(?P/+)$'''), + Message("PP001", SEV_ADVICE, "Unnecessary trailing slash at end of profile path"): + re.compile(r'''.(?P/+)$''') } TAB_ARROW = u'\u2192' @@ -189,7 +241,8 @@ def whitespace_around_operators(line, string_ranges): if not valid: debug_range(start, end, 'WS Operator', True) - line.problems.append(Problem(start, end, message_text)) + message = Message('LC001', SEV_ADVICE, message_text) + line.problems.append(Problem(start, end, message)) return line @@ -310,7 +363,7 @@ def diagnose(start, end): def print_report(line, vi=False): """Print a full report of all problems found with a single line of a processed file""" print(u'') - messages = ', '.join(set([p.message for p in line.problems])) + messages = ', '.join(set([p.message.text for p in line.problems])) print(print_fileinfo(line.filename, line.number, messages, vi=vi)) print(print_line(line.text)) print(print_diagnosis(merge_diagnoses([p.diagnose() for p in line.problems]))) @@ -332,12 +385,15 @@ def get_string_ranges(line): def filestats_table(problem_stats): """Return a formatted table of problem counts per file""" - t = PrettyTable(['Filename', 'Problems']) + t = PrettyTable(['Filename'] + list(SEVERITY_VALUE_MAP) + ['Total']) t.align['Filename'] = 'l' - t.align['Problems'] = 'r' - for filename, problem_count in problem_stats.iteritems(): - if problem_count > 0: - t.add_row([filename, problem_count]) + t.align['Total'] = 'r' + for severity in SEVERITY_VALUE_MAP: + t.align[severity] = 'r' + + for filename, stats in problem_stats.items(): + if stats: + t.add_row([filename] + stats + [sum(stats)]) t.sortby = 'Filename' return t @@ -391,7 +447,8 @@ def check_line_component_use(line, components_included): if match.group('name') not in components_included: start, end = match.span('name') debug_range(start, end, 'ComponentUse', True) - message = 'Component %s in use, but component config has not been included' % match.group('name') + message_text = 'Component %s in use, but component config has not been included' % match.group('name') + message = Message('CU001', SEV_WARNING, message_text) problems.append(Problem(start, end, message)) return problems @@ -453,7 +510,11 @@ def lint_line(line, components_included, first_line=False, allow_mvn_templates=F if not RE_FIRST_LINE.match(line.text): if not (RE_MVN_TEMPLATE.match(line.text) and allow_mvn_templates): line.problems.append( - Problem(0, len(line.text), 'First non-comment line must be the template type and name') + Problem(0, len(line.text), Message( + 'FL001', + SEV_ERROR, + 'First non-comment line must be the template type and name', + )) ) else: string_ranges = get_string_ranges(line) @@ -545,6 +606,7 @@ def main(): problem_count = 0 problem_lines = [] problem_stats = {} + problem_max_severity = 0 if not args.paths: print('No files were provided, not doing anything') @@ -565,6 +627,17 @@ def main(): problem_count += file_problem_count problem_stats[filename] = file_problem_count + file_severities = [0] + for line in file_problem_lines: + for problem in line.problems: + file_severities.append(problem.message.severity) + + problem_stats[filename] = [] + for value in SEVERITY_TEXT: + problem_stats[filename].append(file_severities.count(value)) + + problem_max_severity = max(problem_max_severity, max(file_severities)) + for line in problem_lines: print_report(line, vi=args.vi) diff --git a/panc/src/main/scripts/panlint/tests.py b/panc/src/main/scripts/panlint/tests.py index 794e1ec4..c0e9d516 100755 --- a/panc/src/main/scripts/panlint/tests.py +++ b/panc/src/main/scripts/panlint/tests.py @@ -37,7 +37,7 @@ def _assert_lint_line(self, line, diagnoses, messages, problems, first_line=Fals Parameters: line (panlint.Line): Line of source code to be linted diagnoses (list of str): Expected lines of diagnosis markers - messages (list of str): Expected problem descriptions + messages (list of Message): Expected problem descriptions problems (int): Expected number of problems first_line (bool): Whether this line should be considered the first line of a file (defaults to False) """ @@ -55,10 +55,10 @@ def _assert_lint_line(self, line, diagnoses, messages, problems, first_line=Fals # If messages is set to None, ignore the contents and just check that is not an empty set if messages is None: for p in r_line.problems: - self.assertNotEqual(p.message, '') + self.assertNotEqual(p.message.text, '') else: messages.sort() - r_messages = [p.message for p in r_line.problems] + r_messages = [p.message.text for p in r_line.problems] r_messages.sort() for m1, m2 in zip(messages, r_messages): self.assertEqual(m1, m2) @@ -66,8 +66,25 @@ def _assert_lint_line(self, line, diagnoses, messages, problems, first_line=Fals # first_line must ALWAYS be False when returned self.assertEqual(r_first_line, False) + def test_message_ids(self): + line_pattern_ids = [m.id for m in panlint.LINE_PATTERNS.keys()] + line_pattern_ids.sort() + for p in line_pattern_ids: + self.assertRegex(p, r'LP\d{3}', f'Format of line pattern message ID {p}') + + path_pattern_ids = [m.id for m in panlint.PATH_PATTERNS.keys()] + path_pattern_ids.sort() + for p in path_pattern_ids: + self.assertRegex(p, r'PP\d{3}', f'Format of path pattern message ID {p}') + + all_pattern_ids = line_pattern_ids + path_pattern_ids + all_pattern_ids.sort() + unique_pattern_ids = list(set(all_pattern_ids)) + unique_pattern_ids.sort() + self.assertEqual(unique_pattern_ids, all_pattern_ids) + def test_diagnose(self): - dummy_message = '' + dummy_message = panlint.Message('', 0, '') self.assertEqual(panlint.Problem(0, 0, dummy_message).diagnose(), '') self.assertEqual(panlint.Problem(0, 4, dummy_message).diagnose(), '^^^^') self.assertEqual(panlint.Problem(2, 8, dummy_message).diagnose(), ' ^^^^^^') @@ -196,7 +213,7 @@ def test_whitespace_around_operators(self): for bad_line, bad_message, bad_diag in bad_tests: result = lc.whitespace_around_operators(bad_line, []) self.assertEqual(len(result.problems), 1) - self.assertEqual(result.problems[0].message, bad_message) + self.assertEqual(result.problems[0].message.text, bad_message) self.assertEqual(result.problems[0].diagnose(), bad_diag) # Handling lines that start or end with an operator (i.e. are part of a multi-line expression) should be allowed @@ -494,7 +511,7 @@ def test_check_line_patterns(self): self.assertIsInstance(problems, list) for p in problems: self.assertIsInstance(p, panlint.Problem) - self.assertEqual(set([p.message for p in problems]), messages) + self.assertEqual(set([p.message.text for p in problems]), messages) def test_check_line_paths(self): problems = panlint.check_line_paths( @@ -519,7 +536,7 @@ def test_check_line_methods(self): def test_print_report(self): test_line = panlint.Line('fake.pan', 7, "This is a FAKE line") - test_line.problems = [panlint.Problem(10, 14, u'Everything is Fine')] + test_line.problems = [panlint.Problem(10, 14, panlint.Message('FM000', 1, u'Everything is Fine'))] expected_output = '\n'.join([ '',