Merge pull request #17 from AntonLydike/anton/add-empty-capture-warning

core: Warn on empty captures instead of throwing errors

README.md

@@ -1,7 +1,7 @@
 # filecheck - A Python-native clone of LLVMs FileCheck tool
 
-This tries to be as close a clone of LLVMs FileCheck as possible, without going crazy. It currently passes 1530 out of
-1645 (93%) of LLVMs MLIR filecheck tests.
+This tries to be as close a clone of LLVMs FileCheck as possible, without going crazy. It currently passes 1555 out of
+1645 (94.5%) of LLVMs MLIR filecheck tests.
 
 There are some features that are left out for now (e.g.a
 [pseudo-numeric variables](https://llvm.org/docs/CommandGuide/FileCheck.html#filecheck-pseudo-numeric-variables) and
@@ -47,7 +47,7 @@ Here's an overview of all FileCheck features and their implementation status.
   - [ ] `--color` No color support yet
 - **Base Features:**
   - [X] Regex patterns (Bugs: [#7](https://github.com/AntonLydike/filecheck/issues/7), [#9](https://github.com/AntonLydike/filecheck/issues/9))
-  - [X] Captures and Capture Matches (Diverges: [#5](https://github.com/AntonLydike/filecheck/issues/5), Bug: [#11](https://github.com/AntonLydike/filecheck/issues/11))
+  - [X] Captures and Capture Matches (Bug: [#11](https://github.com/AntonLydike/filecheck/issues/11))
   - [X] Numeric Captures
   - [ ] Numeric Substitutions (jesus christ, wtf man)
   - [X] Literal matching (`CHECK{LITERAL}`)
@@ -112,3 +112,8 @@ can be enabled through the environment variable `FILECHECK_FEATURE_ENABLE=...`.
 
 - `MLIR_REGEX_CLS`: Add additional special regex matchers to match MLIR/LLVM constructs:
   - `\V` will match any SSA value name
+
+### Reject Empty Matches
+
+We introduce a new flag called `reject-empty-vars` that throws an error when a capture expression captures an empty
+string.

filecheck/colors.py

@@ -0,0 +1,46 @@
+import sys
+from enum import Flag, auto
+
+COLOR_SUPPORT = hasattr(sys.stdout, "isatty") and sys.stdout.isatty()
+
+
+class FMT(Flag):
+    RED = auto()
+    BLUE = auto()
+    YELLOW = auto()
+    GREEN = auto()
+    ORANGE = auto()
+    BOLD = auto()
+    GRAY = auto()
+    UNDERLINE = auto()
+    RESET = auto()
+
+    def __str__(self) -> str:
+        if not COLOR_SUPPORT:
+            return ""
+        fmt_str: list[str] = []
+
+        if FMT.RED in self:
+            fmt_str.append("\033[31m")
+        if FMT.ORANGE in self:
+            fmt_str.append("\033[33m")
+        if FMT.GRAY in self:
+            fmt_str.append("\033[37m")
+        if FMT.GREEN in self:
+            fmt_str.append("\033[32m")
+        if FMT.BLUE in self:
+            fmt_str.append("\033[34m")
+        if FMT.YELLOW in self:
+            fmt_str.append("\033[93m")
+        if FMT.BOLD in self:
+            fmt_str.append("\033[1m")
+        if FMT.RESET in self:
+            fmt_str.append("\033[0m")
+        if FMT.UNDERLINE in self:
+            fmt_str.append("\033[4m")
+
+        return "".join(fmt_str)
+
+
+WARN = FMT.ORANGE | FMT.UNDERLINE
+ERR = FMT.RED | FMT.BOLD

filecheck/finput.py

@@ -190,7 +190,7 @@ def find_between(
             if match is not None:
                 return match
 
-    def print_line_with_current_pos(self, pos_override: int | None = None):
+    def print_line_with_current_pos(self, pos_override: int | None = None) -> str:
         """
         Print the current position in the input file.
         """
@@ -205,9 +205,11 @@ def print_line_with_current_pos(self, pos_override: int | None = None):
 
         last_newline_at = self.start_of_line(pos)
         char_pos = pos - last_newline_at
-        print(f"Matching at {fname}:{self.line_no}:{char_pos}")
-        print(self.content[last_newline_at + 1 : next_newline_at])
-        print(" " * (char_pos - 1), end="^\n")
+        return (
+            f"Matching at {fname}:{self.line_no}:{char_pos}\n"
+            f"{self.content[last_newline_at + 1 : next_newline_at]}\n"
+            + f"{'^':>{char_pos}}"
+        )
 
     def start_of_line(self, pos: int | None = None) -> int:
         """

filecheck/help.py

@@ -11,7 +11,7 @@
                                   starting with $ are deleted after each CHECK-LABEL
                                   match.
 --match-full-lines              : Expect every check line to match the whole line.
-
+--reject-empty-vars             : Raise an error when a value captures an empty string.
 
 ARGUMENTS:
 check-file                      : The file from which the check lines are to be read

filecheck/logging.py

@@ -0,0 +1,20 @@
+import sys
+
+from filecheck.colors import WARN, FMT
+from filecheck.ops import CheckOp
+from filecheck.options import Options
+
+
+def warn(
+    msg: str,
+    *,
+    op: CheckOp | None = None,
+    input_loc: str | None = None,
+    opts: Options,
+):
+    print(f"{WARN}Warning: {msg}{FMT.RESET}", end="", file=sys.stderr)
+    if input_loc:
+        print(f" at {input_loc}", end="", file=sys.stderr)
+    print("", file=sys.stderr)
+    if op:
+        print(op.source_repr(opts), file=sys.stderr)

filecheck/matcher.py

@@ -1,10 +1,13 @@
 import re
+import sys
 from dataclasses import dataclass, field
 from typing import Callable
 
 from filecheck.compiler import compile_uops
 from filecheck.error import CheckError, ParseError
 from filecheck.finput import FInput, InputRange
+from filecheck.logging import warn
+from filecheck.colors import ERR, FMT
 from filecheck.ops import CheckOp, CountOp
 from filecheck.options import Options
 from filecheck.parser import Parser
@@ -61,20 +64,27 @@ def run(self) -> int:
         """
         if not self.opts.allow_empty:
             if self.file.content in ("", "\n"):
-                print(f"filecheck error: '{self.opts.readable_input_file()}' is empty.")
+                print(
+                    f"{ERR}filecheck error:{FMT.RESET} '{self.opts.readable_input_file()}' is empty.",
+                    file=sys.stderr,
+                )
                 return 1
 
         try:
             ops = tuple(self.operations)
             if not ops:
                 print(
-                    f"Error: No check strings found with prefix {self.opts.check_prefix}:"
+                    f"{ERR}filecheck error:{FMT.RESET} No check strings found with prefix {self.opts.check_prefix}:",
+                    file=sys.stderr,
                 )
                 return 2
         except ParseError as ex:
-            print(f"{self.opts.match_filename}:{ex.line_no}:{ex.offset} {ex.message}")
-            print(ex.offending_line.rstrip("\n"))
-            print(" " * (ex.offset - 1) + "^")
+            print(
+                f"{self.opts.match_filename}:{ex.line_no}:{ex.offset} {ex.message}",
+                file=sys.stderr,
+            )
+            print(ex.offending_line.rstrip("\n"), file=sys.stderr)
+            print(" " * (ex.offset - 1) + "^", file=sys.stderr)
             return 1
 
         function_table: dict[str, Callable[[CheckOp], None]] = {
@@ -105,15 +115,22 @@ def run(self) -> int:
             self._post_check(CheckOp("NOP", "", -1, []))
         except CheckError as ex:
             print(
-                f"{self.opts.match_filename}:{ex.op.source_line}: error: {ex.message}"
+                f"{self.opts.match_filename}:{ex.op.source_line}: {ERR}error:{FMT.RESET} {ex.message}",
+                file=sys.stderr,
             )
-            self.file.print_line_with_current_pos()
+            print(self.file.print_line_with_current_pos(), file=sys.stderr)
 
             if ex.pattern:
-                print(f"Trying to match with regex '{ex.pattern.pattern}'")
+                print(
+                    f"Trying to match with regex '{ex.pattern.pattern}'",
+                    file=sys.stderr,
+                )
                 if match := self.file.find(ex.pattern):
-                    print("Possible match at:")
-                    self.file.print_line_with_current_pos(match.start(0))
+                    print("Possible match at:", file=sys.stderr)
+                    print(
+                        self.file.print_line_with_current_pos(match.start(0)),
+                        file=sys.stderr,
+                    )
 
             return 1
 
@@ -174,7 +191,7 @@ def check_dag(self, op: CheckOp) -> None:
                 f"{self.opts.check_prefix}-DAG: Can't find match ('{op.arg}')",
                 op,
             )
-        self.capture_results(match, capture)
+        self.capture_results(match, capture, op)
 
     def check_count(self, op: CheckOp) -> None:
         # invariant preserved by parser
@@ -238,7 +255,7 @@ def match_immediately(self, op: CheckOp):
         pattern, repl = compile_uops(op, self.ctx.live_variables, self.opts)
         if match := self.file.match(pattern):
             self.file.move_to(match.end(0))
-            self.capture_results(match, repl)
+            self.capture_results(match, repl, op)
         else:
             raise CheckError(f'Couldn\'t match "{op.arg}".', op, pattern=pattern)
 
@@ -251,7 +268,7 @@ def match_eventually(self, op: CheckOp):
         pattern, repl = compile_uops(op, self.ctx.live_variables, self.opts)
         if match := self.file.find(pattern, op.name == "SAME"):
             self.file.move_to(match.end())
-            self.capture_results(match, repl)
+            self.capture_results(match, repl, op)
         else:
             raise CheckError(f'Couldn\'t match "{op.arg}".', op, pattern=pattern)
 
@@ -273,10 +290,19 @@ def capture_results(
         self,
         match: re.Match[str],
         capture: dict[str, tuple[int, Callable[[str], int] | Callable[[str], str]]],
+        op: CheckOp,
     ):
         """
         Capture the results of a match into variables for string substitution
         """
         for name, (group, mapper) in capture.items():
-            print(f"assigning variable {name} = {match.group(group)}")
-            self.ctx.live_variables[name] = mapper(match.group(group))
+            str_val = match.group(group)
+            self.ctx.live_variables[name] = mapper(str_val)
+            if not str_val:
+                warn(
+                    "Empty pattern capture",
+                    op=op,
+                    opts=self.opts,
+                )
+                if self.opts.reject_empty_vars:
+                    raise CheckError(f'Empty value captured for variable "{name}"', op)

filecheck/ops.py

@@ -1,4 +1,5 @@
 from __future__ import annotations
+
 from dataclasses import dataclass, field
 from typing import Callable, TypeAlias
 
@@ -24,9 +25,11 @@ class CheckOp:
     def check_line_repr(self, prefix: str = "CHECK"):
         return f"{prefix}{self._suffix()}: {self.arg}"
 
-    def print_source_repr(self, opts: Options):
-        print(f"Check rule at {opts.match_filename}:{self.source_line}")
-        print(self.check_line_repr(opts.check_prefix))
+    def source_repr(self, opts: Options) -> str:
+        return (
+            f"Check rule at {opts.match_filename}:{self.source_line}\n"
+            f"{self.check_line_repr(opts.check_prefix)}"
+        )
 
     def _suffix(self):
         suffix = "{LITERAL}" if self.is_literal else ""

filecheck/options.py

@@ -19,6 +19,7 @@ class Options:
     allow_empty: bool = False
     comment_prefixes: list[str] = "COM,RUN"  # type: ignore[reportAssignmentType]
     variables: dict[str, str | int] = field(default_factory=dict)
+    reject_empty_vars: bool = False
 
     extensions: set[Extension] = field(default_factory=set)
 

filecheck/parser.py

@@ -2,9 +2,9 @@
 parser for filecheck syntax
 """
 
+import re
 from dataclasses import dataclass, field
 from typing import Iterator, TextIO
-import re
 
 from filecheck.error import ParseError
 from filecheck.ops import CheckOp, Literal, RE, Capture, Subst, NumSubst, UOp, CountOp
@@ -29,7 +29,7 @@ def pattern_for_opts(opts: Options) -> tuple[re.Pattern[str], re.Pattern[str]]:
 
 
 # see https://llvm.org/docs/CommandGuide/FileCheck.html#filecheck-string-substitution-blocks
-VAR_CAPTURE_PATTERN = re.compile(r"\[\[(\$?[a-zA-Z_][a-zA-Z0-9_]*):([^\n]+)]]")
+VAR_CAPTURE_PATTERN = re.compile(r"\[\[(\$?[a-zA-Z_][a-zA-Z0-9_]*):([^\n]*)]]")
 
 # see https://llvm.org/docs/CommandGuide/FileCheck.html#filecheck-string-substitution-blocks
 VAR_SUBST_PATTERN = re.compile(r"\[\[(\$?[a-zA-Z_][a-zA-Z0-9_]*)]]")
@@ -46,7 +46,6 @@ def pattern_for_opts(opts: Options) -> tuple[re.Pattern[str], re.Pattern[str]]:
     r"\[\[#(\$?[a-zA-Z_][a-zA-Z0-9_]*)([a-z0-9 +\-()]*)]]"
 )
 
-
 LINE_SPLIT_RE = split = re.compile(r"(\{\{|\[\[\$?[#a-zA-Z_]|]|})")
 
 

tests/filecheck/diagnostics/reject-empty-vars.test

@@ -0,0 +1,15 @@
+// RUN:  strip-comments.sh %s | exfail filecheck %s --reject-empty-vars | filecheck %s --check-prefix DIAG
+
+test 123
+// CHECK: test [[VAL:]]
+// CHECK-SAME: [[VAL]]
+
+// the warning printed:
+// DIAG:        Warning: Empty pattern capture
+// DIAG-NEXT:   Check rule at {{.*}}tests/filecheck/diagnostics/reject-empty-vars.test:4
+// DIAG-NEXT:   CHECK: test [[VAL:]]
+// the error printed:
+// DIAG-NEXT:   tests/filecheck/diagnostics/reject-empty-vars.test:4: error: Empty value captured for variable "VAL"
+// DIAG-NEXT:   Matching at <stdin>:1:6
+// DIAG-NEXT:   test 123
+// DIAG-NEXT: {{     }}^

tests/filecheck/variables.test

@@ -49,3 +49,9 @@ test %arg1
 // CHECK:       test [[ARG:%[[:alnum:]]+]]
 test [%arg1][0]
 // CHECK-NEXT:  test [[[ARG]]][0]
+
+
+// we have to replicate FileChecks behaviour here, and allow empty varaible captures:
+test 123
+// CHECK: test [[VAL:]]
+// CHECK-SAME: [[VAL]]