Skip to content

Commit

Permalink
Update ruff check and ruff format to default to the current direc…
Browse files Browse the repository at this point in the history
…tory (#8791)

Closes #7347
Closes #3970 via use of `include`

We could update examples in our documentation, but I worry since we do
not have versioned documentation users on older versions would be
confused. Instead, I'll open an issue to track updating use of `ruff
check .` in the documentation sometime in the future.
  • Loading branch information
zanieb authored Nov 21, 2023
1 parent b61ce7f commit d9151b1
Show file tree
Hide file tree
Showing 16 changed files with 286 additions and 22 deletions.
Empty file.
Empty file.
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[tool.ruff]
select = []
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[tool.ruff]
include = ["a.py", "subdirectory/c.py"]
Empty file.
Empty file.
2 changes: 2 additions & 0 deletions crates/ruff_cli/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ pub enum Command {
#[allow(clippy::struct_excessive_bools)]
pub struct CheckCommand {
/// List of files or directories to check.
#[clap(help = "List of files or directories to check [default: .]")]
pub files: Vec<PathBuf>,
/// Apply fixes to resolve lint violations.
/// Use `--no-fix` to disable or `--unsafe-fixes` to include unsafe fixes.
Expand Down Expand Up @@ -363,6 +364,7 @@ pub struct CheckCommand {
#[allow(clippy::struct_excessive_bools)]
pub struct FormatCommand {
/// List of files or directories to format.
#[clap(help = "List of files or directories to format [default: .]")]
pub files: Vec<PathBuf>,
/// Avoid writing any formatted files back; instead, exit with a non-zero status code if any
/// files would have been modified, and zero otherwise.
Expand Down
9 changes: 5 additions & 4 deletions crates/ruff_cli/src/commands/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use crate::args::{CliOverrides, FormatArguments};
use crate::cache::{Cache, FileCacheKey, PackageCacheMap, PackageCaches};
use crate::panic::{catch_unwind, PanicError};
use crate::resolve::resolve;
use crate::ExitStatus;
use crate::{resolve_default_files, ExitStatus};

#[derive(Debug, Copy, Clone, is_macro::Is)]
pub(crate) enum FormatMode {
Expand All @@ -60,7 +60,7 @@ impl FormatMode {

/// Format a set of files, and return the exit status.
pub(crate) fn format(
cli: &FormatArguments,
cli: FormatArguments,
overrides: &CliOverrides,
log_level: LogLevel,
) -> Result<ExitStatus> {
Expand All @@ -70,8 +70,9 @@ pub(crate) fn format(
overrides,
cli.stdin_filename.as_deref(),
)?;
let mode = FormatMode::from_cli(cli);
let (paths, resolver) = python_files_in_path(&cli.files, &pyproject_config, overrides)?;
let mode = FormatMode::from_cli(&cli);
let files = resolve_default_files(cli.files, false);
let (paths, resolver) = python_files_in_path(&files, &pyproject_config, overrides)?;

if paths.is_empty() {
warn_user_once!("No Python files found under the given path(s)");
Expand Down
40 changes: 24 additions & 16 deletions crates/ruff_cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,19 @@ fn is_stdin(files: &[PathBuf], stdin_filename: Option<&Path>) -> bool {
file == Path::new("-")
}

/// Returns the default set of files if none are provided, otherwise returns `None`.
fn resolve_default_files(files: Vec<PathBuf>, is_stdin: bool) -> Vec<PathBuf> {
if files.is_empty() {
if is_stdin {
vec![Path::new("-").to_path_buf()]
} else {
vec![Path::new(".").to_path_buf()]
}
} else {
files
}
}

/// Get the actual value of the `format` desired from either `output_format`
/// or `format`, and warn the user if they're using the deprecated form.
fn resolve_help_output_format(output_format: HelpFormat, format: Option<HelpFormat>) -> HelpFormat {
Expand Down Expand Up @@ -196,7 +209,7 @@ fn format(args: FormatCommand, log_level: LogLevel) -> Result<ExitStatus> {
if is_stdin(&cli.files, cli.stdin_filename.as_deref()) {
commands::format_stdin::format_stdin(&cli, &overrides)
} else {
commands::format::format(&cli, &overrides, log_level)
commands::format::format(cli, &overrides, log_level)
}
}

Expand All @@ -222,17 +235,15 @@ pub fn check(args: CheckCommand, log_level: LogLevel) -> Result<ExitStatus> {
};
let stderr_writer = Box::new(BufWriter::new(io::stderr()));

let is_stdin = is_stdin(&cli.files, cli.stdin_filename.as_deref());
let files = resolve_default_files(cli.files, is_stdin);

if cli.show_settings {
commands::show_settings::show_settings(
&cli.files,
&pyproject_config,
&overrides,
&mut writer,
)?;
commands::show_settings::show_settings(&files, &pyproject_config, &overrides, &mut writer)?;
return Ok(ExitStatus::Success);
}
if cli.show_files {
commands::show_files::show_files(&cli.files, &pyproject_config, &overrides, &mut writer)?;
commands::show_files::show_files(&files, &pyproject_config, &overrides, &mut writer)?;
return Ok(ExitStatus::Success);
}

Expand Down Expand Up @@ -295,8 +306,7 @@ pub fn check(args: CheckCommand, log_level: LogLevel) -> Result<ExitStatus> {
if !fix_mode.is_generate() {
warn_user!("--fix is incompatible with --add-noqa.");
}
let modifications =
commands::add_noqa::add_noqa(&cli.files, &pyproject_config, &overrides)?;
let modifications = commands::add_noqa::add_noqa(&files, &pyproject_config, &overrides)?;
if modifications > 0 && log_level >= LogLevel::Default {
let s = if modifications == 1 { "" } else { "s" };
#[allow(clippy::print_stderr)]
Expand All @@ -323,7 +333,7 @@ pub fn check(args: CheckCommand, log_level: LogLevel) -> Result<ExitStatus> {
// Configure the file watcher.
let (tx, rx) = channel();
let mut watcher = recommended_watcher(tx)?;
for file in &cli.files {
for file in &files {
watcher.watch(file, RecursiveMode::Recursive)?;
}
if let Some(file) = pyproject_config.path.as_ref() {
Expand All @@ -335,7 +345,7 @@ pub fn check(args: CheckCommand, log_level: LogLevel) -> Result<ExitStatus> {
printer.write_to_user("Starting linter in watch mode...\n");

let messages = commands::check::check(
&cli.files,
&files,
&pyproject_config,
&overrides,
cache.into(),
Expand Down Expand Up @@ -368,7 +378,7 @@ pub fn check(args: CheckCommand, log_level: LogLevel) -> Result<ExitStatus> {
printer.write_to_user("File change detected...\n");

let messages = commands::check::check(
&cli.files,
&files,
&pyproject_config,
&overrides,
cache.into(),
Expand All @@ -382,8 +392,6 @@ pub fn check(args: CheckCommand, log_level: LogLevel) -> Result<ExitStatus> {
}
}
} else {
let is_stdin = is_stdin(&cli.files, cli.stdin_filename.as_deref());

// Generate lint violations.
let diagnostics = if is_stdin {
commands::check_stdin::check_stdin(
Expand All @@ -395,7 +403,7 @@ pub fn check(args: CheckCommand, log_level: LogLevel) -> Result<ExitStatus> {
)?
} else {
commands::check::check(
&cli.files,
&files,
&pyproject_config,
&overrides,
cache.into(),
Expand Down
47 changes: 47 additions & 0 deletions crates/ruff_cli/tests/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,53 @@ if condition:
"###);
}

#[test]
fn default_files() -> Result<()> {
let tempdir = TempDir::new()?;
fs::write(
tempdir.path().join("foo.py"),
r#"
foo = "needs formatting"
"#,
)?;
fs::write(
tempdir.path().join("bar.py"),
r#"
bar = "needs formatting"
"#,
)?;

assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.args(["format", "--isolated", "--no-cache", "--check"]).current_dir(tempdir.path()), @r###"
success: false
exit_code: 1
----- stdout -----
Would reformat: bar.py
Would reformat: foo.py
2 files would be reformatted
----- stderr -----
"###);

Ok(())
}

#[test]
fn format_warn_stdin_filename_with_files() {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.args(["format", "--stdin-filename", "foo.py"])
.arg("foo.py")
.pass_stdin("foo = 1"), @r###"
success: true
exit_code: 0
----- stdout -----
foo = 1
----- stderr -----
warning: Ignoring file foo.py in favor of standard input.
"###);
}

#[test]
fn format_options() -> Result<()> {
let tempdir = TempDir::new()?;
Expand Down
52 changes: 52 additions & 0 deletions crates/ruff_cli/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,57 @@ fn stdin_filename() {
"###);
}

#[test]
fn check_default_files() -> Result<()> {
let tempdir = TempDir::new()?;
fs::write(
tempdir.path().join("foo.py"),
r#"
import foo # unused import
"#,
)?;
fs::write(
tempdir.path().join("bar.py"),
r#"
import bar # unused import
"#,
)?;

assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.args(["check", "--isolated", "--no-cache", "--select", "F401"]).current_dir(tempdir.path()), @r###"
success: false
exit_code: 1
----- stdout -----
bar.py:2:8: F401 [*] `bar` imported but unused
foo.py:2:8: F401 [*] `foo` imported but unused
Found 2 errors.
[*] 2 fixable with the `--fix` option.
----- stderr -----
"###);

Ok(())
}

#[test]
fn check_warn_stdin_filename_with_files() {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.args(STDIN_BASE_OPTIONS)
.args(["--stdin-filename", "F401.py"])
.arg("foo.py")
.pass_stdin("import os\n"), @r###"
success: false
exit_code: 1
----- stdout -----
F401.py:1:8: F401 [*] `os` imported but unused
Found 1 error.
[*] 1 fixable with the `--fix` option.
----- stderr -----
warning: Ignoring file foo.py in favor of standard input.
"###);
}

/// Raise `TCH` errors in `.py` files ...
#[test]
fn stdin_source_type_py() {
Expand Down Expand Up @@ -320,6 +371,7 @@ fn stdin_fix_jupyter() {
Found 2 errors (2 fixed, 0 remaining).
"###);
}

#[test]
fn stdin_override_parser_ipynb() {
let args = ["--extension", "py:ipynb", "--stdin-filename", "Jupyter.py"];
Expand Down
101 changes: 101 additions & 0 deletions crates/ruff_cli/tests/resolve_files.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
#![cfg(not(target_family = "wasm"))]

use std::path::Path;
use std::process::Command;
use std::str;

use insta_cmd::{assert_cmd_snapshot, get_cargo_bin};
const BIN_NAME: &str = "ruff";

#[cfg(not(target_os = "windows"))]
const TEST_FILTERS: &[(&str, &str)] = &[(".*/resources/test/fixtures/", "[BASEPATH]/")];
#[cfg(target_os = "windows")]
const TEST_FILTERS: &[(&str, &str)] = &[
(r".*\\resources\\test\\fixtures\\", "[BASEPATH]\\"),
(r"\\", "/"),
];

#[test]
fn check_project_include_defaults() {
// Defaults to checking the current working directory
//
// The test directory includes:
// - A pyproject.toml which specifies an include
// - A nested pyproject.toml which has a Ruff section
//
// The nested project should all be checked instead of respecting the parent includes

insta::with_settings!({
filters => TEST_FILTERS.to_vec()
}, {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.args(["check", "--show-files"]).current_dir(Path::new("./resources/test/fixtures/include-test")), @r###"
success: true
exit_code: 0
----- stdout -----
[BASEPATH]/include-test/a.py
[BASEPATH]/include-test/nested-project/e.py
[BASEPATH]/include-test/nested-project/pyproject.toml
[BASEPATH]/include-test/subdirectory/c.py
----- stderr -----
"###);
});
}

#[test]
fn check_project_respects_direct_paths() {
// Given a direct path not included in the project `includes`, it should be checked

insta::with_settings!({
filters => TEST_FILTERS.to_vec()
}, {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.args(["check", "--show-files", "b.py"]).current_dir(Path::new("./resources/test/fixtures/include-test")), @r###"
success: true
exit_code: 0
----- stdout -----
[BASEPATH]/include-test/b.py
----- stderr -----
"###);
});
}

#[test]
fn check_project_respects_subdirectory_includes() {
// Given a direct path to a subdirectory, the include should be respected

insta::with_settings!({
filters => TEST_FILTERS.to_vec()
}, {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.args(["check", "--show-files", "subdirectory"]).current_dir(Path::new("./resources/test/fixtures/include-test")), @r###"
success: true
exit_code: 0
----- stdout -----
[BASEPATH]/include-test/subdirectory/c.py
----- stderr -----
"###);
});
}

#[test]
fn check_project_from_project_subdirectory_respects_includes() {
// Run from a project subdirectory, the include specified in the parent directory should be respected

insta::with_settings!({
filters => TEST_FILTERS.to_vec()
}, {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.args(["check", "--show-files"]).current_dir(Path::new("./resources/test/fixtures/include-test/subdirectory")), @r###"
success: true
exit_code: 0
----- stdout -----
[BASEPATH]/include-test/subdirectory/c.py
----- stderr -----
"###);
});
}
Loading

0 comments on commit d9151b1

Please sign in to comment.