From 59ad605e2242944e452d44ae9bc44d31d4e19e9d Mon Sep 17 00:00:00 2001 From: 132ikl <132@ikl.sh> Date: Tue, 15 Jul 2025 08:30:18 -0400 Subject: [PATCH] Add `ShellWarning` (#16147) # Description Adds a proper `ShellWarning` enum which has the same functionality as `ParseWarning`. Also moves the deprecation from #15806 into `ShellWarning::Deprecated` with `ReportMode::FirstUse`, so that warning will only pop up once now. # User-Facing Changes Technically the change to the deprecation warning from #15806 is user facing but it's really not worth listing in the changelog --- crates/nu-cli/src/eval_cmds.rs | 2 +- crates/nu-cli/src/eval_file.rs | 2 +- crates/nu-cli/src/util.rs | 2 +- crates/nu-cmd-base/src/hook.rs | 2 +- crates/nu-command/src/filters/default.rs | 13 ++--- crates/nu-engine/src/command_prelude.rs | 4 +- crates/nu-protocol/src/deprecation.rs | 2 +- crates/nu-protocol/src/engine/engine_state.rs | 2 +- crates/nu-protocol/src/errors/mod.rs | 12 +++-- .../nu-protocol/src/errors/parse_warning.rs | 18 ++++--- .../errors/{cli_error.rs => report_error.rs} | 38 +++++++------ .../nu-protocol/src/errors/shell_warning.rs | 53 +++++++++++++++++++ devdocs/FAQ.md | 2 +- src/experimental_options.rs | 2 +- 14 files changed, 109 insertions(+), 45 deletions(-) rename crates/nu-protocol/src/errors/{cli_error.rs => report_error.rs} (88%) create mode 100644 crates/nu-protocol/src/errors/shell_warning.rs diff --git a/crates/nu-cli/src/eval_cmds.rs b/crates/nu-cli/src/eval_cmds.rs index dce1417507..744604d71e 100644 --- a/crates/nu-cli/src/eval_cmds.rs +++ b/crates/nu-cli/src/eval_cmds.rs @@ -3,9 +3,9 @@ use nu_engine::eval_block; use nu_parser::parse; use nu_protocol::{ PipelineData, ShellError, Spanned, Value, - cli_error::report_compile_error, debugger::WithoutDebug, engine::{EngineState, Stack, StateWorkingSet}, + report_error::report_compile_error, report_parse_error, report_parse_warning, }; use std::sync::Arc; diff --git a/crates/nu-cli/src/eval_file.rs b/crates/nu-cli/src/eval_file.rs index 14d3e3981b..15600c9e9b 100644 --- a/crates/nu-cli/src/eval_file.rs +++ b/crates/nu-cli/src/eval_file.rs @@ -5,9 +5,9 @@ use nu_parser::parse; use nu_path::canonicalize_with; use nu_protocol::{ PipelineData, ShellError, Span, Value, - cli_error::report_compile_error, debugger::WithoutDebug, engine::{EngineState, Stack, StateWorkingSet}, + report_error::report_compile_error, report_parse_error, report_parse_warning, shell_error::io::*, }; diff --git a/crates/nu-cli/src/util.rs b/crates/nu-cli/src/util.rs index 261d7b2ba9..980474b32e 100644 --- a/crates/nu-cli/src/util.rs +++ b/crates/nu-cli/src/util.rs @@ -5,9 +5,9 @@ use nu_engine::{eval_block, eval_block_with_early_return}; use nu_parser::{Token, TokenContents, lex, parse, unescape_unquote_string}; use nu_protocol::{ PipelineData, ShellError, Span, Value, - cli_error::report_compile_error, debugger::WithoutDebug, engine::{EngineState, Stack, StateWorkingSet}, + report_error::report_compile_error, report_parse_error, report_parse_warning, report_shell_error, }; #[cfg(windows)] diff --git a/crates/nu-cmd-base/src/hook.rs b/crates/nu-cmd-base/src/hook.rs index feb912efa4..33072999ec 100644 --- a/crates/nu-cmd-base/src/hook.rs +++ b/crates/nu-cmd-base/src/hook.rs @@ -3,9 +3,9 @@ use nu_engine::{eval_block, eval_block_with_early_return, redirect_env}; use nu_parser::parse; use nu_protocol::{ PipelineData, PositionalArg, ShellError, Span, Type, Value, VarId, - cli_error::{report_parse_error, report_shell_error}, debugger::WithoutDebug, engine::{Closure, EngineState, Stack, StateWorkingSet}, + report_error::{report_parse_error, report_shell_error}, }; use std::{collections::HashMap, sync::Arc}; diff --git a/crates/nu-command/src/filters/default.rs b/crates/nu-command/src/filters/default.rs index 18dd220376..0b9f42df04 100644 --- a/crates/nu-command/src/filters/default.rs +++ b/crates/nu-command/src/filters/default.rs @@ -2,7 +2,7 @@ use std::{borrow::Cow, ops::Deref}; use nu_engine::{ClosureEval, command_prelude::*}; use nu_protocol::{ - ListStream, Signals, + ListStream, ReportMode, ShellWarning, Signals, ast::{Expr, Expression}, report_shell_warning, }; @@ -329,7 +329,7 @@ fn closure_variable_warning( (Value::Closure { .. }, true) => { let span_contents = String::from_utf8_lossy(engine_state.get_span_contents(span)); let carapace_suggestion = "re-run carapace init with version v1.3.3 or later\nor, change this to `{ $carapace_completer }`"; - let suggestion = match span_contents { + let label = match span_contents { Cow::Borrowed("$carapace_completer") => carapace_suggestion.to_string(), Cow::Owned(s) if s.deref() == "$carapace_completer" => { carapace_suggestion.to_string() @@ -339,14 +339,15 @@ fn closure_variable_warning( report_shell_warning( engine_state, - &ShellError::DeprecationWarning { - deprecation_type: "Behavior", - suggestion, + &ShellWarning::Deprecated { + dep_type: "Behavior".to_string(), + label, span, help: Some( r"Since 0.105.0, closure literals passed to default are lazily evaluated, rather than returned as a value. -In a future release, closures passed by variable will also be lazily evaluated.", +In a future release, closures passed by variable will also be lazily evaluated.".to_string(), ), + report_mode: ReportMode::FirstUse, }, ); diff --git a/crates/nu-engine/src/command_prelude.rs b/crates/nu-engine/src/command_prelude.rs index 702a6a4b51..f5c7ca5034 100644 --- a/crates/nu-engine/src/command_prelude.rs +++ b/crates/nu-engine/src/command_prelude.rs @@ -1,8 +1,8 @@ pub use crate::CallExt; pub use nu_protocol::{ ByteStream, ByteStreamType, Category, ErrSpan, Example, IntoInterruptiblePipelineData, - IntoPipelineData, IntoSpanned, IntoValue, PipelineData, Record, ShellError, Signature, Span, - Spanned, SyntaxShape, Type, Value, + IntoPipelineData, IntoSpanned, IntoValue, PipelineData, Record, ShellError, ShellWarning, + Signature, Span, Spanned, SyntaxShape, Type, Value, ast::CellPath, engine::{Call, Command, EngineState, Stack, StateWorkingSet}, record, diff --git a/crates/nu-protocol/src/deprecation.rs b/crates/nu-protocol/src/deprecation.rs index c25ca99aba..d7ab11f52a 100644 --- a/crates/nu-protocol/src/deprecation.rs +++ b/crates/nu-protocol/src/deprecation.rs @@ -133,7 +133,7 @@ impl DeprecationEntry { let label = self.label(command_name); let span = self.span(call); let report_mode = self.report_mode; - Some(ParseWarning::DeprecationWarning { + Some(ParseWarning::Deprecated { dep_type, label, span, diff --git a/crates/nu-protocol/src/engine/engine_state.rs b/crates/nu-protocol/src/engine/engine_state.rs index 6a42014ebd..11335cfe06 100644 --- a/crates/nu-protocol/src/engine/engine_state.rs +++ b/crates/nu-protocol/src/engine/engine_state.rs @@ -3,7 +3,6 @@ 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, @@ -11,6 +10,7 @@ use crate::{ description::{Doccomments, build_desc}, }, eval_const::create_nu_constant, + report_error::ReportLog, shell_error::io::IoError, }; use fancy_regex::Regex; diff --git a/crates/nu-protocol/src/errors/mod.rs b/crates/nu-protocol/src/errors/mod.rs index d453cab3d3..071d0bb211 100644 --- a/crates/nu-protocol/src/errors/mod.rs +++ b/crates/nu-protocol/src/errors/mod.rs @@ -1,19 +1,21 @@ mod chained_error; -pub mod cli_error; mod compile_error; mod config_error; mod labeled_error; mod parse_error; mod parse_warning; +pub mod report_error; pub mod shell_error; +pub mod shell_warning; -pub use cli_error::{ - ReportMode, format_cli_error, report_parse_error, report_parse_warning, report_shell_error, - report_shell_warning, -}; pub use compile_error::CompileError; pub use config_error::ConfigError; pub use labeled_error::{ErrorLabel, LabeledError}; pub use parse_error::{DidYouMean, ParseError}; pub use parse_warning::ParseWarning; +pub use report_error::{ + ReportMode, Reportable, format_cli_error, report_parse_error, report_parse_warning, + report_shell_error, report_shell_warning, +}; pub use shell_error::ShellError; +pub use shell_warning::ShellWarning; diff --git a/crates/nu-protocol/src/errors/parse_warning.rs b/crates/nu-protocol/src/errors/parse_warning.rs index 78fa7ef7a9..24deb41b66 100644 --- a/crates/nu-protocol/src/errors/parse_warning.rs +++ b/crates/nu-protocol/src/errors/parse_warning.rs @@ -4,34 +4,36 @@ use serde::{Deserialize, Serialize}; use std::hash::Hash; use thiserror::Error; -use super::ReportMode; +use crate::{ReportMode, Reportable}; #[derive(Clone, Debug, Error, Diagnostic, Serialize, Deserialize)] #[diagnostic(severity(Warning))] pub enum ParseWarning { #[error("{dep_type} deprecated.")] #[diagnostic(code(nu::parser::deprecated))] - DeprecationWarning { + Deprecated { dep_type: String, + label: String, #[label("{label}")] span: Span, - label: String, - report_mode: ReportMode, #[help] help: Option, + report_mode: ReportMode, }, } impl ParseWarning { pub fn span(&self) -> Span { match self { - ParseWarning::DeprecationWarning { span, .. } => *span, + ParseWarning::Deprecated { span, .. } => *span, } } +} - pub fn report_mode(&self) -> ReportMode { +impl Reportable for ParseWarning { + fn report_mode(&self) -> ReportMode { match self { - ParseWarning::DeprecationWarning { report_mode, .. } => *report_mode, + ParseWarning::Deprecated { report_mode, .. } => *report_mode, } } } @@ -40,7 +42,7 @@ impl ParseWarning { impl Hash for ParseWarning { fn hash(&self, state: &mut H) { match self { - ParseWarning::DeprecationWarning { + ParseWarning::Deprecated { dep_type, label, .. } => { dep_type.hash(state); diff --git a/crates/nu-protocol/src/errors/cli_error.rs b/crates/nu-protocol/src/errors/report_error.rs similarity index 88% rename from crates/nu-protocol/src/errors/cli_error.rs rename to crates/nu-protocol/src/errors/report_error.rs index 2617709742..aeefc62320 100644 --- a/crates/nu-protocol/src/errors/cli_error.rs +++ b/crates/nu-protocol/src/errors/report_error.rs @@ -4,7 +4,7 @@ use std::hash::{DefaultHasher, Hash, Hasher}; use crate::{ - CompileError, ErrorStyle, ParseError, ParseWarning, ShellError, + CompileError, ErrorStyle, ParseError, ParseWarning, ShellError, ShellWarning, engine::{EngineState, StateWorkingSet}, }; use miette::{ @@ -39,13 +39,11 @@ impl<'src> CliError<'src> { } } +/// A bloom-filter like structure to store the hashes of warnings, +/// without actually permanently storing the entire warning in memory. +/// May rarely result in warnings incorrectly being unreported upon hash collision. #[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, -} +pub struct ReportLog(Vec); /// How a warning/error should be reported #[derive(Debug, Clone, Copy, Serialize, Deserialize)] @@ -54,13 +52,21 @@ pub enum ReportMode { EveryUse, } +/// For warnings/errors which have a ReportMode that dictates when they are reported +pub trait Reportable { + fn report_mode(&self) -> ReportMode; +} + /// Returns true if this warning should be reported -fn should_show_warning(engine_state: &EngineState, warning: &ParseWarning) -> bool { - match warning.report_mode() { +fn should_show_reportable(engine_state: &EngineState, reportable: &R) -> bool +where + R: Reportable + Hash, +{ + match reportable.report_mode() { ReportMode::EveryUse => true, ReportMode::FirstUse => { let mut hasher = DefaultHasher::new(); - warning.hash(&mut hasher); + reportable.hash(&mut hasher); let hash = hasher.finish(); let mut report_log = engine_state @@ -68,10 +74,10 @@ fn should_show_warning(engine_state: &EngineState, warning: &ParseWarning) -> bo .lock() .expect("report log lock is poisioned"); - match report_log.parse_warnings.contains(&hash) { + match report_log.0.contains(&hash) { true => false, false => { - report_log.parse_warnings.push(hash); + report_log.0.push(hash); true } } @@ -97,13 +103,13 @@ pub fn report_shell_error(engine_state: &EngineState, error: &ShellError) { } } -pub fn report_shell_warning(engine_state: &EngineState, warning: &ShellError) { - if engine_state.config.display_errors.should_show(warning) { +pub fn report_shell_warning(engine_state: &EngineState, warning: &ShellWarning) { + if should_show_reportable(engine_state, warning) { report_warning( &StateWorkingSet::new(engine_state), warning, "nu::shell::warning", - ) + ); } } @@ -112,7 +118,7 @@ pub fn report_parse_error(working_set: &StateWorkingSet, error: &ParseError) { } pub fn report_parse_warning(working_set: &StateWorkingSet, warning: &ParseWarning) { - if should_show_warning(working_set.permanent(), warning) { + if should_show_reportable(working_set.permanent(), warning) { report_warning(working_set, warning, "nu::parser::warning"); } } diff --git a/crates/nu-protocol/src/errors/shell_warning.rs b/crates/nu-protocol/src/errors/shell_warning.rs new file mode 100644 index 0000000000..50abeaf21f --- /dev/null +++ b/crates/nu-protocol/src/errors/shell_warning.rs @@ -0,0 +1,53 @@ +use crate::Span; +use miette::Diagnostic; +use serde::{Deserialize, Serialize}; +use std::hash::Hash; +use thiserror::Error; + +use crate::{ReportMode, Reportable}; + +#[derive(Clone, Debug, Error, Diagnostic, Serialize, Deserialize)] +#[diagnostic(severity(Warning))] +pub enum ShellWarning { + #[error("{dep_type} deprecated.")] + #[diagnostic(code(nu::shell::deprecated))] + Deprecated { + dep_type: String, + label: String, + #[label("{label}")] + span: Span, + #[help] + help: Option, + report_mode: ReportMode, + }, +} + +impl ShellWarning { + pub fn span(&self) -> Span { + match self { + ShellWarning::Deprecated { span, .. } => *span, + } + } +} + +impl Reportable for ShellWarning { + fn report_mode(&self) -> ReportMode { + match self { + ShellWarning::Deprecated { report_mode, .. } => *report_mode, + } + } +} + +// To keep track of reported warnings +impl Hash for ShellWarning { + fn hash(&self, state: &mut H) { + match self { + ShellWarning::Deprecated { + dep_type, label, .. + } => { + dep_type.hash(state); + label.hash(state); + } + } + } +} diff --git a/devdocs/FAQ.md b/devdocs/FAQ.md index 1527e1642f..c459bcde79 100644 --- a/devdocs/FAQ.md +++ b/devdocs/FAQ.md @@ -29,7 +29,7 @@ Approximate flow: - `return Err(ShellError::...)` and you're done in a `Command::run` 4. Do you want to report a warning but not stop execution? - **NEVER** `println!`, we can write to stderr if necessary but... - - good practice: `nu_protocol::cli_error::report_error` or `report_error_new` + - good practice: `nu_protocol::report_error::report_error` or `report_error_new` - depending on whether you have access to a `StateWorkingSet` - if only relevant to in the field debugging: `log`-crate macros. diff --git a/src/experimental_options.rs b/src/experimental_options.rs index 19954f76e9..d47da1a490 100644 --- a/src/experimental_options.rs +++ b/src/experimental_options.rs @@ -1,8 +1,8 @@ use std::borrow::Borrow; use nu_protocol::{ - cli_error::report_experimental_option_warning, engine::{EngineState, StateWorkingSet}, + report_error::report_experimental_option_warning, }; use crate::command::NushellCliArgs;