Skip to content

Commit

Permalink
Remove LintSource
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Oct 2, 2023
1 parent 347ac87 commit 8bcc8db
Show file tree
Hide file tree
Showing 6 changed files with 89 additions and 132 deletions.
6 changes: 3 additions & 3 deletions crates/ruff_cli/src/commands/add_noqa.rs
Expand Up @@ -7,12 +7,12 @@ use log::{debug, error};
use rayon::prelude::*;

use ruff_linter::linter::add_noqa_to_path;
use ruff_linter::source_kind::SourceKind;
use ruff_linter::warn_user_once;
use ruff_python_ast::{PySourceType, SourceType};
use ruff_workspace::resolver::{python_files_in_path, PyprojectConfig};

use crate::args::CliOverrides;
use crate::diagnostics::LintSource;

/// Add `noqa` directives to a collection of files.
pub(crate) fn add_noqa(
Expand Down Expand Up @@ -57,8 +57,8 @@ pub(crate) fn add_noqa(
.and_then(|parent| package_roots.get(parent))
.and_then(|package| *package);
let settings = resolver.resolve(path, pyproject_config);
let LintSource(source_kind) = match LintSource::try_from_path(path, source_type) {
Ok(Some(source)) => source,
let source_kind = match SourceKind::from_path(path, source_type) {
Ok(Some(source_kind)) => source_kind,
Ok(None) => return None,
Err(e) => {
error!("Failed to extract source from {}: {e}", path.display());
Expand Down
15 changes: 10 additions & 5 deletions crates/ruff_cli/src/commands/format.rs
Expand Up @@ -15,7 +15,7 @@ use tracing::debug;
use ruff_diagnostics::SourceMap;
use ruff_linter::fs;
use ruff_linter::logging::LogLevel;
use ruff_linter::source_kind::{SourceKind, SourceReadError, SourceWriteError};
use ruff_linter::source_kind::{SourceError, SourceKind};
use ruff_linter::warn_user_once;
use ruff_python_ast::{PySourceType, SourceType};
use ruff_python_formatter::{format_module_source, FormatModuleError};
Expand Down Expand Up @@ -147,8 +147,13 @@ fn format_path(
mode: FormatMode,
) -> Result<FormatCommandResult, FormatCommandError> {
// Extract the sources from the file.
let source_kind = SourceKind::from_path(path, source_type)
.map_err(|err| FormatCommandError::Read(Some(path.to_path_buf()), err))?;
let source_kind = match SourceKind::from_path(path, source_type) {
Ok(Some(source_kind)) => source_kind,
Ok(None) => return Ok(FormatCommandResult::Unchanged),
Err(err) => {
return Err(FormatCommandError::Read(Some(path.to_path_buf()), err));
}
};

// Format the source.
if let Some(source_kind) = format_source(&source_kind, source_type, Some(path), settings)? {
Expand Down Expand Up @@ -328,9 +333,9 @@ impl Display for FormatResultSummary {
pub(crate) enum FormatCommandError {
Ignore(#[from] ignore::Error),
Panic(Option<PathBuf>, PanicError),
Read(Option<PathBuf>, SourceReadError),
Read(Option<PathBuf>, SourceError),
Format(Option<PathBuf>, FormatModuleError),
Write(Option<PathBuf>, SourceWriteError),
Write(Option<PathBuf>, SourceError),
}

impl Display for FormatCommandError {
Expand Down
16 changes: 11 additions & 5 deletions crates/ruff_cli/src/commands/format_stdin.rs
Expand Up @@ -74,17 +74,23 @@ fn format_source_code(
) -> Result<FormatCommandResult, FormatCommandError> {
// Read the source from stdin.
let source_code = read_from_stdin()
.map_err(|err| FormatCommandError::Read(path.map(Into::into), err.into()))?;
let source_kind = SourceKind::from_source_code(source_code, source_type)
.map_err(|err| FormatCommandError::Read(path.map(Into::into), err))?;
.map_err(|err| FormatCommandError::Read(path.map(Path::to_path_buf), err.into()))?;

let source_kind = match SourceKind::from_source_code(source_code, source_type) {
Ok(Some(source_kind)) => source_kind,
Ok(None) => return Ok(FormatCommandResult::Unchanged),
Err(err) => {
return Err(FormatCommandError::Read(path.map(Path::to_path_buf), err));
}
};

// Format the source, and write to stdout regardless of the mode.
if let Some(formatted) = format_source(&source_kind, source_type, path, settings)? {
if mode.is_write() {
let mut writer = stdout().lock();
formatted
.write(&mut writer)
.map_err(|err| FormatCommandError::Write(path.map(Into::into), err))?;
.map_err(|err| FormatCommandError::Write(path.map(Path::to_path_buf), err))?;
}

Ok(FormatCommandResult::Formatted)
Expand All @@ -93,7 +99,7 @@ fn format_source_code(
let mut writer = stdout().lock();
source_kind
.write(&mut writer)
.map_err(|err| FormatCommandError::Write(path.map(Into::into), err))?;
.map_err(|err| FormatCommandError::Write(path.map(Path::to_path_buf), err))?;
}

Ok(FormatCommandResult::Unchanged)
Expand Down
134 changes: 39 additions & 95 deletions crates/ruff_cli/src/diagnostics.rs
Expand Up @@ -14,7 +14,6 @@ use filetime::FileTime;
use log::{debug, error, warn};
use rustc_hash::FxHashMap;
use similar::TextDiff;
use thiserror::Error;

use ruff_diagnostics::Diagnostic;
use ruff_linter::linter::{lint_fix, lint_only, FixTable, FixerResult, LinterResult};
Expand All @@ -23,12 +22,12 @@ use ruff_linter::message::Message;
use ruff_linter::pyproject_toml::lint_pyproject_toml;
use ruff_linter::registry::AsRule;
use ruff_linter::settings::{flags, LinterSettings};
use ruff_linter::source_kind::SourceKind;
use ruff_linter::source_kind::{SourceError, SourceKind};
use ruff_linter::{fs, IOError, SyntaxError};
use ruff_macros::CacheKey;
use ruff_notebook::{Cell, Notebook, NotebookError, NotebookIndex};
use ruff_python_ast::imports::ImportMap;
use ruff_python_ast::{PySourceType, SourceType, TomlSourceType};
use ruff_python_ast::{SourceType, TomlSourceType};
use ruff_source_file::{LineIndex, SourceCode, SourceFileBuilder};
use ruff_text_size::{TextRange, TextSize};
use ruff_workspace::Settings;
Expand Down Expand Up @@ -82,15 +81,38 @@ impl Diagnostics {
}
}

/// Generate [`Diagnostics`] based on a [`SourceExtractionError`].
/// Generate [`Diagnostics`] based on a [`SourceError`].
pub(crate) fn from_source_error(
err: &SourceExtractionError,
err: &SourceError,
path: Option<&Path>,
settings: &LinterSettings,
) -> Self {
let diagnostic = Diagnostic::from(err);
let diagnostic = match err {
// IO errors.
SourceError::Io(_)
| SourceError::Notebook(NotebookError::Io(_) | NotebookError::Json(_)) => {
Diagnostic::new(
IOError {
message: err.to_string(),
},
TextRange::default(),
)
}
// Syntax errors.
SourceError::Notebook(
NotebookError::InvalidJson(_)
| NotebookError::InvalidSchema(_)
| NotebookError::InvalidFormat(_),
) => Diagnostic::new(
SyntaxError {
message: err.to_string(),
},
TextRange::default(),
),
};

if settings.rules.enabled(diagnostic.kind.rule()) {
let name = path.map_or_else(|| "-".into(), std::path::Path::to_string_lossy);
let name = path.map_or_else(|| "-".into(), Path::to_string_lossy);
let dummy = SourceFileBuilder::new(name, "").finish();
Self::new(
vec![Message::from_diagnostic(
Expand Down Expand Up @@ -183,13 +205,12 @@ pub(crate) fn lint_path(
.iter_enabled()
.any(|rule_code| rule_code.lint_source().is_pyproject_toml())
{
let contents =
match std::fs::read_to_string(path).map_err(SourceExtractionError::Io) {
Ok(contents) => contents,
Err(err) => {
return Ok(Diagnostics::from_source_error(&err, Some(path), settings));
}
};
let contents = match std::fs::read_to_string(path).map_err(SourceError::from) {
Ok(contents) => contents,
Err(err) => {
return Ok(Diagnostics::from_source_error(&err, Some(path), settings));
}
};
let source_file = SourceFileBuilder::new(path.to_string_lossy(), contents).finish();
lint_pyproject_toml(source_file, settings)
} else {
Expand All @@ -205,8 +226,8 @@ pub(crate) fn lint_path(
};

// Extract the sources from the file.
let LintSource(source_kind) = match LintSource::try_from_path(path, source_type) {
Ok(Some(sources)) => sources,
let source_kind = match SourceKind::from_path(path, source_type) {
Ok(Some(source_kind)) => source_kind,
Ok(None) => return Ok(Diagnostics::default()),
Err(err) => {
return Ok(Diagnostics::from_source_error(&err, Some(path), settings));
Expand Down Expand Up @@ -371,8 +392,8 @@ pub(crate) fn lint_stdin(
};

// Extract the sources from the file.
let LintSource(source_kind) = match LintSource::try_from_source_code(contents, source_type) {
Ok(Some(sources)) => sources,
let source_kind = match SourceKind::from_source_code(contents, source_type) {
Ok(Some(source_kind)) => source_kind,
Ok(None) => return Ok(Diagnostics::default()),
Err(err) => {
return Ok(Diagnostics::from_source_error(&err, path, &settings.linter));
Expand Down Expand Up @@ -487,80 +508,3 @@ pub(crate) fn lint_stdin(
notebook_indexes,
})
}

#[derive(Debug)]
pub(crate) struct LintSource(pub(crate) SourceKind);

impl LintSource {
/// Extract the lint [`LintSource`] from the given file path.
pub(crate) fn try_from_path(
path: &Path,
source_type: PySourceType,
) -> Result<Option<LintSource>, SourceExtractionError> {
if source_type.is_ipynb() {
let notebook = Notebook::from_path(path)?;
Ok(notebook
.is_python_notebook()
.then_some(LintSource(SourceKind::IpyNotebook(notebook))))
} else {
// This is tested by ruff_cli integration test `unreadable_file`
let contents = std::fs::read_to_string(path)?;
Ok(Some(LintSource(SourceKind::Python(contents))))
}
}

/// Extract the lint [`LintSource`] from the raw string contents, optionally accompanied by a
/// file path indicating the path to the file from which the contents were read. If provided,
/// the file path should be used for diagnostics, but not for reading the file from disk.
pub(crate) fn try_from_source_code(
source_code: String,
source_type: PySourceType,
) -> Result<Option<LintSource>, SourceExtractionError> {
if source_type.is_ipynb() {
let notebook = Notebook::from_source_code(&source_code)?;
Ok(notebook
.is_python_notebook()
.then_some(LintSource(SourceKind::IpyNotebook(notebook))))
} else {
Ok(Some(LintSource(SourceKind::Python(source_code))))
}
}
}

#[derive(Error, Debug)]
pub(crate) enum SourceExtractionError {
/// The extraction failed due to an [`io::Error`].
#[error(transparent)]
Io(#[from] io::Error),
/// The extraction failed due to a [`NotebookError`].
#[error(transparent)]
Notebook(#[from] NotebookError),
}

impl From<&SourceExtractionError> for Diagnostic {
fn from(err: &SourceExtractionError) -> Self {
match err {
// IO errors.
SourceExtractionError::Io(_)
| SourceExtractionError::Notebook(NotebookError::Io(_) | NotebookError::Json(_)) => {
Diagnostic::new(
IOError {
message: err.to_string(),
},
TextRange::default(),
)
}
// Syntax errors.
SourceExtractionError::Notebook(
NotebookError::InvalidJson(_)
| NotebookError::InvalidSchema(_)
| NotebookError::InvalidFormat(_),
) => Diagnostic::new(
SyntaxError {
message: err.to_string(),
},
TextRange::default(),
),
}
}
}
4 changes: 1 addition & 3 deletions crates/ruff_cli/tests/integration_test.rs
Expand Up @@ -359,7 +359,7 @@ fn stdin_fix_when_no_issues_should_still_print_contents() {

#[test]
fn stdin_format_jupyter() {
let args = ["format", "--stdin-filename", "Jupyter.ipynb"];
let args = ["format", "--stdin-filename", "Jupyter.ipynb", "--isolated"];
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.args(args)
.pass_stdin(r#"{
Expand Down Expand Up @@ -483,8 +483,6 @@ fn stdin_format_jupyter() {
----- stderr -----
warning: `ruff format` is a work-in-progress, subject to change at any time, and intended only for experimentation.
warning: `one-blank-line-before-class` (D203) and `no-blank-line-before-class` (D211) are incompatible. Ignoring `one-blank-line-before-class`.
warning: `multi-line-summary-first-line` (D212) and `multi-line-summary-second-line` (D213) are incompatible. Ignoring `multi-line-summary-second-line`.
"###);
}

Expand Down
46 changes: 25 additions & 21 deletions crates/ruff_linter/src/source_kind.rs
Expand Up @@ -38,49 +38,53 @@ impl SourceKind {
}
}

/// Read the source kind from the given path.
pub fn from_path(path: &Path, source_type: PySourceType) -> Result<Self, SourceReadError> {
/// Read the [`SourceKind`] from the given path. Returns `None` if the source is not a Python
/// source file.
pub fn from_path(path: &Path, source_type: PySourceType) -> Result<Option<Self>, SourceError> {
if source_type.is_ipynb() {
let notebook = Notebook::from_path(path)?;
Ok(Self::IpyNotebook(notebook))
Ok(notebook
.is_python_notebook()
.then_some(Self::IpyNotebook(notebook)))
} else {
let contents = std::fs::read_to_string(path)?;
Ok(Self::Python(contents))
Ok(Some(Self::Python(contents)))
}
}

/// Read the source kind from the given source code.
/// Read the [`SourceKind`] from the given source code. Returns `None` if the source is not
/// Python source code.
pub fn from_source_code(
source_code: String,
source_type: PySourceType,
) -> Result<Self, SourceReadError> {
) -> Result<Option<Self>, SourceError> {
if source_type.is_ipynb() {
let notebook = Notebook::from_source_code(&source_code)?;
Ok(Self::IpyNotebook(notebook))
Ok(notebook
.is_python_notebook()
.then_some(Self::IpyNotebook(notebook)))
} else {
Ok(Self::Python(source_code))
Ok(Some(Self::Python(source_code)))
}
}

/// Write the source kind to the given writer.
pub fn write(&self, writer: &mut dyn Write) -> Result<(), SourceWriteError> {
/// Write the [`SourceKind`] to the given writer.
pub fn write(&self, writer: &mut dyn Write) -> Result<(), SourceError> {
match self {
SourceKind::Python(source) => writer.write_all(source.as_bytes()).map_err(Into::into),
SourceKind::IpyNotebook(notebook) => notebook.write(writer).map_err(Into::into),
SourceKind::Python(source) => {
writer.write_all(source.as_bytes())?;
Ok(())
}
SourceKind::IpyNotebook(notebook) => {
notebook.write(writer)?;
Ok(())
}
}
}
}

#[derive(Error, Debug)]
pub enum SourceReadError {
#[error(transparent)]
Io(#[from] io::Error),
#[error(transparent)]
Notebook(#[from] NotebookError),
}

#[derive(Error, Debug)]
pub enum SourceWriteError {
pub enum SourceError {
#[error(transparent)]
Io(#[from] io::Error),
#[error(transparent)]
Expand Down

0 comments on commit 8bcc8db

Please sign in to comment.