pw_compilation_testing: Relax PW_NC_EXPECT syntax; add tests

Allow PW_NC_EXPECT() statements to span multiple lines. The statements
must contain only string literals or //-style comments. This prevents
clang-format from breaking PW_NC_EXPECT() statements when it reflows
string literals.

With this change, statements like the following are permitted:

  PW_NC_EXPECT(  // This is a comment
               "This is a \"very\" long string that didn't "  // Comment
               "fit on one line,\nokay?");

Also added tests for negative compilation test parsing.

Change-Id: Ie7ad157c817eaf7f0f576bcd841f00426623b635
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/107678
Reviewed-by: Armando Montanez <[email protected]>
Commit-Queue: Auto-Submit <[email protected]>
Pigweed-Auto-Submit: Wyatt Hepler <[email protected]>

pw_compilation_testing/docs.rst

@@ -94,11 +94,11 @@ Creating a negative compilation test
   line.
 - Execute the tests by running the build.
 
-To simplify parsing, all ``PW_NC_TEST()`` and ``PW_NC_EXPECT()`` statements
-must fit within a single line. If they are too long for one line, disable
-automatic formatting around them with ``// clang-format disable`` or split
-``PW_NC_EXPECT()`` statements into multiple statements on separate lines. This
-restriction may be relaxed in the future.
+To simplify parsing, all ``PW_NC_TEST()`` statements must fit on a single line.
+``PW_NC_EXPECT()`` statements may span multiple lines, but must contain a single
+regular expression as a string literal. The string may be comprised of multiple
+implicitly concatenated string literals. The ``PW_NC_EXPECT()`` statement cannot
+contain anything else except for ``//``-style comments.
 
 Test assertions
 ===============

pw_compilation_testing/py/BUILD.gn

@@ -27,6 +27,7 @@ pw_python_package("py") {
     "pw_compilation_testing/generator.py",
     "pw_compilation_testing/runner.py",
   ]
+  tests = [ "generator_test.py" ]
   python_deps = [ "$dir_pw_cli/py" ]
   pylintrc = "$dir_pigweed/.pylintrc"
 }

pw_compilation_testing/py/generator_test.py

@@ -0,0 +1,107 @@
+# Copyright 2022 The Pigweed Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may not
+# use this file except in compliance with the License. You may obtain a copy of
+# the License at
+#
+#     https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations under
+# the License.
+"""Tests for the negative compilation test generator."""
+
+from pathlib import Path
+import re
+import tempfile
+import unittest
+
+from pw_compilation_testing.generator import (Compiler, Expectation,
+                                              ParseError, TestCase,
+                                              enumerate_tests)
+
+SOURCE = r'''
+#if PW_NC_TEST(FirstTest)
+PW_NC_EXPECT("abcdef");
+
+SomeSourceCode();
+
+#endif  // PW_NC_TEST
+
+#if PW_NC_TEST(SecondTest)
+PW_NC_EXPECT("\"\"abc123"    // Include " and other escapes in the string
+             "def'456\""
+             // Goodbye
+             "ghi\n\t789"  // ???
+); // abc
+
+#endif  // PW_NC_TEST
+'''
+
+ILLEGAL_COMMENT = '''
+#if PW_NC_TEST(FirstTest)
+PW_NC_EXPECT("abcdef" /* illegal comment */);
+
+#endif  // PW_NC_TEST
+'''
+
+UNTERMINATED_EXPECTATION = '#if PW_NC_TEST(FirstTest)\nPW_NC_EXPECT("abcdef"\n'
+
+
+def _write_to_temp_file(contents: str) -> Path:
+    file = tempfile.NamedTemporaryFile('w', delete=False)
+    file.write(contents)
+    file.close()
+    return Path(file.name)
+
+
+class ParserTest(unittest.TestCase):
+    """Tests parsing negative compilation tests from a file."""
+    def test_successful(self) -> None:
+        try:
+            path = _write_to_temp_file(SOURCE)
+
+            self.assertEqual([
+                TestCase(
+                    'TestSuite',
+                    'FirstTest',
+                    (Expectation(Compiler.ANY, re.compile('abcdef'), 3), ),
+                    path,
+                    2,
+                ),
+                TestCase(
+                    'TestSuite',
+                    'SecondTest',
+                    (Expectation(Compiler.ANY,
+                                 re.compile('""abc123def\'456"ghi\\n\\t789'),
+                                 10), ),
+                    path,
+                    9,
+                )
+            ], list(enumerate_tests('TestSuite', [path])))
+        finally:
+            path.unlink()
+
+    def test_illegal_comment(self) -> None:
+        try:
+            path = _write_to_temp_file(ILLEGAL_COMMENT)
+            with self.assertRaises(ParseError):
+                list(enumerate_tests('TestSuite', [path]))
+        finally:
+            path.unlink()
+
+    def test_unterminated_expectation(self) -> None:
+        try:
+            path = _write_to_temp_file(UNTERMINATED_EXPECTATION)
+            with self.assertRaises(ParseError) as err:
+                list(enumerate_tests('TestSuite', [path]))
+        finally:
+            path.unlink()
+
+        self.assertIn('Unterminated', str(err.exception))
+
+
+if __name__ == '__main__':
+    unittest.main()

