diff --git a/src/eval.rs b/src/eval.rs index 233bff3..649ee6b 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -8,9 +8,7 @@ use std::path::{Path, PathBuf}; use crate::error::{Error, Result}; use crate::lexer; -use crate::parser::{ - self, Annotation, BinOp, Entry, Expr, Modifier, Module, Property, StringInterpPart, UnOp, -}; +use crate::parser::{self, BinOp, Entry, Expr, Modifier, Module, Property, StringInterpPart, UnOp}; use crate::value::{ObjectSource, Value}; /// Evaluates pkl source files to [`Value`]. @@ -32,6 +30,10 @@ pub struct Evaluator { /// Converters extracted from `output.renderer.converters`. /// Each entry maps a class name to a converter lambda. converters: Vec<(String, Value)>, + /// Set of (property_name, message) pairs already warned about. + /// Used to deduplicate `@Deprecated` warnings so a deprecated property + /// referenced inside a loop or template doesn't flood stderr. + warned_deprecated: std::collections::HashSet<(String, Option)>, } impl Default for Evaluator { @@ -45,6 +47,7 @@ impl Default for Evaluator { package_dirs: HashMap::new(), http_rewrites: Vec::new(), converters: Vec::new(), + warned_deprecated: std::collections::HashSet::new(), } } } @@ -84,6 +87,28 @@ impl Evaluator { .collect(); } + /// If `field` is marked `@Deprecated` in `source`, emit the warning at + /// most once per (field, message) pair. Called from field-access + /// expressions so the warning fires when a deprecated property is *used*, + /// not when its containing module loads. Per-call dedup mirrors pkl-jvm, + /// which avoids flooding stderr when a deprecated property is referenced + /// inside a loop or template. + fn warn_if_deprecated_access(&mut self, source: &Option>, field: &str) { + let Some(src) = source else { return }; + let Some(message) = src.deprecated.get(field) else { + return; + }; + let key = (field.to_string(), message.clone()); + if !self.warned_deprecated.insert(key) { + return; + } + if let Some(msg) = message { + eprintln!("[pklr] WARNING: property '{field}' is deprecated: {msg}"); + } else { + eprintln!("[pklr] WARNING: property '{field}' is deprecated"); + } + } + /// Apply rewrite rules to a URL. Returns the rewritten URL or the original. pub fn rewrite_url<'a>(&self, url: &'a str) -> std::borrow::Cow<'a, str> { if self.http_rewrites.is_empty() { @@ -674,7 +699,6 @@ impl Evaluator { if has_modifier(mods, Modifier::Local) { continue; // already collected } - check_deprecated(&prop.annotations, &prop.name); // Extract renderer converters from the `output` block AST, // then skip it (it's not included in the output). // Clear any base-inherited converters so child overrides take precedence. @@ -748,7 +772,23 @@ impl Evaluator { } out.retain(|_, v| !matches!(v, Value::Lambda(..))); } - Ok(Value::Object(Arc::new(out), None)) + // If the module declares any `@Deprecated` properties, attach a + // minimal ObjectSource carrying just the deprecation map so field + // access can warn lazily. Modules without @Deprecated keep `None` + // source to avoid changing amend behavior in the common case. + let deprecated = collect_deprecated(&module.body); + let source = if deprecated.is_empty() { + None + } else { + Some(Arc::new(ObjectSource { + entries: Vec::new(), + scope: IndexMap::new(), + is_open: true, + type_name: None, + deprecated, + })) + }; + Ok(Value::Object(Arc::new(out), source)) } #[async_recursion(?Send)] @@ -863,7 +903,6 @@ impl Evaluator { if has_modifier(mods, Modifier::Local) { continue; } - check_deprecated(&prop.annotations, &prop.name); // Skip the `default` property — it's a template, not an output entry if prop.name == "default" && default_template.is_some() { continue; @@ -918,6 +957,7 @@ impl Evaluator { scope: IndexMap::new(), is_open: true, type_name: Some(tn.clone()), + deprecated: merge_deprecated(&src.deprecated, body), }, }; *result_src = Some(std::sync::Arc::new(new_src)); @@ -985,6 +1025,7 @@ impl Evaluator { scope: child_scope.flatten(), is_open: true, // default: allow new properties type_name: None, + deprecated: collect_deprecated(entries), }; Ok(Value::Object(Arc::new(map), Some(Arc::new(source)))) } @@ -1379,11 +1420,13 @@ impl Evaluator { let mut source_scope = scope.flatten(); source_scope.shift_remove("outer"); source_scope.shift_remove("this"); + let deprecated = collect_deprecated(&src_entries); let source = ObjectSource { entries: src_entries, scope: source_scope, is_open: true, type_name: None, + deprecated, }; Ok(Value::Object(Arc::new(map), Some(Arc::new(source)))) } @@ -1469,25 +1512,36 @@ impl Evaluator { scope: IndexMap::new(), is_open, type_name: tn, + deprecated: merge_deprecated(&base_src.deprecated, entries), } }; *src_slot = Some(Arc::new(new_src)); } Ok(result) - } else if let Some(Value::Object(base_map, _)) = base { + } else if let Some(Value::Object(base_map, base_src)) = base { // Fallback: eager merge let overlay = self.eval_entries(entries, scope, depth + 1).await?; let mut merged: IndexMap = (*base_map).clone(); - if let Value::Object(overlay_map, _) = overlay { + let mut deprecated = base_src + .as_ref() + .map(|s| s.deprecated.clone()) + .unwrap_or_default(); + if let Value::Object(overlay_map, overlay_src) = &overlay { merged.extend( overlay_map.iter().map(|(k, v)| (k.clone(), v.clone())), ); + if let Some(os) = overlay_src.as_ref() { + for (k, v) in &os.deprecated { + deprecated.insert(k.clone(), v.clone()); + } + } } let src = ObjectSource { entries: Vec::new(), scope: IndexMap::new(), is_open: true, type_name: type_name.clone(), + deprecated, }; Ok(Value::Object(Arc::new(merged), Some(Arc::new(src)))) } else { @@ -1540,10 +1594,14 @@ impl Evaluator { _ => {} } match &obj { - Value::Object(map, _) => map - .get(field) - .cloned() - .ok_or_else(|| Error::Eval(format!("field not found: {field}"))), + Value::Object(map, source) => { + let val = map + .get(field) + .cloned() + .ok_or_else(|| Error::Eval(format!("field not found: {field}")))?; + self.warn_if_deprecated_access(source, field); + Ok(val) + } _ => Err(Error::Eval(format!( "cannot access field '{field}' on {}", value_type_name(&obj) @@ -1554,7 +1612,13 @@ impl Evaluator { let obj = self.eval_expr(obj_expr, scope, depth + 1).await?; match &obj { Value::Null => Ok(Value::Null), - Value::Object(map, _) => Ok(map.get(field).cloned().unwrap_or(Value::Null)), + Value::Object(map, source) => { + let val = map.get(field).cloned().unwrap_or(Value::Null); + if !matches!(val, Value::Null) { + self.warn_if_deprecated_access(source, field); + } + Ok(val) + } _ => Err(Error::Eval(format!( "cannot access field '{field}' on {}", value_type_name(&obj) @@ -2204,6 +2268,7 @@ impl Evaluator { scope: IndexMap::new(), is_open: true, type_name: Some(tn.clone()), + deprecated: merge_deprecated(&src.deprecated, body), }, }; *result_src = Some(std::sync::Arc::new(new_src)); @@ -2472,26 +2537,47 @@ fn resolve_package_uri(uri: &str) -> Result { Err(Error::Eval(format!("unsupported package URI: {uri}"))) } -fn check_deprecated(annotations: &[Annotation], prop_name: &str) { - for ann in annotations { - if ann.name == "Deprecated" { - // Look for a "message" property in the annotation body - let mut message = None; - for entry in &ann.body { - if let Entry::Property(p) = entry - && p.name == "message" - && let Some(Expr::String(s)) = &p.value - { - message = Some(s.clone()); +/// Collect `@Deprecated` annotations from a list of entries into a map of +/// property name → optional message. Used to populate `ObjectSource.deprecated` +/// so field access can warn lazily, instead of warning eagerly when a module +/// or object body is evaluated. +fn collect_deprecated(entries: &[Entry]) -> IndexMap> { + let mut out: IndexMap> = IndexMap::new(); + for entry in entries { + if let Entry::Property(prop) = entry { + for ann in &prop.annotations { + if ann.name != "Deprecated" { + continue; } - } - if let Some(msg) = message { - eprintln!("[pklr] WARNING: property '{prop_name}' is deprecated: {msg}"); - } else { - eprintln!("[pklr] WARNING: property '{prop_name}' is deprecated"); + let mut message = None; + for e in &ann.body { + if let Entry::Property(p) = e + && p.name == "message" + && let Some(Expr::String(s)) = &p.value + { + message = Some(s.clone()); + } + } + out.insert(prop.name.clone(), message); } } } + out +} + +/// Combine a base deprecation map with any `@Deprecated` annotations on a +/// list of overlay entries, with overlay winning on conflict. Used by the +/// amend/merge code paths so an amended object's `ObjectSource.deprecated` +/// reflects deprecations from both the base and the overlay. +fn merge_deprecated( + base: &IndexMap>, + overlay_entries: &[Entry], +) -> IndexMap> { + let mut out = base.clone(); + for (k, v) in collect_deprecated(overlay_entries) { + out.insert(k, v); + } + out } /// Resolve a potentially dotted name (e.g. "Foo.Bar") in scope. diff --git a/src/value.rs b/src/value.rs index 477f409..d8dd394 100644 --- a/src/value.rs +++ b/src/value.rs @@ -18,6 +18,11 @@ pub struct ObjectSource { /// The pkl class name this object was instantiated from (e.g., "Step", "Group"). /// Used by `output.renderer.converters` to apply type-specific transforms. pub type_name: Option, + /// Map of property name → optional deprecation message for properties + /// annotated with `@Deprecated`. Consulted on field access so the + /// warning fires when a deprecated property is *used*, not when the + /// containing module is loaded. + pub deprecated: IndexMap>, } /// A pkl runtime value.