From 371c16c2d58d354c8a9e70c60b6e8d120e82c7d2 Mon Sep 17 00:00:00 2001
From: James Adams <james.adams@stfc.ac.uk>
Date: Mon, 21 Jun 2021 11:16:12 +0100
Subject: [PATCH] Add check for features being included from outside their tree

---
 panc/src/main/scripts/panlint/panlint.py | 32 ++++++++++++++++++++++++
 panc/src/main/scripts/panlint/tests.py   | 13 ++++++++++
 2 files changed, 45 insertions(+)

diff --git a/panc/src/main/scripts/panlint/panlint.py b/panc/src/main/scripts/panlint/panlint.py
index 0e2dc7ff..611a2884 100755
--- a/panc/src/main/scripts/panlint/panlint.py
+++ b/panc/src/main/scripts/panlint/panlint.py
@@ -23,6 +23,7 @@
 from glob import glob
 from sys import stdout, exit as sys_exit
 from inspect import getmembers, isfunction
+from os.path import dirname
 import six
 from colorama import Fore, Style, init as colorama_init
 from prettytable import PrettyTable
@@ -81,6 +82,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
@@ -189,6 +193,28 @@ def whitespace_around_operators(line, string_ranges):
 
         return line
 
+    def features_standalone(self, 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"""
+        passed = True
+        diagnosis = ''
+        message = ''
+
+        if line.filename.startswith('features/'):
+            includes = RE_FEATURE_INCLUDE.finditer(line.text)
+            for include in includes:
+                this_file = line.filename
+                this_dir = dirname(this_file)
+                incl_file = include.group('name')
+                incl_dir = dirname(incl_file)
+                passed = passed and incl_dir.startswith(this_dir)
+                diagnosis = diagnose(*include.span('name'))
+
+            if not passed:
+                message = 'Feature template includes a template which is not a child of the feature'
+
+        return (passed, diagnosis, message)
+
 
 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"""
@@ -496,6 +522,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')
@@ -518,6 +546,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)
diff --git a/panc/src/main/scripts/panlint/tests.py b/panc/src/main/scripts/panlint/tests.py
index 5a66c9d3..ecdfd45b 100755
--- a/panc/src/main/scripts/panlint/tests.py
+++ b/panc/src/main/scripts/panlint/tests.py
@@ -533,6 +533,19 @@ def test_print_report(self):
             panlint.print_report(test_line)
             self.assertEqual(mock_stdout.getvalue(), expected_output)
 
+    def test_feature_child_inclusion(self):
+        lc = panlint.LineChecks()
+        passed, _, message = lc.feature_child_inclusion(
+            panlint.Line('features/test/foo/config.pan', 3, "include 'features/test/foo/service/users';"),
+            [],
+        );
+        self.assertTrue(passed);
+        passed, _, message = lc.feature_child_inclusion(
+            panlint.Line('features/test/foo/config.pan', 3, "include 'features/test/bar/webserver';"),
+            [],
+        );
+        self.assertFalse(passed);
+
 
 if __name__ == '__main__':
     unittest.main()