Add Files to model. Ignore dotfiles in packages

problemtools/checks/problem_package.py

@@ -5,75 +5,68 @@
 import difflib
 import os
 import re
-import stat
 from pathlib import Path
 
-from ..diagnostics import Diagnostics
+from ..diagnostics import Diagnostics, pluralize
 from ..formatversion import FormatVersion
+from ..model import BadReason, EntryKind, Files
 
 _NAME_REGEX = re.compile(r'^[a-zA-Z0-9_][a-zA-Z0-9_.-]{0,254}$')
 
+_SYMLINK_PROBLEMS = {
+    BadReason.ABSOLUTE_SYMLINK: 'which is an absolute path. Symlinks must be relative.',
+    BadReason.OUTSIDE_PACKAGE: 'which is outside of problem package',
+    BadReason.BROKEN_SYMLINK: 'which does not exist',
+    BadReason.SYMLINK_LOOP: 'which creates a symlink loop',
+    BadReason.IGNORED_TARGET: "which is ignored (its name, or the name of a directory it is in, starts with '.')",
+}
 
-def check_problem_package(probdir: Path, format_version: FormatVersion, diag: Diagnostics) -> None:
+
+def check_problem_package(files: Files, format_version: FormatVersion, diag: Diagnostics) -> None:
     """Run all checks on the structure of a problem package."""
-    _check_symlinks(probdir, diag)
-    _check_file_types(probdir, diag)
-    _check_file_and_directory_names(probdir, diag)
-    _check_root_directory_names(probdir, format_version, diag)
-
-
-def _check_symlinks(probdir: Path, diag: Diagnostics) -> None:
-    """Check that all symlinks point to something existing within the problem package"""
-    real_probdir = os.path.realpath(probdir)
-    for root, dirs, files in os.walk(real_probdir):
-        for file in dirs + files:
-            filename = os.path.join(root, file)
-            if os.path.islink(filename):
-                target = os.path.realpath(filename)
-                # relfile is the filename of the symlink, relative to the problem root (only used for nicer error messages)
-                relfile = os.path.relpath(filename, probdir)
-                # reltarget is what the symlink points to (absolute, or relative to where the symlink is)
-                reltarget = os.readlink(filename)
-                if not os.path.exists(target):
-                    diag.error(f'Symlink {relfile} links to {reltarget} which does not exist')
-                if os.path.commonpath([real_probdir, target]) != real_probdir:
-                    diag.error(f'Symlink {relfile} links to {reltarget} which is outside of problem package')
-                if os.path.isabs(reltarget):
-                    diag.error(f'Symlink {relfile} links to {reltarget} which is an absolute path. Symlinks must be relative.')
-
-
-def _check_file_types(probdir: Path, diag: Diagnostics) -> None:
-    """Check that the package only contains regular files, directories and symlinks
-    (symlinks are checked by _check_symlinks), and warn about empty directories."""
-    for root, dirs, files in os.walk(probdir):
-        if root == str(probdir) and '.git' in dirs:
-            dirs.remove('.git')
-        # Path of the directory we're in, starting with problem shortname. Only used for nicer error messages.
-        reldir = os.path.relpath(root, probdir.parent)
-        if not dirs and not files:
-            diag.warning(f'Empty directory {reldir}')
-        for file in files:
-            mode = os.lstat(os.path.join(root, file)).st_mode
-            if not stat.S_ISREG(mode) and not stat.S_ISLNK(mode):
-                diag.error(f"'{file}' in {reldir} is not a regular file, directory or symlink")
-
-
-def _check_file_and_directory_names(probdir: Path, diag: Diagnostics) -> None:
-    def _special_case_allowed_files(file: str, reldir: str) -> bool:
-        return file == '.gitignore' or (file == '.timelimit' and reldir == probdir.name)
-
-    def _special_case_allowed_dirs(directory: str, reldir: str) -> bool:
-        return directory == '.git' and reldir == probdir.name
-
-    for root, dirs, files in os.walk(probdir):
-        # 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 _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 _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}")
+    num_files = len(files.walk(files.root, EntryKind.FILE))
+    num_dirs = len(files.walk(files.root, EntryKind.DIR))
+    msg = f'Checking package structure: {pluralize(num_files, "file")}, {pluralize(num_dirs, "directory", "directories")}'
+    extra = [f'{len(files.ignored)} ignored'] if files.ignored else []
+    extra += [f'{len(files.bad)} invalid'] if files.bad else []
+    if extra:
+        msg += f' ({", ".join(extra)})'
+    diag.msg(msg)
+
+    _check_bad_entries(files, diag)
+    _check_empty_directories(files, diag)
+    _check_file_and_directory_names(files, diag)
+    _check_root_directory_names(files, format_version, diag)
+
+
+def _display_path(files: Files, path: Path) -> Path:
+    """path, starting with the problem shortname. Only used for nicer error messages."""
+    return path.relative_to(files.root.parent)
+
+
+def _check_bad_entries(files: Files, diag: Diagnostics) -> None:
+    """Report entries which can not be part of a valid package: bad symlinks and special files."""
+    for entry in files.bad:
+        if entry.reason is BadReason.SPECIAL_FILE:
+            diag.error(f'{_display_path(files, entry.path)} is not a regular file, directory or symlink')
+        else:
+            target = os.readlink(entry.path)
+            diag.error(f'Symlink {_display_path(files, entry.path)} links to {target} {_SYMLINK_PROBLEMS[entry.reason]}')
+
+
+def _check_empty_directories(files: Files, diag: Diagnostics) -> None:
+    for directory in files.walk(files.root, EntryKind.DIR):
+        if not files.listdir(directory):
+            diag.warning(f'Empty directory {_display_path(files, directory)}')
+
+
+def _check_file_and_directory_names(files: Files, diag: Diagnostics) -> None:
+    for path, kind in files.entries.items():
+        if not _NAME_REGEX.match(path.name):
+            what = 'file' if kind is EntryKind.FILE else 'directory'
+            diag.error(
+                f"Invalid {what} name '{path.name}' in {_display_path(files, path.parent)}, should match {_NAME_REGEX.pattern}"
+            )
 
 
 def _warn_renamed_directory(found_name: str, format_version: FormatVersion, diag: Diagnostics) -> bool:
