diff --git a/Cargo.lock b/Cargo.lock index 4475d92..d27621d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2367,6 +2367,7 @@ version = "0.8.23" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dc1beb996b9d83529a9e75c17a1686767d148d70663143c7854d8b4a09ced362" dependencies = [ + "indexmap", "serde", "serde_spanned", "toml_datetime", diff --git a/crates/cph-model/Cargo.toml b/crates/cph-model/Cargo.toml index 6953601..145ca5c 100644 --- a/crates/cph-model/Cargo.toml +++ b/crates/cph-model/Cargo.toml @@ -7,4 +7,6 @@ license.workspace = true [dependencies] cph-diag = { workspace = true } serde = { workspace = true } -toml = { workspace = true } +# `preserve_order` keeps `[targets.*]` tables in TOML document order, which is +# the declared order this crate exposes via `Lesson.targets` (ADR-0009). +toml = { workspace = true, features = ["preserve_order"] } diff --git a/crates/cph-model/src/lib.rs b/crates/cph-model/src/lib.rs index 13263ce..2b46004 100644 --- a/crates/cph-model/src/lib.rs +++ b/crates/cph-model/src/lib.rs @@ -34,13 +34,93 @@ pub struct Lesson { pub info: Info, /// The ordered parts — the lesson's element sequence. pub parts: Vec, - /// Declared export target names, e.g. `["student", "teacher"]`, collected - /// from the `[targets.]` tables. - pub targets: Vec, + /// Declared export targets, collected from the `[targets.]` tables. + /// + /// Per ADR-0009 an export target is a *build* producing a typed artifact, + /// with a file-resident, overridable presentation config. This carries the + /// file-resident slice of that (artifact choice + presentation overrides), + /// in declared (TOML document) order. See [`TargetConfig`]. + pub targets: Vec, /// Engineering-file root (absolute), for resolving part paths. pub root: PathBuf, } +impl Lesson { + /// The declared export-target names, in declared order. + /// + /// Convenience for callers that only need the names (the shape this crate + /// exposed before ADR-0009 turned `targets` into structured + /// [`TargetConfig`]s). + pub fn target_names(&self) -> Vec<&str> { + self.targets.iter().map(|t| t.name.as_str()).collect() + } +} + +/// One declared export target's *file-resident* build config (ADR-0009). +/// +/// An export target is a build producing a typed [`ArtifactKind`]; its defaults +/// are seeded at project creation and **overridable in the engineering file**. +/// This struct models the slice of that build config that lives in the file: +/// the artifact choice and presentation overrides (currently heading +/// numbering). The per-kind render *map* itself stays in the render package +/// (`Spec.Courseware.TargetSpec.renders`), not here. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct TargetConfig { + /// Target name, e.g. `"student"` (the `[targets.]` table key). + pub name: String, + /// The artifact shape this target produces. Defaults to + /// [`ArtifactKind::SingleFile`] when the `artifact` field is absent. + pub artifact: ArtifactKind, + /// Presentation override for this target. `None` when no presentation + /// config is given in the file (the render layer applies its defaults). + pub numbering: Option, +} + +/// The shape of the artifact an export target produces (ADR-0009). +/// +/// **Mirrors `Spec.Courseware.Artifact`** in the Lean semantic master +/// (`spec/Spec/Courseware/Artifact.lean`), whose definition is exactly: +/// +/// ```text +/// inductive Artifact where +/// | singleFile +/// | fileTree +/// ``` +/// +/// `SingleFile` is one bundled document (a 讲义 / 教案 PDF); `FileTree` is a +/// tree of files plus an index (a third-party-platform archive). The on-disk +/// `artifact` string maps `"single-file"` → [`ArtifactKind::SingleFile`] and +/// `"file-tree"` → [`ArtifactKind::FileTree`]. +/// +/// As with `cph-diag`'s `Severity`, there is no CI gate enforcing this +/// alignment (see the repo constitution) — it is maintained by review, which is +/// why the correspondence is documented here. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +pub enum ArtifactKind { + /// One bundled document. Mirrors Lean `Artifact.singleFile`. Default. + SingleFile, + /// A tree of files plus an index. Mirrors Lean `Artifact.fileTree`. + FileTree, +} + +impl ArtifactKind { + /// The default artifact shape when `artifact` is omitted: `SingleFile`. + const DEFAULT: ArtifactKind = ArtifactKind::SingleFile; +} + +/// File-resident presentation overrides for a target (ADR-0009). +/// +/// Each field is `Option` so **absence is distinguishable from a value**: when +/// a field is `None`, the render package applies its framework default; only an +/// explicitly-given value overrides it. This crate deliberately invents no +/// presentation defaults — that ownership stays in the render layer. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct NumberingConfig { + /// Per-heading-level numbering patterns (index 0 = level 1, etc.), e.g. + /// `["{1:一}、", "{1:1}.{2:1}"]`. `None` when `numbering.heading` is absent. + pub heading: Option>, +} + /// `[project]` table. #[derive(Debug, Clone, PartialEq, Eq, Serialize)] pub struct Project { @@ -215,9 +295,15 @@ pub fn load(root: &Path) -> (Option, Vec) { } }; - // Target names are just the keys of the [targets.*] table; order is not - // semantically meaningful, but we preserve the TOML document order. - let targets: Vec = raw.targets.keys().cloned().collect(); + // Parse each [targets.] table into a structured build config + // (ADR-0009). Order is the TOML document order, as before. Malformed + // target config is non-fatal: it is reported and the target is kept with + // defaults so loading continues. + let targets: Vec = raw + .targets + .into_iter() + .map(|(name, value)| parse_target(name, value, &mut diags)) + .collect(); // Load each part. Problems are collected, not fatal: we still build the // Part (with a best-effort descriptor) so order/membership is observable. @@ -399,6 +485,149 @@ fn load_descriptor( } } +/// Parse one `[targets.]` table into a [`TargetConfig`] (ADR-0009), +/// collecting [`DiagCode::SchemaViolation`] diagnostics for malformed fields. +/// +/// Malformed config is **non-fatal**: a bad `artifact` value falls back to the +/// default ([`ArtifactKind::SingleFile`]); a malformed `numbering.heading` +/// leaves the override absent — both reported, never dropping the target. An +/// empty `[targets.]` body is valid (all defaults, no diagnostics). +fn parse_target(name: String, value: toml::Value, diags: &mut Vec) -> TargetConfig { + // A target body must be a table; anything else (`student = 1`) is malformed. + let mut table = match value { + toml::Value::Table(t) => t, + _ => { + diags.push( + Diagnostic::error( + DiagCode::SchemaViolation, + format!("target '{name}' must be a table, e.g. `[targets.{name}]`"), + ) + .with_hint("declare a target as a `[targets.]` table"), + ); + return TargetConfig { + name, + artifact: ArtifactKind::DEFAULT, + numbering: None, + }; + } + }; + + let artifact = match table.remove("artifact") { + None => ArtifactKind::DEFAULT, + Some(toml::Value::String(s)) => match s.as_str() { + "single-file" => ArtifactKind::SingleFile, + "file-tree" => ArtifactKind::FileTree, + other => { + diags.push( + Diagnostic::error( + DiagCode::SchemaViolation, + format!( + "target '{name}' has an invalid artifact '{other}'; \ + valid values are \"single-file\", \"file-tree\"" + ), + ) + .with_hint("set `artifact` to \"single-file\" or \"file-tree\""), + ); + ArtifactKind::DEFAULT + } + }, + Some(_) => { + diags.push( + Diagnostic::error( + DiagCode::SchemaViolation, + format!( + "target '{name}' has a non-string `artifact`; \ + valid values are \"single-file\", \"file-tree\"" + ), + ) + .with_hint("set `artifact` to \"single-file\" or \"file-tree\""), + ); + ArtifactKind::DEFAULT + } + }; + + let numbering = parse_numbering(&name, &mut table, diags); + + TargetConfig { + name, + artifact, + numbering, + } +} + +/// Parse the optional `[targets..numbering]` sub-table into a +/// [`NumberingConfig`]. Returns `None` when no `numbering` table is present. +/// A malformed `heading` (not an array of strings) is reported as a +/// [`DiagCode::SchemaViolation`] and left absent (`None`). +fn parse_numbering( + name: &str, + table: &mut toml::Table, + diags: &mut Vec, +) -> Option { + let numbering = table.remove("numbering")?; + + let mut numbering_table = match numbering { + toml::Value::Table(t) => t, + _ => { + diags.push( + Diagnostic::error( + DiagCode::SchemaViolation, + format!("target '{name}' has a non-table `numbering`"), + ) + .with_hint(format!("declare numbering as `[targets.{name}.numbering]`")), + ); + return Some(NumberingConfig { heading: None }); + } + }; + + let heading = match numbering_table.remove("heading") { + None => None, + Some(toml::Value::Array(items)) => { + let mut patterns = Vec::with_capacity(items.len()); + let mut malformed = false; + for item in items { + match item { + toml::Value::String(s) => patterns.push(s), + _ => { + malformed = true; + break; + } + } + } + if malformed { + diags.push( + Diagnostic::error( + DiagCode::SchemaViolation, + format!( + "target '{name}' has a `numbering.heading` that is not an array of strings" + ), + ) + .with_hint( + "`numbering.heading` must be an array of per-level pattern strings", + ), + ); + None + } else { + Some(patterns) + } + } + Some(_) => { + diags.push( + Diagnostic::error( + DiagCode::SchemaViolation, + format!( + "target '{name}' has a `numbering.heading` that is not an array of strings" + ), + ) + .with_hint("`numbering.heading` must be an array of per-level pattern strings"), + ); + None + } + }; + + Some(NumberingConfig { heading }) +} + /// True if `path` contains any `..` component (parent-dir traversal). fn has_parent_traversal(path: &Path) -> bool { path.components().any(|c| matches!(c, Component::ParentDir)) diff --git a/crates/cph-model/tests/fixtures/bad-target-config/manifest.toml b/crates/cph-model/tests/fixtures/bad-target-config/manifest.toml new file mode 100644 index 0000000..d47aa1b --- /dev/null +++ b/crates/cph-model/tests/fixtures/bad-target-config/manifest.toml @@ -0,0 +1,16 @@ +[project] +id = "fixture-bad-target-config" +name = "bad-target-config" + +[info] +title = "测试课：非法导出目标配置" + +# Invalid artifact value → SchemaViolation, falls back to default (single-file). +[targets.student] +artifact = "pdf-thing" + +# numbering.heading is not an array of strings → SchemaViolation, heading None. +[targets.teacher] +artifact = "file-tree" +[targets.teacher.numbering] +heading = "not-an-array" diff --git a/crates/cph-model/tests/fixtures/target-configs/manifest.toml b/crates/cph-model/tests/fixtures/target-configs/manifest.toml new file mode 100644 index 0000000..fe06232 --- /dev/null +++ b/crates/cph-model/tests/fixtures/target-configs/manifest.toml @@ -0,0 +1,19 @@ +[project] +id = "fixture-target-configs" +name = "target-configs" + +[info] +title = "测试课：导出目标的结构化 build 配置" + +# Full config: explicit artifact + numbering.heading override. +[targets.student] +artifact = "single-file" +[targets.student.numbering] +heading = ["{1:一}、", "{1:1}.{2:1}", "{1:1}.{2:1}.{3:1}"] + +# file-tree artifact, no presentation override. +[targets.archive] +artifact = "file-tree" + +# Empty body → all defaults. +[targets.teacher] diff --git a/crates/cph-model/tests/load.rs b/crates/cph-model/tests/load.rs index 3ab446b..511e53e 100644 --- a/crates/cph-model/tests/load.rs +++ b/crates/cph-model/tests/load.rs @@ -46,10 +46,13 @@ fn valid_two_part_lesson_loads_in_order_with_no_errors() { ); assert!(scalars.get("kind").is_none()); - // Targets are collected from [targets.*]. - let mut targets = lesson.targets.clone(); - targets.sort(); - assert_eq!(targets, vec!["student".to_string(), "teacher".to_string()]); + // Targets are collected from [targets.*], in declared order. + assert_eq!(lesson.target_names(), vec!["student", "teacher"]); + // Empty `[targets.x]` bodies → all-default build config, no diagnostics. + for target in &lesson.targets { + assert_eq!(target.artifact, cph_model::ArtifactKind::SingleFile); + assert_eq!(target.numbering, None); + } // Descriptor dir is absolute, anchored under the root. assert!(lesson.parts[0].descriptor.dir.is_absolute()); @@ -123,3 +126,94 @@ fn missing_manifest_is_a_hard_failure() { assert_eq!(diags.len(), 1); assert_eq!(diags[0].code, DiagCode::SchemaViolation); } + +#[test] +fn structured_target_configs_parse_into_typed_builds() { + use cph_model::ArtifactKind; + + let (lesson, diags) = load(&fixture("target-configs")); + let lesson = lesson.expect("target-configs fixture must produce a Lesson"); + assert!( + diags.is_empty(), + "well-formed target configs must have no diagnostics, got: {diags:?}" + ); + + // Declared order is preserved. + assert_eq!(lesson.target_names(), vec!["student", "archive", "teacher"]); + + // student: explicit single-file + heading numbering override, in order. + let student = &lesson.targets[0]; + assert_eq!(student.name, "student"); + assert_eq!(student.artifact, ArtifactKind::SingleFile); + let numbering = student + .numbering + .as_ref() + .expect("student declares a numbering override"); + assert_eq!( + numbering.heading.as_deref(), + Some( + &[ + "{1:一}、".to_string(), + "{1:1}.{2:1}".to_string(), + "{1:1}.{2:1}.{3:1}".to_string(), + ][..] + ) + ); + + // archive: file-tree artifact, no presentation override. + let archive = &lesson.targets[1]; + assert_eq!(archive.name, "archive"); + assert_eq!(archive.artifact, ArtifactKind::FileTree); + assert_eq!(archive.numbering, None); + + // teacher: empty body → all defaults, numbering absent. + let teacher = &lesson.targets[2]; + assert_eq!(teacher.name, "teacher"); + assert_eq!(teacher.artifact, ArtifactKind::SingleFile); + assert_eq!(teacher.numbering, None); +} + +#[test] +fn malformed_target_config_is_non_fatal_with_schema_violations() { + use cph_model::ArtifactKind; + + let (lesson, diags) = load(&fixture("bad-target-config")); + let lesson = lesson.expect("bad-target-config must still produce a Lesson"); + + // Both targets survive despite their malformed fields. + assert_eq!(lesson.target_names(), vec!["student", "teacher"]); + + // Bad `artifact` value → SchemaViolation, target kept with default artifact. + let student = &lesson.targets[0]; + assert_eq!(student.artifact, ArtifactKind::SingleFile); + + // Bad `numbering.heading` (not an array of strings) → heading stays None, + // but the numbering table itself is present. + let teacher = &lesson.targets[1]; + assert_eq!(teacher.artifact, ArtifactKind::FileTree); + let teacher_numbering = teacher + .numbering + .as_ref() + .expect("teacher declares a numbering table"); + assert_eq!(teacher_numbering.heading, None); + + let violations: Vec<_> = diags + .iter() + .filter(|d| d.code == DiagCode::SchemaViolation) + .collect(); + assert_eq!( + violations.len(), + 2, + "expected one SchemaViolation per malformed field, got: {diags:?}" + ); + assert!( + violations.iter().any(|d| d.message.contains("pdf-thing")), + "a diagnostic should name the bad artifact value, got: {diags:?}" + ); + assert!( + violations + .iter() + .any(|d| d.message.contains("numbering.heading")), + "a diagnostic should name the malformed numbering.heading, got: {diags:?}" + ); +}