panlint: Add class to model messages more richly

- Update tests to handle new message format
- Drop severity of uppercase global variable check
- Add test to check that message IDs have a consistent format and are unique
- Fix _assert_lint_line docstring for messages being objects

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'''('.*?'|".*?"(?<!\\"))''')
@@ -88,29 +105,64 @@ def diagnose(self):
 
 LINE_LENGTH_LIMIT = 120
 
+SEV_ADVICE = 1
+SEV_WARNING = 2
+SEV_ERROR = 3
+
+SEVERITY_TEXT = {
+    SEV_ADVICE: 'Advice',
+    SEV_WARNING: 'Warning',
+    SEV_ERROR: 'Error',
+}
+
+SEVERITY_VALUE_MAP = dict(zip(SEVERITY_TEXT.values(), SEVERITY_TEXT.keys()))
+
 # Simple regular-expression based checks that will be performed against all non-ignored lines
 # Every pattern must provide a single capturing group named "error"
 LINE_PATTERNS = {
-    "Indentation should be a multiple of four spaces": re.compile(r'^(?!( {4})*(\S|$))(?P<error>\s*)'),
-    "Spaces should be used instead of tabs": re.compile(r'(?P<error>\t+)'),
-    "Trailing whitespace": re.compile(r'(?P<error>\s+$)'),
-    "Use dicts instead of nlists": re.compile(r'\b(?P<error>(?:is_)?nlist)\s*\('),
-    "Include statements no longer need curly braces": re.compile(r'''include\s+(?P<error>{[^;]+})'''),
-    "Line is longer than %s characters" % LINE_LENGTH_LIMIT:
-    re.compile(r'''^(?!bind ).{0,%s}(?P<error>.*?)$''' % LINE_LENGTH_LIMIT),
-    "Commas should be followed by exactly one space": re.compile(r'(?P<error>,(?:\S|\s{2,}))'),
-    "Whitespace before semicolon": re.compile(r'(?P<error>\s+;)'),
-    "Semicolons should be followed exactly one space or end-of-line": re.compile(r';(?P<error>(?:\S|\s{2,}))'),
-    "Global variables should be uppercase": re.compile(r'variable\s+(?P<error>[\w]+)(?<=\S[a-z]\S)'),
-    "Global variables should be five or more characters": re.compile(r'variable\s+(?P<error>\w{1,4})\b'),
-    "Redundant use of format within error or debug call": re.compile(r'(?:error|debug)\s*\(\s*(?P<error>format)\s*\('),
+    Message("LP001", SEV_ADVICE, "Indentation should be a multiple of four spaces"):
+        re.compile(r'^(?!( {4})*(\S|$))(?P<error>\s*)'),
+
+    Message("LP002", SEV_ADVICE, "Spaces should be used instead of tabs"):
+        re.compile(r'(?P<error>\t+)'),
+
+    Message("LP003", SEV_ADVICE, "Trailing whitespace"):
+        re.compile(r'(?P<error>\s+$)'),
+
+    Message("LP004", SEV_WARNING, "Use dicts instead of nlists"):
+        re.compile(r'\b(?P<error>(?:is_)?nlist)\s*\('),
+
+    Message("LP005", SEV_ADVICE, "Include statements no longer need curly braces"):
+        re.compile(r'''include\s+(?P<error>{[^;]+})'''),
+
+    Message("LP006", SEV_ADVICE, f"Line is longer than {LINE_LENGTH_LIMIT} characters"):
+        re.compile(r'''^(?!bind ).{0,%s}(?P<error>.*?)$''' % LINE_LENGTH_LIMIT),
+
+    Message("LP007", SEV_ADVICE, "Commas should be followed by exactly one space"):
+        re.compile(r'(?P<error>,(?:\S|\s{2,}))'),
+
+    Message("LP008", SEV_ADVICE, "Whitespace before semicolon"):
+        re.compile(r'(?P<error>\s+;)'),
+
+    Message("LP009", SEV_ADVICE, "Semicolons should be followed exactly one space or end-of-line"):
+        re.compile(r';(?P<error>(?:\S|\s{2,}))'),
+
+    Message("LP010", SEV_ADVICE, "Global variables should be uppercase"):
+        re.compile(r'variable\s+(?P<error>[\w]+)(?<=\S[a-z]\S)'),
+
+    Message("LP011", SEV_ADVICE, "Global variables should be five or more characters"):
+        re.compile(r'variable\s+(?P<error>\w{1,4})\b'),
+
+    Message("LP012", SEV_WARNING, "Redundant use of format within error or debug call"):
+        re.compile(r'(?:error|debug)\s*\(\s*(?P<error>format)\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<error>/+)$'''),
+    Message("PP001", SEV_ADVICE, "Unnecessary trailing slash at end of profile path"):
+        re.compile(r'''.(?P<error>/+)$''')
 }
 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])))
@@ -391,7 +444,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 +507,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)

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,19 +55,36 @@ 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)
 
         # 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([
             '',