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

@@ -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;

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::*,
 };

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)]

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};
 

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 {
+                &ShellWarning::Deprecated {
-                    deprecation_type: "Behavior",
+                    dep_type: "Behavior".to_string(),
-                    suggestion,
+                    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,
                 },
             );
 

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,
+    IntoPipelineData, IntoSpanned, IntoValue, PipelineData, Record, ShellError, ShellWarning,
-    Spanned, SyntaxShape, Type, Value,
+    Signature, Span, Spanned, SyntaxShape, Type, Value,
     ast::CellPath,
     engine::{Call, Command, EngineState, Stack, StateWorkingSet},
     record,

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,

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;

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;

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<String>,
+        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<H: std::hash::Hasher>(&self, state: &mut H) {
         match self {
-            ParseWarning::DeprecationWarning {
+            ParseWarning::Deprecated {
                 dep_type, label, ..
             } => {
                 dep_type.hash(state);

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 {
+pub struct ReportLog(Vec<u64>);
-    // 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<u64>,
-}
 
 /// 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 {
+fn should_show_reportable<R>(engine_state: &EngineState, reportable: &R) -> bool
-    match warning.report_mode() {
+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) {
+pub fn report_shell_warning(engine_state: &EngineState, warning: &ShellWarning) {
-    if engine_state.config.display_errors.should_show(warning) {
+    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");
     }
 }

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<String>,
+        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<H: std::hash::Hasher>(&self, state: &mut H) {
+        match self {
+            ShellWarning::Deprecated {
+                dep_type, label, ..
+            } => {
+                dep_type.hash(state);
+                label.hash(state);
+            }
+        }
+    }
+}

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.
 

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;