Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update CLI to respect fix applicability #7769

Merged
merged 29 commits into from
Oct 6, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
d01969c
Update CLI to respect fix applicability
evanrittenhouse Oct 2, 2023
74acdf5
Fixups for CI
zanieb Oct 2, 2023
85b5efe
Update CLI documentation for fixes
zanieb Oct 3, 2023
7c05fb5
Use `self.flags.intersects` consistently
zanieb Oct 3, 2023
87def58
Revert change to notebook cell iteration
zanieb Oct 3, 2023
6577833
Remove stale todo from test
zanieb Oct 3, 2023
d004ddc
Cleanup internal commentary on the fix rules
zanieb Oct 3, 2023
96b8ce1
Refactor `fix_file` implementation to use `Fix.applies`
zanieb Oct 3, 2023
9949fa0
Use different symbols for fix messages depending on applicability
zanieb Oct 3, 2023
ee903bf
Update snapshots to reflect changes in fix messages
zanieb Oct 3, 2023
b24d9dd
Remove comment detailing ordering
zanieb Oct 4, 2023
a5ef7d6
Refactor `RuleCodeAndBody.fmt` to avoid using `unwrap`
zanieb Oct 4, 2023
cb6c25e
Do not display fixes with manual applicability
zanieb Oct 4, 2023
8683340
Rename `--suggested-fix` to `--unsafe-fixes` and fix interaction with…
zanieb Oct 4, 2023
d3f0558
Improve messaging around unsafe fixes
zanieb Oct 4, 2023
d35d68d
Refactor `UnsafeFixes` out of `FixMode`
zanieb Oct 5, 2023
953ee4a
Merge branch 'main' into zanie/applicability
zanieb Oct 5, 2023
98c2c29
Update order of applicability levels for `Ord`
zanieb Oct 5, 2023
f57b853
Fix merge compile errors
zanieb Oct 5, 2023
a57e3d3
Refactor unsafe fix display to avoid using flags
zanieb Oct 5, 2023
d3cb905
Fix unsafe fixes in grouped emitter
zanieb Oct 5, 2023
2a5f454
Refactor `FixableStatistics`
zanieb Oct 5, 2023
8d89499
Lint
zanieb Oct 5, 2023
b72ab24
Fixup `CheckCommand`
zanieb Oct 5, 2023
356f409
`FixableStatistics` -> `FixableSummary`
zanieb Oct 5, 2023
1bbc858
Wrap --fix and --unsafe-fixes in code quotes
zanieb Oct 5, 2023
b6e3a9a
Clean up integration tests
zanieb Oct 5, 2023
47b146b
Commit missing snapshot
zanieb Oct 5, 2023
859ac7d
Avoid reading `unsafe_fixes` from nested configuration files
zanieb Oct 6, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fixups for CI
  • Loading branch information
