From a9e88b9cd67abe8bca421389cdb905a654a8ad7a Mon Sep 17 00:00:00 2001 From: Francesco Vercellesi Date: Fri, 15 May 2026 14:58:00 +0200 Subject: [PATCH 1/6] The checker must now output printable UTF-8 --- task-maker-format/src/ioi/dag/checker.rs | 46 ++++++++++++++++++++---- 1 file changed, 40 insertions(+), 6 deletions(-) diff --git a/task-maker-format/src/ioi/dag/checker.rs b/task-maker-format/src/ioi/dag/checker.rs index c88c60022..2bcc54e35 100644 --- a/task-maker-format/src/ioi/dag/checker.rs +++ b/task-maker-format/src/ioi/dag/checker.rs @@ -96,7 +96,7 @@ impl Checker { group.priority = EVALUATION_PRIORITY - testcase_id.unwrap_or_default() as Priority; let sender = eval.sender.clone(); eval.dag.on_execution_done(&group.uuid, move |results| { - let res = &results[0]; + let res = &results[0]; let stdout = res .stdout .as_ref() @@ -105,8 +105,6 @@ impl Checker { .stderr .as_ref() .ok_or_else(|| anyhow!("Checker stderr not captured"))?; - let message = String::from_utf8_lossy(stderr).trim().to_string(); - let message = Self::translate_checker_message(message); if !res.status.is_success() { let message = if let Some(testcase_id) = testcase_id { format!( @@ -122,9 +120,30 @@ impl Checker { sender.add_diagnostic(diagnostic)?; return Ok(()); } - let score = String::from_utf8_lossy(stdout); - let score: f64 = match score.trim().parse() { - Ok(score) => score, + let message = str::from_utf8(stderr) + .map(|s| Self::translate_checker_message(s.trim().to_string())); + let score = str::from_utf8(stdout) + .map(|s| s.trim().parse::()); + + let Ok(score) = score else { + let message = "The checked return a non UTF-8 score".to_string(); + let diagnostic = Diagnostic::error(message) + .with_note(description) + .with_help_attachment(stdout.clone()); + sender.add_diagnostic(diagnostic)?; + return Ok(()); + }; + let Ok(message) = message else { + let message = "The checked return a non UTF-8 message".to_string(); + let diagnostic = Diagnostic::error(message) + .with_note(description) + .with_help_attachment(stdout.clone()); + sender.add_diagnostic(diagnostic)?; + return Ok(()); + }; + + let score: f64 = match &score { + Ok(score) => *score, Err(e) => { let message = if let Some(testcase_id) = testcase_id { format!( @@ -141,6 +160,16 @@ impl Checker { return Ok(()); } }; + + if !Self::is_unicode_printable(&message) { + let message = format!("The checked return a non printable message ({message})"); + let diagnostic = Diagnostic::error(message) + .with_note(description) + .with_help_attachment(stdout.clone()); + sender.add_diagnostic(diagnostic)?; + return Ok(()); + } + callback(score, message) }); Ok(group) @@ -195,6 +224,11 @@ impl Checker { Ok(()) } + /// Checks that a string is a printable message without control characters. + fn is_unicode_printable(s: &str) -> bool { + s.chars().all(|c| !c.is_control() || c.is_whitespace()) + } + /// The checker may return a message to be translated. This function maps the message /// placeholders to actual messages. pub fn translate_checker_message(message: String) -> String { From 3b6f4a49ee3b88408264624980661f669d07bf6c Mon Sep 17 00:00:00 2001 From: Francesco Vercellesi Date: Fri, 15 May 2026 16:07:23 +0200 Subject: [PATCH 2/6] fixed grader/stub difference in attachments --- .../src/ioi/sanity_checks/att.rs | 12 +++++++----- .../src/ioi/sanity_checks/mod.rs | 19 ++++++------------- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/task-maker-format/src/ioi/sanity_checks/att.rs b/task-maker-format/src/ioi/sanity_checks/att.rs index 9007fc632..d19d00391 100644 --- a/task-maker-format/src/ioi/sanity_checks/att.rs +++ b/task-maker-format/src/ioi/sanity_checks/att.rs @@ -1,4 +1,5 @@ use std::collections::{BTreeSet, HashMap}; +use std::ffi::OsStr; use std::io::{Read, Seek, SeekFrom}; use std::path::PathBuf; use std::sync::{Arc, Mutex}; @@ -552,11 +553,12 @@ impl SanityCheck for AttTemplatesShouldCompile { let att_name = format!("att/{}.{}", task.name, ext); let template = task.path.join(&att_name); - let grader_name = grader - .file_name() - .ok_or_else(|| anyhow!("Grader has no file name"))? - .to_string_lossy(); - let att_grader_name = format!("att/{grader_name}"); + let grader_ext= grader + .extension() + .map(OsStr::to_str) + .flatten() + .unwrap_or(""); + let att_grader_name = format!("att/grader.{grader_ext}",); let att_grader = task.path.join(&att_grader_name); // Only run the check if the grader is not a symlink, as otherwise we are already diff --git a/task-maker-format/src/ioi/sanity_checks/mod.rs b/task-maker-format/src/ioi/sanity_checks/mod.rs index 81288c67c..761c7ca9d 100644 --- a/task-maker-format/src/ioi/sanity_checks/mod.rs +++ b/task-maker-format/src/ioi/sanity_checks/mod.rs @@ -45,27 +45,20 @@ fn check_missing_graders>( if !has_grader(task) { return Ok(()); } - // some task formats use stub.* others use grader.* - // To avoid confusion emit warnings only either for stubs or graders. - let is_stub = task - .grader_map - .all_paths() - .filter_map(|p| p.file_stem()) - .any(|p| p == "stub"); let mut by_ext = HashMap::new(); for file in list_files(task.path.join(folder.as_ref()), vec!["*.*"]) { let file = task.path_of(&file); - let stem = match file.file_stem() { - Some(stem) => stem, - None => continue, + let Some(stem) = file.file_stem() else { + continue; }; - // do not check the graders - if stem == "grader" || stem == "stub" { + // Only check if the file name is that of a template + if *stem != *task.name { continue; } if let Some(lang) = LanguageManager::detect_language(file) { let ext = lang.extensions()[0]; - let name = format!("{}.{}", if is_stub { "stub" } else { "grader" }, ext); + // Even if some tasks want `stubs`, we always check for `grader`s. + let name = format!("grader.{ext}"); let grader_name = file.with_file_name(name); let grader_path = task.path.join(&grader_name); by_ext.insert(ext, (grader_path, grader_name, file.to_owned())); From 0b3f4a0060d47ef28aca37fdd38458adaa9168f9 Mon Sep 17 00:00:00 2001 From: Francesco Vercellesi Date: Fri, 15 May 2026 16:47:44 +0200 Subject: [PATCH 3/6] Added warning for st checks on samples --- .../src/ioi/sanity_checks/checker.rs | 10 +++- .../src/ioi/sanity_checks/sol.rs | 57 ++++++++++++++++--- 2 files changed, 57 insertions(+), 10 deletions(-) diff --git a/task-maker-format/src/ioi/sanity_checks/checker.rs b/task-maker-format/src/ioi/sanity_checks/checker.rs index 0c7cf02a9..ccb1b006e 100644 --- a/task-maker-format/src/ioi/sanity_checks/checker.rs +++ b/task-maker-format/src/ioi/sanity_checks/checker.rs @@ -81,9 +81,13 @@ impl SanityCheck for FuzzCheckerWithJunkOutput { test_output_uuid, move |score, outcome| { if score != 0.0 { - sender.add_diagnostic(Diagnostic::error(format!( - "Junk file '{description}' scored {score} (with message '{outcome}')" - )))?; + sender.add_diagnostic( + Diagnostic::warning(format!( + "Junk file '{description}' scored {score} (with message '{outcome}')" + )).with_note( + "This almost certainly indicates a bug in the checker, unless it's intended that such a file scores points." + ) + )?; } Ok(()) }, diff --git a/task-maker-format/src/ioi/sanity_checks/sol.rs b/task-maker-format/src/ioi/sanity_checks/sol.rs index bf891cdc0..b5013bc79 100644 --- a/task-maker-format/src/ioi/sanity_checks/sol.rs +++ b/task-maker-format/src/ioi/sanity_checks/sol.rs @@ -81,12 +81,10 @@ impl SanityCheck for SolutionsWithNoChecks { } fn pre_hook(&self, task: &IOITask, eval: &mut EvaluationData) -> Result<(), Error> { - for subtask in task.subtasks.values() { - if subtask.name.is_none() { - // If not all the subtasks have a name, do not bother with the solutions, it's much - // more important to give everything a name before. - return Ok(()); - } + // If not all the subtasks have a name, do not bother with the solutions, it's much + // more important to give everything a name before. + if task.subtasks.values().any(|st| st.name.is_none()) { + return Ok(()); } let mut solutions = vec![]; @@ -111,7 +109,52 @@ impl SanityCheck for SolutionsWithNoChecks { "The following solutions are missing the subtask checks: {}", solutions.join(", ") )) - .with_help("Try running task-maker-tools add-solution-checks"), + )?; + } + Ok(()) + } +} + +/// Check that all the solutions (that are not symlinks) contain at least one check. +#[derive(Debug, Default)] +pub struct SolutionsWithSamplesCheck; +make_sanity_check!(SolutionsWithSamplesCheck); + +impl SanityCheck for SolutionsWithSamplesCheck { + type Task = IOITask; + + fn name(&self) -> &'static str { + "SolutionsWithSamplesChecks" + } + + fn category(&self) -> SanityCheckCategory { + SanityCheckCategory::Solutions + } + + fn pre_hook(&self, task: &IOITask, eval: &mut EvaluationData) -> Result<(), Error> { + // If no first subtask exists, or it has no name, skip this check. + let Some(Some(samples)) = task.subtasks.get(&0).map(|st| st.name.clone()) else { + return Ok(()); + }; + + let mut solutions = vec![]; + for solution in eval.solutions.iter() { + let samples_checked = solution.checks.iter().any(|check| check.subtask_name_pattern == samples); + if samples_checked { + solutions.push(format!( + "{}", + solution.source_file.relative_path().display() + )) + } + } + if !solutions.is_empty() { + eval.add_diagnostic( + Diagnostic::warning(format!( + "The following solutions have solution checks for the samples: {}", + solutions.join(", ") + )).with_note( + "Subtask checks on samples are rarely useful." + ) )?; } Ok(()) From fcd580d6ba86490465a6ac975eb84e533e23cd16 Mon Sep 17 00:00:00 2001 From: Francesco Vercellesi Date: Fri, 15 May 2026 16:49:50 +0200 Subject: [PATCH 4/6] Removed st ordering check --- .../src/ioi/sanity_checks/task.rs | 140 +----------------- 1 file changed, 1 insertion(+), 139 deletions(-) diff --git a/task-maker-format/src/ioi/sanity_checks/task.rs b/task-maker-format/src/ioi/sanity_checks/task.rs index fe7420081..164428598 100644 --- a/task-maker-format/src/ioi/sanity_checks/task.rs +++ b/task-maker-format/src/ioi/sanity_checks/task.rs @@ -1,4 +1,3 @@ -use std::collections::{HashMap, HashSet}; use std::fs; use anyhow::Error; @@ -7,7 +6,7 @@ use task_maker_diagnostics::{CodeSpan, Diagnostic}; use crate::ioi::IOITask; use crate::sanity_checks::{make_sanity_check, SanityCheck, SanityCheckCategory}; -use crate::{list_files, EvaluationData, SolutionCheckResult}; +use crate::{list_files, EvaluationData}; /// The default maximum score of a task. const DEFAULT_TASK_MAX_SCORE: f64 = 100.0; @@ -129,143 +128,6 @@ impl SanityCheck for NoBitsStdCpp { } } -/// Check that "implied" subtasks dependecies do not form cycles. -#[derive(Debug, Default)] -pub struct SubtaskDependencies; -make_sanity_check!(SubtaskDependencies); - -impl SanityCheck for SubtaskDependencies { - type Task = IOITask; - - fn name(&self) -> &'static str { - "SubtaskDependencies" - } - - fn category(&self) -> SanityCheckCategory { - SanityCheckCategory::Task - } - - fn pre_hook(&self, task: &Self::Task, eval: &mut EvaluationData) -> Result<(), Error> { - let non_zero_sts = task - .subtasks - .keys() - .copied() - .filter(|st| task.subtasks[st].max_score > 0.0) - .collect::>(); - - let mut st_dependencies = HashSet::new(); - for &st1 in &non_zero_sts { - for &st2 in &non_zero_sts { - if st1 != st2 { - st_dependencies.insert((st1, st2)); - } - } - } - - let mut any_st_check = false; - for sol in &eval.solutions { - let mut score_range = HashMap::new(); - for check in &sol.checks { - any_st_check = true; - - let val = match check.result { - SolutionCheckResult::Accepted => (1.0, 1.0), - SolutionCheckResult::PartialScore => (0.0, 1.0), - _ => (0.0, 0.0), - }; - - for subtask in task.find_subtasks_by_pattern_name(&check.subtask_name_pattern) { - score_range.insert(subtask.id, val); - } - } - - for &st1 in &non_zero_sts { - for &st2 in &non_zero_sts { - if let (Some(v1), Some(v2)) = (score_range.get(&st1), score_range.get(&st2)) { - if v1.1 > v2.0 { - st_dependencies.remove(&(st1, st2)); - } - } - } - } - } - - if !any_st_check { - return Ok(()); - } - - 'outer: for &st1 in &non_zero_sts { - let mut sts = vec![st1]; - for &st2 in &non_zero_sts { - if st_dependencies.contains(&(st1, st2)) && st_dependencies.contains(&(st2, st1)) { - if st1 < st2 { - sts.push(st2); - } else { - continue 'outer; - } - } - } - if sts.len() > 1 { - sts.sort(); - let st_names: Vec<_> = sts - .iter() - .map(|st| { - task.subtasks[st] - .name - .as_ref() - .map(|s| s as &dyn std::fmt::Debug) - .unwrap_or(st) - }) - .collect(); - eval.add_diagnostic( - Diagnostic::warning(format!( - "Subtasks {st_names:?} are solved by the same set of solutions", - )) - .with_note("Add a solution that solves only one of them"), - )?; - } - } - - let mut to_swap = Vec::new(); - for &st1 in &non_zero_sts { - for &st2 in &non_zero_sts { - if st1 < st2 - && st_dependencies.contains(&(st1, st2)) - && !st_dependencies.contains(&(st2, st1)) - { - to_swap.push((st1, st2)); - } - } - } - - if !to_swap.is_empty() { - let to_swap_names = to_swap - .iter() - .map(|(st1, st2)| { - let st1_name = task.subtasks[st1] - .name - .as_ref() - .map(|s| s as &dyn std::fmt::Debug) - .unwrap_or(st1); - let st2_name = task.subtasks[st2] - .name - .as_ref() - .map(|s| s as &dyn std::fmt::Debug) - .unwrap_or(st2); - (st1_name, st2_name) - }) - .collect::>(); - eval.add_diagnostic( - Diagnostic::warning("Subtasks are not in order of difficulty").with_note(format!( - "Based on the current solutions the following pairs of subtasks seems to be ordered incorrectly {to_swap_names:?}" - )), - )?; - } - - Ok(()) - } -} - /// Check that the task title is not emtpy. #[derive(Debug, Default)] pub struct EmptyTitle; From a6577e2b41e49ace6a1159c6ff892d0a3ff13048 Mon Sep 17 00:00:00 2001 From: Francesco Vercellesi Date: Fri, 15 May 2026 17:01:12 +0200 Subject: [PATCH 5/6] clippy and format --- .../src/ioi/sanity_checks/att.rs | 6 +----- .../src/ioi/sanity_checks/sol.rs | 20 +++++++++---------- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/task-maker-format/src/ioi/sanity_checks/att.rs b/task-maker-format/src/ioi/sanity_checks/att.rs index d19d00391..84a3061ea 100644 --- a/task-maker-format/src/ioi/sanity_checks/att.rs +++ b/task-maker-format/src/ioi/sanity_checks/att.rs @@ -553,11 +553,7 @@ impl SanityCheck for AttTemplatesShouldCompile { let att_name = format!("att/{}.{}", task.name, ext); let template = task.path.join(&att_name); - let grader_ext= grader - .extension() - .map(OsStr::to_str) - .flatten() - .unwrap_or(""); + let grader_ext = grader.extension().and_then(OsStr::to_str).unwrap_or(""); let att_grader_name = format!("att/grader.{grader_ext}",); let att_grader = task.path.join(&att_grader_name); diff --git a/task-maker-format/src/ioi/sanity_checks/sol.rs b/task-maker-format/src/ioi/sanity_checks/sol.rs index b5013bc79..854fc348b 100644 --- a/task-maker-format/src/ioi/sanity_checks/sol.rs +++ b/task-maker-format/src/ioi/sanity_checks/sol.rs @@ -104,12 +104,10 @@ impl SanityCheck for SolutionsWithNoChecks { )) } if !solutions.is_empty() { - eval.add_diagnostic( - Diagnostic::warning(format!( - "The following solutions are missing the subtask checks: {}", - solutions.join(", ") - )) - )?; + eval.add_diagnostic(Diagnostic::warning(format!( + "The following solutions are missing the subtask checks: {}", + solutions.join(", ") + )))?; } Ok(()) } @@ -139,7 +137,10 @@ impl SanityCheck for SolutionsWithSamplesCheck { let mut solutions = vec![]; for solution in eval.solutions.iter() { - let samples_checked = solution.checks.iter().any(|check| check.subtask_name_pattern == samples); + let samples_checked = solution + .checks + .iter() + .any(|check| check.subtask_name_pattern == samples); if samples_checked { solutions.push(format!( "{}", @@ -152,9 +153,8 @@ impl SanityCheck for SolutionsWithSamplesCheck { Diagnostic::warning(format!( "The following solutions have solution checks for the samples: {}", solutions.join(", ") - )).with_note( - "Subtask checks on samples are rarely useful." - ) + )) + .with_note("Subtask checks on samples are rarely useful."), )?; } Ok(()) From 3667d075b0b90230c216e259bca5f484270a7cf3 Mon Sep 17 00:00:00 2001 From: Francesco Vercellesi Date: Fri, 15 May 2026 17:21:35 +0200 Subject: [PATCH 6/6] fixed tests --- task-maker-format/tests/ioi_sanity_checks.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/task-maker-format/tests/ioi_sanity_checks.rs b/task-maker-format/tests/ioi_sanity_checks.rs index 09d303849..106041d3e 100644 --- a/task-maker-format/tests/ioi_sanity_checks.rs +++ b/task-maker-format/tests/ioi_sanity_checks.rs @@ -64,7 +64,7 @@ fn test_sanity_checks_att_graders() { std::fs::create_dir(tmpdir.path().join("sol")).unwrap(); std::fs::create_dir(tmpdir.path().join("att")).unwrap(); std::fs::write(tmpdir.path().join("sol/grader.cpp"), "x").unwrap(); - std::fs::write(tmpdir.path().join("att/template.cpp"), "x").unwrap(); + std::fs::write(tmpdir.path().join("att/task.cpp"), "x").unwrap(); task.grader_map = Arc::new(GraderMap::new(vec![tmpdir.path().join("sol/grader.cpp")])); let warnings = get_warnings(&task); @@ -185,8 +185,8 @@ fn test_sanity_checks_sol_graders() { let mut task = utils::new_task_with_context(tmpdir.path()); std::fs::create_dir(tmpdir.path().join("sol")).unwrap(); std::fs::write(tmpdir.path().join("sol/grader.cpp"), "x").unwrap(); - std::fs::write(tmpdir.path().join("sol/template.cpp"), "x").unwrap(); - std::fs::write(tmpdir.path().join("sol/template.c"), "x").unwrap(); + std::fs::write(tmpdir.path().join("sol/task.cpp"), "x").unwrap(); + std::fs::write(tmpdir.path().join("sol/task.c"), "x").unwrap(); task.grader_map = Arc::new(GraderMap::new(vec![tmpdir.path().join("sol/grader.cpp")])); let warnings = get_warnings(&task);