feat: Issue a warning when unsupported characters are used in wildcard names (#1587)

SichongP · johanneskoester · web-flow · commit fa5735555dab · 2025-09-04T16:07:55.000+02:00
### Description Currently, wildcard names can only include alphanumeric characters and underscores. When an unsupported character such as `-` is used in a wildcard name, it is silently ignored. Making matters worse, the entire wildcard name is truncated, leading to confusing error messages like seen in #1349. This PR should close #1349 and #1579 #### Solution When registering wildcard names, attempt is first made to match "legal" regex pattern for a whilcard name. When that matching fails and returns `None`, it's either because there isn't a wildcard in the file name, or an "illegal" name is used for the wildcard. We then match again to a relaxed regex pattern, where we specifically look for any non-word characters (`\W`) before any commas in wildcard definition. If this returns a match, we issue a warning informing users of illegal names. #### Alternative Alternatively, an error can be raised in lieu of a warning. I opted not to do this because I'm not sure if there could be any legitimate case where `{}` are used but not for wildcard difinition (`{{}}` escaped brackets are already accounted for in relaxed regex matching) and therefore I'm not comfortable raising an exception at this stage. But if not, then we should probably raise a `WildcardError` instead. I see that `WildcardError` is currently not implemented. Do we have a plan for this exception class yet? ### QC  * [ ] The PR contains a test case for the changes or the changes are already covered by an existing test case. * [ ] The documentation (`docs/`) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake).  ## Summary by CodeRabbit * **New Features** * Added warnings when file patterns contain illegal wildcard names. * **Bug Fixes** * Validation now runs earlier to catch wildcard/name issues sooner. * Ensures outputs, logs, and benchmarks use a consistent wildcard set with clearer errors. * **Refactor** * Public wildcard-registration API updated; integrations or extensions relying on previous behavior may need small adjustments.  --------- Co-authored-by: Johannes Köster <johannes.koester@tu-dortmund.de> Co-authored-by: Johannes Koester <johannes.koester@uni-due.de>
diff --git a/src/snakemake/io/__init__.py b/src/snakemake/io/__init__.py
@@ -487,6 +487,11 @@ def check(self):
                 f"File path {self._file} contains double '{os.path.sep}'. "
                 f"This is likely unintended. {hint}"
             )
+        if _illegal_wildcard_name_regex.search(self._file) is not None:
+            logger.warning(
+                f"File path '{self._file}' contains illegal characters in a wildcard "
+                f"name (only alphanumerics and underscores are allowed)."
+            )
 
     async def exists(self):
         if self.is_storage:
@@ -991,6 +996,17 @@ def remove_flag(value: MaybeAnnotated, flag: str) -> MaybeAnnotated:
 
 _CONSIDER_LOCAL_DEFAULT = frozenset()
 
+_illegal_wildcard_name_regex = re.compile(
+    r"""
+    \{(?!\{) # Start matching from the second {, otherwise \W will match the second {
+        \s*
+        (?P<name>
+            .*?\W[^,\{\}]*
+        ),?[^,\{\}]*? # Do we see any non-word character before comma?
+    \}
+    """,
+)
+
 
 async def wait_for_files(
     files,
diff --git a/src/snakemake/rules.py b/src/snakemake/rules.py
@@ -246,7 +246,8 @@ def benchmark(self, benchmark):
             benchmark = self._update_item_wildcard_constraints(benchmark)
 
         self._benchmark = IOFile(benchmark, rule=self)
-        self.register_wildcards(self._benchmark.get_wildcard_names())
+        self._benchmark.check()
+        self.register_wildcards(self._benchmark)
 
     @property
     def conda_env(self):
@@ -318,7 +319,8 @@ def get_some_product(self):
     def has_products(self):
         return self.get_some_product() is not None
 
-    def register_wildcards(self, wildcard_names):
+    def register_wildcards(self, item):
+        wildcard_names = item.get_wildcard_names()
         if self._wildcard_names is None:
             self._wildcard_names = wildcard_names
         else:
@@ -363,7 +365,7 @@ def set_output(self, *output, **kwoutput):
             self._set_inoutput_item(item, output=True, name=name)
 
         for item in self.output:
-            self.register_wildcards(item.get_wildcard_names())
+            self.register_wildcards(item)
         # Check output file name list for duplicates
         self.check_output_duplicates()
         self.check_caching()
@@ -531,6 +533,7 @@ def _set_inoutput_item(self, item, output=False, name=None, mark_ancient=False):
 
             # record rule if this is an output file output
             _item = IOFile(item, rule=self)
+            _item.check()
 
             if is_flagged(item, "temp"):
                 if output:
@@ -614,7 +617,7 @@ def set_log(self, *logs, **kwlogs):
             self._set_log_item(item, name=name)
 
         for item in self.log:
-            self.register_wildcards(item.get_wildcard_names())
+            self.register_wildcards(item)
 
     def _set_log_item(self, item, name=None):
         # Pathlib compatibility
@@ -625,7 +628,11 @@ def _set_log_item(self, item, name=None):
                 item = self.apply_path_modifier(item, self.log_modifier, property="log")
                 item = self._update_item_wildcard_constraints(item)
 
-            self.log.append(IOFile(item, rule=self) if isinstance(item, str) else item)
+            if isinstance(item, str):
+                item = IOFile(item, rule=self)
+                item.check()
+
+            self.log.append(item)
             if name:
                 self.log._add_name(name)
         else:
