From eb2e2e63703cdcba629d6443869b0c471ac8c7a6 Mon Sep 17 00:00:00 2001
From: WindSoilder <WindSoilder@outlook.com>
Date: Fri, 17 Mar 2023 20:37:59 +0800
Subject: [PATCH] make `else if` generate helpful error when condition have an
 issue (#8274)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

# Description

Fixes: #7575

# User-Facing Changes

Previously:
```
if❯ if false { "aaa" } else if $a { 'a' }
Error: nu::parser::parse_mismatch

  × Parse mismatch during operation.
   ╭─[entry #10:1:1]
 1 │ if false { "aaa" } else if $a { 'a' }
   ·                         ─┬
   ·                          ╰── expected block, closure or record
   ╰────

```

After:
```
❯ if false { "aaa" } else if $a { 'a' }
Error: nu::parser::variable_not_found

  × Variable not found.
   ╭─[entry #1:1:1]
 1 │ if false { "aaa" } else if $a { 'a' }
   ·                            ─┬
   ·                             ╰── variable not found
   ╰────

```


# Tests + Formatting

Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
clippy::needless_collect` to check that you're using the standard code
style
- `cargo test --workspace` to check that all tests pass

# After Submitting

If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
---
 crates/nu-parser/src/parser.rs        | 23 ++++++++++++++++++++++-
 crates/nu-parser/tests/test_parser.rs | 19 +++++++++++++++++++
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs
index 37e6cacc8..d98a29218 100644
--- a/crates/nu-parser/src/parser.rs
+++ b/crates/nu-parser/src/parser.rs
@@ -660,6 +660,8 @@ pub fn parse_multispan_value(
             (arg, error)
         }
         SyntaxShape::OneOf(shapes) => {
+            // handle for `if` command.
+            let block_then_exp = shapes.as_slice() == [SyntaxShape::Block, SyntaxShape::Expression];
             let mut err = None;
             for shape in shapes.iter() {
                 let (s, option_err) = parse_multispan_value(
@@ -671,7 +673,26 @@ pub fn parse_multispan_value(
                 );
                 match option_err {
                     None => return (s, None),
-                    e => err = err.or(e),
+                    e => {
+                        // `if` is parsing block first and then expression.
+                        // when we're writing something like `else if $a`, parsing as a
+                        // block will result to error(because it's not a block)
+                        //
+                        // If parse as a expression also failed, user is more likely concerned
+                        // about expression failure rather than "expect block failure"".
+                        if block_then_exp {
+                            match &err {
+                                Some(ParseError::Expected(expected, _)) => {
+                                    if expected.starts_with("block") {
+                                        err = e
+                                    }
+                                }
+                                _ => err = err.or(e),
+                            }
+                        } else {
+                            err = err.or(e)
+                        }
+                    }
                 }
             }
             let span = spans[*spans_idx];
diff --git a/crates/nu-parser/tests/test_parser.rs b/crates/nu-parser/tests/test_parser.rs
index b2c125b47..9297302db 100644
--- a/crates/nu-parser/tests/test_parser.rs
+++ b/crates/nu-parser/tests/test_parser.rs
@@ -1972,4 +1972,23 @@ mod input_types {
 
         assert!(matches!(err, ParseError::VariableNotFound(_)));
     }
+
+    #[test]
+    fn else_if_errors_correctly() {
+        let mut engine_state = EngineState::new();
+        add_declarations(&mut engine_state);
+
+        let mut working_set = StateWorkingSet::new(&engine_state);
+        let (_, err) = parse(
+            &mut working_set,
+            None,
+            b"if false { 'a' } else $foo { 'b' }",
+            true,
+            &[],
+        );
+
+        let err = err.unwrap();
+
+        assert!(matches!(err, ParseError::VariableNotFound(_)));
+    }
 }