Skip to content

Commit

Permalink
Allow string to copmpare with another string (nushell#11590)
Browse files Browse the repository at this point in the history
# Description

Nushell parser now reject comparison operator with 2 strings (e.g.
`"abc" < "cba"`). This pr fixes it.

## before

```nu
~
❯ "abc" < "bca"
Error: nu::parser::unsupported_operation

  × less-than comparison is not supported on values of type string
   ╭─[entry nushell#43:1:1]
 1 │ "abc" < "bca"
   · ──┬── ┬
   ·   │   ╰── doesn't support this value
   ·   ╰── string
   ╰────


~
❯ def foo []: nothing -> string { "abc" }

~
❯ (foo) < "bca"
Error: nu::parser::unsupported_operation

  × less-than comparison is not supported on values of type string
   ╭─[entry nushell#53:1:1]
 1 │ (foo) < "bca"
   · ──┬── ┬
   ·   │   ╰── doesn't support this value
   ·   ╰── string
   ╰────
```

## after

```nu
~
❯ "abc" < "bca"
true

~
❯ def foo []: nothing -> string { "abc" }

~
❯ (foo) < "bca"
true
```

# User-Facing Changes

Following pattern will be allowed.

| operator | type of lhs | type of rhs | result |
| -------- | ----------- | ----------- | ------ |
| `<`      | string      | string      | bool   |
| `<=`     | string      | string      | bool   |
| `>`      | string      | string      | bool   |
| `>=`     | string      | string      | bool   |

# Tests + Formatting

- [x] `cargo fmt --all -- --check` to check standard code formatting
(`cargo fmt --all` applies these changes)
- [x] `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used`
to check that you're using the standard code style
- [x] `cargo test --workspace` to check that all tests pass (on Windows
make sure to [enable developer
mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging))
- [x] `cargo run -- -c "use std testing; testing run-tests --path
crates/nu-std"` to run the tests for the standard library

# After Submitting
  • Loading branch information
yukitomoda authored and dmatos2012 committed Feb 20, 2024
1 parent 18b1b2a commit 46bc47a
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 0 deletions.
4 changes: 4 additions & 0 deletions crates/nu-parser/src/type_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,7 @@ pub fn math_result_type(
(Type::Int, Type::Number) => (Type::Bool, None),
(Type::Number, Type::Float) => (Type::Bool, None),
(Type::Float, Type::Number) => (Type::Bool, None),
(Type::String, Type::String) => (Type::Bool, None),
(Type::Duration, Type::Duration) => (Type::Bool, None),
(Type::Date, Type::Date) => (Type::Bool, None),
(Type::Filesize, Type::Filesize) => (Type::Bool, None),
Expand Down Expand Up @@ -478,6 +479,7 @@ pub fn math_result_type(
(Type::Int, Type::Number) => (Type::Bool, None),
(Type::Number, Type::Float) => (Type::Bool, None),
(Type::Float, Type::Number) => (Type::Bool, None),
(Type::String, Type::String) => (Type::Bool, None),
(Type::Duration, Type::Duration) => (Type::Bool, None),
(Type::Date, Type::Date) => (Type::Bool, None),
(Type::Filesize, Type::Filesize) => (Type::Bool, None),
Expand Down Expand Up @@ -527,6 +529,7 @@ pub fn math_result_type(
(Type::Int, Type::Number) => (Type::Bool, None),
(Type::Number, Type::Float) => (Type::Bool, None),
(Type::Float, Type::Number) => (Type::Bool, None),
(Type::String, Type::String) => (Type::Bool, None),
(Type::Duration, Type::Duration) => (Type::Bool, None),
(Type::Date, Type::Date) => (Type::Bool, None),
(Type::Filesize, Type::Filesize) => (Type::Bool, None),
Expand Down Expand Up @@ -576,6 +579,7 @@ pub fn math_result_type(
(Type::Int, Type::Number) => (Type::Bool, None),
(Type::Number, Type::Float) => (Type::Bool, None),
(Type::Float, Type::Number) => (Type::Bool, None),
(Type::String, Type::String) => (Type::Bool, None),
(Type::Duration, Type::Duration) => (Type::Bool, None),
(Type::Date, Type::Date) => (Type::Bool, None),
(Type::Filesize, Type::Filesize) => (Type::Bool, None),
Expand Down
24 changes: 24 additions & 0 deletions crates/nu-parser/tests/test_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1915,3 +1915,27 @@ mod input_types {
)
}
}

#[cfg(test)]
mod operator {
use super::*;

#[rstest]
#[case(br#""abc" < "bca""#, "string < string")]
#[case(br#""abc" <= "bca""#, "string <= string")]
#[case(br#""abc" > "bca""#, "string > string")]
#[case(br#""abc" >= "bca""#, "string >= string")]
fn parse_comparison_operators_with_string_and_string(
#[case] expr: &[u8],
#[case] test_tag: &str,
) {
let engine_state = EngineState::new();
let mut working_set = StateWorkingSet::new(&engine_state);
parse(&mut working_set, None, expr, false);
assert_eq!(
working_set.parse_errors.len(),
0,
"{test_tag}: expected to be parsed successfully, but failed."
);
}
}

0 comments on commit 46bc47a

Please sign in to comment.