drop bad support (#445)

* drop bad support

* fix

* Fix log messages when renaming data/bad/*

* [stats] Rename "bad" to "inv" and "good" to "v_o" to stay closer to their actual names

---------

Co-authored-by: Maarten Sijm <[email protected]>

Author: mzuenni

bin/config.py

@@ -86,7 +86,6 @@
     "invalid_input",
     "invalid_answer",
     "invalid_output",
-    "bad",
 ]
 
 

bin/generate.py

@@ -484,12 +484,6 @@ def __init__(
 
         # root in /data
         self.root = self.path.parts[0]
-        if self.root == "bad":
-            message(
-                "bad is deprecated. Use {invalid_input,invalid_answer} instead.",
-                self.path,
-                color_type=MessageType.WARN,
-            )
 
         if not config.COMPILED_FILE_NAME_REGEX.fullmatch(name + ".in"):
             raise ParseException("Testcase does not have a valid name.")
@@ -1124,7 +1118,6 @@ def add_testdata_to_cache():
 
             # consider specific files for the uniqueness of this testcase
             relevant_files = {
-                "bad": [".in", ".ans"],
                 "invalid_answer": [".in", ".ans"],
                 "invalid_output": [".in", ".ans", ".out"],
                 "valid_output": [".in", ".ans", ".out"],

bin/stats.py

@@ -68,12 +68,12 @@ def problem_stats(problems):
             100,
         ),
         (
-            "bad",
+            "inv",
             [lambda s: {x.stem for x in s if x.parts[2] in config.INVALID_CASE_DIRECTORIES}],
             0,
         ),
         (
-            "good",
+            "v_o",
             [lambda s: {x.stem for x in s if x.parts[2] in ["valid_output"]}],
             0,
         ),

bin/testcase.py

@@ -10,7 +10,6 @@
     fatal,
     print_name,
     shorten_path,
-    warn,
 )
 import config
 import validate
@@ -97,12 +96,6 @@ def __init__(self, base_problem, path, *, short_path=None, print_warn=False):
         # Display name: everything after data/.
         self.name = str(self.short_path.with_suffix(""))
 
-        # Backwards compatibility support for `data/bad`.
-        if self.root == "bad":
-            if print_warn:
-                warn("data/bad is deprecated. Use data/{invalid_input,invalid_answer} instead.")
-            self.root = "invalid_answer" if self.ans_path.is_file() else "invalid_input"
-
     def __repr__(self):
         return self.name
 
@@ -204,7 +197,7 @@ def validate_format(
                     warn_instead_of_error=warn_instead_of_error,
                 )
             case validate.Mode.INVALID:
-                assert self.root in config.INVALID_CASE_DIRECTORIES[:-1]
+                assert self.root in config.INVALID_CASE_DIRECTORIES
 
                 ok = self.validate_format(
                     validate.Mode.INPUT,

bin/upgrade.py

@@ -3,6 +3,7 @@
 from util import *
 from validate import InputValidator, AnswerValidator, OutputValidator
 
+import secrets
 import shutil
 from typing import Any
 
@@ -28,6 +29,34 @@ def upgrade_data(problem_path: Path, bar: ProgressBar) -> None:
             bar.log(f"renaming '{old_name}' to '{new_name}'")
             old_path.rename(new_path)
 
+    def rename_testcase(old_base: Path, new_dir: Path) -> None:
+        new_dir.mkdir(parents=True, exist_ok=True)
+        new_base = new_dir / old_base.name
+        for ext in config.KNOWN_TEXT_DATA_EXTENSIONS:
+            old_path = old_base.with_suffix(ext)
+            new_path = new_base.with_suffix(ext)
+            if old_path.is_file():
+                old_rel_path, new_rel_path = [
+                    p.relative_to(problem_path) for p in (old_path, new_path)
+                ]
+                if new_path.exists():
+                    bar.error(
+                        f"can't rename '{old_rel_path}', '{new_rel_path}' already exists",
+                        resume=True,
+                    )
+                    continue
+                bar.log(f"renaming '{old_rel_path}' to '{new_rel_path}'")
+                old_path.rename(new_path)
+
+    bad_dir = problem_path / "data" / "bad"
+    for file in bad_dir.glob("*.in"):
+        if file.with_suffix(".ans").is_file():
+            rename_testcase(file, problem_path / "data" / "invalid_answer")
+        else:
+            rename_testcase(file, problem_path / "data" / "invalid_input")
+    if bad_dir.is_dir() and not any(bad_dir.iterdir()):
+        bad_dir.rmdir()
+
 
 def upgrade_testdata_yaml(problem_path: Path, bar: ProgressBar) -> None:
     rename = [
@@ -62,14 +91,16 @@ def upgrade_generators_yaml(problem_path: Path, bar: ProgressBar) -> None:
 
     changed = False
 
-    rename = [
-        ("invalid_inputs", "invalid_input"),
-        ("invalid_answers", "invalid_answer"),
-        ("invalid_outputs", "invalid_output"),
-        ("valid_outputs", "valid_output"),
-    ]
     if "data" in yaml_data and isinstance(yaml_data["data"], dict):
         data = yaml_data["data"]
+        assert isinstance(data, CommentedMap)
+
+        rename = [
+            ("invalid_inputs", "invalid_input"),
+            ("invalid_answers", "invalid_answer"),
+            ("invalid_outputs", "invalid_output"),
+            ("valid_outputs", "valid_output"),
+        ]
         for old_name, new_name in rename:
             if old_name in data:
                 if new_name in data:
@@ -82,6 +113,40 @@ def upgrade_generators_yaml(problem_path: Path, bar: ProgressBar) -> None:
                 ryaml_replace(data, old_name, new_name)
                 changed = True
 
+        # this breaks comments... but that is fine
+        if "bad" in data:
+
+            def move_testcase(name: str, value: Any, new_parent: str) -> None:
+                parent = ryaml_get_or_add(data, new_parent)
+                if "data" not in parent:
+                    parent[data] = CommentedSeq
+                parent = parent["data"]
+                new_name = name
+                if isinstance(parent, list):
+                    parent.append(CommentedMap())
+                    parent[-1][new_name] = value
+                else:
+                    if new_name in parent:
+                        new_name = f"bad_{new_name}"
+                    if new_name in parent:
+                        new_name = f"{new_name}_{secrets.token_hex(6)}"
+                    assert new_name not in parent
+                    parent[new_name] = value
+                bar.log(f"renaming 'bad.{name}' to '{new_parent}.{new_name}' in generators.yaml")
+
+            bad = data["bad"]
+            if "data" in bad and bad["data"]:
+                children = bad["data"] if isinstance(bad["data"], list) else [bad["data"]]
+                for dictionary in children:
+                    for child_name, child_data in sorted(dictionary.items()):
+                        if "ans" in child_data:
+                            move_testcase(child_name, child_data, "invalid_answer")
+                        else:
+                            move_testcase(child_name, child_data, "invalid_input")
+
+            ryaml_filter(data, "bad")
+            changed = True
+
     def upgrade_generated_testdata_yaml(data: dict[str, Any], path: str) -> bool:
         changed = False
         if "testdata.yaml" in data:
@@ -374,7 +439,7 @@ def add_args(new_data: dict[str, Any]) -> bool:
                 data["limits"] = CommentedMap()
             if "time_limit" in data["limits"]:
                 bar.error(
-                    "can't change '.timelimit' file, 'limits.time_limit' already exists in problem.yaml",
+                    "can't change 'domjudge-problem.ini' file, 'limits.time_limit' already exists in problem.yaml",
                     resume=True,
                 )
             else: