Skip to content

Commit

Permalink
panlint: Add class to model messages more richly
Browse files Browse the repository at this point in the history
- Update tests to handle new message format
- Drop severity of uppercase global variable check
  • Loading branch information
jrha committed Dec 9, 2024
1 parent 49acca6 commit b570b22
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 25 deletions.
91 changes: 71 additions & 20 deletions panc/src/main/scripts/panlint/panlint.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -63,6 +63,16 @@ def diagnose(self):
return (' ' * self.start) + ('^' * (self.end - self.start))


class Message(object):
def __init__(self, message_id, severity, text):
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'''('.*?'|".*?"(?<!\\"))''')
Expand All @@ -88,29 +98,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 = 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, "Line is longer than %s characters" % LINE_LENGTH_LIMIT):
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'

Expand Down Expand Up @@ -189,7 +234,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

Expand Down Expand Up @@ -310,7 +356,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])))
Expand Down Expand Up @@ -391,7 +437,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

Expand Down Expand Up @@ -453,7 +500,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)
Expand Down
10 changes: 5 additions & 5 deletions panc/src/main/scripts/panlint/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -196,7 +196,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
Expand Down Expand Up @@ -494,7 +494,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(
Expand All @@ -519,7 +519,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([
'',
Expand Down

0 comments on commit b570b22

Please sign in to comment.