From cfbe8359105d08c852bcd411bc68a3c59de44344 Mon Sep 17 00:00:00 2001 From: 132ikl <132@ikl.sh> Date: Sun, 1 Jun 2025 09:55:47 -0400 Subject: [PATCH] Add unified deprecation system and @deprecated attribute (#15770) --- Cargo.lock | 2 + crates/nu-cli/tests/completions/mod.rs | 17 +- crates/nu-cmd-base/src/lib.rs | 3 + crates/nu-cmd-base/src/wrap_call.rs | 101 ++++++++++++ crates/nu-cmd-lang/Cargo.toml | 2 + .../src/core_commands/attr/deprecated.rs | 148 ++++++++++++++++++ .../nu-cmd-lang/src/core_commands/attr/mod.rs | 2 + crates/nu-cmd-lang/src/default_context.rs | 1 + .../tests/commands/attr/deprecated.rs | 114 ++++++++++++++ crates/nu-cmd-lang/tests/commands/attr/mod.rs | 1 + crates/nu-cmd-lang/tests/commands/mod.rs | 1 + crates/nu-cmd-lang/tests/main.rs | 1 + crates/nu-derive-value/src/attributes.rs | 4 + crates/nu-derive-value/src/from.rs | 14 +- crates/nu-parser/src/parse_keywords.rs | 41 +++-- crates/nu-parser/src/parser.rs | 13 +- crates/nu-protocol/src/deprecation.rs | 144 +++++++++++++++++ crates/nu-protocol/src/engine/command.rs | 7 +- crates/nu-protocol/src/engine/engine_state.rs | 3 + crates/nu-protocol/src/errors/cli_error.rs | 59 ++++++- crates/nu-protocol/src/errors/mod.rs | 2 +- .../nu-protocol/src/errors/parse_warning.rs | 43 +++-- crates/nu-protocol/src/lib.rs | 2 + crates/nu-protocol/src/signature.rs | 19 ++- crates/nu-protocol/src/value/from_value.rs | 2 + crates/nu-protocol/src/value/test_derive.rs | 27 ++++ 26 files changed, 719 insertions(+), 54 deletions(-) create mode 100644 crates/nu-cmd-base/src/wrap_call.rs create mode 100644 crates/nu-cmd-lang/src/core_commands/attr/deprecated.rs create mode 100644 crates/nu-cmd-lang/tests/commands/attr/deprecated.rs create mode 100644 crates/nu-cmd-lang/tests/commands/attr/mod.rs create mode 100644 crates/nu-cmd-lang/tests/commands/mod.rs create mode 100644 crates/nu-cmd-lang/tests/main.rs create mode 100644 crates/nu-protocol/src/deprecation.rs diff --git a/Cargo.lock b/Cargo.lock index 6186a3439d..f033961070 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3661,6 +3661,8 @@ name = "nu-cmd-lang" version = "0.104.2" dependencies = [ "itertools 0.13.0", + "miette", + "nu-cmd-base", "nu-engine", "nu-parser", "nu-protocol", diff --git a/crates/nu-cli/tests/completions/mod.rs b/crates/nu-cli/tests/completions/mod.rs index 360bde8797..e27d627905 100644 --- a/crates/nu-cli/tests/completions/mod.rs +++ b/crates/nu-cli/tests/completions/mod.rs @@ -1579,16 +1579,25 @@ fn attribute_completions() { // Create a new engine let (_, _, engine, stack) = new_engine(); + // Compile a list of built-in attribute names (without the "attr " prefix) + let attribute_names: Vec = engine + .get_signatures_and_declids(false) + .into_iter() + .map(|(sig, _)| sig.name) + .filter(|name| name.starts_with("attr ")) + .map(|name| name[5..].to_string()) + .collect(); + + // Make sure we actually found some attributes so the test is valid + assert!(attribute_names.contains(&String::from("example"))); + // Instantiate a new completer let mut completer = NuCompleter::new(Arc::new(engine), Arc::new(stack)); // Test completions for the 'ls' flags let suggestions = completer.complete("@", 1); - // Only checking for the builtins and not the std attributes - let expected: Vec<_> = vec!["category", "example", "search-terms"]; - // Match results - match_suggestions(&expected, &suggestions); + match_suggestions_by_string(&attribute_names, &suggestions); } #[test] diff --git a/crates/nu-cmd-base/src/lib.rs b/crates/nu-cmd-base/src/lib.rs index c2608f8466..b0c15a16b8 100644 --- a/crates/nu-cmd-base/src/lib.rs +++ b/crates/nu-cmd-base/src/lib.rs @@ -3,3 +3,6 @@ pub mod formats; pub mod hook; pub mod input_handler; pub mod util; +mod wrap_call; + +pub use wrap_call::*; diff --git a/crates/nu-cmd-base/src/wrap_call.rs b/crates/nu-cmd-base/src/wrap_call.rs new file mode 100644 index 0000000000..f9e9b1703e --- /dev/null +++ b/crates/nu-cmd-base/src/wrap_call.rs @@ -0,0 +1,101 @@ +use nu_engine::CallExt; +use nu_protocol::{ + DeclId, FromValue, ShellError, Span, + engine::{Call, EngineState, Stack, StateWorkingSet}, +}; + +/// A helper utility to aid in implementing commands which have the same behavior for `run` and `run_const`. +/// +/// Only supports functions in [`Call`] and [`CallExt`] which have a `const` suffix. +/// +/// To use, the actual command logic should be moved to a function. Then, `eval` and `eval_const` can be implemented like this: +/// ```rust +/// # use nu_engine::command_prelude::*; +/// # use nu_cmd_base::WrapCall; +/// # fn do_command_logic(call: WrapCall) -> Result { Ok(PipelineData::Empty) } +/// +/// # struct Command {} +/// # impl Command { +/// fn run(&self, engine_state: &EngineState, stack: &mut Stack, call: &Call) -> Result { +/// let call = WrapCall::Eval(engine_state, stack, call); +/// do_command_logic(call) +/// } +/// +/// fn run_const(&self, working_set: &StateWorkingSet, call: &Call) -> Result { +/// let call = WrapCall::ConstEval(working_set, call); +/// do_command_logic(call) +/// } +/// # } +/// ``` +/// +/// Then, the typical [`Call`] and [`CallExt`] operations can be called using destructuring: +/// +/// ```rust +/// # use nu_engine::command_prelude::*; +/// # use nu_cmd_base::WrapCall; +/// # let call = WrapCall::Eval(&EngineState::new(), &mut Stack::new(), &Call::new(Span::unknown())); +/// # fn do_command_logic(call: WrapCall) -> Result<(), ShellError> { +/// let (call, required): (_, String) = call.req(0)?; +/// let (call, flag): (_, Option) = call.get_flag("number")?; +/// # Ok(()) +/// # } +/// ``` +/// +/// A new `WrapCall` instance has to be returned after each function to ensure +/// that there is only ever one copy of mutable [`Stack`] reference. +pub enum WrapCall<'a> { + Eval(&'a EngineState, &'a mut Stack, &'a Call<'a>), + ConstEval(&'a StateWorkingSet<'a>, &'a Call<'a>), +} + +/// Macro to choose between the non-const and const versions of each [`Call`]/[`CallExt`] function +macro_rules! proxy { + ($self:ident , $eval:ident , $const:ident , $( $args:expr ),*) => { + match $self { + WrapCall::Eval(engine_state, stack, call) => { + Call::$eval(call, engine_state, stack, $( $args ),*) + .map(|val| (WrapCall::Eval(engine_state, stack, call), val)) + }, + WrapCall::ConstEval(working_set, call) => { + Call::$const(call, working_set, $( $args ),*) + .map(|val| (WrapCall::ConstEval(working_set, call), val)) + }, + } + }; +} + +impl WrapCall<'_> { + pub fn head(&self) -> Span { + match self { + WrapCall::Eval(_, _, call) => call.head, + WrapCall::ConstEval(_, call) => call.head, + } + } + + pub fn decl_id(&self) -> DeclId { + match self { + WrapCall::Eval(_, _, call) => call.decl_id, + WrapCall::ConstEval(_, call) => call.decl_id, + } + } + + pub fn has_flag(self, flag_name: &str) -> Result<(Self, bool), ShellError> { + proxy!(self, has_flag, has_flag_const, flag_name) + } + + pub fn get_flag(self, name: &str) -> Result<(Self, Option), ShellError> { + proxy!(self, get_flag, get_flag_const, name) + } + + pub fn req(self, pos: usize) -> Result<(Self, T), ShellError> { + proxy!(self, req, req_const, pos) + } + + pub fn rest(self, pos: usize) -> Result<(Self, Vec), ShellError> { + proxy!(self, rest, rest_const, pos) + } + + pub fn opt(self, pos: usize) -> Result<(Self, Option), ShellError> { + proxy!(self, opt, opt_const, pos) + } +} diff --git a/crates/nu-cmd-lang/Cargo.toml b/crates/nu-cmd-lang/Cargo.toml index 29acd7316d..b93dab441c 100644 --- a/crates/nu-cmd-lang/Cargo.toml +++ b/crates/nu-cmd-lang/Cargo.toml @@ -19,6 +19,7 @@ nu-engine = { path = "../nu-engine", version = "0.104.2", default-features = fal nu-parser = { path = "../nu-parser", version = "0.104.2" } nu-protocol = { path = "../nu-protocol", version = "0.104.2", default-features = false } nu-utils = { path = "../nu-utils", version = "0.104.2", default-features = false } +nu-cmd-base = { path = "../nu-cmd-base", version = "0.104.2" } itertools = { workspace = true } shadow-rs = { version = "1.1", default-features = false } @@ -29,6 +30,7 @@ shadow-rs = { version = "1.1", default-features = false, features = ["build"] } [dev-dependencies] quickcheck = { workspace = true } quickcheck_macros = { workspace = true } +miette = { workspace = true } [features] default = ["os"] diff --git a/crates/nu-cmd-lang/src/core_commands/attr/deprecated.rs b/crates/nu-cmd-lang/src/core_commands/attr/deprecated.rs new file mode 100644 index 0000000000..67a1298530 --- /dev/null +++ b/crates/nu-cmd-lang/src/core_commands/attr/deprecated.rs @@ -0,0 +1,148 @@ +use nu_cmd_base::WrapCall; +use nu_engine::command_prelude::*; + +#[derive(Clone)] +pub struct AttrDeprecated; + +impl Command for AttrDeprecated { + fn name(&self) -> &str { + "attr deprecated" + } + + fn signature(&self) -> Signature { + Signature::build("attr deprecated") + .input_output_types(vec![ + (Type::Nothing, Type::Nothing), + (Type::Nothing, Type::String), + ]) + .optional( + "message", + SyntaxShape::String, + "Help message to include with deprecation warning.", + ) + .named( + "flag", + SyntaxShape::String, + "Mark a flag as deprecated rather than the command", + None, + ) + .named( + "since", + SyntaxShape::String, + "Denote a version when this item was deprecated", + Some('s'), + ) + .named( + "remove", + SyntaxShape::String, + "Denote a version when this item will be removed", + Some('r'), + ) + .named( + "report", + SyntaxShape::String, + "How to warn about this item. One of: first (default), every", + None, + ) + .category(Category::Core) + } + + fn description(&self) -> &str { + "Attribute for marking a command or flag as deprecated." + } + + fn extra_description(&self) -> &str { + "Mark a command (default) or flag/switch (--flag) as deprecated. By default, only the first usage will trigger a deprecation warning. + +A help message can be included to provide more context for the deprecation, such as what to use as a replacement. + +Also consider setting the category to deprecated with @category deprecated" + } + + fn run( + &self, + engine_state: &EngineState, + stack: &mut Stack, + call: &Call, + _input: PipelineData, + ) -> Result { + let call = WrapCall::Eval(engine_state, stack, call); + Ok(deprecated_record(call)?.into_pipeline_data()) + } + + fn run_const( + &self, + working_set: &StateWorkingSet, + call: &Call, + _input: PipelineData, + ) -> Result { + let call = WrapCall::ConstEval(working_set, call); + Ok(deprecated_record(call)?.into_pipeline_data()) + } + + fn is_const(&self) -> bool { + true + } + + fn examples(&self) -> Vec { + vec![ + Example { + description: "Add a deprecation warning to a custom command", + example: r###"@deprecated + def outdated [] {}"###, + result: Some(Value::nothing(Span::test_data())), + }, + Example { + description: "Add a deprecation warning with a custom message", + example: r###"@deprecated "Use my-new-command instead." + @category deprecated + def my-old-command [] {}"###, + result: Some(Value::string( + "Use my-new-command instead.", + Span::test_data(), + )), + }, + ] + } +} + +fn deprecated_record(call: WrapCall) -> Result { + let (call, message): (_, Option>) = call.opt(0)?; + let (call, flag): (_, Option>) = call.get_flag("flag")?; + let (call, since): (_, Option>) = call.get_flag("since")?; + let (call, remove): (_, Option>) = call.get_flag("remove")?; + let (call, report): (_, Option>) = call.get_flag("report")?; + + let mut record = Record::new(); + if let Some(message) = message { + record.push("help", Value::string(message.item, message.span)) + } + if let Some(flag) = flag { + record.push("flag", Value::string(flag.item, flag.span)) + } + if let Some(since) = since { + record.push("since", Value::string(since.item, since.span)) + } + if let Some(remove) = remove { + record.push("expected_removal", Value::string(remove.item, remove.span)) + } + + let report = if let Some(Spanned { item, span }) = report { + match item.as_str() { + "every" => Value::string(item, span), + "first" => Value::string(item, span), + _ => { + return Err(ShellError::IncorrectValue { + msg: "The report mode must be one of: every, first".into(), + val_span: span, + call_span: call.head(), + }); + } + } + } else { + Value::string("first", call.head()) + }; + record.push("report", report); + + Ok(Value::record(record, call.head())) +} diff --git a/crates/nu-cmd-lang/src/core_commands/attr/mod.rs b/crates/nu-cmd-lang/src/core_commands/attr/mod.rs index 6d8c641b35..8791bb4d68 100644 --- a/crates/nu-cmd-lang/src/core_commands/attr/mod.rs +++ b/crates/nu-cmd-lang/src/core_commands/attr/mod.rs @@ -1,7 +1,9 @@ mod category; +mod deprecated; mod example; mod search_terms; pub use category::AttrCategory; +pub use deprecated::AttrDeprecated; pub use example::AttrExample; pub use search_terms::AttrSearchTerms; diff --git a/crates/nu-cmd-lang/src/default_context.rs b/crates/nu-cmd-lang/src/default_context.rs index 8929f75fa0..e3b0d2d2e8 100644 --- a/crates/nu-cmd-lang/src/default_context.rs +++ b/crates/nu-cmd-lang/src/default_context.rs @@ -17,6 +17,7 @@ pub fn create_default_context() -> EngineState { bind_command! { Alias, AttrCategory, + AttrDeprecated, AttrExample, AttrSearchTerms, Break, diff --git a/crates/nu-cmd-lang/tests/commands/attr/deprecated.rs b/crates/nu-cmd-lang/tests/commands/attr/deprecated.rs new file mode 100644 index 0000000000..7ddda3af58 --- /dev/null +++ b/crates/nu-cmd-lang/tests/commands/attr/deprecated.rs @@ -0,0 +1,114 @@ +use miette::{Diagnostic, LabeledSpan}; +use nu_cmd_lang::{Alias, Def}; +use nu_parser::parse; +use nu_protocol::engine::{EngineState, StateWorkingSet}; + +use nu_cmd_lang::AttrDeprecated; + +#[test] +pub fn test_deprecated_attribute() { + let engine_state = EngineState::new(); + let mut working_set = StateWorkingSet::new(&engine_state); + + working_set.add_decl(Box::new(Def)); + working_set.add_decl(Box::new(Alias)); + working_set.add_decl(Box::new(AttrDeprecated)); + + // test deprecation with no message + let source = br#" + @deprecated + def foo [] {} + "#; + let _ = parse(&mut working_set, None, source, false); + + // there should be no warning until the command is called + assert!(working_set.parse_errors.is_empty()); + assert!(working_set.parse_warnings.is_empty()); + + let source = b"foo"; + let _ = parse(&mut working_set, None, source, false); + + // command called, there should be a deprecation warning + assert!(working_set.parse_errors.is_empty()); + assert!(!working_set.parse_warnings.is_empty()); + let labels: Vec = working_set.parse_warnings[0].labels().unwrap().collect(); + let label = labels.first().unwrap().label().unwrap(); + assert!(label.contains("foo is deprecated")); + working_set.parse_warnings.clear(); + + // test deprecation with message + let source = br#" + @deprecated "Use new-command instead" + def old-command [] {} + + old-command + "#; + let _ = parse(&mut working_set, None, source, false); + + assert!(working_set.parse_errors.is_empty()); + assert!(!working_set.parse_warnings.is_empty()); + + let help = &working_set.parse_warnings[0].help().unwrap().to_string(); + assert!(help.contains("Use new-command instead")); +} + +#[test] +pub fn test_deprecated_attribute_flag() { + let engine_state = EngineState::new(); + let mut working_set = StateWorkingSet::new(&engine_state); + + working_set.add_decl(Box::new(Def)); + working_set.add_decl(Box::new(Alias)); + working_set.add_decl(Box::new(AttrDeprecated)); + + let source = br#" + @deprecated "Use foo instead of bar" --flag bar + @deprecated "Use foo instead of baz" --flag baz + def old-command [--foo, --bar, --baz] {} + old-command --foo + old-command --bar + old-command --baz + old-command --foo --bar --baz + "#; + let _ = parse(&mut working_set, None, source, false); + + assert!(working_set.parse_errors.is_empty()); + assert!(!working_set.parse_warnings.is_empty()); + + let help = &working_set.parse_warnings[0].help().unwrap().to_string(); + assert!(help.contains("Use foo instead of bar")); + + let help = &working_set.parse_warnings[1].help().unwrap().to_string(); + assert!(help.contains("Use foo instead of baz")); + + let help = &working_set.parse_warnings[2].help().unwrap().to_string(); + assert!(help.contains("Use foo instead of bar")); + + let help = &working_set.parse_warnings[3].help().unwrap().to_string(); + assert!(help.contains("Use foo instead of baz")); +} + +#[test] +pub fn test_deprecated_attribute_since_remove() { + let engine_state = EngineState::new(); + let mut working_set = StateWorkingSet::new(&engine_state); + + working_set.add_decl(Box::new(Def)); + working_set.add_decl(Box::new(Alias)); + working_set.add_decl(Box::new(AttrDeprecated)); + + let source = br#" + @deprecated --since 0.10000.0 --remove 1.0 + def old-command [] {} + old-command + "#; + let _ = parse(&mut working_set, None, source, false); + + assert!(working_set.parse_errors.is_empty()); + assert!(!working_set.parse_warnings.is_empty()); + + let labels: Vec = working_set.parse_warnings[0].labels().unwrap().collect(); + let label = labels.first().unwrap().label().unwrap(); + assert!(label.contains("0.10000.0")); + assert!(label.contains("1.0")); +} diff --git a/crates/nu-cmd-lang/tests/commands/attr/mod.rs b/crates/nu-cmd-lang/tests/commands/attr/mod.rs new file mode 100644 index 0000000000..312dcd95f5 --- /dev/null +++ b/crates/nu-cmd-lang/tests/commands/attr/mod.rs @@ -0,0 +1 @@ +mod deprecated; diff --git a/crates/nu-cmd-lang/tests/commands/mod.rs b/crates/nu-cmd-lang/tests/commands/mod.rs new file mode 100644 index 0000000000..ddd623bcc3 --- /dev/null +++ b/crates/nu-cmd-lang/tests/commands/mod.rs @@ -0,0 +1 @@ +mod attr; diff --git a/crates/nu-cmd-lang/tests/main.rs b/crates/nu-cmd-lang/tests/main.rs new file mode 100644 index 0000000000..f3d44688d6 --- /dev/null +++ b/crates/nu-cmd-lang/tests/main.rs @@ -0,0 +1 @@ +mod commands; diff --git a/crates/nu-derive-value/src/attributes.rs b/crates/nu-derive-value/src/attributes.rs index 37d1988890..6718b01369 100644 --- a/crates/nu-derive-value/src/attributes.rs +++ b/crates/nu-derive-value/src/attributes.rs @@ -68,6 +68,7 @@ impl ParseAttrs for ContainerAttributes { #[derive(Debug, Default)] pub struct MemberAttributes { pub rename: Option, + pub default: bool, } impl ParseAttrs for MemberAttributes { @@ -79,6 +80,9 @@ impl ParseAttrs for MemberAttributes { let rename = rename.value(); self.rename = Some(rename); } + "default" => { + self.default = true; + } ident => { return Err(DeriveError::UnexpectedAttribute { meta_span: ident.span(), diff --git a/crates/nu-derive-value/src/from.rs b/crates/nu-derive-value/src/from.rs index dc5f837f77..41ad461bff 100644 --- a/crates/nu-derive-value/src/from.rs +++ b/crates/nu-derive-value/src/from.rs @@ -570,16 +570,15 @@ fn parse_value_via_fields( let ident_s = name_resolver.resolve_ident(ident, container_attrs, &member_attrs, None)?; let ty = &field.ty; - fields_ts.push(match type_is_option(ty) { - true => quote! { + fields_ts.push(match (type_is_option(ty), member_attrs.default) { + (true, _) => quote! { #ident: record .remove(#ident_s) .map(|v| <#ty as nu_protocol::FromValue>::from_value(v)) .transpose()? .flatten() }, - - false => quote! { + (false, false) => quote! { #ident: <#ty as nu_protocol::FromValue>::from_value( record .remove(#ident_s) @@ -590,6 +589,13 @@ fn parse_value_via_fields( })?, )? }, + (false, true) => quote! { + #ident: record + .remove(#ident_s) + .map(|v| <#ty as nu_protocol::FromValue>::from_value(v)) + .transpose()? + .unwrap_or_default() + }, }); } Ok(quote! { diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 7f8416cbd3..4b44181181 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -9,7 +9,7 @@ use log::trace; use nu_path::canonicalize_with; use nu_protocol::{ Alias, BlockId, CustomExample, DeclId, FromValue, Module, ModuleId, ParseError, PositionalArg, - ResolvedImportPattern, ShellError, Span, Spanned, SyntaxShape, Type, Value, VarId, + ResolvedImportPattern, ShellError, Signature, Span, Spanned, SyntaxShape, Type, Value, VarId, ast::{ Argument, AttributeBlock, Block, Call, Expr, Expression, ImportPattern, ImportPatternHead, ImportPatternMember, Pipeline, PipelineElement, @@ -521,9 +521,6 @@ fn parse_def_inner( let (desc, extra_desc) = working_set.build_desc(&lite_command.comments); - let (attribute_vals, examples, search_terms, category) = - handle_special_attributes(attributes, working_set); - // Checking that the function is used with the correct name // Maybe this is not necessary but it is a sanity check // Note: "export def" is treated the same as "def" @@ -724,8 +721,6 @@ fn parse_def_inner( } if let Some(decl_id) = working_set.find_predecl(name.as_bytes()) { - let declaration = working_set.get_decl_mut(decl_id); - signature.name.clone_from(&name); if !has_wrapped { *signature = signature.add_help(); @@ -733,8 +728,11 @@ fn parse_def_inner( signature.description = desc; signature.extra_description = extra_desc; signature.allows_unknown_args = has_wrapped; - signature.search_terms = search_terms; - signature.category = category_from_string(&category); + + let (attribute_vals, examples) = + handle_special_attributes(attributes, working_set, &mut signature); + + let declaration = working_set.get_decl_mut(decl_id); *declaration = signature .clone() @@ -788,9 +786,6 @@ fn parse_extern_inner( let (description, extra_description) = working_set.build_desc(&lite_command.comments); - let (attribute_vals, examples, search_terms, category) = - handle_special_attributes(attributes, working_set); - // Checking that the function is used with the correct name // Maybe this is not necessary but it is a sanity check @@ -876,8 +871,6 @@ fn parse_extern_inner( } if let Some(decl_id) = working_set.find_predecl(name.as_bytes()) { - let declaration = working_set.get_decl_mut(decl_id); - let external_name = if let Some(mod_name) = module_name { if name.as_bytes() == b"main" { String::from_utf8_lossy(mod_name).to_string() @@ -891,9 +884,12 @@ fn parse_extern_inner( signature.name = external_name; signature.description = description; signature.extra_description = extra_description; - signature.search_terms = search_terms; signature.allows_unknown_args = true; - signature.category = category_from_string(&category); + + let (attribute_vals, examples) = + handle_special_attributes(attributes, working_set, &mut signature); + + let declaration = working_set.get_decl_mut(decl_id); if let Some(block_id) = body.and_then(|x| x.as_block()) { if signature.rest_positional.is_none() { @@ -950,16 +946,11 @@ fn parse_extern_inner( Expression::new(working_set, Expr::Call(call), call_span, Type::Any) } -#[allow(clippy::type_complexity)] fn handle_special_attributes( attributes: Vec<(String, Value)>, working_set: &mut StateWorkingSet<'_>, -) -> ( - Vec<(String, Value)>, - Vec, - Vec, - String, -) { + signature: &mut Signature, +) -> (Vec<(String, Value)>, Vec) { let mut attribute_vals = vec![]; let mut examples = vec![]; let mut search_terms = vec![]; @@ -1016,7 +1007,11 @@ fn handle_special_attributes( } } } - (attribute_vals, examples, search_terms, category) + + signature.search_terms = search_terms; + signature.category = category_from_string(&category); + + (attribute_vals, examples) } fn check_alias_name<'a>(working_set: &mut StateWorkingSet, spans: &'a [Span]) -> Option<&'a Span> { diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 185142d139..e01c9dd10d 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -971,6 +971,8 @@ pub fn parse_internal_call( let signature = working_set.get_signature(decl); let output = signature.get_output_type(); + let deprecation = decl.deprecation_info(); + // storing the var ID for later due to borrowing issues let lib_dirs_var_id = match decl.name() { "use" | "overlay use" | "source-env" if decl.is_keyword() => { @@ -1264,6 +1266,16 @@ pub fn parse_internal_call( check_call(working_set, command_span, &signature, &call); + deprecation + .into_iter() + .filter_map(|entry| entry.parse_warning(&signature.name, &call)) + .for_each(|warning| { + // FIXME: if two flags are deprecated and both are used in one command, + // the second flag's deprecation won't show until the first flag is removed + // (but it won't be flagged as reported until it is actually reported) + working_set.warning(warning); + }); + if signature.creates_scope { working_set.exit_scope(); } @@ -1304,7 +1316,6 @@ pub fn parse_call(working_set: &mut StateWorkingSet, spans: &[Span], head: Span) } } - // TODO: Try to remove the clone let decl = working_set.get_decl(decl_id); let parsed_call = if let Some(alias) = decl.as_alias() { diff --git a/crates/nu-protocol/src/deprecation.rs b/crates/nu-protocol/src/deprecation.rs new file mode 100644 index 0000000000..c25ca99aba --- /dev/null +++ b/crates/nu-protocol/src/deprecation.rs @@ -0,0 +1,144 @@ +use crate::{FromValue, ParseWarning, ShellError, Type, Value, ast::Call}; + +// Make nu_protocol available in this namespace, consumers of this crate will +// have this without such an export. +// The `FromValue` derive macro fully qualifies paths to "nu_protocol". +use crate::{self as nu_protocol, ReportMode, Span}; + +/// A entry which indicates that some part of, or all of, a command is deprecated +/// +/// Commands can implement [`Command::deprecation_info`] to return deprecation entries, +/// which will cause a parse-time warning. Additionally, custom commands can use the +/// @deprecated attribute to add a `DeprecationEntry`. +#[derive(FromValue)] +pub struct DeprecationEntry { + /// The type of deprecation + // might need to revisit this if we added additional DeprecationTypes + #[nu_value(rename = "flag", default)] + pub ty: DeprecationType, + /// How this deprecation should be reported + #[nu_value(rename = "report")] + pub report_mode: ReportMode, + /// When this deprecation started + pub since: Option, + /// When this item is expected to be removed + pub expected_removal: Option, + /// Help text, possibly including a suggestion for what to use instead + pub help: Option, +} + +/// What this deprecation affects +#[derive(Default)] +pub enum DeprecationType { + /// Deprecation of whole command + #[default] + Command, + /// Deprecation of a flag/switch + Flag(String), +} + +impl FromValue for DeprecationType { + fn from_value(v: Value) -> Result { + match v { + Value::String { val, .. } => Ok(DeprecationType::Flag(val)), + Value::Nothing { .. } => Ok(DeprecationType::Command), + v => Err(ShellError::CantConvert { + to_type: Self::expected_type().to_string(), + from_type: v.get_type().to_string(), + span: v.span(), + help: None, + }), + } + } + + fn expected_type() -> Type { + Type::String + } +} + +impl FromValue for ReportMode { + fn from_value(v: Value) -> Result { + let span = v.span(); + let Value::String { val, .. } = v else { + return Err(ShellError::CantConvert { + to_type: Self::expected_type().to_string(), + from_type: v.get_type().to_string(), + span: v.span(), + help: None, + }); + }; + match val.as_str() { + "first" => Ok(ReportMode::FirstUse), + "every" => Ok(ReportMode::EveryUse), + _ => Err(ShellError::InvalidValue { + valid: "first or every".into(), + actual: val, + span, + }), + } + } + + fn expected_type() -> Type { + Type::String + } +} + +impl DeprecationEntry { + fn check(&self, call: &Call) -> bool { + match &self.ty { + DeprecationType::Command => true, + DeprecationType::Flag(flag) => call.get_named_arg(flag).is_some(), + } + } + + fn type_name(&self) -> String { + match &self.ty { + DeprecationType::Command => "Command".to_string(), + DeprecationType::Flag(_) => "Flag".to_string(), + } + } + + fn label(&self, command_name: &str) -> String { + let name = match &self.ty { + DeprecationType::Command => command_name, + DeprecationType::Flag(flag) => &format!("{command_name} --{flag}"), + }; + let since = match &self.since { + Some(since) => format!("was deprecated in {since}"), + None => "is deprecated".to_string(), + }; + let removal = match &self.expected_removal { + Some(expected) => format!("and will be removed in {expected}"), + None => "and will be removed in a future release".to_string(), + }; + format!("{name} {since} {removal}.") + } + + fn span(&self, call: &Call) -> Span { + match &self.ty { + DeprecationType::Command => call.span(), + DeprecationType::Flag(flag) => call + .get_named_arg(flag) + .map(|arg| arg.span) + .unwrap_or(Span::unknown()), + } + } + + pub fn parse_warning(self, command_name: &str, call: &Call) -> Option { + if !self.check(call) { + return None; + } + + let dep_type = self.type_name(); + let label = self.label(command_name); + let span = self.span(call); + let report_mode = self.report_mode; + Some(ParseWarning::DeprecationWarning { + dep_type, + label, + span, + report_mode, + help: self.help, + }) + } +} diff --git a/crates/nu-protocol/src/engine/command.rs b/crates/nu-protocol/src/engine/command.rs index 167e05f3ac..59039e665f 100644 --- a/crates/nu-protocol/src/engine/command.rs +++ b/crates/nu-protocol/src/engine/command.rs @@ -1,6 +1,7 @@ use super::{EngineState, Stack, StateWorkingSet}; use crate::{ - Alias, BlockId, Example, OutDest, PipelineData, ShellError, Signature, Value, engine::Call, + Alias, BlockId, DeprecationEntry, Example, OutDest, PipelineData, ShellError, Signature, Value, + engine::Call, }; use std::fmt::Display; @@ -133,6 +134,10 @@ pub trait Command: Send + Sync + CommandClone { self.command_type() == CommandType::Plugin } + fn deprecation_info(&self) -> Vec { + vec![] + } + fn pipe_redirection(&self) -> (Option, Option) { (None, None) } diff --git a/crates/nu-protocol/src/engine/engine_state.rs b/crates/nu-protocol/src/engine/engine_state.rs index c2496c1d3a..9d00ddd62c 100644 --- a/crates/nu-protocol/src/engine/engine_state.rs +++ b/crates/nu-protocol/src/engine/engine_state.rs @@ -3,6 +3,7 @@ use crate::{ ModuleId, OverlayId, ShellError, SignalAction, Signals, Signature, Span, SpanId, Type, Value, VarId, VirtualPathId, ast::Block, + cli_error::ReportLog, debugger::{Debugger, NoopDebugger}, engine::{ CachedFile, Command, CommandType, DEFAULT_OVERLAY_NAME, EnvVars, OverlayFrame, ScopeFrame, @@ -115,6 +116,7 @@ pub struct EngineState { startup_time: i64, is_debugging: IsDebugging, pub debugger: Arc>>, + pub report_log: Arc>, pub jobs: Arc>, @@ -201,6 +203,7 @@ impl EngineState { startup_time: -1, is_debugging: IsDebugging::new(false), debugger: Arc::new(Mutex::new(Box::new(NoopDebugger))), + report_log: Arc::default(), jobs: Arc::new(Mutex::new(Jobs::default())), current_job: CurrentJob { id: JobId::new(0), diff --git a/crates/nu-protocol/src/errors/cli_error.rs b/crates/nu-protocol/src/errors/cli_error.rs index fa17f89043..06f02cd895 100644 --- a/crates/nu-protocol/src/errors/cli_error.rs +++ b/crates/nu-protocol/src/errors/cli_error.rs @@ -1,6 +1,8 @@ //! This module manages the step of turning error types into printed error messages //! //! Relies on the `miette` crate for pretty layout +use std::hash::{DefaultHasher, Hash, Hasher}; + use crate::{ CompileError, ErrorStyle, ParseError, ParseWarning, ShellError, engine::{EngineState, StateWorkingSet}, @@ -9,6 +11,7 @@ use miette::{ LabeledSpan, MietteHandlerOpts, NarratableReportHandler, ReportHandler, RgbColors, Severity, SourceCode, }; +use serde::{Deserialize, Serialize}; use thiserror::Error; /// This error exists so that we can defer SourceCode handling. It simply @@ -20,6 +23,46 @@ struct CliError<'src>( pub &'src StateWorkingSet<'src>, ); +#[derive(Default)] +pub struct ReportLog { + // A bloom-filter like structure to store the hashes of `ParseWarning`s, + // without actually permanently storing the entire warning in memory. + // May rarely result in warnings incorrectly being unreported upon hash collision. + parse_warnings: Vec, +} + +/// How a warning/error should be reported +#[derive(Debug, Clone, Copy, Serialize, Deserialize)] +pub enum ReportMode { + FirstUse, + EveryUse, +} + +/// Returns true if this warning should be reported +fn should_show_warning(engine_state: &EngineState, warning: &ParseWarning) -> bool { + match warning.report_mode() { + ReportMode::EveryUse => true, + ReportMode::FirstUse => { + let mut hasher = DefaultHasher::new(); + warning.hash(&mut hasher); + let hash = hasher.finish(); + + let mut report_log = engine_state + .report_log + .lock() + .expect("report log lock is poisioned"); + + match report_log.parse_warnings.contains(&hash) { + true => false, + false => { + report_log.parse_warnings.push(hash); + true + } + } + } + } +} + pub fn format_shell_error(working_set: &StateWorkingSet, error: &ShellError) -> String { format!("Error: {:?}", CliError(error, working_set)) } @@ -30,9 +73,9 @@ pub fn report_shell_error(engine_state: &EngineState, error: &ShellError) { } } -pub fn report_shell_warning(engine_state: &EngineState, error: &ShellError) { - if engine_state.config.display_errors.should_show(error) { - report_warning(&StateWorkingSet::new(engine_state), error) +pub fn report_shell_warning(engine_state: &EngineState, warning: &ShellError) { + if engine_state.config.display_errors.should_show(warning) { + report_warning(&StateWorkingSet::new(engine_state), warning) } } @@ -40,8 +83,10 @@ pub fn report_parse_error(working_set: &StateWorkingSet, error: &ParseError) { report_error(working_set, error); } -pub fn report_parse_warning(working_set: &StateWorkingSet, error: &ParseWarning) { - report_warning(working_set, error); +pub fn report_parse_warning(working_set: &StateWorkingSet, warning: &ParseWarning) { + if should_show_warning(working_set.permanent(), warning) { + report_warning(working_set, warning); + } } pub fn report_compile_error(working_set: &StateWorkingSet, error: &CompileError) { @@ -57,8 +102,8 @@ fn report_error(working_set: &StateWorkingSet, error: &dyn miette::Diagnostic) { } } -fn report_warning(working_set: &StateWorkingSet, error: &dyn miette::Diagnostic) { - eprintln!("Warning: {:?}", CliError(error, working_set)); +fn report_warning(working_set: &StateWorkingSet, warning: &dyn miette::Diagnostic) { + eprintln!("Warning: {:?}", CliError(warning, working_set)); // reset vt processing, aka ansi because illbehaved externals can break it #[cfg(windows)] { diff --git a/crates/nu-protocol/src/errors/mod.rs b/crates/nu-protocol/src/errors/mod.rs index 9c729dc8f4..ed5a553287 100644 --- a/crates/nu-protocol/src/errors/mod.rs +++ b/crates/nu-protocol/src/errors/mod.rs @@ -8,7 +8,7 @@ mod parse_warning; pub mod shell_error; pub use cli_error::{ - format_shell_error, report_parse_error, report_parse_warning, report_shell_error, + ReportMode, format_shell_error, report_parse_error, report_parse_warning, report_shell_error, report_shell_warning, }; pub use compile_error::CompileError; diff --git a/crates/nu-protocol/src/errors/parse_warning.rs b/crates/nu-protocol/src/errors/parse_warning.rs index 6fa4ee790d..c5d66e46ff 100644 --- a/crates/nu-protocol/src/errors/parse_warning.rs +++ b/crates/nu-protocol/src/errors/parse_warning.rs @@ -1,27 +1,50 @@ use crate::Span; use miette::Diagnostic; use serde::{Deserialize, Serialize}; +use std::hash::Hash; use thiserror::Error; +use super::ReportMode; + #[derive(Clone, Debug, Error, Diagnostic, Serialize, Deserialize)] pub enum ParseWarning { - #[error("Deprecated: {old_command}")] - #[diagnostic(help("for more info see {url}"))] - DeprecatedWarning { - old_command: String, - new_suggestion: String, - #[label( - "`{old_command}` is deprecated and will be removed in a future release. Please {new_suggestion} instead." - )] + #[error("{dep_type} deprecated.")] + #[diagnostic(code(nu::parser::deprecated))] + DeprecationWarning { + dep_type: String, + #[label("{label}")] span: Span, - url: String, + label: String, + report_mode: ReportMode, + #[help] + help: Option, }, } impl ParseWarning { pub fn span(&self) -> Span { match self { - ParseWarning::DeprecatedWarning { span, .. } => *span, + ParseWarning::DeprecationWarning { span, .. } => *span, + } + } + + pub fn report_mode(&self) -> ReportMode { + match self { + ParseWarning::DeprecationWarning { report_mode, .. } => *report_mode, + } + } +} + +// To keep track of reported warnings +impl Hash for ParseWarning { + fn hash(&self, state: &mut H) { + match self { + ParseWarning::DeprecationWarning { + dep_type, label, .. + } => { + dep_type.hash(state); + label.hash(state); + } } } } diff --git a/crates/nu-protocol/src/lib.rs b/crates/nu-protocol/src/lib.rs index f1d58c272d..73d83694d9 100644 --- a/crates/nu-protocol/src/lib.rs +++ b/crates/nu-protocol/src/lib.rs @@ -5,6 +5,7 @@ pub mod ast; pub mod casing; pub mod config; pub mod debugger; +mod deprecation; mod did_you_mean; pub mod engine; mod errors; @@ -30,6 +31,7 @@ mod value; pub use alias::*; pub use ast::unit::*; pub use config::*; +pub use deprecation::*; pub use did_you_mean::did_you_mean; pub use engine::{ENV_VARIABLE_ID, IN_VARIABLE_ID, NU_VARIABLE_ID}; pub use errors::*; diff --git a/crates/nu-protocol/src/signature.rs b/crates/nu-protocol/src/signature.rs index ba576dc7b6..cae7787a86 100644 --- a/crates/nu-protocol/src/signature.rs +++ b/crates/nu-protocol/src/signature.rs @@ -1,8 +1,9 @@ use crate::{ - BlockId, Example, PipelineData, ShellError, SyntaxShape, Type, Value, VarId, + BlockId, DeprecationEntry, Example, FromValue, PipelineData, ShellError, SyntaxShape, Type, + Value, VarId, engine::{Call, Command, CommandType, EngineState, Stack}, }; -use nu_derive_value::FromValue; +use nu_derive_value::FromValue as DeriveFromValue; use serde::{Deserialize, Serialize}; use std::fmt::Write; @@ -701,7 +702,7 @@ fn get_positional_short_name(arg: &PositionalArg, is_required: bool) -> String { } } -#[derive(Clone, FromValue)] +#[derive(Clone, DeriveFromValue)] pub struct CustomExample { pub example: String, pub description: String, @@ -785,4 +786,16 @@ impl Command for BlockCommand { .map(String::as_str) .collect() } + + fn deprecation_info(&self) -> Vec { + self.attributes + .iter() + .filter_map(|(key, value)| { + (key == "deprecated") + .then_some(value.clone()) + .map(DeprecationEntry::from_value) + .and_then(Result::ok) + }) + .collect() + } } diff --git a/crates/nu-protocol/src/value/from_value.rs b/crates/nu-protocol/src/value/from_value.rs index fa4ee132bf..7893df4730 100644 --- a/crates/nu-protocol/src/value/from_value.rs +++ b/crates/nu-protocol/src/value/from_value.rs @@ -27,6 +27,8 @@ use std::{ /// - If `#[nu_value(rename_all = "...")]` is applied on the container (struct) the key of the /// field will be case-converted accordingly. /// - If neither attribute is applied, the field name is used as is. +/// - If `#[nu_value(default)]` is applied to a field, the field type's [`Default`] implementation +/// will be used if the corresponding record field is missing /// /// Supported case conversions include those provided by [`heck`], such as /// "snake_case", "kebab-case", "PascalCase", and others. diff --git a/crates/nu-protocol/src/value/test_derive.rs b/crates/nu-protocol/src/value/test_derive.rs index bdf8c058fa..2029de8d77 100644 --- a/crates/nu-protocol/src/value/test_derive.rs +++ b/crates/nu-protocol/src/value/test_derive.rs @@ -669,3 +669,30 @@ fn renamed_variant_enum_roundtrip() { .into_test_value(); assert_eq!(expected, actual); } + +#[derive(IntoValue, FromValue, Default, Debug, PartialEq)] +struct DefaultFieldStruct { + #[nu_value(default)] + field: String, + #[nu_value(rename = "renamed", default)] + field_two: String, +} + +#[test] +fn default_field_struct_from_value() { + let populated = DefaultFieldStruct { + field: "hello".into(), + field_two: "world".into(), + }; + let populated_record = Value::test_record(record! { + "field" => Value::test_string("hello"), + "renamed" => Value::test_string("world"), + }); + let actual = DefaultFieldStruct::from_value(populated_record).unwrap(); + assert_eq!(populated, actual); + + let default = DefaultFieldStruct::default(); + let default_record = Value::test_record(Record::new()); + let actual = DefaultFieldStruct::from_value(default_record).unwrap(); + assert_eq!(default, actual); +}