Skip to content

Commit

Permalink
Rename fix_unsafe to fix_suggested
Browse files Browse the repository at this point in the history
  • Loading branch information
evanrittenhouse committed Sep 24, 2023
1 parent 27f5ce6 commit aaf677b
Show file tree
Hide file tree
Showing 9 changed files with 36 additions and 42 deletions.
6 changes: 3 additions & 3 deletions crates/ruff/src/settings/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ pub struct Configuration {
pub external: Option<Vec<String>>,
pub fix: Option<bool>,
pub fix_only: Option<bool>,
pub fix_unsafe: Option<bool>,
pub fix_suggested: Option<bool>,
pub force_exclude: Option<bool>,
pub format: Option<SerializationFormat>,
pub ignore_init_module_imports: Option<bool>,
Expand Down Expand Up @@ -189,7 +189,7 @@ impl Configuration {
external: options.external,
fix: options.fix,
fix_only: options.fix_only,
fix_unsafe: options.r#unsafe,
fix_suggested: options.fix_suggested,
format: options.format,
force_exclude: options.force_exclude,
ignore_init_module_imports: options.ignore_init_module_imports,
Expand Down Expand Up @@ -288,7 +288,7 @@ impl Configuration {
external: self.external.or(config.external),
fix: self.fix.or(config.fix),
fix_only: self.fix_only.or(config.fix_only),
fix_unsafe: self.fix_unsafe.or(config.fix_unsafe),
fix_suggested: self.fix_suggested.or(config.fix_suggested),
format: self.format.or(config.format),
force_exclude: self.force_exclude.or(config.force_exclude),
include: self.include.or(config.include),
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff/src/settings/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ impl AllSettings {
.unwrap_or_else(|| cache_dir(project_root)),
fix: config.fix.unwrap_or(false),
fix_only: config.fix_only.unwrap_or(false),
fix_unsafe: config.fix_unsafe.unwrap_or(false),
fix_suggested: config.fix_suggested.unwrap_or(false),
format: config.format.unwrap_or_default(),
show_fixes: config.show_fixes.unwrap_or(false),
show_source: config.show_source.unwrap_or(false),
Expand All @@ -73,7 +73,7 @@ pub struct CliSettings {
pub cache_dir: PathBuf,
pub fix: bool,
pub fix_only: bool,
pub fix_unsafe: bool,
pub fix_suggested: bool,
pub format: SerializationFormat,
pub show_fixes: bool,
pub show_source: bool,
Expand Down
3 changes: 1 addition & 2 deletions crates/ruff/src/settings/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,7 @@ pub struct Options {
pub fix_only: Option<bool>,
/// 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>,
pub fix_suggested: Option<bool>,
#[option(
default = r#"["ALL"]"#,
value_type = "list[RuleSelector]",
Expand Down
17 changes: 9 additions & 8 deletions crates/ruff_cli/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,12 @@ pub struct CheckArgs {
pub files: Vec<PathBuf>,
/// Attempt to automatically fix safe lint violations. Safe fixes are exactly what the developer
/// wants and/or maintain the exact meaning of the existing code.
#[arg(long, overrides_with("no_fix"))]
#[arg(long, overrides_with_all(["fix_suggested", "no_fix"]))]
fix: bool,
#[clap(long, overrides_with("fix"), hide = true)]
/// Attempt to automatically fix both automatic and suggested lint violations.
#[arg(long, overrides_with_all(["fix", "no_fix"]))]
fix_suggested: bool,
#[clap(long, overrides_with_all(["fix", "fix_suggested"]), hide = true)]
no_fix: bool,
/// Show violations with source code.
#[arg(long, overrides_with("no_show_source"))]
Expand Down Expand Up @@ -197,8 +200,6 @@ pub struct CheckArgs {
pub extend_exclude: Option<Vec<FilePattern>>,
/// Attempt to automatically fix both safe and unsafe lint violations. Only applicable when
/// autofix itself is enabled (e.g., via `--fix`).
#[arg(long, requires("fix_args"))]
pub r#unsafe: bool,
/// List of rule codes to treat as eligible for autofix. Only applicable
/// when autofix itself is enabled (e.g., via `--fix`).
#[arg(
Expand Down Expand Up @@ -449,7 +450,7 @@ impl CheckArgs {
cache_dir: self.cache_dir,
fix: resolve_bool_arg(self.fix, self.no_fix),
fix_only: resolve_bool_arg(self.fix_only, self.no_fix_only),
fix_unsafe: Some(self.r#unsafe),
fix_suggested: Some(self.fix_suggested),
force_exclude: resolve_bool_arg(self.force_exclude, self.no_force_exclude),
format: self.format,
show_fixes: resolve_bool_arg(self.show_fixes, self.no_show_fixes),
Expand Down Expand Up @@ -518,7 +519,7 @@ pub struct Overrides {
pub cache_dir: Option<PathBuf>,
pub fix: Option<bool>,
pub fix_only: Option<bool>,
pub fix_unsafe: Option<bool>,
pub fix_suggested: Option<bool>,
pub force_exclude: Option<bool>,
pub format: Option<SerializationFormat>,
pub show_fixes: Option<bool>,
Expand All @@ -544,8 +545,8 @@ impl ConfigProcessor for &Overrides {
if let Some(fix_only) = &self.fix_only {
config.fix_only = Some(*fix_only);
}
if let Some(fix_unsafe) = &self.fix_unsafe {
config.fix_unsafe = Some(*fix_unsafe);
if self.fix_suggested.is_some() {
config.fix_suggested = self.fix_suggested;
}
config.rule_selections.push(RuleSelection {
select: self.select.clone(),
Expand Down
13 changes: 6 additions & 7 deletions crates/ruff_cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ pub fn check(args: CheckArgs, log_level: LogLevel) -> Result<ExitStatus> {
let CliSettings {
fix,
fix_only,
fix_unsafe,
fix_suggested,
format,
show_fixes,
show_source,
Expand All @@ -234,15 +234,14 @@ pub fn check(args: CheckArgs, log_level: LogLevel) -> Result<ExitStatus> {
// Autofix rules are as follows:
// - By default, generate all fixes, but don't apply them to the filesystem.
// - If `--fix` or `--fix-only` is set, always apply fixes to the filesystem (or
// print them to stdout, if we're reading from stdin).
// print them to stdout, if we're reading from stdin) for [`Applicability::Automatic`] rules
// only.
// - If `--diff` or `--fix-only` are set, don't print any violations (only
// fixes).
// - If `--unsafe` is set, the above rules will apply to both [`Applicability::Suggested`] and
// [`Applicability::Automatic`] fixes. If it is not set, the rules will only apply to
// fixes) for [`Applicability::Automatic`] rules only.
// - If `--fix--suggested` is set, the above rules will apply to both [`Applicability::Suggested`] and
// [`Applicability::Automatic`] fixes.

// TODO: can't fix this until @zanieb changes the CLI
let fix_suggested = if fix_unsafe {
let fix_suggested = if fix_suggested {
SuggestedFixes::Apply
} else {
SuggestedFixes::Disable
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_cli/src/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,7 @@ impl<'a> FixableStatistics<'a> {

let total = self.automatic + self.suggested;
fix_status = format!(
"{fix_status}{line_break}{extra_prefix} {total} potentially fixable with the --fix --unsafe options."
"{fix_status}{line_break}{extra_prefix} {total} potentially fixable with the --fix-suggested option."
);
}

Expand Down
27 changes: 11 additions & 16 deletions crates/ruff_cli/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -353,8 +353,8 @@ fn unreadable_dir() -> Result<()> {
fn display_different_safety_levels() -> Result<()> {
let mut cmd = Command::cargo_bin(BIN_NAME)?;

// `--fix` should only apply safe fixes, but should tell the user about `--fix --unsafe` if
// there are remaining unsafe fixes.
// `--fix` should only apply automatic fixes, but should tell the user about `--fix-suggested` if
// there are remaining suggested fixes.
let output = cmd
.args([
"-",
Expand All @@ -373,15 +373,15 @@ fn display_different_safety_levels() -> Result<()> {
-:2:7: UP034 [*] Avoid extraneous parentheses
Found 2 errors.
[*] 1 potentially fixable with the --fix option.
[*] 2 potentially fixable with the --fix --unsafe options.
[*] 2 potentially fixable with the --fix-suggested option.
"#
);

Ok(())
}

#[test]
fn display_unsafe_fixes_remain() -> Result<()> {
fn display_suggested_fixes_remain() -> Result<()> {
let mut cmd = Command::cargo_bin(BIN_NAME)?;

let output = cmd
Expand All @@ -393,7 +393,7 @@ fn display_unsafe_fixes_remain() -> Result<()> {
str::from_utf8(&output.get_output().stdout)?,
r#"-:1:14: F601 [*] Dictionary key literal `'a'` repeated
Found 1 error.
[*] 1 potentially fixable with the --fix --unsafe options.
[*] 1 potentially fixable with the --fix-suggested option.
"#
);

Expand Down Expand Up @@ -428,10 +428,9 @@ fn fix_applies_safe_fixes_only() -> Result<()> {
}

#[test]
fn fix_applies_all_fixes() -> Result<()> {
fn fix_suggested_applies_all_fixes() -> Result<()> {
let mut cmd = Command::cargo_bin(BIN_NAME)?;

// `--fix --unsafe` should apply both safe and unsafe fixes.
let output = cmd
.args([
"-",
Expand All @@ -440,8 +439,7 @@ fn fix_applies_all_fixes() -> Result<()> {
"--isolated",
"--select",
"F601,UP034",
"--fix",
"--unsafe",
"--fix-suggested",
])
.write_stdin("x = {'a': 1, 'a': 1}\nprint(('foo'))\n")
.assert()
Expand All @@ -458,7 +456,6 @@ fn fix_applies_all_fixes() -> Result<()> {
fn diff_diffs_all_fixes() -> Result<()> {
let mut cmd = Command::cargo_bin(BIN_NAME)?;

// `--fix --unsafe` should apply both safe and unsafe fixes.
let output = cmd
.args([
"-",
Expand All @@ -468,7 +465,7 @@ fn diff_diffs_all_fixes() -> Result<()> {
"--select",
"F601,UP034",
"--diff",
"--unsafe",
"--fix-suggested",
])
.write_stdin("x = {'a': 1, 'a': 1}\nprint(('foo'))\n")
.assert()
Expand All @@ -488,10 +485,9 @@ fn diff_diffs_all_fixes() -> Result<()> {
}

#[test]
fn diff_diffs_safe_fixes_only() -> Result<()> {
fn diff_diffs_automatic_fixes_only() -> Result<()> {
let mut cmd = Command::cargo_bin(BIN_NAME)?;

// `--fix --unsafe` should apply both safe and unsafe fixes.
let output = cmd
.args([
"-",
Expand Down Expand Up @@ -532,7 +528,7 @@ fn fix_only_applies_all_fixes() -> Result<()> {
"--select",
"F601,UP034",
"--fix-only",
"--unsafe",
"--fix-suggested",
])
.write_stdin("x = {'a': 1, 'a': 1}\nprint(('foo'))\n")
.assert()
Expand All @@ -546,10 +542,9 @@ fn fix_only_applies_all_fixes() -> Result<()> {
}

#[test]
fn fix_only_applies_safe_fixes_only() -> Result<()> {
fn fix_only_applies_automatic_fixes_only() -> Result<()> {
let mut cmd = Command::cargo_bin(BIN_NAME)?;

// `--fix --unsafe` should apply both safe and unsafe fixes.
let output = cmd
.args([
"-",
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_wasm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ impl Workspace {
extend_per_file_ignores: None,
fix: None,
fix_only: None,
r#unsafe: None,
fix_suggested: None,
fixable: None,
force_exclude: None,
format: None,
Expand Down
4 changes: 2 additions & 2 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,8 @@ Options:
The minimum Python version that should be supported [possible values: py37, py38, py39, py310, py311, py312]
--config <CONFIG>
Path to the `pyproject.toml` or `ruff.toml` file to use for configuration
--unsafe
Attempt to automatically fix both safe and unsafe lint violations. Only applicable when autofix itself is enabled (e.g., via `--fix`)
--fix--suggested
Attempt to automatically fix both automatic and suggested lint violations. Implies `--fix`
--statistics
Show counts for every rule with at least one violation
--add-noqa
Expand Down

0 comments on commit aaf677b

Please sign in to comment.