diff --git a/problemtools/checks/config.py b/problemtools/checks/config.py index da26163a..acf428d8 100644 --- a/problemtools/checks/config.py +++ b/problemtools/checks/config.py @@ -19,8 +19,8 @@ # 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) @@ -28,7 +28,7 @@ def _error_in_2023_07(format: FormatVersion, diag: Diagnostics, msg: str, additi 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' diff --git a/problemtools/checks/problem_package.py b/problemtools/checks/problem_package.py index b40e7be2..100a19f0 100644 --- a/problemtools/checks/problem_package.py +++ b/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') diff --git a/problemtools/checks/statements.py b/problemtools/checks/statements.py index 701ba81c..34cbc771 100644 --- a/problemtools/checks/statements.py +++ b/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..{ext}' for ext in format.statement_extensions) + if format_version is FormatVersion.LEGACY: + allowed_statements = ', '.join( + f'problem.{ext}, problem..{ext}' for ext in format_version.statement_extensions + ) else: - allowed_statements = ', '.join(f'problem..{ext}' for ext in format.statement_extensions) + allowed_statements = ', '.join(f'problem..{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: diff --git a/problemtools/checks/testdata.py b/problemtools/checks/testdata.py index c70456f8..1e395f91 100644 --- a/problemtools/checks/testdata.py +++ b/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') diff --git a/problemtools/checks/validators.py b/problemtools/checks/validators.py index 3d9933af..96684cba 100644 --- a/problemtools/checks/validators.py +++ b/problemtools/checks/validators.py @@ -75,19 +75,8 @@ 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) @@ -95,9 +84,6 @@ def _error_in_2023_07(format: FormatVersion, diag: Diagnostics, msg: str, additi 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,22 +184,21 @@ 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') @@ -221,15 +206,15 @@ def check_output_validators( 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: diff --git a/problemtools/formatversion.py b/problemtools/formatversion.py index 407d04e6..5bf568ed 100644 --- a/problemtools/formatversion.py +++ b/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 diff --git a/problemtools/model/statements.py b/problemtools/model/statements.py index e465c5a4..18d0f9df 100644 --- a/problemtools/model/statements.py +++ b/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)) diff --git a/problemtools/model/validators.py b/problemtools/model/validators.py index 978cc2dc..36034920 100644 --- a/problemtools/model/validators.py +++ b/problemtools/model/validators.py @@ -17,15 +17,17 @@ class InputValidators: """A problem's input format validators.""" validators: list[Program] = field(default_factory=list) - uses_old_path: bool = False def load_input_validators(probdir: Path, language_config: Languages) -> InputValidators: - old_path = probdir / 'input_format_validators' - uses_old_path = old_path.is_dir() - validators_path = old_path if uses_old_path else probdir / 'input_validators' - validators = find_programs(str(validators_path), language_config=language_config, allow_validation_script=True) - return InputValidators(validators=validators, uses_old_path=uses_old_path) + # input_format_validators is a deprecated name for input_validators. We just load + # from both and let _check_root_directory_names warn about the deprecated directory + validators = [ + program + for directory in ('input_format_validators', 'input_validators') + for program in find_programs(str(probdir / directory), language_config=language_config, allow_validation_script=True) + ] + return InputValidators(validators=validators) @dataclass(frozen=True) @@ -34,20 +36,20 @@ class OutputValidators: validators: list[Program] = field(default_factory=list) - def uses_default(self, format: FormatVersion, metadata: Metadata) -> bool: + def uses_default(self, format_version: FormatVersion, metadata: Metadata) -> bool: """Whether the default validator is used, rather than a custom one.""" - if format is FormatVersion.LEGACY: + if format_version is FormatVersion.LEGACY: return metadata.legacy_validation == 'default' return not self.validators - def select(self, format: FormatVersion, metadata: Metadata) -> Program | None: + def select(self, format_version: FormatVersion, metadata: Metadata) -> Program | None: """The output validator that will actually be used, or None if the default validator is required but not available on this problemtools install.""" - if self.uses_default(format, metadata) or not self.validators: + if self.uses_default(format_version, metadata) or not self.validators: return DEFAULT_VALIDATOR return self.validators[0] -def load_output_validators(probdir: Path, format: FormatVersion, language_config: Languages) -> OutputValidators: - validators = find_programs(str(probdir / format.output_validator_directory), language_config=language_config) +def load_output_validators(probdir: Path, format_version: FormatVersion, language_config: Languages) -> OutputValidators: + validators = find_programs(str(probdir / format_version.output_validator_directory), language_config=language_config) return OutputValidators(validators=validators) diff --git a/problemtools/verifyproblem.py b/problemtools/verifyproblem.py index d38395a6..e456d408 100644 --- a/problemtools/verifyproblem.py +++ b/problemtools/verifyproblem.py @@ -190,7 +190,6 @@ def start_submissions(context: Context) -> None: problem.format_version, problem.metadata, problem.testdata, - problem.probdir, work_dir, diag, ),