enhance easyconfigs style check to also check grouping & order of easyconfig parameters (WIP)

easybuild/framework/easyconfig/style.py

@@ -32,7 +32,8 @@
 import sys
 from vsc.utils import fancylogger
 
-from easybuild.tools.build_log import print_msg
+from easybuild.framework.easyconfig.default import DEFAULT_CONFIG
+from easybuild.tools.build_log import EasyBuildError, print_msg
 from easybuild.tools.utilities import only_if_module_is_available
 
 try:
@@ -50,10 +51,20 @@
 
 EB_CHECK = '_eb_check_'
 
+BLANK_LINE_REGEX = re.compile(r'^\s*$')
 COMMENT_REGEX = re.compile(r'^\s*#')
 PARAM_DEF_REGEX = re.compile(r"^(?P<key>[a-z_]+)\s*=\s*")
 
 
+PARAM_GROUPS_HEAD = [
+    ((), ('easyblock',)),
+    (('name', 'version'), ()),
+    (('homepage', 'description'), ()),
+    (('toolchain',), ('toolchainopts',)),
+    (('source_urls', 'sources'), ('patches', 'checksums')),
+]
+
+
 # Any function starting with _eb_check_ (see EB_CHECK variable) will be
 # added to the tests if the test number is added to the select list.
 #
@@ -68,6 +79,104 @@
 # https://pycodestyle.readthedocs.io or more specifically:
 # https://pycodestyle.readthedocs.io/en/latest/developer.html#contribute
 
