do not attempt to glob expand if the file path is wrapped in quotes (#11569)

# Description
Fixes: #11455

### For arguments which is annotated with `:path/:directory/:glob`
To fix the issue, we need to have a way to know if a path is originally
quoted during runtime. So the information needed to be added at several
levels:
* parse time (from user input to expression)
We need to add quoted information into `Expr::Filepath`,
`Expr::Directory`, `Expr::GlobPattern`
* eval time
When convert from `Expr::Filepath`, `Expr::Directory`,
`Expr::GlobPattern` to `Value::String` during runtime, we won't auto
expanded the path if it's quoted

### For `ls`
It's really special, because it accepts a `String` as a pattern, and it
generates `glob` expression inside the command itself.

So the idea behind the change is introducing a special SyntaxShape to
ls: `SyntaxShape::LsGlobPattern`. So we can track if the pattern is
originally quoted easier, and we don't auto expand the path either.

Then when constructing a glob pattern inside ls, we check if input
pattern is quoted, if so: we escape the input pattern, so we can run `ls
a[123]b`, because it's already escaped.
Finally, to accomplish the checking process, we also need to introduce a
new value type called `Value::QuotedString` to differ from
`Value::String`, it's used to generate an enum called `NuPath`, which is
finally used in `ls` function. `ls` learned from `NuPath` to know if
user input is quoted.

# User-Facing Changes
Actually it contains several changes
### For arguments which is annotated with `:path/:directory/:glob`
#### Before
```nushell
> def foo [p: path] { echo $p }; print (foo "~/a"); print (foo '~/a')
/home/windsoilder/a
/home/windsoilder/a
> def foo [p: directory] { echo $p }; print (foo "~/a"); print (foo '~/a')
/home/windsoilder/a
/home/windsoilder/a
> def foo [p: glob] { echo $p }; print (foo "~/a"); print (foo '~/a')
/home/windsoilder/a
/home/windsoilder/a
```
#### After
```nushell
> def foo [p: path] { echo $p }; print (foo "~/a"); print (foo '~/a')
~/a
~/a
> def foo [p: directory] { echo $p }; print (foo "~/a"); print (foo '~/a')
~/a
~/a
> def foo [p: glob] { echo $p }; print (foo "~/a"); print (foo '~/a')
~/a
~/a
```
### For ls command
`touch '[uwu]'`
#### Before
```
❯ ls -D "[uwu]"
Error:   × No matches found for [uwu]
   ╭─[entry #6:1:1]
 1 │ ls -D "[uwu]"
   ·       ───┬───
   ·          ╰── Pattern, file or folder not found
   ╰────
  help: no matches found
```

#### After
```
❯ ls -D "[uwu]"
╭───┬───────┬──────┬──────┬──────────╮
│ # │ name  │ type │ size │ modified │
├───┼───────┼──────┼──────┼──────────┤
│ 0 │ [uwu] │ file │  0 B │ now      │
╰───┴───────┴──────┴──────┴──────────╯
```

# Tests + Formatting
Done

# After Submitting
NaN
This commit is contained in:
WindSoilder
2024-01-21 23:22:25 +08:00
committed by GitHub
parent 64f34e0287
commit c59d6d31bc
26 changed files with 310 additions and 60 deletions

View File

@ -37,9 +37,10 @@ pub enum Expr {
Keyword(Vec<u8>, Span, Box<Expression>),
ValueWithUnit(Box<Expression>, Spanned<Unit>),
DateTime(chrono::DateTime<FixedOffset>),
Filepath(String),
Directory(String),
GlobPattern(String),
Filepath(String, bool),
Directory(String, bool),
GlobPattern(String, bool),
LsGlobPattern(String, bool),
String(String),
CellPath(CellPath),
FullCellPath(Box<FullCellPath>),

View File

@ -197,8 +197,8 @@ impl Expression {
}
Expr::ImportPattern(_) => false,
Expr::Overlay(_) => false,
Expr::Filepath(_) => false,
Expr::Directory(_) => false,
Expr::Filepath(_, _) => false,
Expr::Directory(_, _) => false,
Expr::Float(_) => false,
Expr::FullCellPath(full_cell_path) => {
if full_cell_path.head.has_in_variable(working_set) {
@ -208,7 +208,8 @@ impl Expression {
}
Expr::Garbage => false,
Expr::Nothing => false,
Expr::GlobPattern(_) => false,
Expr::GlobPattern(_, _) => false,
Expr::LsGlobPattern(_, _) => false,
Expr::Int(_) => false,
Expr::Keyword(_, _, expr) => expr.has_in_variable(working_set),
Expr::List(list) => {
@ -375,8 +376,8 @@ impl Expression {
expr.replace_span(working_set, replaced, new_span);
}
}
Expr::Filepath(_) => {}
Expr::Directory(_) => {}
Expr::Filepath(_, _) => {}
Expr::Directory(_, _) => {}
Expr::Float(_) => {}
Expr::FullCellPath(full_cell_path) => {
full_cell_path
@ -387,7 +388,8 @@ impl Expression {
Expr::Overlay(_) => {}
Expr::Garbage => {}
Expr::Nothing => {}
Expr::GlobPattern(_) => {}
Expr::GlobPattern(_, _) => {}
Expr::LsGlobPattern(_, _) => {}
Expr::MatchBlock(_) => {}
Expr::Int(_) => {}
Expr::Keyword(_, _, expr) => expr.replace_span(working_set, replaced, new_span),

View File

@ -27,9 +27,9 @@ pub trait Eval {
Expr::Int(i) => Ok(Value::int(*i, expr.span)),
Expr::Float(f) => Ok(Value::float(*f, expr.span)),
Expr::Binary(b) => Ok(Value::binary(b.clone(), expr.span)),
Expr::Filepath(path) => Self::eval_filepath(state, mut_state, path.clone(), expr.span),
Expr::Directory(path) => {
Self::eval_directory(state, mut_state, path.clone(), expr.span)
Expr::Filepath(path, quoted) => Self::eval_filepath(state, mut_state, path.clone(), *quoted, expr.span),
Expr::Directory(path, quoted) => {
Self::eval_directory(state, mut_state, path.clone(), *quoted, expr.span)
}
Expr::Var(var_id) => Self::eval_var(state, mut_state, *var_id, expr.span),
Expr::CellPath(cell_path) => Ok(Value::cell_path(cell_path.clone(), expr.span)),
@ -274,8 +274,15 @@ pub trait Eval {
Self::eval_string_interpolation(state, mut_state, exprs, expr.span)
}
Expr::Overlay(_) => Self::eval_overlay(state, expr.span),
Expr::GlobPattern(pattern) => {
Self::eval_glob_pattern(state, mut_state, pattern.clone(), expr.span)
Expr::GlobPattern(pattern, quoted) => {
Self::eval_glob_pattern(state, mut_state, pattern.clone(), *quoted, expr.span)
}
Expr::LsGlobPattern(pattern, quoted) => {
if *quoted {
Ok(Value::quoted_string(pattern, expr.span))
} else {
Ok(Value::string(pattern, expr.span))
}
}
Expr::MatchBlock(_) // match blocks are handled by `match`
| Expr::VarDecl(_)
@ -291,6 +298,7 @@ pub trait Eval {
state: Self::State<'_>,
mut_state: &mut Self::MutState,
path: String,
quoted: bool,
span: Span,
) -> Result<Value, ShellError>;
@ -298,6 +306,7 @@ pub trait Eval {
state: Self::State<'_>,
mut_state: &mut Self::MutState,
path: String,
quoted: bool,
span: Span,
) -> Result<Value, ShellError>;
@ -370,6 +379,7 @@ pub trait Eval {
state: Self::State<'_>,
mut_state: &mut Self::MutState,
pattern: String,
quoted: bool,
span: Span,
) -> Result<Value, ShellError>;

View File

@ -282,6 +282,7 @@ impl Eval for EvalConst {
_: &StateWorkingSet,
_: &mut (),
path: String,
_: bool,
span: Span,
) -> Result<Value, ShellError> {
Ok(Value::string(path, span))
@ -291,6 +292,7 @@ impl Eval for EvalConst {
_: &StateWorkingSet,
_: &mut (),
_: String,
_: bool,
span: Span,
) -> Result<Value, ShellError> {
Err(ShellError::NotAConstant { span })
@ -392,6 +394,7 @@ impl Eval for EvalConst {
_: &StateWorkingSet,
_: &mut (),
_: String,
_: bool,
span: Span,
) -> Result<Value, ShellError> {
Err(ShellError::NotAConstant { span })

View File

@ -64,6 +64,9 @@ pub enum SyntaxShape {
/// A glob pattern is allowed, eg `foo*`
GlobPattern,
/// A special glob pattern for ls.
LsGlobPattern,
/// Only an integer value is allowed
Int,
@ -151,6 +154,7 @@ impl SyntaxShape {
SyntaxShape::Filesize => Type::Filesize,
SyntaxShape::FullCellPath => Type::Any,
SyntaxShape::GlobPattern => Type::String,
SyntaxShape::LsGlobPattern => Type::String,
SyntaxShape::Error => Type::Error,
SyntaxShape::ImportPattern => Type::Any,
SyntaxShape::Int => Type::Int,
@ -201,6 +205,7 @@ impl Display for SyntaxShape {
SyntaxShape::Filepath => write!(f, "path"),
SyntaxShape::Directory => write!(f, "directory"),
SyntaxShape::GlobPattern => write!(f, "glob"),
SyntaxShape::LsGlobPattern => write!(f, "glob"),
SyntaxShape::ImportPattern => write!(f, "import"),
SyntaxShape::Block => write!(f, "block"),
SyntaxShape::Closure(args) => {

View File

@ -1,5 +1,6 @@
use std::path::PathBuf;
use super::NuPath;
use crate::ast::{CellPath, PathMember};
use crate::engine::{Block, Closure};
use crate::{Range, Record, ShellError, Spanned, Value};
@ -203,6 +204,45 @@ impl FromValue for Spanned<String> {
}
}
impl FromValue for NuPath {
fn from_value(v: Value) -> Result<Self, ShellError> {
// FIXME: we may want to fail a little nicer here
match v {
Value::CellPath { val, .. } => Ok(NuPath::UnQuoted(val.to_string())),
Value::String { val, .. } => Ok(NuPath::UnQuoted(val)),
Value::QuotedString { val, .. } => Ok(NuPath::Quoted(val)),
v => Err(ShellError::CantConvert {
to_type: "string".into(),
from_type: v.get_type().to_string(),
span: v.span(),
help: None,
}),
}
}
}
impl FromValue for Spanned<NuPath> {
fn from_value(v: Value) -> Result<Self, ShellError> {
let span = v.span();
Ok(Spanned {
item: match v {
Value::CellPath { val, .. } => NuPath::UnQuoted(val.to_string()),
Value::String { val, .. } => NuPath::UnQuoted(val),
Value::QuotedString { val, .. } => NuPath::Quoted(val),
v => {
return Err(ShellError::CantConvert {
to_type: "string".into(),
from_type: v.get_type().to_string(),
span: v.span(),
help: None,
})
}
},
span,
})
}
}
impl FromValue for Vec<String> {
fn from_value(v: Value) -> Result<Self, ShellError> {
// FIXME: we may want to fail a little nicer here

View File

@ -2,6 +2,7 @@ mod custom_value;
mod from;
mod from_value;
mod lazy_record;
mod path;
mod range;
mod record;
mod stream;
@ -24,6 +25,7 @@ use nu_utils::{
contains_emoji, get_system_locale, locale::get_system_locale_string, IgnoreCaseExt,
};
use num_format::ToFormattedString;
pub use path::*;
pub use range::*;
pub use record::Record;
use serde::{Deserialize, Serialize};
@ -90,6 +92,12 @@ pub enum Value {
// please use .span() instead of matching this span value
internal_span: Span,
},
QuotedString {
val: String,
// note: spans are being refactored out of Value
// please use .span() instead of matching this span value
internal_span: Span,
},
Record {
val: Record,
// note: spans are being refactored out of Value
@ -179,6 +187,10 @@ impl Clone for Value {
val: val.clone(),
internal_span: *internal_span,
},
Value::QuotedString { val, internal_span } => Value::QuotedString {
val: val.clone(),
internal_span: *internal_span,
},
Value::Record { val, internal_span } => Value::Record {
val: val.clone(),
internal_span: *internal_span,
@ -509,6 +521,7 @@ impl Value {
| Value::Date { internal_span, .. }
| Value::Range { internal_span, .. }
| Value::String { internal_span, .. }
| Value::QuotedString { internal_span, .. }
| Value::Record { internal_span, .. }
| Value::List { internal_span, .. }
| Value::Block { internal_span, .. }
@ -533,6 +546,7 @@ impl Value {
| Value::Date { internal_span, .. }
| Value::Range { internal_span, .. }
| Value::String { internal_span, .. }
| Value::QuotedString { internal_span, .. }
| Value::Record { internal_span, .. }
| Value::LazyRecord { internal_span, .. }
| Value::List { internal_span, .. }
@ -559,6 +573,7 @@ impl Value {
Value::Date { .. } => Type::Date,
Value::Range { .. } => Type::Range,
Value::String { .. } => Type::String,
Value::QuotedString { .. } => Type::String,
Value::Record { val, .. } => {
Type::Record(val.iter().map(|(x, y)| (x.clone(), y.get_type())).collect())
}
@ -672,6 +687,7 @@ impl Value {
)
}
Value::String { val, .. } => val.clone(),
Value::QuotedString { val, .. } => val.clone(),
Value::List { vals: val, .. } => format!(
"[{}]",
val.iter()
@ -726,6 +742,7 @@ impl Value {
)
}
Value::String { val, .. } => val.to_string(),
Value::QuotedString { val, .. } => val.to_string(),
Value::List { ref vals, .. } => {
if !vals.is_empty() && vals.iter().all(|x| matches!(x, Value::Record { .. })) {
format!(
@ -852,6 +869,7 @@ impl Value {
)
}
Value::String { val, .. } => val.clone(),
Value::QuotedString { val, .. } => val.clone(),
Value::List { vals: val, .. } => format!(
"[{}]",
val.iter()
@ -1750,6 +1768,13 @@ impl Value {
}
}
pub fn quoted_string(val: impl Into<String>, span: Span) -> Value {
Value::QuotedString {
val: val.into(),
internal_span: span,
}
}
pub fn record(val: Record, span: Span) -> Value {
Value::Record {
val,
@ -1955,6 +1980,7 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Less),
Value::Range { .. } => Some(Ordering::Less),
Value::String { .. } => Some(Ordering::Less),
Value::QuotedString { .. } => Some(Ordering::Less),
Value::Record { .. } => Some(Ordering::Less),
Value::LazyRecord { .. } => Some(Ordering::Less),
Value::List { .. } => Some(Ordering::Less),
@ -1975,6 +2001,7 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Less),
Value::Range { .. } => Some(Ordering::Less),
Value::String { .. } => Some(Ordering::Less),
Value::QuotedString { .. } => Some(Ordering::Less),
Value::Record { .. } => Some(Ordering::Less),
Value::LazyRecord { .. } => Some(Ordering::Less),
Value::List { .. } => Some(Ordering::Less),
@ -1995,6 +2022,7 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Less),
Value::Range { .. } => Some(Ordering::Less),
Value::String { .. } => Some(Ordering::Less),
Value::QuotedString { .. } => Some(Ordering::Less),
Value::Record { .. } => Some(Ordering::Less),
Value::LazyRecord { .. } => Some(Ordering::Less),
Value::List { .. } => Some(Ordering::Less),
@ -2015,6 +2043,7 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Less),
Value::Range { .. } => Some(Ordering::Less),
Value::String { .. } => Some(Ordering::Less),
Value::QuotedString { .. } => Some(Ordering::Less),
Value::Record { .. } => Some(Ordering::Less),
Value::LazyRecord { .. } => Some(Ordering::Less),
Value::List { .. } => Some(Ordering::Less),
@ -2035,6 +2064,7 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Less),
Value::Range { .. } => Some(Ordering::Less),
Value::String { .. } => Some(Ordering::Less),
Value::QuotedString { .. } => Some(Ordering::Less),
Value::Record { .. } => Some(Ordering::Less),
Value::LazyRecord { .. } => Some(Ordering::Less),
Value::List { .. } => Some(Ordering::Less),
@ -2055,6 +2085,7 @@ impl PartialOrd for Value {
Value::Date { val: rhs, .. } => lhs.partial_cmp(rhs),
Value::Range { .. } => Some(Ordering::Less),
Value::String { .. } => Some(Ordering::Less),
Value::QuotedString { .. } => Some(Ordering::Less),
Value::Record { .. } => Some(Ordering::Less),
Value::LazyRecord { .. } => Some(Ordering::Less),
Value::List { .. } => Some(Ordering::Less),
@ -2075,6 +2106,7 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Greater),
Value::Range { val: rhs, .. } => lhs.partial_cmp(rhs),
Value::String { .. } => Some(Ordering::Less),
Value::QuotedString { .. } => Some(Ordering::Less),
Value::Record { .. } => Some(Ordering::Less),
Value::LazyRecord { .. } => Some(Ordering::Less),
Value::List { .. } => Some(Ordering::Less),
@ -2095,6 +2127,28 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Greater),
Value::Range { .. } => Some(Ordering::Greater),
Value::String { val: rhs, .. } => lhs.partial_cmp(rhs),
Value::QuotedString { val: rhs, .. } => lhs.partial_cmp(rhs),
Value::Record { .. } => Some(Ordering::Less),
Value::LazyRecord { .. } => Some(Ordering::Less),
Value::List { .. } => Some(Ordering::Less),
Value::Block { .. } => Some(Ordering::Less),
Value::Closure { .. } => Some(Ordering::Less),
Value::Nothing { .. } => Some(Ordering::Less),
Value::Error { .. } => Some(Ordering::Less),
Value::Binary { .. } => Some(Ordering::Less),
Value::CellPath { .. } => Some(Ordering::Less),
Value::CustomValue { .. } => Some(Ordering::Less),
},
(Value::QuotedString { val: lhs, .. }, rhs) => match rhs {
Value::Bool { .. } => Some(Ordering::Greater),
Value::Int { .. } => Some(Ordering::Greater),
Value::Float { .. } => Some(Ordering::Greater),
Value::Filesize { .. } => Some(Ordering::Greater),
Value::Duration { .. } => Some(Ordering::Greater),
Value::Date { .. } => Some(Ordering::Greater),
Value::Range { .. } => Some(Ordering::Greater),
Value::String { val: rhs, .. } => lhs.partial_cmp(rhs),
Value::QuotedString { val: rhs, .. } => lhs.partial_cmp(rhs),
Value::Record { .. } => Some(Ordering::Less),
Value::LazyRecord { .. } => Some(Ordering::Less),
Value::List { .. } => Some(Ordering::Less),
@ -2115,6 +2169,7 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Greater),
Value::Range { .. } => Some(Ordering::Greater),
Value::String { .. } => Some(Ordering::Greater),
Value::QuotedString { .. } => Some(Ordering::Greater),
Value::Record { val: rhs, .. } => {
// reorder cols and vals to make more logically compare.
// more general, if two record have same col and values,
@ -2154,6 +2209,7 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Greater),
Value::Range { .. } => Some(Ordering::Greater),
Value::String { .. } => Some(Ordering::Greater),
Value::QuotedString { .. } => Some(Ordering::Greater),
Value::Record { .. } => Some(Ordering::Greater),
Value::LazyRecord { .. } => Some(Ordering::Greater),
Value::List { vals: rhs, .. } => lhs.partial_cmp(rhs),
@ -2174,6 +2230,7 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Greater),
Value::Range { .. } => Some(Ordering::Greater),
Value::String { .. } => Some(Ordering::Greater),
Value::QuotedString { .. } => Some(Ordering::Greater),
Value::Record { .. } => Some(Ordering::Greater),
Value::List { .. } => Some(Ordering::Greater),
Value::LazyRecord { .. } => Some(Ordering::Greater),
@ -2194,6 +2251,7 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Greater),
Value::Range { .. } => Some(Ordering::Greater),
Value::String { .. } => Some(Ordering::Greater),
Value::QuotedString { .. } => Some(Ordering::Greater),
Value::Record { .. } => Some(Ordering::Greater),
Value::LazyRecord { .. } => Some(Ordering::Greater),
Value::List { .. } => Some(Ordering::Greater),
@ -2214,6 +2272,7 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Greater),
Value::Range { .. } => Some(Ordering::Greater),
Value::String { .. } => Some(Ordering::Greater),
Value::QuotedString { .. } => Some(Ordering::Greater),
Value::Record { .. } => Some(Ordering::Greater),
Value::LazyRecord { .. } => Some(Ordering::Greater),
Value::List { .. } => Some(Ordering::Greater),
@ -2234,6 +2293,7 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Greater),
Value::Range { .. } => Some(Ordering::Greater),
Value::String { .. } => Some(Ordering::Greater),
Value::QuotedString { .. } => Some(Ordering::Greater),
Value::Record { .. } => Some(Ordering::Greater),
Value::LazyRecord { .. } => Some(Ordering::Greater),
Value::List { .. } => Some(Ordering::Greater),
@ -2254,6 +2314,7 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Greater),
Value::Range { .. } => Some(Ordering::Greater),
Value::String { .. } => Some(Ordering::Greater),
Value::QuotedString { .. } => Some(Ordering::Greater),
Value::Record { .. } => Some(Ordering::Greater),
Value::LazyRecord { .. } => Some(Ordering::Greater),
Value::List { .. } => Some(Ordering::Greater),
@ -2274,6 +2335,7 @@ impl PartialOrd for Value {
Value::Date { .. } => Some(Ordering::Greater),
Value::Range { .. } => Some(Ordering::Greater),
Value::String { .. } => Some(Ordering::Greater),
Value::QuotedString { .. } => Some(Ordering::Greater),
Value::Record { .. } => Some(Ordering::Greater),
Value::LazyRecord { .. } => Some(Ordering::Greater),
Value::List { .. } => Some(Ordering::Greater),

View File

@ -0,0 +1,19 @@
/// A simple wrapper to String.
///
/// But it tracks if the string is originally quoted.
/// So commands can make decision on path auto-expanding behavior.
#[derive(Debug, Clone)]
pub enum NuPath {
/// A quoted path(except backtick), in this case, nushell shouldn't auto-expand path.
Quoted(String),
/// An unquoted path, in this case, nushell should auto-expand path.
UnQuoted(String),
}
impl AsRef<str> for NuPath {
fn as_ref(&self) -> &str {
match self {
NuPath::Quoted(s) | NuPath::UnQuoted(s) => s,
}
}
}