zanieb committed Oct 3, 2023
commit 74acdf5a22d5763a478e6fef1b06905866e1f579
1 change: 1 addition & 0 deletions crates/ruff_cli/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ pub enum Command {

// The `Parser` derive is for ruff_dev, for ruff_cli `Args` would be sufficient
#[derive(Clone, Debug, clap::Parser)]
#[allow(clippy::struct_excessive_bools)]
pub struct CheckCommand {
/// List of files or directories to check.
pub files: Vec<PathBuf>,
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_cli/src/commands/check_stdin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub(crate) fn check_stdin(
let mut diagnostics = lint_stdin(
filename,
package_root,
stdin,
&stdin,
&pyproject_config.settings,
noqa,
fix_mode,
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_cli/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ pub(crate) fn lint_path(
pub(crate) fn lint_stdin(
path: Option<&Path>,
package: Option<&Path>,
contents: String,
contents: &str,
settings: &Settings,
noqa: flags::Noqa,
fix_mode: flags::FixMode,
Expand All @@ -406,7 +406,7 @@ pub(crate) fn lint_stdin(
};

// Extract the sources from the file.
let source_kind = match SourceKind::from_source_code(contents.clone(), source_type) {
let source_kind = match SourceKind::from_source_code(contents.to_string(), source_type) {
Ok(Some(source_kind)) => source_kind,
Ok(None) => return Ok(Diagnostics::default()),
Err(err) => {
Expand Down
8 changes: 4 additions & 4 deletions crates/ruff_cli/src/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ impl Printer {
}
SerializationFormat::Text => {
TextEmitter::default()
.with_show_fix_status(show_fix_status(self.fix_mode, fixables))
.with_show_fix_status(show_fix_status(self.fix_mode, &fixables))
.with_show_fix_diff(self.flags.contains(Flags::SHOW_FIX_DIFF))
.with_show_source(self.flags.contains(Flags::SHOW_SOURCE))
.emit(writer, &diagnostics.messages, &context)?;
Expand All @@ -202,7 +202,7 @@ impl Printer {
SerializationFormat::Grouped => {
GroupedEmitter::default()
.with_show_source(self.flags.intersects(Flags::SHOW_SOURCE))
.with_show_fix_status(show_fix_status(self.fix_mode, fixables))
.with_show_fix_status(show_fix_status(self.fix_mode, &fixables))
.emit(writer, &diagnostics.messages, &context)?;

if self.flags.intersects(Flags::SHOW_FIX_SUMMARY) {
Expand Down Expand Up @@ -361,7 +361,7 @@ impl Printer {

let context = EmitterContext::new(&diagnostics.notebook_indexes);
TextEmitter::default()
.with_show_fix_status(show_fix_status(self.fix_mode, fixables))
.with_show_fix_status(show_fix_status(self.fix_mode, &fixables))
.with_show_source(self.flags.intersects(Flags::SHOW_SOURCE))
.emit(writer, &diagnostics.messages, &context)?;
}
Expand All @@ -385,7 +385,7 @@ fn num_digits(n: usize) -> usize {
}

/// Return `true` if the [`Printer`] should indicate that a rule is fixable.
fn show_fix_status(fix_mode: flags::FixMode, fixables: FixableStatistics) -> bool {
fn show_fix_status(fix_mode: flags::FixMode, fixables: &FixableStatistics) -> bool {
// If we're in application mode, avoid indicating that a rule is fixable.
// If the specific violation were truly fixable, it would've been fixed in
// this pass! (We're occasionally unable to determine whether a specific
Expand Down
32 changes: 8 additions & 24 deletions crates/ruff_cli/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -799,7 +799,7 @@ fn check_input_from_argfile() -> Result<()> {
}

#[test]
fn displays_fix_applicability_levels() -> Result<()> {
fn displays_fix_applicability_levels() {
// `--fix` should only apply automatic fixes, but should tell the user about `--fix --unautomatic` if
// there are remaining unautomatic fixes.
// TODO: this should be a failure but we don't have a way to track that
Expand All @@ -825,12 +825,10 @@ fn displays_fix_applicability_levels() -> Result<()> {

----- stderr -----
"###);

Ok(())
}

#[test]
fn displays_remaining_suggested_fixes() -> Result<()> {
fn displays_remaining_suggested_fixes() {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.args(["-", "--output-format", "text", "--no-cache", "--isolated", "--select", "F601"])
.pass_stdin("x = {'a': 1, 'a': 1}\n"),
Expand All @@ -844,12 +842,10 @@ fn displays_remaining_suggested_fixes() -> Result<()> {

----- stderr -----
"###);

Ok(())
}

#[test]
fn fix_applies_automatic_fixes_only() -> Result<()> {
fn fix_applies_automatic_fixes_only() {
// `--fix` should only apply automatic fixes. Since we're runnnig in `stdin` mode, output shouldn't
// be printed.
assert_cmd_snapshot!(
Expand All @@ -873,12 +869,10 @@ fn fix_applies_automatic_fixes_only() -> Result<()> {

----- stderr -----
"###);

Ok(())
}

#[test]
fn fix_applies_automatic_and_suggested_fixes_with_fix_suggested() -> Result<()> {
fn fix_applies_automatic_and_suggested_fixes_with_fix_suggested() {
assert_cmd_snapshot!(
Command::new(get_cargo_bin(BIN_NAME))
.args([
Expand All @@ -904,12 +898,10 @@ fn fix_applies_automatic_and_suggested_fixes_with_fix_suggested() -> Result<()>

----- stderr -----
"###);

Ok(())
}

#[test]
fn diff_shows_automatic_and_suggested_fixes_with_fix_suggested() -> Result<()> {
fn diff_shows_automatic_and_suggested_fixes_with_fix_suggested() {
assert_cmd_snapshot!(
Command::new(get_cargo_bin(BIN_NAME))
.args([
Expand Down Expand Up @@ -937,12 +929,10 @@ fn diff_shows_automatic_and_suggested_fixes_with_fix_suggested() -> Result<()> {
----- stderr -----
"###
);

Ok(())
}

#[test]
fn diff_shows_automatic_fixes_only() -> Result<()> {
fn diff_shows_automatic_fixes_only() {
assert_cmd_snapshot!(
Command::new(get_cargo_bin(BIN_NAME))
.args([
Expand All @@ -968,12 +958,10 @@ fn diff_shows_automatic_fixes_only() -> Result<()> {
----- stderr -----
"###
);

Ok(())
}

#[test]
fn fix_only_applies_automatic_and_suggested_fixes_with_fix_suggested() -> Result<()> {
fn fix_only_applies_automatic_and_suggested_fixes_with_fix_suggested() {
assert_cmd_snapshot!(
Command::new(get_cargo_bin(BIN_NAME))
.args([
Expand All @@ -996,12 +984,10 @@ fn fix_only_applies_automatic_and_suggested_fixes_with_fix_suggested() -> Result

----- stderr -----
"###);

Ok(())
}

#[test]
fn fix_only_applies_automatic_fixes_only() -> Result<()> {
fn fix_only_applies_automatic_fixes_only() {
assert_cmd_snapshot!(
Command::new(get_cargo_bin(BIN_NAME))
.args([
Expand All @@ -1023,6 +1009,4 @@ fn fix_only_applies_automatic_fixes_only() -> Result<()> {

----- stderr -----
"###);

Ok(())
}