Skip to content

Commit

Permalink
Respect --force-exclude for lint.exclude and format.exclude (#8393)
Browse files Browse the repository at this point in the history
## Summary

We typically avoid enforcing exclusions if a file was passed to Ruff
directly on the CLI. However, we also allow `--force-exclude`, which
ignores excluded files _even_ if they're passed to Ruff directly. This
is really important for pre-commit, which always passes changed files --
we need to exclude files passed by pre-commit if they're in the
`exclude` lists.

Turns out the new `lint.exclude` and `format.exclude` settings weren't
respecting `--force-exclude`.

Closes #8391.
  • Loading branch information
charliermarsh authored Oct 31, 2023
1 parent 3835898 commit 1642f4d
Show file tree
Hide file tree
Showing 7 changed files with 139 additions and 37 deletions.
2 changes: 1 addition & 1 deletion crates/ruff_cli/src/commands/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ pub(crate) fn check(

let settings = resolver.resolve(path, pyproject_config);

if !resolved_file.is_root()
if (settings.file_resolver.force_exclude || !resolved_file.is_root())
&& match_exclusion(
resolved_file.path(),
resolved_file.file_name(),
Expand Down
22 changes: 12 additions & 10 deletions crates/ruff_cli/src/commands/check_stdin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,19 @@ pub(crate) fn check_stdin(
noqa: flags::Noqa,
fix_mode: flags::FixMode,
) -> Result<Diagnostics> {
if let Some(filename) = filename {
if !python_file_at_path(filename, pyproject_config, overrides)? {
return Ok(Diagnostics::default());
}
if pyproject_config.settings.file_resolver.force_exclude {
if let Some(filename) = filename {
if !python_file_at_path(filename, pyproject_config, overrides)? {
return Ok(Diagnostics::default());
}

let lint_settings = &pyproject_config.settings.linter;
if filename
.file_name()
.is_some_and(|name| match_exclusion(filename, name, &lint_settings.exclude))
{
return Ok(Diagnostics::default());
let lint_settings = &pyproject_config.settings.linter;
if filename
.file_name()
.is_some_and(|name| match_exclusion(filename, name, &lint_settings.exclude))
{
return Ok(Diagnostics::default());
}
}
}
let package_root = filename.and_then(Path::parent).and_then(|path| {
Expand Down
14 changes: 4 additions & 10 deletions crates/ruff_cli/src/commands/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,14 +117,14 @@ pub(crate) fn format(
return None;
};

let resolved_settings = resolver.resolve(path, &pyproject_config);
let settings = resolver.resolve(path, &pyproject_config);

// Ignore files that are excluded from formatting
if !resolved_file.is_root()
if (settings.file_resolver.force_exclude || !resolved_file.is_root())
&& match_exclusion(
path,
resolved_file.file_name(),
&resolved_settings.formatter.exclude,
&settings.formatter.exclude,
)
{
return None;
Expand All @@ -139,13 +139,7 @@ pub(crate) fn format(

Some(
match catch_unwind(|| {
format_path(
path,
&resolved_settings.formatter,
source_type,
mode,
cache,
)
format_path(path, &settings.formatter, source_type, mode, cache)
}) {
Ok(inner) => inner.map(|result| FormatPathResult {
path: resolved_file.path().to_path_buf(),
Expand Down
22 changes: 12 additions & 10 deletions crates/ruff_cli/src/commands/format_stdin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,19 @@ pub(crate) fn format_stdin(cli: &FormatArguments, overrides: &CliOverrides) -> R

let mode = FormatMode::from_cli(cli);

if let Some(filename) = cli.stdin_filename.as_deref() {
if !python_file_at_path(filename, &pyproject_config, overrides)? {
return Ok(ExitStatus::Success);
}
if pyproject_config.settings.file_resolver.force_exclude {
if let Some(filename) = cli.stdin_filename.as_deref() {
if !python_file_at_path(filename, &pyproject_config, overrides)? {
return Ok(ExitStatus::Success);
}

let format_settings = &pyproject_config.settings.formatter;
if filename
.file_name()
.is_some_and(|name| match_exclusion(filename, name, &format_settings.exclude))
{
return Ok(ExitStatus::Success);
let format_settings = &pyproject_config.settings.formatter;
if filename
.file_name()
.is_some_and(|name| match_exclusion(filename, name, &format_settings.exclude))
{
return Ok(ExitStatus::Success);
}
}
}

Expand Down
104 changes: 104 additions & 0 deletions crates/ruff_cli/tests/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,73 @@ OTHER = "OTHER"
Ok(())
}

#[test]
fn force_exclude() -> Result<()> {
let tempdir = TempDir::new()?;
let ruff_toml = tempdir.path().join("ruff.toml");
fs::write(
&ruff_toml,
r#"
extend-exclude = ["out"]
[format]
exclude = ["test.py", "generated.py"]
"#,
)?;

fs::write(
tempdir.path().join("main.py"),
r#"
from test import say_hy
if __name__ == "__main__":
say_hy("dear Ruff contributor")
"#,
)?;

// Excluded file but passed to the CLI directly, should be formatted
let test_path = tempdir.path().join("test.py");
fs::write(
&test_path,
r#"
def say_hy(name: str):
print(f"Hy {name}")"#,
)?;

fs::write(
tempdir.path().join("generated.py"),
r#"NUMBERS = [
0, 1, 2, 3, 4, 5, 6, 7, 8, 9,
10, 11, 12, 13, 14, 15, 16, 17, 18, 19
]
OTHER = "OTHER"
"#,
)?;

let out_dir = tempdir.path().join("out");
fs::create_dir(&out_dir)?;

fs::write(out_dir.join("a.py"), "a = a")?;

assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.current_dir(tempdir.path())
.args(["format", "--no-cache", "--force-exclude", "--check", "--config"])
.arg(ruff_toml.file_name().unwrap())
// Explicitly pass test.py, should be respect the `format.exclude` when `--force-exclude` is present
.arg(test_path.file_name().unwrap())
// Format all other files in the directory, should respect the `exclude` and `format.exclude` options
.arg("."), @r###"
success: false
exit_code: 1
----- stdout -----
Would reformat: main.py
1 file would be reformatted
----- stderr -----
"###);
Ok(())
}

#[test]
fn exclude_stdin() -> Result<()> {
let tempdir = TempDir::new()?;
Expand All @@ -209,6 +276,43 @@ exclude = ["generated.py"]
.pass_stdin(r#"
from test import say_hy
if __name__ == '__main__':
say_hy("dear Ruff contributor")
"#), @r###"
success: true
exit_code: 0
----- stdout -----
from test import say_hy
if __name__ == "__main__":
say_hy("dear Ruff contributor")
----- stderr -----
"###);
Ok(())
}

#[test]
fn force_exclude_stdin() -> Result<()> {
let tempdir = TempDir::new()?;
let ruff_toml = tempdir.path().join("ruff.toml");
fs::write(
&ruff_toml,
r#"
extend-select = ["B", "Q"]
ignore = ["Q000", "Q001", "Q002", "Q003"]
[format]
exclude = ["generated.py"]
"#,
)?;

assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.current_dir(tempdir.path())
.args(["format", "--config", &ruff_toml.file_name().unwrap().to_string_lossy(), "--stdin-filename", "generated.py", "--force-exclude", "-"])
.pass_stdin(r#"
from test import say_hy
if __name__ == '__main__':
say_hy("dear Ruff contributor")
"#), @r###"
Expand Down
8 changes: 6 additions & 2 deletions crates/ruff_cli/tests/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,9 +262,13 @@ from test import say_hy
if __name__ == "__main__":
say_hy("dear Ruff contributor")
"#), @r###"
success: true
exit_code: 0
success: false
exit_code: 1
----- stdout -----
generated.py:4:16: Q000 [*] Double quotes found but single quotes preferred
generated.py:5:12: Q000 [*] Double quotes found but single quotes preferred
Found 2 errors.
[*] 2 fixable with the `--fix` option.
----- stderr -----
"###);
Expand Down
4 changes: 0 additions & 4 deletions crates/ruff_workspace/src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -483,10 +483,6 @@ pub fn python_file_at_path(
pyproject_config: &PyprojectConfig,
transformer: &dyn ConfigurationTransformer,
) -> Result<bool> {
if !pyproject_config.settings.file_resolver.force_exclude {
return Ok(true);
}

// Normalize the path (e.g., convert from relative to absolute).
let path = fs::normalize_path(path);

Expand Down

0 comments on commit 1642f4d

Please sign in to comment.