Skip to content

Commit

Permalink
Implement basic reviewer comments
Browse files Browse the repository at this point in the history
  • Loading branch information
evanrittenhouse committed Aug 14, 2023
1 parent 542f755 commit c7a753a
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 24 deletions.
6 changes: 3 additions & 3 deletions crates/ruff/src/settings/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,14 +211,14 @@ pub struct Options {
pub external: Option<Vec<String>>,
#[option(default = "false", value_type = "bool", example = "fix = true")]
/// Enable autofix behavior by-default when running `ruff` (overridden
/// by the `--fix` and `--no-fix` command-line flags). This will only implement "safe" fixes
/// by the `--fix` and `--no-fix` command-line flags). This will only apply "safe" fixes.
/// Safe fixes are exactly what the developer wants and/or maintain the exact meaning of the
/// existing code).
/// existing code.
pub fix: Option<bool>,
#[option(default = "false", value_type = "bool", example = "fix-only = true")]
/// Like `fix`, but disables reporting on leftover violation. Implies `fix`.
pub fix_only: Option<bool>,
/// Like `fix`, but will also implement "unsafe" fixes. Unsafe fixes are those which may not be
/// Like `fix`, but will also implement "suggested" fixes. Suggested fixes are those which may not be
/// exactly what you intend, but should still result in valid code. Implies `fix`.
#[option(default = "false", value_type = "bool", example = "unsafe = true")]
pub r#unsafe: Option<bool>,
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use ruff_python_parser::lexer::LexResult;

use rustc_hash::FxHashMap;

use ruff_diagnostics::{AutofixKind, Diagnostic};
use ruff_diagnostics::{Applicability, AutofixKind, Diagnostic};
use ruff_python_ast::PySourceType;
use ruff_python_codegen::Stylist;
use ruff_python_index::Indexer;
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_cli/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ pub(crate) fn lint_stdin(
},
fixed,
) = match autofix {
flags::FixMode::Apply(fix_unsafe) | flags::FixMode::Diff(fix_unsafe) => {
flags::FixMode::Apply(fix_suggested) | flags::FixMode::Diff(fix_suggested) => {
if let Ok(FixerResult {
result,
transformed,
Expand Down
15 changes: 7 additions & 8 deletions crates/ruff_cli/src/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -389,13 +389,13 @@ fn num_digits(n: usize) -> usize {
.max(1)
}

/// Return the number of safe ([`Applicability::Automatic`]) and unsafe ([`Applicability::Suggested`])
/// Return the number of safe ([`Applicability::Automatic`]) and suggested ([`Applicability::Suggested`])
/// fixes generated from a given [`Diagnostics`], contingent upon the mode/applicability levels
/// passed by the user. Returns [`None`] if there are no applicable fixes.
fn applicable_fixes(
autofix_level: flags::FixMode,
diagnostics: &Diagnostics,
) -> Option<(i32, i32)> {
) -> Option<(u32, u32)> {
let mut safe_remaining = 0;
let mut unsafe_remaining = 0;
diagnostics.messages.iter().for_each(|msg| {
Expand All @@ -409,12 +409,12 @@ fn applicable_fixes(
});

// If we're in apply mode, calculate the status based on the applicability level of the
// fixes in question. If the user passed `--fix --unsafe`, we can safely say that all fixes
// fixes in question. If the user passed `--fix --suggested`, we can safely say that all fixes
// should've been applied in this pass, so we do not want to show the status (We're occasionally
// unable to determine whether a specific violation is fixable without trying to fix it, so
// if autofix is not enabled, we may inadvertently indicate that a rule is fixable.).
//
// If the user passed `--fix`, unsafe fixes (if any were generated) would not have been
// If the user passed `--fix`, suggested fixes (if any were generated) would not have been
// applied. If this is the case, the fix status message will depend on the types of fixes that
// remain, so we'll have to return both.
//
Expand Down Expand Up @@ -484,17 +484,16 @@ fn print_fix_summary(writer: &mut dyn Write, fixed: &FxHashMap<String, FixTable>
}

/// Build the displayed fix status message depending on the types of the remaining fixes.
fn display_violations(safe_remaining: i32, unsafe_remaining: i32) -> String {
fn display_violations(safe_remaining: u32, unsafe_remaining: u32) -> String {
let prefix = format!("[{}]", "*".cyan());
let mut fix_status = prefix;

if safe_remaining > 0 {
fix_status =
format!("{fix_status} {safe_remaining} potentially fixable with the --fix option.");
fix_status = format!("{fix_status} {safe_remaining} potentially fixable with the --fix option.");
}

if unsafe_remaining > 0 {
let (line_break, extra_prefix) = if safe_remaining > 0 {
// Avoid re-allocating prefix.
("\n", format!("[{}]", "*".cyan()))
} else {
("", String::new())
Expand Down
27 changes: 16 additions & 11 deletions crates/ruff_diagnostics/src/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,23 @@ use crate::edit::Edit;
#[derive(Default, Copy, Clone, Debug, Hash, PartialEq, Eq, PartialOrd, Ord)]
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
pub enum Applicability {
/// The fix is definitely what the user intended, or maintains the exact meaning of the code.
/// This fix should be automatically applied.
Automatic = 2,

/// The fix may be what the user intended, but it is uncertain.
/// The fix should result in valid code if it is applied.
/// The fix can be applied with user opt-in.
Suggested = 1,

/// The fix has a good chance of being incorrect or the code be incomplete.
/// The fix may result in invalid code if it is applied.
/// The fix should only be manually applied by the user.
Manual = -1,
Manual,

/// The applicability of the fix is unknown or not relevant.
#[default]
Unspecified = 0,
Unspecified,

/// The fix may be what the user intended, but it is uncertain.
/// The fix should result in valid code if it is applied.
/// The fix can be applied with user opt-in.
Suggested,

/// The fix is definitely what the user intended, or maintains the exact meaning of the code.
/// This fix should be automatically applied.
Automatic,
}

/// Indicates the level of isolation required to apply a fix.
Expand Down Expand Up @@ -162,4 +162,9 @@ impl Fix {
self.isolation_level = isolation;
self
}

/// Return [`true`] if this [`Fix`] should be applied with a given [`Applicability`].
pub fn is_applied(&self, applicability: Applicability) -> bool {
self.applicability >= applicability
}
}

0 comments on commit c7a753a

Please sign in to comment.