Avoid displaying syntax error as log message (#11902)

## Summary

Follow-up to #11901 

This PR avoids displaying the syntax errors as log message now that the
`E999` diagnostic cannot be disabled.

For context on why this was added, refer to
https://github.com/astral-sh/ruff/pull/2505. Basically, we would allow
ignoring the syntax error diagnostic because certain syntax feature
weren't supported back then like `match` statement. And, if a user
ignored `E999`, Ruff would give no feedback if the source code contained
any syntax error. So, this log message was a way to indicate to the user
even if `E999` was disabled.

The current state of the parser is such that (a) it matches with the
latest grammar and (b) it's easy to add support for any new syntax.

**Note:** This PR doesn't remove the `DisplayParseError` struct because
it's still being used by the formatter.

## Test Plan

Update existing snapshots from the integration tests.
This commit is contained in:
Dhruv Manilawala 2024-06-27 07:54:06 +05:30 committed by Micha Reiser
parent e7b49694a7
commit 73851e73ab
3 changed files with 60 additions and 101 deletions

View file

@ -9,13 +9,12 @@ use std::path::Path;
use anyhow::{Context, Result}; use anyhow::{Context, Result};
use colored::Colorize; use colored::Colorize;
use log::{debug, error, warn}; use log::{debug, warn};
use ruff_linter::codes::Rule;
use rustc_hash::FxHashMap; use rustc_hash::FxHashMap;
use ruff_diagnostics::Diagnostic; use ruff_diagnostics::Diagnostic;
use ruff_linter::codes::Rule;
use ruff_linter::linter::{lint_fix, lint_only, FixTable, FixerResult, LinterResult, ParseSource}; use ruff_linter::linter::{lint_fix, lint_only, FixTable, FixerResult, LinterResult, ParseSource};
use ruff_linter::logging::DisplayParseError;
use ruff_linter::message::{Message, SyntaxErrorMessage}; use ruff_linter::message::{Message, SyntaxErrorMessage};
use ruff_linter::pyproject_toml::lint_pyproject_toml; use ruff_linter::pyproject_toml::lint_pyproject_toml;
use ruff_linter::settings::types::UnsafeFixes; use ruff_linter::settings::types::UnsafeFixes;
@ -357,13 +356,6 @@ pub(crate) fn lint_path(
} }
} }
if let Some(error) = parse_error {
error!(
"{}",
DisplayParseError::from_source_kind(error, Some(path.to_path_buf()), &transformed)
);
}
let notebook_indexes = if let SourceKind::IpyNotebook(notebook) = transformed { let notebook_indexes = if let SourceKind::IpyNotebook(notebook) = transformed {
FxHashMap::from_iter([(path.to_string_lossy().to_string(), notebook.into_index())]) FxHashMap::from_iter([(path.to_string_lossy().to_string(), notebook.into_index())])
} else { } else {
@ -408,52 +400,66 @@ pub(crate) fn lint_stdin(
}; };
// Lint the inputs. // Lint the inputs.
let ( let (LinterResult { data: messages, .. }, transformed, fixed) =
LinterResult { if matches!(fix_mode, flags::FixMode::Apply | flags::FixMode::Diff) {
data: messages, if let Ok(FixerResult {
error: parse_error, result,
}, transformed,
transformed, fixed,
fixed, }) = lint_fix(
) = if matches!(fix_mode, flags::FixMode::Apply | flags::FixMode::Diff) { path.unwrap_or_else(|| Path::new("-")),
if let Ok(FixerResult { package,
result, noqa,
transformed, settings.unsafe_fixes,
fixed, &settings.linter,
}) = lint_fix( &source_kind,
path.unwrap_or_else(|| Path::new("-")), source_type,
package, ) {
noqa, match fix_mode {
settings.unsafe_fixes, flags::FixMode::Apply => {
&settings.linter, // Write the contents to stdout, regardless of whether any errors were fixed.
&source_kind, transformed.write(&mut io::stdout().lock())?;
source_type,
) {
match fix_mode {
flags::FixMode::Apply => {
// Write the contents to stdout, regardless of whether any errors were fixed.
transformed.write(&mut io::stdout().lock())?;
}
flags::FixMode::Diff => {
// But only write a diff if it's non-empty.
if !fixed.is_empty() {
write!(
&mut io::stdout().lock(),
"{}",
source_kind.diff(&transformed, path).unwrap()
)?;
} }
flags::FixMode::Diff => {
// But only write a diff if it's non-empty.
if !fixed.is_empty() {
write!(
&mut io::stdout().lock(),
"{}",
source_kind.diff(&transformed, path).unwrap()
)?;
}
}
flags::FixMode::Generate => {}
} }
flags::FixMode::Generate => {} let transformed = if let Cow::Owned(transformed) = transformed {
} transformed
let transformed = if let Cow::Owned(transformed) = transformed { } else {
transformed source_kind
};
(result, transformed, fixed)
} else { } else {
source_kind // If we fail to fix, lint the original source code.
}; let result = lint_only(
(result, transformed, fixed) path.unwrap_or_else(|| Path::new("-")),
package,
&settings.linter,
noqa,
&source_kind,
source_type,
ParseSource::None,
);
// Write the contents to stdout anyway.
if fix_mode.is_apply() {
source_kind.write(&mut io::stdout().lock())?;
}
let transformed = source_kind;
let fixed = FxHashMap::default();
(result, transformed, fixed)
}
} else { } else {
// If we fail to fix, lint the original source code.
let result = lint_only( let result = lint_only(
path.unwrap_or_else(|| Path::new("-")), path.unwrap_or_else(|| Path::new("-")),
package, package,
@ -463,37 +469,10 @@ pub(crate) fn lint_stdin(
source_type, source_type,
ParseSource::None, ParseSource::None,
); );
// Write the contents to stdout anyway.
if fix_mode.is_apply() {
source_kind.write(&mut io::stdout().lock())?;
}
let transformed = source_kind; let transformed = source_kind;
let fixed = FxHashMap::default(); let fixed = FxHashMap::default();
(result, transformed, fixed) (result, transformed, fixed)
} };
} else {
let result = lint_only(
path.unwrap_or_else(|| Path::new("-")),
package,
&settings.linter,
noqa,
&source_kind,
source_type,
ParseSource::None,
);
let transformed = source_kind;
let fixed = FxHashMap::default();
(result, transformed, fixed)
};
if let Some(error) = parse_error {
error!(
"{}",
DisplayParseError::from_source_kind(error, path.map(Path::to_path_buf), &transformed)
);
}
let notebook_indexes = if let SourceKind::IpyNotebook(notebook) = transformed { let notebook_indexes = if let SourceKind::IpyNotebook(notebook) = transformed {
FxHashMap::from_iter([( FxHashMap::from_iter([(

View file

@ -807,7 +807,6 @@ fn stdin_parse_error() {
Found 1 error. Found 1 error.
----- stderr ----- ----- stderr -----
error: Failed to parse at 1:16: Expected one or more symbol names after import
"###); "###);
} }
@ -836,7 +835,6 @@ fn stdin_multiple_parse_error() {
Found 2 errors. Found 2 errors.
----- stderr ----- ----- stderr -----
error: Failed to parse at 1:16: Expected one or more symbol names after import
"###); "###);
} }
@ -858,7 +856,6 @@ fn parse_error_not_included() {
Found 1 error. Found 1 error.
----- stderr ----- ----- stderr -----
error: Failed to parse at 1:6: Expected an expression
"###); "###);
} }
@ -880,7 +877,6 @@ fn deprecated_parse_error_selection() {
----- stderr ----- ----- stderr -----
warning: Rule `E999` is deprecated and will be removed in a future release. Syntax errors will always be shown regardless of whether this rule is selected or not. warning: Rule `E999` is deprecated and will be removed in a future release. Syntax errors will always be shown regardless of whether this rule is selected or not.
error: Failed to parse at 1:6: Expected an expression
"###); "###);
} }

View file

@ -5,7 +5,6 @@ use std::path::Path;
use anyhow::{anyhow, Result}; use anyhow::{anyhow, Result};
use colored::Colorize; use colored::Colorize;
use itertools::Itertools; use itertools::Itertools;
use log::error;
use rustc_hash::FxHashMap; use rustc_hash::FxHashMap;
use ruff_diagnostics::Diagnostic; use ruff_diagnostics::Diagnostic;
@ -26,7 +25,6 @@ use crate::checkers::tokens::check_tokens;
use crate::directives::Directives; use crate::directives::Directives;
use crate::doc_lines::{doc_lines_from_ast, doc_lines_from_tokens}; use crate::doc_lines::{doc_lines_from_ast, doc_lines_from_tokens};
use crate::fix::{fix_file, FixResult}; use crate::fix::{fix_file, FixResult};
use crate::logging::DisplayParseError;
use crate::message::Message; use crate::message::Message;
use crate::noqa::add_noqa; use crate::noqa::add_noqa;
use crate::registry::{AsRule, Rule, RuleSet}; use crate::registry::{AsRule, Rule, RuleSet};
@ -354,8 +352,7 @@ pub fn add_noqa_to_path(
// Generate diagnostics, ignoring any existing `noqa` directives. // Generate diagnostics, ignoring any existing `noqa` directives.
let LinterResult { let LinterResult {
data: diagnostics, data: diagnostics, ..
error,
} = check_path( } = check_path(
path, path,
package, package,
@ -370,19 +367,6 @@ pub fn add_noqa_to_path(
&parsed, &parsed,
); );
// Log any parse errors.
if let Some(error) = error {
error!(
"{}",
DisplayParseError::from_source_code(
error,
Some(path.to_path_buf()),
&locator.to_source_code(),
source_kind,
)
);
}
// Add any missing `# noqa` pragmas. // Add any missing `# noqa` pragmas.
// TODO(dhruvmanila): Add support for Jupyter Notebooks // TODO(dhruvmanila): Add support for Jupyter Notebooks
add_noqa( add_noqa(