@@ -88,13 +81,13 @@ def _warn_renamed_directory(found_name: str, format_version: FormatVersion, diag
     return False
 
 
-def _check_root_directory_names(probdir: Path, format_version: FormatVersion, diag: Diagnostics) -> None:
+def _check_root_directory_names(files: Files, 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
-    existing_dirs = {entry.name for entry in probdir.iterdir() if entry.is_dir()}
+    existing_dirs = {path.name for path in files.listdir(files.root, EntryKind.DIR)}
     typo_candidates = known - existing_dirs
 
     for name in sorted(existing_dirs - known):

problemtools/checks/statements.py

@@ -2,7 +2,6 @@
 
 from __future__ import annotations
 
-import glob
 import os
 import traceback
 from pathlib import Path
@@ -11,24 +10,27 @@
 from ..diagnostics import Diagnostics, pluralize
 from ..formatversion import FormatVersion
 from ..metadata import Metadata
-from ..model import Statements
+from ..model import EntryKind, Files, Statements
 
 
 def check_statements(
     statements: Statements,
     metadata: Metadata,
     format_version: FormatVersion,
-    probdir: Path,
+    files: Files,
     work_dir: Path,
     diag: Diagnostics,
 ) -> None:
     """Run all checks on a problem's statements."""
     diag.msg(f'Checking problem statements in {pluralize(len(statements.by_language), "language")}')
 
-    for ifilename in glob.glob(os.path.join(str(probdir), 'data/sample/*.interaction')):
+    for path in files.listdir(files.root / 'data' / 'sample', EntryKind.FILE):
+        if path.suffix != '.interaction':
+            continue
+        ifilename = path.relative_to(files.root)
         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:
+        with open(path, '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):
@@ -52,9 +54,11 @@ def check_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)}')
+    for lang, statement_files in statements.by_language.items():
+        if len(statement_files) > 1:
+            diag.error(
+                f'Found multiple statements in the same language {lang}: {", ".join(file.name for file in statement_files)}'
+            )
 
         if lang not in metadata.name:
             diag.error(f'No problem name given in language {lang}')
@@ -65,10 +69,10 @@ def _latex_heuristic(name: str) -> bool:
         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:
+        for file in statement_files:
             try:
                 options = problem2pdf.get_parser().parse_args([''])
-                options.problem = probdir
+                options.problem = files.root
                 options.language = lang
                 options.nopdf = True
                 options.quiet = True
@@ -82,7 +86,7 @@ def _latex_heuristic(name: str) -> bool:
 
             try:
                 options = problem2html.get_parser().parse_args([''])
-                options.problem = probdir
+                options.problem = files.root
                 options.destdir = os.path.join(work_dir, 'html')
                 options.language = lang
                 options.quiet = True

problemtools/checks/testdata.py

@@ -3,9 +3,7 @@
 from __future__ import annotations
 
 import collections
-import glob
 import hashlib
-import os
 from collections.abc import Callable, Iterator
 from pathlib import Path
 
@@ -18,6 +16,8 @@
     DEFAULT_CONFIG,
     DEFAULT_GRADER,
     SCORING_ONLY_KEYS,
+    EntryKind,
+    Files,
     Graders,
     InputValidators,
     OutputValidators,
@@ -32,7 +32,7 @@ def check_testdata(
     testdata: TestDataGroup,
     context: Context,
     metadata: Metadata,
-    probdir: Path,
+    files: Files,
     graders: Graders,
     input_validators: InputValidators,
     output_validators: OutputValidators,
@@ -72,7 +72,7 @@ def check_testdata(
         testdata,
         context,
         metadata,
-        probdir,
+        files,
         has_custom_grader,
         has_default_grader,
         input_validation,
@@ -221,7 +221,7 @@ def _check_group(
     group: TestDataGroup,
     context: Context,
     metadata: Metadata,
-    probdir: Path,
+    files: Files,
     has_custom_grader: bool,
     has_default_grader: bool,
     input_validation: InputValidationCache,
@@ -278,48 +278,39 @@ def _check_group(
         if not seen_sample:
             diag.warning('No sample data provided')
 
-        hashes = collections.defaultdict(list)
-        for root, _dirs, files in os.walk(group.datadir):
-            for filename in files:
-                filepath = os.path.join(root, filename)
-                if filepath.endswith('.in') and not os.path.islink(filepath):
-                    md5 = hashlib.md5(usedforsecurity=False)
-                    with open(filepath, 'rb') as f:
-                        for buf in iter(lambda: f.read(1024), b''):
-                            md5.update(buf)
-                    filehash = md5.digest()
-                    hashes[filehash].append(os.path.relpath(filepath, probdir))
-        for files in hashes.values():
-            if len(files) > 1:
-                diag.warning(f"Identical input files: '{files!s}'")
-
-    infiles = glob.glob(os.path.join(str(group.datadir), '*.in'))
-    ansfiles = glob.glob(os.path.join(str(group.datadir), '*.ans'))
-
-    for infile in infiles:
-        if os.path.isdir(infile):
-            continue
-        if f'{infile[:-3]}.ans' not in ansfiles:
-            diag.error(f"No matching answer file for input '{infile}'")
-    for ansfile in ansfiles:
-        if os.path.isdir(ansfile):
-            continue
-        if f'{ansfile[:-4]}.in' not in infiles:
-            diag.error(f"No matching input file for answer '{ansfile}'")
+        hashes: dict[bytes, list[Path]] = collections.defaultdict(list)
+        for testcase in group.get_all_testcases():
+            if testcase.infile.is_symlink():
+                continue
+            with open(testcase.infile, 'rb') as f:
+                digest = hashlib.file_digest(f, lambda: hashlib.sha256(usedforsecurity=False)).digest()
+            hashes[digest].append(testcase.infile.relative_to(files.root))
+        for identical in hashes.values():
+            if len(identical) > 1:
+                diag.warning(
+                    f'Identical input files: {", ".join(str(path) for path in identical)}. If this is intentional, use symlinks.'
+                )
+
+    for path in files.listdir(group.datadir, EntryKind.FILE):
+        if path.suffix == '.in' and not files.is_file(path.with_suffix('.ans')):
+            diag.error(f"No matching answer file for input '{path.relative_to(files.root)}'")
+        elif path.suffix == '.ans' and not files.is_file(path.with_suffix('.in')):
+            diag.error(f"No matching input file for answer '{path.relative_to(files.root)}'")
 
     if not group.get_subgroups() and not group.get_testcases():
+        group_dir = group.datadir.relative_to(files.root)
         if group.datadir.name != 'sample':
-            diag.error(f'Testcase group {group.datadir} exists, but does not contain any testcases')
+            diag.error(f'Testcase group {group_dir} exists, but does not contain any testcases')
         else:
             if not (
                 (metadata.is_interactive() or metadata.is_multi_pass())
-                and glob.glob(os.path.join(str(group.datadir), '*.interaction'))
+                and any(path.suffix == '.interaction' for path in files.listdir(group.datadir, EntryKind.FILE))
             ):
-                diag.warning(f'Sample testcase group {group.datadir} exists, but does not contain any testcases')
+                diag.warning(f'Sample testcase group {group_dir} exists, but does not contain any testcases')
 
     last_testgroup_name = ''
     for subgroup in group.get_subgroups():
-        name = os.path.relpath(subgroup.datadir, probdir)
+        name = str(subgroup.datadir.relative_to(files.root))
         if _natural_sort_le(name, last_testgroup_name):
             diag.warning(f"Test data group '{last_testgroup_name}' will be ordered before '{name}'; consider zero-padding")
         last_testgroup_name = name
@@ -332,7 +323,7 @@ def _check_group(
                 child,
                 context,
                 metadata,
-                probdir,
+                files,
                 has_custom_grader,
                 has_default_grader,
                 input_validation,

problemtools/diagnostics.py

@@ -14,9 +14,12 @@ class VerifyError(Exception):
     pass
 
 
-def pluralize(n: int, word: str) -> str:
-    """Format a count together with a regular English noun, e.g. pluralize(3, 'submission') -> '3 submissions'."""
-    return f'{n} {word}' if n == 1 else f'{n} {word}s'
+def pluralize(n: int, word: str, plural: str | None = None) -> str:
+    """Format a count together with an English noun, e.g. pluralize(3, 'submission') -> '3 submissions'.
+
+    plural is the plural form of word, for irregular nouns (defaults to word + 's').
+    """
+    return f'{n} {word}' if n == 1 else f'{n} {plural or word + "s"}'
 
 
 class Diagnostics(ABC):

problemtools/model/__init__.py

@@ -4,6 +4,7 @@
 Verdict = Literal['AC', 'TLE', 'OLE', 'MLE', 'RTE', 'WA', 'PAC', 'JE']
 
 from .attachments import Attachments, load_attachments
+from .files import BadEntry, BadReason, EntryKind, Files, load_files
 from .graders import DEFAULT_GRADER, Graders, load_graders
 from .includes import DEFAULT_LANGUAGE, Includes, LanguageIncludes, load_includes
 from .problem import Problem, load_problem
@@ -20,6 +21,10 @@
     'DEFAULT_VALIDATOR',
     'SCORING_ONLY_KEYS',
     'Attachments',
+    'BadEntry',
+    'BadReason',
+    'EntryKind',
+    'Files',
     'Graders',
     'Includes',
     'InputValidators',
@@ -36,6 +41,7 @@
     'TestDataGroup',
     'Verdict',
     'load_attachments',
+    'load_files',
     'load_graders',
     'load_includes',
     'load_input_validators',