From b617d906510ab6ea04e019f73b5aa56fe3692e0a Mon Sep 17 00:00:00 2001
From: Dhruv Manilawala <dhruvmanila@gmail.com>
Date: Wed, 19 Jun 2024 13:09:54 +0530
Subject: [PATCH] Update `E999` to show all syntax errors (#11900)

## Summary

This PR updates the linter to show all the parse errors as diagnostics
instead of just the first one.

Note that this doesn't affect the parse error displayed as error log
message. This will be removed in a follow-up PR.

### Breaking?

I don't think this is a breaking change even though this might give more
diagnostics. The main reason is that this shouldn't affect any users
because it'll only give additional diagnostics in the case of multiple
syntax errors.

## Test Plan

Add an integration test case which would raise more than one parse
error.
---
 crates/ruff/tests/integration_test.rs       | 23 ++++++++++++++++++---
 crates/ruff_linter/src/linter.rs            |  8 ++++---
 crates/ruff_python_parser/src/lib.rs        |  8 +++----
 crates/ruff_python_parser/tests/fixtures.rs |  3 +--
 4 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/crates/ruff/tests/integration_test.rs b/crates/ruff/tests/integration_test.rs
index 52f1b09df1368..dc096f47181f0 100644
--- a/crates/ruff/tests/integration_test.rs
+++ b/crates/ruff/tests/integration_test.rs
@@ -727,15 +727,32 @@ fn stdin_format_jupyter() {
 fn stdin_parse_error() {
     let mut cmd = RuffCheck::default().build();
     assert_cmd_snapshot!(cmd
-        .pass_stdin("from foo import =\n"), @r###"
+        .pass_stdin("from foo import\n"), @r###"
     success: false
     exit_code: 1
     ----- stdout -----
-    -:1:17: E999 SyntaxError: Expected an import name
+    -:1:16: E999 SyntaxError: Expected one or more symbol names after import
     Found 1 error.
 
     ----- stderr -----
-    error: Failed to parse at 1:17: Expected an import name
+    error: Failed to parse at 1:16: Expected one or more symbol names after import
+    "###);
+}
+
+#[test]
+fn stdin_multiple_parse_error() {
+    let mut cmd = RuffCheck::default().build();
+    assert_cmd_snapshot!(cmd
+        .pass_stdin("from foo import\nbar =\n"), @r###"
+    success: false
+    exit_code: 1
+    ----- stdout -----
+    -:1:16: E999 SyntaxError: Expected one or more symbol names after import
+    -:2:6: E999 SyntaxError: Expected an expression
+    Found 2 errors.
+
+    ----- stderr -----
+    error: Failed to parse at 1:16: Expected one or more symbol names after import
     "###);
 }
 
diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs
index fdfe380fb6325..1717d5116d1b2 100644
--- a/crates/ruff_linter/src/linter.rs
+++ b/crates/ruff_linter/src/linter.rs
@@ -192,15 +192,17 @@ pub fn check_path(
                     doc_lines.extend(doc_lines_from_ast(parsed.suite(), locator));
                 }
             }
-            Err(parse_error) => {
+            Err(parse_errors) => {
                 // Always add a diagnostic for the syntax error, regardless of whether
                 // `Rule::SyntaxError` is enabled. We avoid propagating the syntax error
                 // if it's disabled via any of the usual mechanisms (e.g., `noqa`,
                 // `per-file-ignores`), and the easiest way to detect that suppression is
                 // to see if the diagnostic persists to the end of the function.
-                pycodestyle::rules::syntax_error(&mut diagnostics, parse_error, locator);
+                for parse_error in parse_errors {
+                    pycodestyle::rules::syntax_error(&mut diagnostics, parse_error, locator);
+                }
                 // TODO(dhruvmanila): Remove this clone
-                error = Some(parse_error.clone());
+                error = parse_errors.iter().next().cloned();
             }
         }
     }
diff --git a/crates/ruff_python_parser/src/lib.rs b/crates/ruff_python_parser/src/lib.rs
index 17af39f96a031..0add53e446260 100644
--- a/crates/ruff_python_parser/src/lib.rs
+++ b/crates/ruff_python_parser/src/lib.rs
@@ -274,11 +274,11 @@ impl<T> Parsed<T> {
 
     /// Returns the [`Parsed`] output as a [`Result`], returning [`Ok`] if it has no syntax errors,
     /// or [`Err`] containing the first [`ParseError`] encountered.
-    pub fn as_result(&self) -> Result<&Parsed<T>, &ParseError> {
-        if let [error, ..] = self.errors() {
-            Err(error)
-        } else {
+    pub fn as_result(&self) -> Result<&Parsed<T>, &[ParseError]> {
+        if self.is_valid() {
             Ok(self)
+        } else {
+            Err(&self.errors)
         }
     }
 
diff --git a/crates/ruff_python_parser/tests/fixtures.rs b/crates/ruff_python_parser/tests/fixtures.rs
index 63b7bec85743a..2b8d9acfc1c04 100644
--- a/crates/ruff_python_parser/tests/fixtures.rs
+++ b/crates/ruff_python_parser/tests/fixtures.rs
@@ -126,8 +126,7 @@ fn test_invalid_syntax(input_path: &Path) {
 #[allow(clippy::print_stdout)]
 fn parser_quick_test() {
     let source = "\
-def foo()
-    pass
+from foo import
 ";
 
     let parsed = parse_unchecked(source, Mode::Module);