Warn for typo'd folders #399

problemtools/checks/config.py

@@ -19,16 +19,16 @@
 # promote to a shared helper if a third one shows up.
 
 
-def _error_in_2023_07(format: FormatVersion, diag: Diagnostics, msg: str, additional_info: str | None = None) -> None:
-    if format is FormatVersion.LEGACY:
+def _error_in_2023_07(format_version: FormatVersion, diag: Diagnostics, msg: str, additional_info: str | None = None) -> None:
+    if format_version is FormatVersion.LEGACY:
         diag.warning(msg, additional_info)
     else:
         diag.error(msg, additional_info)
 
 
 def check_config(
     metadata: Metadata,
-    format: FormatVersion,
+    format_version: FormatVersion,
     statements: Statements,
     testdata: TestDataGroup,
     diag: Diagnostics,
@@ -61,7 +61,7 @@ def check_config(
         diag.warning("License is 'unknown'")
 
     if metadata.uuid is None:
-        _error_in_2023_07(format, diag, f'Missing uuid from problem.yaml. Add "uuid: {uuid.uuid4()}" to problem.yaml.')
+        _error_in_2023_07(format_version, diag, f'Missing uuid from problem.yaml. Add "uuid: {uuid.uuid4()}" to problem.yaml.')
 
     names_with_no_statement = [lang for lang in metadata.name if lang not in statements.by_language]
     if names_with_no_statement:
@@ -73,7 +73,7 @@ def check_config(
         not metadata.is_pass_fail()
         and testdata.has_custom_groups()
         and not metadata.show_test_data_groups_explicitly_set
-        and format is FormatVersion.LEGACY
+        and format_version is FormatVersion.LEGACY
     ):
         diag.warning(
             'Problem has custom testcase groups, but does not specify a value for grading.show_test_data_groups; defaulting to false'

problemtools/checks/problem_package.py

@@ -10,12 +10,14 @@
 from ..diagnostics import Diagnostics
 from ..formatversion import FormatVersion
 
+_NAME_REGEX = re.compile(r'^[a-zA-Z0-9_][a-zA-Z0-9_.-]{0,254}$')
 
-def check_problem_package(probdir: Path, format: FormatVersion, diag: Diagnostics) -> None:
+
+def check_problem_package(probdir: Path, format_version: FormatVersion, diag: Diagnostics) -> None:
     """Run all checks on the structure of a problem package."""
     _check_symlinks(probdir, diag)
     _check_file_and_directory_names(probdir, diag)
-    _check_submission_directory_names(probdir, format, diag)
+    _check_root_directory_names(probdir, format_version, diag)
 
 
 def _check_symlinks(probdir: Path, diag: Diagnostics) -> None:
@@ -39,8 +41,6 @@ def _check_symlinks(probdir: Path, diag: Diagnostics) -> None:
 
 
 def _check_file_and_directory_names(probdir: Path, diag: Diagnostics) -> None:
-    regex = re.compile(r'^[a-zA-Z0-9_][a-zA-Z0-9_.-]{0,254}$')
-
     def _special_case_allowed_files(file: str, reldir: str) -> bool:
         return file == '.gitignore' or (file == '.timelimit' and reldir == probdir.name)
 
@@ -51,40 +51,52 @@ def _special_case_allowed_dirs(directory: str, reldir: str) -> bool:
         # Path of the directory we're in, starting with problem shortname. Only used for nicer error messages.
         reldir = os.path.relpath(root, probdir.parent)
         for file in files:
-            if not regex.match(file) and not _special_case_allowed_files(file, reldir):
-                diag.error(f"Invalid file name '{file}' in {reldir}, should match {regex.pattern}")
+            if not _NAME_REGEX.match(file) and not _special_case_allowed_files(file, reldir):
+                diag.error(f"Invalid file name '{file}' in {reldir}, should match {_NAME_REGEX.pattern}")
         for directory in dirs:
-            if not regex.match(directory) and not _special_case_allowed_dirs(directory, reldir):
-                diag.error(f"Invalid directory name '{directory}' in {reldir}, should match {regex.pattern}")
-
-
-def _check_submission_directory_names(probdir: Path, format: FormatVersion, diag: Diagnostics) -> None:
-    """Heuristically check if submissions contain any directories that will be ignored because of typos or format mismatches"""
-    submission_directories = [p.name for p in (probdir / 'submissions').glob('*') if p.is_dir()]
-    if len(submission_directories) == 0:
-        return
-
-    def most_similar(present_dir: str, format_version: FormatVersion) -> tuple[str, float]:
-        similarities = [
-            (spec_dir, difflib.SequenceMatcher(None, present_dir, spec_dir).ratio())
-            for spec_dir in format_version.submission_directories
-        ]
-        return max(similarities, key=lambda x: x[1])
-
-    for present_dir in submission_directories:
-        most_similar_dir, max_similarity = most_similar(present_dir, format)
-
-        if max_similarity == 1:
-            # Exact match, no typo
+            if not _NAME_REGEX.match(directory) and not _special_case_allowed_dirs(directory, reldir):
+                diag.error(f"Invalid directory name '{directory}' in {reldir}, should match {_NAME_REGEX.pattern}")
+
+
+def _warn_renamed_directory(found_name: str, format_version: FormatVersion, diag: Diagnostics) -> bool:
+    """If found_name is what some other format version calls a directory that was renamed
+    across format versions, warn that it's been renamed and return True."""
+    for prop in ('statement_directory', 'output_validator_directory'):
+        good_dir = getattr(format_version, prop)
+        bad_dirs = {getattr(version, prop) for version in FormatVersion} - {good_dir}
+        if found_name in bad_dirs:
+            diag.warning(f'Found directory "{found_name}". Version {format_version} looks for this as "{good_dir}"')
+            return True
+    return False
+
+
+def _check_root_directory_names(probdir: Path, format_version: FormatVersion, diag: Diagnostics) -> None:
+    """Warn about unrecognized directories at the problem root: deprecated names,
+    directories renamed between format versions, directories belonging to other
+    format versions, and likely typos."""
+    known = format_version.root_directories
+    other_known = {directory for version in FormatVersion for directory in version.root_directories} - known
+
+    for entry in probdir.iterdir():
+        name = entry.name
+        if not entry.is_dir() or name in known or name == '.git':
+            continue
+        if not _NAME_REGEX.match(name):
+            # Already flagged as an invalid name by _check_file_and_directory_names.
             continue
 
-        if 0.75 <= max_similarity:
-            diag.warning(f'Potential typo: directory submissions/{present_dir} is similar to {most_similar_dir}')
+        if name == 'input_format_validators':
+            diag.warning('input_format_validators is a deprecated name; please use input_validators instead')
+        elif _warn_renamed_directory(name, format_version, diag):
+            pass
+        elif name in other_known:
+            diag.warning(f'Directory "{name}" is not part of format version {format_version}')
         else:
-            for other_version in [v for v in FormatVersion if v != format]:
-                _, max_similarity = most_similar(present_dir, other_version)
-                if max_similarity == 1:
-                    diag.warning(
-                        f'Directory submissions/{present_dir} is not part of format version {format}, but part of {other_version}'
-                    )
-                    break
+            closest, similarity = max(
+                ((d, difflib.SequenceMatcher(None, name, d).ratio()) for d in known),
+                key=lambda x: x[1],
+            )
+            if similarity >= 0.75:
+                diag.warning(f'Potential typo: directory "{name}" is similar to "{closest}"')
+            else:
+                diag.warning(f'Unrecognized directory "{name}" at problem root')

problemtools/checks/statements.py

@@ -13,29 +13,16 @@
 from ..metadata import Metadata
 from ..model import Statements
 
-# Temporary local copy of checks/validators.py's _warn_directory. Only two consumers so far;
-# promote to a shared helper if a third one shows up.
-
-
-def _warn_directory(format: FormatVersion, probdir: Path, name: str, prop: str, diag: Diagnostics) -> None:
-    good_dir = getattr(format, prop)
-    bad_dirs = {getattr(version, prop) for version in FormatVersion} - {good_dir}
-    for directory in bad_dirs:
-        if (probdir / directory).exists():
-            diag.warning(f'Found directory "{directory}". Version {format} looks for {name} in "{good_dir}"')
-
 
 def check_statements(
     statements: Statements,
     metadata: Metadata,
-    format: FormatVersion,
+    format_version: FormatVersion,
     probdir: Path,
     work_dir: Path,
     diag: Diagnostics,
 ) -> None:
     """Run all checks on a problem's statements."""
-    _warn_directory(format, probdir, 'problem statements', 'statement_directory', diag)
-
     for ifilename in glob.glob(os.path.join(str(probdir), 'data/sample/*.interaction')):
         if not metadata.is_interactive() and not metadata.is_multi_pass():
             diag.error(f'Problem is not interactive, but there is an interaction sample {ifilename}')
@@ -49,13 +36,15 @@ def check_statements(
                     break
 
     if not statements.by_language:
-        if format is FormatVersion.LEGACY:
-            allowed_statements = ', '.join(f'problem.{ext}, problem.<language>.{ext}' for ext in format.statement_extensions)
+        if format_version is FormatVersion.LEGACY:
+            allowed_statements = ', '.join(
+                f'problem.{ext}, problem.<language>.{ext}' for ext in format_version.statement_extensions
+            )
         else:
-            allowed_statements = ', '.join(f'problem.<language>.{ext}' for ext in format.statement_extensions)
+            allowed_statements = ', '.join(f'problem.<language>.{ext}' for ext in format_version.statement_extensions)
 
         diag.error(
-            f'No problem statements found (expected file of one of following forms in directory {format.statement_directory}/: {allowed_statements})'
+            f'No problem statements found (expected file of one of following forms in directory {format_version.statement_directory}/: {allowed_statements})'
         )
 
     def _latex_heuristic(name: str) -> bool:
@@ -71,7 +60,7 @@ def _latex_heuristic(name: str) -> bool:
             diag.error(f'Problem name in language {lang} is empty')
         elif not metadata.name[lang].strip():
             diag.error(f'Problem name in language {lang} contains only whitespace')
-        elif format is FormatVersion.LEGACY and _latex_heuristic(metadata.name[lang]):
+        elif format_version is FormatVersion.LEGACY and _latex_heuristic(metadata.name[lang]):
             diag.warning(f'Problem name in language {lang} looks like LaTeX. Consider using plainproblemname.')
 
         for file in files:

problemtools/checks/testdata.py

@@ -35,12 +35,12 @@ def check_testdata(
     graders: Graders,
     input_validators: InputValidators,
     output_validators: OutputValidators,
-    format: FormatVersion,
+    format_version: FormatVersion,
     work_dir: Path,
     diag: Diagnostics,
 ) -> None:
     """Run all checks on a problem's test data."""
-    output_validator = output_validators.select(format, metadata)
+    output_validator = output_validators.select(format_version, metadata)
     if output_validator is None:
         diag.fatal('Unable to locate default validator')
 

problemtools/checks/validators.py

@@ -75,29 +75,15 @@ def _build_junk_modifier(
 ]
 
 
-# Temporary helpers to keep code structure as similar as possible to old code from
-# verifyproblem when extracting this to a separate module; ProblemAspect still owns the
-# "real" versions of these, used by parts not yet extracted (e.g. ProblemStatement, ProblemConfig).
-def _warn_directory(format: FormatVersion, probdir: Path, name: str, prop: str, diag: Diagnostics) -> None:
-    good_dir = getattr(format, prop)
-    bad_dirs = {getattr(version, prop) for version in FormatVersion} - {good_dir}
-    for directory in bad_dirs:
-        if (probdir / directory).exists():
-            diag.warning(f'Found directory "{directory}". Version {format} looks for {name} in "{good_dir}"')
-
-
-def _error_in_2023_07(format: FormatVersion, diag: Diagnostics, msg: str, additional_info: str | None = None) -> None:
-    if format is FormatVersion.LEGACY:
+def _error_in_2023_07(format_version: FormatVersion, diag: Diagnostics, msg: str, additional_info: str | None = None) -> None:
+    if format_version is FormatVersion.LEGACY:
         diag.warning(msg, additional_info)
     else:
         diag.error(msg, additional_info)
 
 
 def check_input_validators(validators: InputValidators, testdata: TestDataGroup, work_dir: Path, diag: Diagnostics) -> None:
     """Run all checks on a problem's input format validators."""
-    if validators.uses_old_path:
-        diag.warning('input_format_validators is a deprecated name; please use input_validators instead')
-
     errors_before = diag.errors
     if len(validators.validators) == 0:
         diag.error('No input format validators found')
@@ -198,38 +184,37 @@ def check_testcase_input(validators: InputValidators, testcase: TestCase, work_d
 
 def check_output_validators(
     validators: OutputValidators,
-    format: FormatVersion,
+    format_version: FormatVersion,
     metadata: Metadata,
     testdata: TestDataGroup,
-    probdir: Path,
     work_dir: Path,
     diag: Diagnostics,
 ) -> None:
     """Run all checks on a problem's output validators."""
-    _warn_directory(format, probdir, 'output validators', 'output_validator_directory', diag)
-
     errors_before = diag.errors
 
-    selected = validators.select(format, metadata)
+    selected = validators.select(format_version, metadata)
 
     if len(validators.validators) > 1:
-        _error_in_2023_07(format, diag, f'Support for multiple output validators has been dropped. will only use {selected}')
+        _error_in_2023_07(
+            format_version, diag, f'Support for multiple output validators has been dropped. will only use {selected}'
+        )
 
     if selected is None:
         diag.fatal('Unable to locate default validator')
 
     safe_output_validator_languages = {'c', 'cpp', 'python3'}
     if isinstance(selected, SourceCode) and selected.language.lang_id not in safe_output_validator_languages:
         _error_in_2023_07(
-            format,
+            format_version,
             diag,
             f'Output validator in {selected.language.name}. Only {safe_output_validator_languages} are standardized. '
             'Check carefully if your CCS supports more (Kattis does not).',
         )
 
-    if validators.uses_default(format, metadata) and validators.validators:
+    if validators.uses_default(format_version, metadata) and validators.validators:
         diag.error('There are validator programs but problem.yaml has validation = "default"')
-    elif not validators.uses_default(format, metadata) and not validators.validators:
+    elif not validators.uses_default(format_version, metadata) and not validators.validators:
         diag.fatal('problem.yaml specifies custom validator but no validator programs found')
 
     try:

problemtools/formatversion.py

@@ -34,14 +34,38 @@ def output_validator_directory(self) -> str:
                 return 'output_validator'
 
     @property
-    def submission_directories(self) -> list[str]:
+    def root_directories(self) -> frozenset[str]:
+        """Known directories directly under the problem root for this format version."""
         match self:
             case FormatVersion.LEGACY:
-                return ['accepted', 'partially_accepted', 'wrong_answer', 'time_limit_exceeded', 'run_time_error']
+                return frozenset(
+                    {
+                        self.statement_directory,
+                        'attachments',
+                        'data',
+                        'include',
+                        'submissions',
+                        'input_validators',
+                        self.output_validator_directory,
+                    }
+                )
             case FormatVersion.V_2023_07:
-                # TODO: parse submissions.yaml if applicable, since
-                # 2023-07 and later formats support adding more submission directories
-                return ['accepted', 'rejected', 'wrong_answer', 'time_limit_exceeded', 'run_time_error', 'brute_force']
+                return frozenset(
+                    {
+                        self.statement_directory,
+                        'attachments',
+                        'solution',
+                        'data',
+                        'generators',
+                        'include',
+                        'submissions',
+                        'input_validators',
+                        'static_validator',
+                        self.output_validator_directory,
+                        'input_visualizer',
+                        'output_visualizer',
+                    }
+                )
 
     # Support 2023-07 and 2023-07-draft strings.
     # This method should be replaced with an alias once we require python 3.13

problemtools/model/statements.py

@@ -17,5 +17,5 @@ class Statements:
     by_language: dict[str, list[Path]] = field(default_factory=dict)
 
 
-def load_statements(probdir: Path, format: FormatVersion) -> Statements:
-    return Statements(by_language=statement_util.find_statements(probdir, format))
+def load_statements(probdir: Path, format_version: FormatVersion) -> Statements:
+    return Statements(by_language=statement_util.find_statements(probdir, format_version))