use checker_state in trailing whitespace style check + add dedicated test

easybuild/framework/easyconfig/style.py

@@ -43,17 +43,25 @@
 
 EB_CHECK = '_eb_check_'
 
+COMMENT_REGEX = re.compile(r'^\s*#')
+PARAM_DEF_REGEX = re.compile(r"^(?P<key>[a-z_]+)\s*=\s*")
 
-# any function starting with _eb_check_ (see EB_CHECK variable) will be
+
+# 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.
+#
+# Note: only functions that have a first argument named 'physical_line' or 'logical_line'
+# will actually be used!
+#
 # The test number is definied as WXXX and EXXX (for warnings and errors)
 # where XXX is a 3 digit number.
 #
 # It should be mentioned in the docstring as a single word.
 # Read the pep8 docs to understand the arguments of these functions:
 # https://pep8.readthedocs.org or more specifically:
 # https://pep8.readthedocs.org/en/latest/developer.html#contribute
-def _eb_check_trailing_whitespace(physical_line, lines, line_number, total_lines):  # pylint:disable=unused-argument
+
+def _eb_check_trailing_whitespace(physical_line, lines, line_number, checker_state):  # pylint:disable=unused-argument
     """
     W299
     Warn about trailing whitespace, except for the description and comments.
@@ -62,27 +70,25 @@ def _eb_check_trailing_whitespace(physical_line, lines, line_number, total_lines
     The arguments are explained at
     https://pep8.readthedocs.org/en/latest/developer.html#contribute
     """
-    comment_re = re.compile(r'^\s*#')
-    if comment_re.match(physical_line):
+    # apparently this is not the same as physical_line line?!
+    line = lines[line_number-1]
+
+    if COMMENT_REGEX.match(line):
         return None
 
-    result = pep8.trailing_whitespace(physical_line)
+    result = pep8.trailing_whitespace(line)
     if result:
-        result = (result[0], result[1].replace("W291", "W299"))
+        result = (result[0], result[1].replace('W291', 'W299'))
+
+    # keep track of name of last parameter that was defined
+    param_def = PARAM_DEF_REGEX.search(line)
+    if param_def:
+        checker_state['eb_last_key'] = param_def.group('key')
 
     # if the warning is about the multiline string of description
     # we will not issue a warning
-    keys_re = re.compile(r"^(?P<key>[a-z_]+)\s*=\s*")
-
-    # starting from the current line and going to the top,
-    # check that if the first `key = value` that is found, has
-    # key == description, then let the test pass, else return
-    # the result of the general pep8 check.
-    for line in reversed(lines[:line_number]):
-        res = keys_re.match(line)
-        if res and res.group("key") == "description":
-            result = None
-            break
+    if checker_state.get('eb_last_key') == 'description':
+        result = None
 
     return result
 
@@ -112,7 +118,7 @@ def check_easyconfigs_style(easyconfigs, verbose=False):
     # on a line: the default of 79 is too narrow.
     options.max_line_length = 120
     # we ignore some tests
-    # note that W291 has be replaced by our custom W299
+    # note that W291 has been replaced by our custom W299
     options.ignore = (
         'W291',  # replaced by W299
     )

test/framework/style.py

@@ -34,7 +34,7 @@
 from test.framework.utilities import EnhancedTestCase, TestLoaderFiltered
 from unittest import TextTestRunner
 from vsc.utils import fancylogger
-from easybuild.framework.easyconfig.style import check_easyconfigs_style
+from easybuild.framework.easyconfig.style import _eb_check_trailing_whitespace, check_easyconfigs_style
 
 try:
     import pep8
@@ -60,6 +60,33 @@ def test_style_conformance(self):
 
         self.assertEqual(result, 0, "No code style errors (and/or warnings) found.")
 
+    def test_check_trailing_whitespace(self):
+        """Test for trailing whitespace check."""
+        lines = [
+            "name = 'foo'",  # no trailing whitespace
+            "version = '1.2.3'  ",  # trailing whitespace
+            "   ",  # blank line with whitespace included
+            '''description = """start of long description, ''',  # trailing whitespace, but allowed in description
+            ''' continuation of long description ''',  # trailing whitespace, but allowed in continued description
+            ''' end of long description"""''',
+            "moduleclass = 'tools'   ",  # trailing whitespace
+            '',
+        ]
+        line_numbers = range(1, len(lines) + 1)
+        test_cases = [
+            ({}, None),
+            ({}, (17, "W299 trailing whitespace")),
+            ({}, (0, "W293 blank line contains whitespace")),
+            ({}, None),
+            ({'eb_last_key': 'description'}, None),
+            ({'eb_last_key': 'description'}, None),
+            ({'eb_last_key': 'description'}, (21, "W299 trailing whitespace")),
+        ]
+
+        for (line, line_number, (checker_state, expected_result)) in zip(lines, line_numbers, test_cases):
+            result = _eb_check_trailing_whitespace(line, lines, line_number, checker_state)
+            self.assertEqual(result, expected_result)
+
 
 def suite():
     """Return all style tests for easyconfigs."""