Skip to content

Commit

Permalink
Add check that features are standalone
Browse files Browse the repository at this point in the history
i.e. they do not include templates from feature paths outside their tree

Whether this is desirable or not is down to site policy, so this is optional and disabled by default.
  • Loading branch information
jrha committed Dec 4, 2024
1 parent 658cfe3 commit 5932acc
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 1 deletion.
41 changes: 40 additions & 1 deletion panc/src/main/scripts/panlint/panlint.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
from glob import glob
from sys import stdout, exit as sys_exit
from inspect import getmembers, isfunction
from os import sep as os_sep
from os.path import dirname
import six
from colorama import Fore, Style, init as colorama_init
from prettytable import PrettyTable
Expand Down Expand Up @@ -81,6 +83,9 @@ def diagnose(self):
# Detect whether a file is part of the source tree of a component
RE_COMPONENT_SOURCE_FILE = re.compile(r'^/?(?:\S+/)?(?:core/components/|ncm-)(?P<name>\w+)/\S+$')

# Find where templates belonging to features have been included
RE_FEATURE_INCLUDE = re.compile(r'^\s*[^#]?\s*include.*(?P<name>features/\S+\w)', re.M)

LINE_LENGTH_LIMIT = 120

# Simple regular-expression based checks that will be performed against all non-ignored lines
Expand All @@ -107,7 +112,6 @@ def diagnose(self):
PATH_PATTERNS = {
"Unnecessary trailing slash at end of profile path": re.compile(r'''.(?P<error>/+)$'''),
}

TAB_ARROW = u'\u2192'

DEBUG = False
Expand Down Expand Up @@ -189,6 +193,35 @@ def whitespace_around_operators(line, string_ranges):

return line

@staticmethod
def features_standalone(line, _):
"""If the current file looks like a feature, then check a line for includes of feature templates
to ensure that they are children of the current feature"""
message_text = 'Feature template includes a template which is not a child of the feature'

file_path = line.filename.split(os_sep)
try:
file_feature = file_path.index('features')
except ValueError:
# Return immediately if this file isn't part of a feature
return line

if file_feature >= 0:
file_path = file_path[file_feature:]
includes = RE_FEATURE_INCLUDE.finditer(line.text)
for include in includes:
this_file = os_sep.join(file_path)
this_dir = dirname(this_file)

incl_file = include.group('name')
incl_dir = dirname(incl_file)

if not incl_dir.startswith(this_dir):
start, end = include.span('name')
line.problems.append(Problem(start, end, message_text))

return line


def inside_string(i, j, string_ranges):
"""Returns true if the range described by i and j is contained within a range in the list string_ranges"""
Expand Down Expand Up @@ -496,6 +529,8 @@ def main():
help='Always exit cleanly even if problems are found')
parser.add_argument('--ignore-components', type=str,
help='List of component to ignore when checking included components')
parser.add_argument('--features_standalone', action='store_true',
help='Check that features don\'t include other features')
group_output = parser.add_mutually_exclusive_group()
group_output.add_argument('--debug', action='store_true', help='Enable debug output')
group_output.add_argument('--ide', action='store_true', help='Output machine-readable results for use by IDEs')
Expand All @@ -518,6 +553,10 @@ def main():
if args.ignore_components:
ignore_components = args.ignore_components.split(',')

# This check is based on site-specific policy, so it is optional
if not args.features_standalone:
del LineChecks.features_standalone

for path in args.paths:
for filename in glob(path):
file_problem_lines, file_problem_count = lint_file(filename, args.allow_mvn_templates, ignore_components)
Expand Down
22 changes: 22 additions & 0 deletions panc/src/main/scripts/panlint/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,28 @@ def test_print_report(self):
panlint.print_report(test_line)
self.assertEqual(mock_stdout.getvalue(), expected_output)

def test_features_standalone(self):
lc = panlint.LineChecks()

self.assertEqual(
lc.features_standalone(
panlint.Line('./linux/features/test/foo/config.pan', 3, "include 'features/test/foo/service/users';"),
[],
).problems,
[],
)

problems = lc.features_standalone(
panlint.Line('./linux/features/test/foo/config.pan', 3, "include 'features/test/bar/webserver';"),
[],
).problems
self.assertEqual(len(problems), 1)
for p in problems:
self.assertIsInstance(p, panlint.Problem)
self.assertEqual(p.message, 'Feature template includes a template which is not a child of the feature')
self.assertEqual(p.start, 9)
self.assertEqual(p.end, 36)


if __name__ == '__main__':
unittest.main()

0 comments on commit 5932acc

Please sign in to comment.