pw_compilation_testing/py/pw_compilation_testing/generator.py

@@ -30,24 +30,24 @@
 import pickle
 import re
 import sys
-from typing import (Iterable, Iterator, List, NamedTuple, NoReturn, Pattern,
-                    Sequence, Set, Tuple)
+from typing import (Iterable, Iterator, List, NamedTuple, NoReturn, Optional,
+                    Pattern, Sequence, Set, Tuple)
 
 # Matches the #if or #elif statement that starts a compile fail test.
-_TEST_START = re.compile(br'^[ \t]*#[ \t]*(?:el)?if[ \t]+PW_NC_TEST\([ \t]*')
+_TEST_START = re.compile(r'^[ \t]*#[ \t]*(?:el)?if[ \t]+PW_NC_TEST\([ \t]*')
 
 # Matches the name of a test case.
 _TEST_NAME = re.compile(
-    br'(?P<name>[a-zA-Z0-9_]+)[ \t]*\)[ \t]*(?://.*|/\*.*)?$')
+    r'(?P<name>[a-zA-Z0-9_]+)[ \t]*\)[ \t]*(?://.*|/\*.*)?$')
 
 # Negative compilation test commands take the form PW_NC_EXPECT("regex"),
 # PW_NC_EXPECT_GCC("regex"), or PW_NC_EXPECT_CLANG("regex"). PW_NC_EXPECT() is
 # an error.
-_EXPECT_START = re.compile(br'^[ \t]*PW_NC_EXPECT(?P<compiler>_GCC|_CLANG)?\(')
+_EXPECT_START = re.compile(r'^[ \t]*PW_NC_EXPECT(?P<compiler>_GCC|_CLANG)?\(')
 
 # EXPECT statements are regular expressions that must match the compiler output.
 # They must fit on a single line.
-_EXPECT_REGEX = re.compile(br'(?P<regex>"[^\n]+")\);[ \t]*(?://.*|/\*.*)?$')
+_EXPECT_REGEX = re.compile(r'(?P<regex>"[^\n]+")\);[ \t]*(?://.*|/\*.*)?$')
 
 
 class Compiler(Enum):
@@ -99,60 +99,139 @@ def deserialize(cls, serialized: str) -> 'Expectation':
 
 class ParseError(Exception):
     """Failed to parse a PW_NC_TEST."""
