Skip to content

Commit

Permalink
Review
Browse files Browse the repository at this point in the history
  • Loading branch information
konstin committed Jul 19, 2023
1 parent d7dd854 commit 75b33f6
Show file tree
Hide file tree
Showing 8 changed files with 90 additions and 40 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ filetime = { version = "0.2.20" }
glob = { version = "0.3.1" }
globset = { version = "0.4.10" }
ignore = { version = "0.4.20" }
insta = { version = "1.31.0" }
insta = { version = "1.31.0", feature = ["filters", "glob"] }
is-macro = { version = "0.2.2" }
itertools = { version = "0.10.5" }
log = { version = "0.4.17" }
Expand Down
25 changes: 16 additions & 9 deletions crates/ruff/src/pyproject_toml.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use colored::Colorize;
use log::warn;
use pyproject_toml::{BuildSystem, Project};
use ruff_text_size::{TextRange, TextSize};
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -26,28 +28,33 @@ pub fn lint_pyproject_toml(source_file: SourceFile, settings: &Settings) -> Vec<
return Vec::default();
};

let mut messages = vec![];
let mut messages = Vec::new();
let range = match err.span() {
// This is bad but sometimes toml and/or serde just don't give us spans
// TODO(konstin,micha): https://github.com/astral-sh/ruff/issues/4571
None => TextRange::default(),
Some(range) => {
let Ok(end) = TextSize::try_from(range.end) else {
return if settings.rules.enabled(Rule::IOError) {
let message = format!(
"{} is larger than 4GB, but ruff assumes all files to be smaller",
source_file.name(),
);
let message = format!(
"{} is larger than 4GB, but ruff assumes all files to be smaller",
source_file.name(),
);
if settings.rules.enabled(Rule::IOError) {
let diagnostic = Diagnostic::new(IOError { message }, TextRange::default());
messages.push(Message::from_diagnostic(
diagnostic,
source_file,
TextSize::default(),
));
messages
} else {
Vec::new()
};
warn!(
"{}{}{} {message}",
"Failed to lint ".bold(),
source_file.name().bold(),
":".bold()
);
}
return messages;
};
TextRange::new(
// start <= end, so if end < 4GB follows start < 4GB
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ wild = { version = "2" }

[dev-dependencies]
assert_cmd = { version = "2.0.8" }
insta = { workspace = true }
insta = { workspace = true, features = ["filters"] }
tempfile = "3.6.0"
ureq = { version = "2.6.2", features = [] }

Expand Down
29 changes: 16 additions & 13 deletions crates/ruff_cli/src/commands/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,6 @@ pub(crate) fn run(
}
.unwrap_or_else(|(path, message)| {
if let Some(path) = &path {
error!(
"{}{}{} {message}",
"Failed to lint ".bold(),
fs::relativize_path(path).bold(),
":".bold()
);
let settings = resolver.resolve(path, pyproject_config);
if settings.rules.enabled(Rule::IOError) {
let dummy =
Expand All @@ -163,10 +157,16 @@ pub(crate) fn run(
ImportMap::default(),
)
} else {
warn!(
"{}{}{} {message}",
"Failed to lint ".bold(),
fs::relativize_path(path).bold(),
":".bold()
);
Diagnostics::default()
}
} else {
error!("{} {message}", "Encountered error:".bold());
warn!("{} {message}", "Encountered error:".bold());
Diagnostics::default()
}
})
Expand Down Expand Up @@ -228,7 +228,7 @@ with the relevant file contents, the `pyproject.toml` settings, and the followin
}

#[cfg(test)]
#[cfg(unix)] // Remove when adding a second test
#[cfg(unix)] // If you need to add a second test, move this cfg
mod test {
use super::run;
use crate::args::Overrides;
Expand Down Expand Up @@ -294,12 +294,15 @@ mod test {
)
.unwrap();

// Remove the tempdir from the messages
let messages = String::from_utf8(output)
.unwrap()
.replace(tempdir.path().to_str().unwrap(), "");
let messages = String::from_utf8(output).unwrap();

insta::with_settings!({ omit_expression => true }, {
insta::with_settings!({
omit_expression => true,
filters => vec![
// The tempdir is always different (and platform dependent)
(tempdir.path().to_str().unwrap(), "/home/ferris/project"),
]
}, {
insta::assert_snapshot!(snapshot, messages);
});
Ok(())
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
---
source: crates/ruff_cli/src/commands/run.rs
---
/code.py:1:1: E902 Permission denied (os error 13)
/notebook.ipynb:1:1: E902 Permission denied (os error 13)
/pyproject.toml:1:1: E902 Permission denied (os error 13)
/home/ferris/project/code.py:1:1: E902 Permission denied (os error 13)
/home/ferris/project/notebook.ipynb:1:1: E902 Permission denied (os error 13)
/home/ferris/project/pyproject.toml:1:1: E902 Permission denied (os error 13)

35 changes: 23 additions & 12 deletions crates/ruff_cli/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::path::Path;

use anyhow::{anyhow, Result};
use colored::Colorize;
use log::{debug, error};
use log::{debug, error, warn};
use ruff_text_size::{TextRange, TextSize};
use rustc_hash::FxHashMap;
use similar::TextDiff;
Expand All @@ -18,6 +18,7 @@ use ruff::linter::{lint_fix, lint_only, FixTable, FixerResult, LinterResult};
use ruff::logging::DisplayParseError;
use ruff::message::Message;
use ruff::pyproject_toml::lint_pyproject_toml;
use ruff::registry::Rule;
use ruff::settings::{flags, AllSettings, Settings};
use ruff::source_kind::SourceKind;
use ruff::{fs, IOError};
Expand Down Expand Up @@ -130,17 +131,27 @@ pub(crate) fn lint_path(

// In case of an io error we want to exit early
let io_error_diagnostics = |err: io::Error, path: &Path| -> Diagnostics {
let io_err = Diagnostic::new(
IOError {
message: err.to_string(),
},
TextRange::default(),
);
let dummy = SourceFileBuilder::new(path.to_string_lossy().as_ref(), "").finish();
Diagnostics::new(
vec![Message::from_diagnostic(io_err, dummy, TextSize::default())],
ImportMap::default(),
)
if settings.lib.rules.enabled(Rule::IOError) {
let io_err = Diagnostic::new(
IOError {
message: err.to_string(),
},
TextRange::default(),
);
let dummy = SourceFileBuilder::new(path.to_string_lossy().as_ref(), "").finish();
Diagnostics::new(
vec![Message::from_diagnostic(io_err, dummy, TextSize::default())],
ImportMap::default(),
)
} else {
warn!(
"{}{}{} {err}",
"Failed to lint ".bold(),
fs::relativize_path(path).bold(),
":".bold()
);
Diagnostics::default()
}
};

// We have to special case this here since the Python tokenizer doesn't work with TOML.
Expand Down
30 changes: 29 additions & 1 deletion crates/ruff_cli/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
#[cfg(unix)]
use std::fs;
#[cfg(unix)]
use std::os::unix::fs::OpenOptionsExt;
use std::fs::Permissions;
#[cfg(unix)]
use std::os::unix::fs::{OpenOptionsExt, PermissionsExt};
#[cfg(unix)]
use std::path::Path;
use std::str;
Expand Down Expand Up @@ -317,3 +319,29 @@ fn unreadable_pyproject_toml() -> Result<()> {
);
Ok(())
}

/// Check the output with an unreadable directory
#[cfg(unix)]
#[test]
fn unreadable_dir() -> Result<()> {
// Create a directory with 000 (not iterable/readable) permissions
let tempdir = TempDir::new()?;
let unreadable_dir = tempdir.path().join("unreadable_dir");
fs::create_dir(&unreadable_dir)?;
fs::set_permissions(&unreadable_dir, Permissions::from_mode(0o000))?;

// We (currently?) have to use a subcommand to check exit status (currently wrong) and logging
// output
let mut cmd = Command::cargo_bin(BIN_NAME)?;
let output = cmd
.args(["--no-cache", "--isolated"])
.arg(&unreadable_dir)
.assert()
// TODO(konstin): This should be a failure, but we currently can't track that
.success();
assert_eq!(
str::from_utf8(&output.get_output().stderr)?,
"warning: Encountered error: Permission denied (os error 13)\n"
);
Ok(())
}

0 comments on commit 75b33f6

Please sign in to comment.