Skip to content

Commit

Permalink
Avoid displaying syntax error as log message
Browse files Browse the repository at this point in the history
  • Loading branch information
dhruvmanila committed Jun 18, 2024
1 parent 1121283 commit 9bc3076
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 100 deletions.
137 changes: 58 additions & 79 deletions crates/ruff/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,11 @@ use std::path::Path;

use anyhow::{Context, Result};
use colored::Colorize;
use log::{debug, error, warn};
use log::{debug, warn};
use rustc_hash::FxHashMap;

use ruff_diagnostics::Diagnostic;
use ruff_linter::linter::{lint_fix, lint_only, FixTable, FixerResult, LinterResult, ParseSource};
use ruff_linter::logging::DisplayParseError;
use ruff_linter::message::Message;
use ruff_linter::pyproject_toml::lint_pyproject_toml;
use ruff_linter::registry::AsRule;
Expand Down Expand Up @@ -353,13 +352,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 {
FxHashMap::from_iter([(path.to_string_lossy().to_string(), notebook.into_index())])
} else {
Expand Down Expand Up @@ -404,52 +396,66 @@ pub(crate) fn lint_stdin(
};

// Lint the inputs.
let (
LinterResult {
data: messages,
error: parse_error,
},
transformed,
fixed,
) = if matches!(fix_mode, flags::FixMode::Apply | flags::FixMode::Diff) {
if let Ok(FixerResult {
result,
transformed,
fixed,
}) = lint_fix(
path.unwrap_or_else(|| Path::new("-")),
package,
noqa,
settings.unsafe_fixes,
&settings.linter,
&source_kind,
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()
)?;
let (LinterResult { data: messages, .. }, transformed, fixed) =
if matches!(fix_mode, flags::FixMode::Apply | flags::FixMode::Diff) {
if let Ok(FixerResult {
result,
transformed,
fixed,
}) = lint_fix(
path.unwrap_or_else(|| Path::new("-")),
package,
noqa,
settings.unsafe_fixes,
&settings.linter,
&source_kind,
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::Generate => {}
}
flags::FixMode::Generate => {}
}
let transformed = if let Cow::Owned(transformed) = transformed {
transformed
let transformed = if let Cow::Owned(transformed) = transformed {
transformed
} else {
source_kind
};
(result, transformed, fixed)
} else {
source_kind
};
(result, transformed, fixed)
// If we fail to fix, lint the original source code.
let result = lint_only(
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 {
// If we fail to fix, lint the original source code.
let result = lint_only(
path.unwrap_or_else(|| Path::new("-")),
package,
Expand All @@ -459,37 +465,10 @@ pub(crate) fn lint_stdin(
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 {
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 {
FxHashMap::from_iter([(
Expand Down
4 changes: 0 additions & 4 deletions crates/ruff/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -735,7 +735,6 @@ fn stdin_parse_error() {
Found 1 error.
----- stderr -----
error: Failed to parse at 1:16: Expected one or more symbol names after import
"###);
}

Expand All @@ -752,7 +751,6 @@ fn stdin_multiple_parse_error() {
Found 2 errors.
----- stderr -----
error: Failed to parse at 1:16: Expected one or more symbol names after import
"###);
}

Expand All @@ -769,7 +767,6 @@ fn parse_error_not_included() {
Found 1 error.
----- stderr -----
error: Failed to parse at 1:6: Expected an expression
"###);
}

Expand All @@ -786,7 +783,6 @@ fn parse_error_ignored() {
Found 1 error.
----- stderr -----
error: Failed to parse at 1:6: Expected an expression
"###);
}

Expand Down
18 changes: 1 addition & 17 deletions crates/ruff_linter/src/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use std::path::Path;
use anyhow::{anyhow, Result};
use colored::Colorize;
use itertools::Itertools;
use log::error;
use rustc_hash::FxHashMap;

use ruff_diagnostics::Diagnostic;
Expand All @@ -26,7 +25,6 @@ use crate::checkers::tokens::check_tokens;
use crate::directives::Directives;
use crate::doc_lines::{doc_lines_from_ast, doc_lines_from_tokens};
use crate::fix::{fix_file, FixResult};
use crate::logging::DisplayParseError;
use crate::message::Message;
use crate::noqa::add_noqa;
use crate::registry::{AsRule, Rule, RuleSet};
Expand Down Expand Up @@ -365,8 +363,7 @@ pub fn add_noqa_to_path(

// Generate diagnostics, ignoring any existing `noqa` directives.
let LinterResult {
data: diagnostics,
error,
data: diagnostics, ..
} = check_path(
path,
package,
Expand All @@ -381,19 +378,6 @@ pub fn add_noqa_to_path(
&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.
// TODO(dhruvmanila): Add support for Jupyter Notebooks
add_noqa(
Expand Down

0 comments on commit 9bc3076

Please sign in to comment.