Deprecate --flag: bool in custom command (#11365)

# Description
While #11057 is merged, it's hard to tell the difference between
`--flag: bool` and `--flag`, and it makes user hard to read custom
commands' signature, and hard to use them correctly.

After discussion, I think we can deprecate `--flag: bool` usage, and
encourage using `--flag` instead.

# User-Facing Changes
The following code will raise warning message, but don't stop from
running.
```nushell
❯ def florb [--dry-run: bool, --another-flag] { "aaa" };  florb
Error:   × Deprecated: --flag: bool
   ╭─[entry #7:1:1]
 1 │ def florb [--dry-run: bool, --another-flag] { "aaa" };  florb
   ·                       ──┬─
   ·                         ╰── `--flag: bool` is deprecated. Please use `--flag` instead, more info: https://www.nushell.sh/book/custom_commands.html
   ╰────

aaa
```

cc @kubouch 

# Tests + Formatting
Done

# After Submitting
- [ ] Add more information under
https://www.nushell.sh/book/custom_commands.html to indicate `--dry-run:
bool` is not allowed,
- [ ] remove `: bool` from custom commands between 0.89 and 0.90

---------

Co-authored-by: Antoine Stevan <44101798+amtoine@users.noreply.github.com>
This commit is contained in:
WindSoilder 2023-12-21 17:07:08 +08:00 committed by GitHub
parent 109f629cb6
commit 5d98a727ca
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 56 additions and 3 deletions

View File

@ -35,6 +35,10 @@ pub fn evaluate_commands(
let mut working_set = StateWorkingSet::new(engine_state); let mut working_set = StateWorkingSet::new(engine_state);
let output = parse(&mut working_set, None, commands.item.as_bytes(), false); let output = parse(&mut working_set, None, commands.item.as_bytes(), false);
if let Some(warning) = working_set.parse_warnings.first() {
report_error(&working_set, warning);
}
if let Some(err) = working_set.parse_errors.first() { if let Some(err) = working_set.parse_errors.first() {
report_error(&working_set, err); report_error(&working_set, err);

View File

@ -220,6 +220,10 @@ pub fn eval_source(
source, source,
false, false,
); );
if let Some(warning) = working_set.parse_warnings.first() {
report_error(&working_set, warning);
}
if let Some(err) = working_set.parse_errors.first() { if let Some(err) = working_set.parse_errors.first() {
set_last_exit_code(stack, 1); set_last_exit_code(stack, 1);
report_error(&working_set, err); report_error(&working_set, err);

View File

@ -18,8 +18,8 @@ use nu_protocol::{
}, },
engine::StateWorkingSet, engine::StateWorkingSet,
eval_const::eval_constant, eval_const::eval_constant,
span, BlockId, DidYouMean, Flag, ParseError, PositionalArg, Signature, Span, Spanned, span, BlockId, DidYouMean, Flag, ParseError, ParseWarning, PositionalArg, Signature, Span,
SyntaxShape, Type, Unit, VarId, ENV_VARIABLE_ID, IN_VARIABLE_ID, Spanned, SyntaxShape, Type, Unit, VarId, ENV_VARIABLE_ID, IN_VARIABLE_ID,
}; };
use crate::parse_keywords::{ use crate::parse_keywords::{
@ -3520,6 +3520,13 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) ->
type_annotated, type_annotated,
} => { } => {
working_set.set_variable_type(var_id.expect("internal error: all custom parameters must have var_ids"), syntax_shape.to_type()); working_set.set_variable_type(var_id.expect("internal error: all custom parameters must have var_ids"), syntax_shape.to_type());
if syntax_shape == SyntaxShape::Boolean {
working_set.warning(ParseWarning::DeprecatedWarning(
"--flag: bool".to_string(),
"--flag".to_string(),
span,
));
}
*arg = Some(syntax_shape); *arg = Some(syntax_shape);
*type_annotated = true; *type_annotated = true;
} }

View File

@ -6,7 +6,7 @@ use crate::ast::Block;
use crate::{ use crate::{
BlockId, Config, DeclId, FileId, Module, ModuleId, Span, Type, VarId, Variable, VirtualPathId, BlockId, Config, DeclId, FileId, Module, ModuleId, Span, Type, VarId, Variable, VirtualPathId,
}; };
use crate::{Category, ParseError, Value}; use crate::{Category, ParseError, ParseWarning, Value};
use core::panic; use core::panic;
use std::collections::{HashMap, HashSet}; use std::collections::{HashMap, HashSet};
use std::path::PathBuf; use std::path::PathBuf;
@ -27,6 +27,7 @@ pub struct StateWorkingSet<'a> {
/// Whether or not predeclarations are searched when looking up a command (used with aliases) /// Whether or not predeclarations are searched when looking up a command (used with aliases)
pub search_predecls: bool, pub search_predecls: bool,
pub parse_errors: Vec<ParseError>, pub parse_errors: Vec<ParseError>,
pub parse_warnings: Vec<ParseWarning>,
} }
impl<'a> StateWorkingSet<'a> { impl<'a> StateWorkingSet<'a> {
@ -39,6 +40,7 @@ impl<'a> StateWorkingSet<'a> {
parsed_module_files: vec![], parsed_module_files: vec![],
search_predecls: true, search_predecls: true,
parse_errors: vec![], parse_errors: vec![],
parse_warnings: vec![],
} }
} }
@ -50,6 +52,10 @@ impl<'a> StateWorkingSet<'a> {
self.parse_errors.push(parse_error) self.parse_errors.push(parse_error)
} }
pub fn warning(&mut self, parse_warning: ParseWarning) {
self.parse_warnings.push(parse_warning)
}
pub fn num_files(&self) -> usize { pub fn num_files(&self) -> usize {
self.delta.num_files() + self.permanent_state.num_files() self.delta.num_files() + self.permanent_state.num_files()
} }

View File

@ -12,6 +12,7 @@ mod id;
mod lev_distance; mod lev_distance;
mod module; mod module;
mod parse_error; mod parse_error;
mod parse_warning;
mod pipeline_data; mod pipeline_data;
#[cfg(feature = "plugin")] #[cfg(feature = "plugin")]
mod plugin_signature; mod plugin_signature;
@ -35,6 +36,7 @@ pub use id::*;
pub use lev_distance::levenshtein_distance; pub use lev_distance::levenshtein_distance;
pub use module::*; pub use module::*;
pub use parse_error::{DidYouMean, ParseError}; pub use parse_error::{DidYouMean, ParseError};
pub use parse_warning::ParseWarning;
pub use pipeline_data::*; pub use pipeline_data::*;
#[cfg(feature = "plugin")] #[cfg(feature = "plugin")]
pub use plugin_signature::*; pub use plugin_signature::*;

View File

@ -0,0 +1,23 @@
use crate::Span;
use miette::Diagnostic;
use serde::{Deserialize, Serialize};
use thiserror::Error;
#[derive(Clone, Debug, Error, Diagnostic, Serialize, Deserialize)]
pub enum ParseWarning {
#[error("Deprecated: {0}")]
DeprecatedWarning(
String,
String,
#[label = "`{0}` is deprecated and will be removed in 0.90. Please use `{1}` instead, more info: https://www.nushell.sh/book/custom_commands.html"]
Span,
),
}
impl ParseWarning {
pub fn span(&self) -> Span {
match self {
ParseWarning::DeprecatedWarning(_, _, s) => *s,
}
}
}

View File

@ -146,6 +146,13 @@ fn custom_flag2() -> TestResult {
) )
} }
#[test]
fn deprecated_boolean_flag() {
let actual = nu!(r#"def florb [--dry-run: bool, --another-flag] { "aaa" }; florb"#);
assert!(actual.err.contains("Deprecated"));
assert_eq!(actual.out, "aaa");
}
#[test] #[test]
fn simple_var_closing() -> TestResult { fn simple_var_closing() -> TestResult {
run_test("let $x = 10; def foo [] { $x }; foo", "10") run_test("let $x = 10; def foo [] { $x }; foo", "10")