-    def __init__(self, message: str, file: Path, lines: Sequence[bytes],
+    def __init__(self, message: str, file: Path, lines: Sequence[str],
                  error_lines: Sequence[int]) -> None:
         for i in error_lines:
-            message += (
-                f'\n{file.name}:{i + 1}: {lines[i].decode(errors="replace")}')
+            message += f'\n{file.name}:{i + 1}: {lines[i]}'
         super().__init__(message)
 
 
+class _ExpectationParser:
+    """Parses expecatations from 'PW_NC_EXPECT(' to the final ');'."""
+    class _State:
+        SPACE = 0  # Space characters, which are ignored
+        COMMENT_START = 1  # First / in a //-style comment
+        COMMENT = 2  # Everything after // on a line
+        OPEN_QUOTE = 3  # Starting quote for a string literal
+        CHARACTERS = 4  # Characters within a string literal
+        ESCAPE = 5  # \ within a string literal, which may escape a "
+        CLOSE_PAREN = 6  # Closing parenthesis to the PW_NC_EXPECT statement.
+
+    def __init__(self, index: int, compiler: Compiler) -> None:
+        self.index = index
+        self._compiler = compiler
+        self._state = self._State.SPACE
+        self._contents: List[str] = []
+
+    def parse(self, chars: str) -> Optional[Expectation]:
+        """State machine that parses characters in PW_NC_EXPECT()."""
+        for char in chars:
+            if self._state is self._State.SPACE:
+                if char == '"':
+                    self._state = self._State.CHARACTERS
+                elif char == ')':
+                    self._state = self._State.CLOSE_PAREN
+                elif char == '/':
+                    self._state = self._State.COMMENT_START
+                elif not char.isspace():
+                    raise ValueError(f'Unexpected character "{char}"')
+            elif self._state is self._State.COMMENT_START:
+                if char == '*':
+                    raise ValueError(
+                        '"/* */" comments are not supported; use // instead')
+                if char != '/':
+                    raise ValueError(f'Unexpected character "{char}"')
+                self._state = self._State.COMMENT
+            elif self._state is self._State.COMMENT:
+                if char == '\n':
+                    self._state = self._State.SPACE
+            elif self._state is self._State.CHARACTERS:
+                if char == '"':
+                    self._state = self._State.SPACE
+                elif char == '\\':
+                    self._state = self._State.ESCAPE
+                else:
+                    self._contents.append(char)
+            elif self._state is self._State.ESCAPE:
+                # Include escaped " directly. Restore the \ for other chars.
+                if char != '"':
+                    self._contents.append('\\')
+                self._contents.append(char)
+                self._state = self._State.CHARACTERS
+            elif self._state is self._State.CLOSE_PAREN:
+                if char != ';':
+                    raise ValueError(f'Expected ";", found "{char}"')
+
+                return self._expectation(''.join(self._contents))
+
+        return None
+
+    def _expectation(self, regex: str) -> Expectation:
+        if '"""' in regex:
+            raise ValueError('The regular expression cannot contain """')
+
+        # Evaluate the string from the C++ source as a raw literal.
+        re_string = eval(f'r"""{regex}"""')  # pylint: disable=eval-used
+        if not isinstance(re_string, str):
+            raise ValueError('The regular expression must be a string!')
+
+        try:
+            return Expectation(self._compiler, re.compile(re_string),
+                               self.index + 1)
+        except re.error as error:
+            raise ValueError('Invalid regular expression: ' + error.msg)
+
+
 class _NegativeCompilationTestSource:
     def __init__(self, file: Path) -> None:
         self._file = file
-        self._lines = self._file.read_bytes().splitlines()
+        self._lines = self._file.read_text().splitlines(keepends=True)
 
         self._parsed_expectations: Set[int] = set()
 
     def _error(self, message: str, *error_lines: int) -> NoReturn:
         raise ParseError(message, self._file, self._lines, error_lines)
 
     def _parse_expectations(self, start: int) -> Iterator[Expectation]:
+        expectation: Optional[_ExpectationParser] = None
+
         for index in range(start, len(self._lines)):
             line = self._lines[index]
 
-            expect_match = _EXPECT_START.match(line)
-
-            if not expect_match:
-                # Skip empty lines or lines with comments
-                if not line or line.isspace() or line.lstrip().startswith(
-                    (b'//', b'/*')):
-                    continue
-                break
+            # Skip empty or comment lines
+            if not line or line.isspace() or line.lstrip().startswith('//'):
+                continue
 
-            compiler_str = expect_match['compiler'] or b'ANY'
-            compiler = Compiler[compiler_str.lstrip(b'_').decode()]
+            # Look for a 'PW_NC_EXPECT(' in the code.
+            if not expectation:
+                expect_match = _EXPECT_START.match(line)
+                if not expect_match:
+                    break  # No expectation found, stop processing.
 
-            self._parsed_expectations.add(index)
+                compiler = expect_match['compiler'] or 'ANY'
+                expectation = _ExpectationParser(
+                    index, Compiler[compiler.lstrip('_')])
 
-            regex_match = _EXPECT_REGEX.match(line, expect_match.end())
-            if not regex_match:
-                self._error(
-                    'Failed to parse PW_EXPECT_NC() statement. PW_EXPECT_NC() '
-                    'statements must fit on one line and contain a single, '
-                    'non-empty string literal.', index)
+                self._parsed_expectations.add(index)
 
-            # Evaluate the string from the C++ source as a raw literal.
-            re_string = eval(b'r' + regex_match['regex'])  # pylint: disable=eval-used
-            if not isinstance(re_string, str):
-                self._error('The regular expression must be a string!', index)
+                # Remove the 'PW_NC_EXPECT(' so the line starts with the regex.
+                line = line[expect_match.end():]
 
+            # Find the regex after previously finding 'PW_NC_EXPECT('.
             try:
-                yield Expectation(compiler, re.compile(re_string), index + 1)
-            except re.error as error:
-                self._error('Invalid regular expression: ' + error.msg, index)
+                if parsed_expectation := expectation.parse(line.lstrip()):
+                    yield parsed_expectation
+
+                    expectation = None
+            except ValueError as err:
+                self._error(
+                    f'Failed to parse PW_NC_EXPECT() statement: {err}. '
+                    'PW_NC_EXPECT() must contain only a string literal and '
+                    '//-style comments.', index)
 
-            start = regex_match.end()
+        if expectation:
+            self._error('Unterminated PW_NC_EXPECT() statement!',
+                        expectation.index)
 
     def _check_for_stray_expectations(self) -> None:
         all_expectations = frozenset(i for i in range(len(self._lines))
@@ -173,13 +252,12 @@ def parse(self, suite: str) -> Iterator[TestCase]:
             if not name_match:
                 self._error(
                     'Negative compilation test syntax error. '
-                    'Expected test name, found '
-                    f"'{line[case_match.end():].decode(errors='replace')}'",
+                    f"Expected test name, found '{line[case_match.end():]}'",
                     index)
 
             expectations = tuple(self._parse_expectations(index + 1))
-            yield TestCase(suite, name_match['name'].decode(), expectations,
-                           self._file, index + 1)
+            yield TestCase(suite, name_match['name'], expectations, self._file,
+                           index + 1)
 
         self._check_for_stray_expectations()