+
+def _check_param_group(params, done_params):
+    """
+    Check whether the specified group of parameters conforms to the style guide w.r.t. order & grouping of parameters
+    """
+    result, fail_msgs = None, []
+
+    fail_msgs.extend(_check_param_group_head(params, done_params))
+
+    if fail_msgs:
+        result = (0, "W001 %s" % ', '.join(fail_msgs))
+
+    return result
+
+
+def _check_param_group_head(params, done_params):
+    """
+    Check whether the specified group of parameters conforms to the style guide w.r.t. order & grouping of parameters,
+    for head of easyconfig file
+    """
+    fail_msgs = []
+
+    for param_group in PARAM_GROUPS_HEAD:
+        full_param_group = param_group[0] + param_group[1]
+        if any(p in params for p in full_param_group):
+            fail_msgs.extend(_check_specific_param_group_isolation_order(param_group[0], param_group[1], params))
+            done_params.extend(full_param_group)
+
+    # check whether any unexpected parameter definitions are found in the head of the easyconfig file
+    expected_params_head = [p for (pg1, pg2) in PARAM_GROUPS_HEAD for p in pg1 + pg2]
+    unexpected = [p for p in params if p not in expected_params_head]
+    if unexpected:
+        fail_msgs.append("found unexpected parameter definitions in head of easyconfig: %s" % ', '.join(unexpected))
+
+    return fail_msgs
+
+
+def _check_specific_param_group_isolation_order(required_params, optional_params, params):
+    """
+    Check whether provided parameters adher to style of specified parameter group w.r.t. isolation, order, ...
+    """
+    fail_msgs = []
+
+    expected_params = required_params + optional_params
+    is_are = (' is', 's are')[len(expected_params) > 1]
+
+    if any(p not in params for p in required_params):
+        fail_msgs.append("Not all required parameters found in group: %s" % '.'.join(required_params))
+
+    if sorted(params) != sorted(expected_params):
+        fail_msgs.append("%s parameter definition%s not isolated" % ('/'.join(expected_params), is_are))
+
+    if tuple(p for p in params if p in expected_params) != expected_params:
+        fail_msgs.append("%s parameter definition%s out of order" % ('/'.join(expected_params), is_are))
+
+    return fail_msgs
+
+
+def _eb_check_order_grouping_params(physical_line, lines, line_number, total_lines, checker_state):
+    """
+    W001
+    Check order and grouping easyconfig parameter definitions
+    The arguments are explained at
+    https://pep8.readthedocs.org/en/latest/developer.html#contribute
+    """
+    result = None
+
+    # apparently this is not the same as physical_line line?!
+    line = lines[line_number-1]
+
+    # list of parameters that should to be defined already at this point
+    done_params = checker_state.setdefault('eb_done_params', [])
+
+    # list of groups of already defined parameters
+    defined_params = checker_state.setdefault('eb_defined_params', [[]])
+
+    # keep track of order parameter definitions via checker state
+    param_def = PARAM_DEF_REGEX.search(line)
+    if param_def:
+        key = param_def.group('key')
+
+        # include key in last group of parameters;
+        # only if its a known easyconfig parameter, easyconfigs may include local variables
+        if key in DEFAULT_CONFIG:
+            defined_params[-1].append(key)
+
+    # if we're at the end of the file, or if the next line is blank, check this group of parameters
+    if line_number == total_lines or BLANK_LINE_REGEX.match(lines[line_number]):
+        if defined_params[-1]:
+            # check whether last group of parameters is in the expected order
+            result = _check_param_group(defined_params[-1], done_params)
+
+            # blank line starts a new group of parameters
+            defined_params.append([])
+
+    return result
+
+
 def _eb_check_trailing_whitespace(physical_line, lines, line_number, checker_state):  # pylint:disable=unused-argument
     """
     W299

test/framework/style.py

@@ -34,7 +34,8 @@
 from test.framework.utilities import EnhancedTestCase, TestLoaderFiltered
 from unittest import TextTestRunner
 from vsc.utils import fancylogger
-from easybuild.framework.easyconfig.style import _eb_check_trailing_whitespace, check_easyconfigs_style
+from easybuild.framework.easyconfig.style import _eb_check_trailing_whitespace, _eb_check_order_grouping_params
+from easybuild.framework.easyconfig.style import check_easyconfigs_style
 
 try:
     import pycodestyle
@@ -91,6 +92,71 @@ def test_check_trailing_whitespace(self):
             result = _eb_check_trailing_whitespace(line, lines, line_number, state)
             self.assertEqual(result, expected_result)
 
+    def test_check_order_grouping_params_head(self):
+        """Test for check on order/grouping of parameters in head of easyconfig file."""
+
+        def run_test(fail_idx):
+            """Helper function to run the actual test"""
+            state = {}
+            for idx in range(total_lines):
+                # error state should be detected at specified line
+                if idx == fail_idx:
+                    expected = (0, "W001 " + ', '.join(expected_errors))
+                else:
+                    expected = None
+
+                res = _eb_check_order_grouping_params(lines[idx], lines, idx+1, total_lines, state)
+                self.assertEqual(res, expected)
+
+
+        # easyblock shouldn't be in same group as name/version
+        # name/version are out of order
+        keys = ['easyblock', 'version', 'name']
+        lines = ["%s = '...'" % k for k in keys]
+        total_lines = len(lines)
+
+        expected_errors = [
+            "easyblock parameter definition is not isolated",
+            "name/version parameter definitions are not isolated",
+            "name/version parameter definitions are out of order",
+        ]
+
+        # failure should occur at index 2 (3rd line)
+        run_test(2)
+
+        # error state detected as soon as group ends, not necessary at end of file
+        lines.append('')
+        total_lines += 1
+
+        # failure should still occur at index 2 (3rd line)
+        run_test(2)
+
+        # switch name/version lines, inject empty line after easyblock line
+        lines[1], lines[2] = lines[2], lines[1]
+        lines.insert(1, '')
+        total_lines += 1
+
+        # no failures
+        run_test(-1)
+
+        # add homepage/description out of order
+        lines.extend([
+            '',
+            'description = """..."""',
+            'homepage = "..."',
+        ])
+
+        # add unexpected parameter definition in head of easyconfig
+        lines.append("moduleclass = '...'")
+        total_lines = len(lines)
+
+        expected_errors = [
+            "homepage/description parameter definitions are not isolated",
+            "homepage/description parameter definitions are out of order",
+            "found unexpected parameter definitions in head of easyconfig: moduleclass",
+        ]
+        run_test(8)
+
 
 def suite():
     """Return all style tests for easyconfigs."""