Skip to content

Commit

Permalink
Update E999 to show all syntax errors (#11900)
Browse files Browse the repository at this point in the history
## 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.
  • Loading branch information
dhruvmanila authored Jun 19, 2024
1 parent cdc7c71 commit b617d90
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 12 deletions.
23 changes: 20 additions & 3 deletions crates/ruff/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
"###);
}

Expand Down
8 changes: 5 additions & 3 deletions crates/ruff_linter/src/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions crates/ruff_python_parser/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
3 changes: 1 addition & 2 deletions crates/ruff_python_parser/tests/fixtures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit b617d90

Please sign in to comment.