diff --git a/problemtools/checks/__init__.py b/problemtools/checks/__init__.py index 7c9bd291..b40dffe7 100644 --- a/problemtools/checks/__init__.py +++ b/problemtools/checks/__init__.py @@ -1,14 +1,18 @@ +from .attachments import check_attachments from .graders import check_graders from .includes import check_includes +from .statements import check_statements from .submissions import check_submissions from .testdata import check_testdata from .validators import check_input_validators, check_output_validators __all__ = [ + 'check_attachments', 'check_graders', 'check_includes', 'check_input_validators', 'check_output_validators', + 'check_statements', 'check_submissions', 'check_testdata', ] diff --git a/problemtools/checks/attachments.py b/problemtools/checks/attachments.py new file mode 100644 index 00000000..3f8683d1 --- /dev/null +++ b/problemtools/checks/attachments.py @@ -0,0 +1,13 @@ +"""Checks for a problem package's attachments.""" + +from __future__ import annotations + +from ..diagnostics import Diagnostics +from ..model import Attachments + + +def check_attachments(attachments: Attachments, diag: Diagnostics) -> None: + """Run all checks on a problem's attachments.""" + for attachment_path in attachments.paths: + if attachment_path.is_dir(): + diag.error(f'Directories are not allowed as attachments ({attachment_path} is a directory)') diff --git a/problemtools/checks/statements.py b/problemtools/checks/statements.py new file mode 100644 index 00000000..3ebc0f0c --- /dev/null +++ b/problemtools/checks/statements.py @@ -0,0 +1,101 @@ +"""Checks for a problem package's statements.""" + +from __future__ import annotations + +import glob +import os +import traceback +from pathlib import Path + +from .. import problem2html, problem2pdf +from ..diagnostics import Diagnostics +from ..formatversion import FormatVersion +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, + probdir: Path, + work_dir: str, + 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}') + with open(ifilename, 'r') as interaction: + for i, line in enumerate(interaction): + valid_new_pass = metadata.is_multi_pass() and line.strip() == '---' + if len(line) == 0 or (line[0] != '<' and line[0] != '>' and not valid_new_pass): + diag.error( + f'Interaction {ifilename}: line {i + 1} does not start with < or > {"or ---" if metadata.is_multi_pass() else ""}' + ) + 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) + else: + allowed_statements = ', '.join(f'problem..{ext}' for ext in format.statement_extensions) + + diag.error( + f'No problem statements found (expected file of one of following forms in directory {format.statement_directory}/: {allowed_statements})' + ) + + def _latex_heuristic(name: str) -> bool: + return '\\' in name or '$' in name + + for lang, files in statements.by_language.items(): + if len(files) > 1: + diag.error(f'Found multiple statements in the same language {lang}: {", ".join(file.name for file in files)}') + + if lang not in metadata.name: + diag.error(f'No problem name given in language {lang}') + elif not metadata.name[lang]: + 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]): + diag.warning(f'Problem name in language {lang} looks like LaTeX. Consider using plainproblemname.') + + for file in files: + try: + options = problem2pdf.get_parser().parse_args(['']) + options.problem = probdir + options.language = lang + options.nopdf = True + options.quiet = True + if not problem2pdf.convert(options, file): + diag.error( + f'Could not compile problem statement for language "{lang}". Run problem2pdf --language {lang} on the problem to diagnose.' + ) + except Exception as e: + diag.error(f'Error raised when checking problem statement for language {lang}:\n{e}\n{traceback.format_exc()}') + + try: + options = problem2html.get_parser().parse_args(['']) + options.problem = probdir + options.destdir = os.path.join(work_dir, 'html') + options.language = lang + options.quiet = True + problem2html.convert(options, file) + except Exception as e: + diag.error( + f'Could not convert problem statement to html for language "{lang}". Run problem2html --language {lang} on the problem to diagnose.\n{e}\n{traceback.format_exc()}' + ) diff --git a/problemtools/model/__init__.py b/problemtools/model/__init__.py index dd50598d..185c6a4c 100644 --- a/problemtools/model/__init__.py +++ b/problemtools/model/__init__.py @@ -3,8 +3,10 @@ #: A submission's (or testcase's) verdict, e.g. as expected by policy or produced by judging. Verdict = Literal['AC', 'TLE', 'OLE', 'MLE', 'RTE', 'WA', 'PAC', 'JE'] +from .attachments import Attachments, load_attachments from .graders import DEFAULT_GRADER, Graders, load_graders from .includes import DEFAULT_LANGUAGE, IncludeFile, Includes, LanguageIncludes, load_includes +from .statements import Statements, load_statements from .submissions import LegacyPolicy, Submission, Submissions, load_submissions from .testdata import DEFAULT_CONFIG, SCORING_ONLY_KEYS, TestCase, TestDataGroup, load_testdata from .validators import DEFAULT_VALIDATOR, InputValidators, OutputValidators, load_input_validators, load_output_validators @@ -15,6 +17,7 @@ 'DEFAULT_LANGUAGE', 'DEFAULT_VALIDATOR', 'SCORING_ONLY_KEYS', + 'Attachments', 'Graders', 'IncludeFile', 'Includes', @@ -22,15 +25,18 @@ 'LanguageIncludes', 'LegacyPolicy', 'OutputValidators', + 'Statements', 'Submission', 'Submissions', 'TestCase', 'TestDataGroup', 'Verdict', + 'load_attachments', 'load_graders', 'load_includes', 'load_input_validators', 'load_output_validators', + 'load_statements', 'load_submissions', 'load_testdata', ] diff --git a/problemtools/model/attachments.py b/problemtools/model/attachments.py new file mode 100644 index 00000000..d2d63ae2 --- /dev/null +++ b/problemtools/model/attachments.py @@ -0,0 +1,17 @@ +from __future__ import annotations + +from dataclasses import dataclass, field +from pathlib import Path + + +@dataclass(frozen=True) +class Attachments: + """A problem's attachments: files found under the attachments/ directory.""" + + paths: list[Path] = field(default_factory=list) + + +def load_attachments(probdir: Path) -> Attachments: + attachments_dir = probdir / 'attachments' + paths = list(attachments_dir.iterdir()) if attachments_dir.is_dir() else [] + return Attachments(paths=paths) diff --git a/problemtools/model/statements.py b/problemtools/model/statements.py new file mode 100644 index 00000000..e465c5a4 --- /dev/null +++ b/problemtools/model/statements.py @@ -0,0 +1,21 @@ +from __future__ import annotations + +from dataclasses import dataclass, field +from pathlib import Path + +from .. import statement_util +from ..formatversion import FormatVersion + + +@dataclass(frozen=True) +class Statements: + """A problem's statements, keyed by language code. + + Well-formed packages have exactly one statement per language; this keeps every + file found (rather than just the first) so checks can report duplicates.""" + + 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)) diff --git a/problemtools/verifyproblem.py b/problemtools/verifyproblem.py index f9ed4c9b..48fee2ed 100644 --- a/problemtools/verifyproblem.py +++ b/problemtools/verifyproblem.py @@ -3,7 +3,6 @@ import argparse import difflib -import glob import logging import os import random @@ -11,7 +10,6 @@ import shutil import sys import tempfile -import traceback import uuid from abc import ABC from pathlib import Path @@ -21,7 +19,7 @@ from pydantic import ValidationError -from . import checks, languages, metadata, model, problem2html, problem2pdf, run, statement_util +from . import checks, languages, metadata, model, run from .context import PROBLEM_PARTS, Context from .diagnostics import Diagnostics, LoggingDiagnostics, VerifyError from .formatversion import FormatVersion, get_format_version @@ -74,15 +72,6 @@ def debug(self, msg: str) -> None: def msg(self, msg: str) -> None: print(msg) - def warn_directory(self, name: str, prop: str) -> None: - """Warns if a directory meant for a different problem format version exists""" - good_dir = getattr(self.problem.format, prop) - bad_dirs = {getattr(version, prop) for version in FormatVersion} - {good_dir} - problem_root = Path(self.problem.probdir) - for directory in bad_dirs: - if (problem_root / directory).exists(): - self.warning(f'Found directory "{directory}". Version {self.problem.format} looks for {name} in "{good_dir}"') - class ProblemPart(ProblemAspect): """Baseclass for all parts that can be included in a problem-format.""" @@ -109,87 +98,30 @@ def check(self, context: Context) -> bool: class ProblemStatement(ProblemPart): - statements: dict[str, list[Path]] # Maps language code -> statement(s) + """Seam to integrate a model + checks setup into verifyproblem in a somewhat clean way""" + PART_NAME = 'statement' def setup(self) -> None: self.debug(' Loading problem statement') - self.statements = statement_util.find_statements(Path(self.problem.probdir), self.problem.format) + self.statements = model.load_statements(Path(self.problem.probdir), self.problem.format) def check(self, context: Context) -> bool: if self._check_res is not None: return self._check_res self._check_res = True - self.warn_directory('problem statements', 'statement_directory') - - for ifilename in glob.glob(os.path.join(self.problem.probdir, 'data/sample/*.interaction')): - if not self.problem.is_interactive() and not self.problem.is_multi_pass(): - self.error(f'Problem is not interactive, but there is an interaction sample {ifilename}') - with open(ifilename, 'r') as interaction: - for i, line in enumerate(interaction): - valid_new_pass = self.problem.is_multi_pass() and line.strip() == '---' - if len(line) == 0 or (line[0] != '<' and line[0] != '>' and not valid_new_pass): - self.error( - f'Interaction {ifilename}: line {i + 1} does not start with < or > {"or ---" if self.problem.is_multi_pass() else ""}' - ) - break - - if not self.statements: - if self.problem.format is FormatVersion.LEGACY: - allowed_statements = ', '.join( - f'problem.{ext}, problem..{ext}' for ext in self.problem.format.statement_extensions - ) - else: - allowed_statements = ', '.join(f'problem..{ext}' for ext in self.problem.format.statement_extensions) - - self.error( - f'No problem statements found (expected file of one of following forms in directory {self.problem.format.statement_directory}/: {allowed_statements})' - ) - - def _latex_heuristic(name: str) -> bool: - return '\\' in name or '$' in name - - for lang, files in self.statements.items(): - if len(files) > 1: - self.error(f'Found multiple statements in the same language {lang}: {", ".join(file.name for file in files)}') - - if lang not in self.problem.metadata.name: - self.error(f'No problem name given in language {lang}') - elif not self.problem.metadata.name[lang]: - self.error(f'Problem name in language {lang} is empty') - elif not self.problem.metadata.name[lang].strip(): - self.error(f'Problem name in language {lang} contains only whitespace') - elif self.problem.format is FormatVersion.LEGACY and _latex_heuristic(self.problem.metadata.name[lang]): - self.warning(f'Problem name in language {lang} looks like LaTeX. Consider using plainproblemname.') - - for file in files: - try: - options = problem2pdf.get_parser().parse_args(['']) - options.problem = self.problem.probdir - options.language = lang - options.nopdf = True - options.quiet = True - if not problem2pdf.convert(options, file): - self.error( - f'Could not compile problem statement for language "{lang}". Run problem2pdf --language {lang} on the problem to diagnose.' - ) - except Exception as e: - self.error( - f'Error raised when checking problem statement for language {lang}:\n{e}\n{traceback.format_exc()}' - ) - - try: - options = problem2html.get_parser().parse_args(['']) - options.problem = self.problem.probdir - options.destdir = os.path.join(self.problem.tmpdir, 'html') - options.language = lang - options.quiet = True - problem2html.convert(options, file) - except Exception as e: - self.error( - f'Could not convert problem statement to html for language "{lang}". Run problem2html --language {lang} on the problem to diagnose.\n{e}\n{traceback.format_exc()}' - ) + errors_before = self.errors + checks.check_statements( + self.statements, + self.problem.metadata, + self.problem.format, + Path(self.problem.probdir), + self.problem.tmpdir, + self._diag, + ) + if self.errors > errors_before: + self._check_res = False return self._check_res @@ -253,7 +185,9 @@ def check(self, context: Context) -> bool: if self._metadata.uuid is None: self.error_in_2023_07(f'Missing uuid from problem.yaml. Add "uuid: {uuid.uuid4()}" to problem.yaml.') - names_with_no_statement = [lang for lang in self._metadata.name if lang not in self.problem.statement.statements] + names_with_no_statement = [ + lang for lang in self._metadata.name if lang not in self.problem.statement.statements.by_language + ] if names_with_no_statement: self.error(f'Names exist for languages without problem statements: {", ".join(names_with_no_statement)}') @@ -307,34 +241,28 @@ def check(self, context: Context) -> bool: class Attachments(ProblemPart): - """Represents the attachments of a problem. - - Attributes: - attachments: The absolute paths to the attachment files for this problem. - """ - - attachments: list[Path] + """Seam to integrate a model + checks setup into verifyproblem in a somewhat clean way""" PART_NAME = 'attachments' def setup(self) -> None: - attachments_dir = Path(self.problem.probdir) / 'attachments' - self.attachments = [p for p in attachments_dir.iterdir()] if attachments_dir.is_dir() else [] - self.debug(f'Adding attachments {self.attachments!s}') + self.attachments = model.load_attachments(Path(self.problem.probdir)) + self.debug(f'Adding attachments {self.attachments.paths!s}') def check(self, context: Context) -> bool: if self._check_res is not None: return self._check_res self._check_res = True - for attachment_path in self.attachments: - if os.path.isdir(attachment_path): - self.error(f'Directories are not allowed as attachments ({attachment_path} is a directory)') + errors_before = self.errors + checks.check_attachments(self.attachments, self._diag) + if self.errors > errors_before: + self._check_res = False return self._check_res def get_attachment_paths(self) -> list[Path]: - return self.attachments + return self.attachments.paths def __str__(self) -> str: return 'attachments'