Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incomplete parser of boolean expressions ending with bracket #11982

Open
DoumanAsh opened this issue Feb 26, 2024 · 3 comments
Open

Incomplete parser of boolean expressions ending with bracket #11982

DoumanAsh opened this issue Feb 26, 2024 · 3 comments
Labels
🐛 bug Something isn't working error-handling How errors in externals/nu code are caught or handled programmatically (see also unhelpful-error) needs-triage An issue that hasn't had any proper look parser Issues related to parsing

Comments

@DoumanAsh
Copy link

DoumanAsh commented Feb 26, 2024

Describe the bug

Following expression will evaluate incorrectly, without syntax error, even though syntax wise you have superfluous bracket at the end

if ((sys | get host | get name) == Windows)) { echo "WINDOWS" } else { echo "NOT WINDOWS" }

I did few experiments, and it seems parser just ignores trailing brackets, even if you put tons of them

Interestingly enough, extra bracket will reverse result of expression inside.
I.e. my example above will be NOT WINDOWS even on windows system

From my observation parser seems to mind brackets, but it seems to be pretty lax about its number, resulting in expression to become incorrect or outright not making any sense

How to reproduce

Use above example in console

Expected behavior

Indicate error in case of invalid number of brackets at the end

Screenshots

No response

Configuration

key value
version 0.90.1
branch
commit_hash c2992d5
build_os windows-x86_64
build_target x86_64-pc-windows-msvc
rust_version rustc 1.73.0 (cc66ad468 2023-10-03)
rust_channel 1.73.0-x86_64-pc-windows-msvc
cargo_version cargo 1.73.0 (9c4383fb5 2023-08-26)
build_time 2024-02-07 02:25:54 +00:00
build_rust_channel release
allocator mimalloc
features dataframe, default, extra, sqlite, trash, which, zip
installed_plugins

Additional context

No response

@DoumanAsh DoumanAsh added the needs-triage An issue that hasn't had any proper look label Feb 26, 2024
@fdncred
Copy link
Collaborator

fdncred commented Feb 26, 2024

I agree that this should produce a parse error. However, it was confusing to figure out because there are no brackets in your example. There are parentheses () and braces {} but no brackets [].

This has one extra closing parenthesis and it seems like it should produce an error, but does not.

if ((sys | get host | get name) == Windows)) { echo "WINDOWS" } else { echo "NOT WINDOWS" }
                                          ^
                                          |
                                          -- should be removed and our parser should say that

@fdncred fdncred added 🐛 bug Something isn't working parser Issues related to parsing error-handling How errors in externals/nu code are caught or handled programmatically (see also unhelpful-error) labels Feb 26, 2024
@DoumanAsh
Copy link
Author

Ah, sorry, yeah, I'm calling everything brackets, but it is indeed parentheses
On side note I didn't notice issues with {}

But expression like: [1, 2]] produces:

╭───┬────╮
│ 0 │  1 │
│ 1 │ 2] │
╰───┴────╯

This would produce list<any> due to nu interpreting 2] as string
I'm not sure if it is intended behavior

@fdncred
Copy link
Collaborator

fdncred commented Feb 26, 2024

I think [1, 2]] is probably working as intended because 2] is a bareword string in this instance. I'm guessing our paren parsing needs to be cleaned up a bit to match our brace parsing. there may be places in bracket parsing that need to be cleaned up too.

fdncred pushed a commit that referenced this issue Mar 7, 2024
…1982) (#12098)

- Fixes issue #11982 

# Description
Expressions with unbalanced parenthesis [excess closing ')' parenthesis]
will throw an error instead of interpreting ')' as a string.

Solved he same way as closing braces '}' are handled.

![Screenshot 2024-03-06 at 14 53
46](https://github.com/nushell/nushell/assets/56027726/86834e47-a1e5-484d-881d-0e3b80fecef8)

![Screenshot 2024-03-06 at 14 48
27](https://github.com/nushell/nushell/assets/56027726/bb27c969-6a3b-4735-8a1e-a5881d9096d3)

# User-Facing Changes
- Trailing closing parentheses ')' which do not match the number of
opening parentheses '(' will lead to a parse error.
- From what I have found in the documentation this is the intended
behavior, thus no documentation has been updated on my part

# Tests + Formatting
- Two tests added in src/tests/test_parser.rs
- All previous tests are still passing
- cargo fmt, clippy and test have been run

Unable to get the following command run
- `cargo run -- -c "use std testing; testing run-tests --path
crates/nu-std"` to run the tests for the standard library
![Screenshot 2024-03-06 at 20 06
25](https://github.com/nushell/nushell/assets/56027726/91724fb9-d7d0-472b-bf14-bfa2a7618d09)

---------

Co-authored-by: Noak Jönsson <noakj@kth.se>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working error-handling How errors in externals/nu code are caught or handled programmatically (see also unhelpful-error) needs-triage An issue that hasn't had any proper look parser Issues related to parsing
Projects
None yet
Development

No branches or pull requests

2 participants