Skip to content

Commit

Permalink
Remove autofix behavior for uncapitalized-environment-variables (SIM112)
Browse files Browse the repository at this point in the history
  • Loading branch information
@charliermarsh
charliermarsh committed Apr 16, 2023
1 parent 289289b commit fc5aa58
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 85 deletions.
33 changes: 8 additions & 25 deletions crates/ruff/src/rules/flake8_simplify/rules/ast_expr.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use rustpython_parser::ast::{Constant, Expr, ExprKind};

use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit};
use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::{create_expr, unparse_expr};
use ruff_python_ast::types::Range;
Expand Down Expand Up @@ -33,35 +33,30 @@ pub struct DictGetWithNoneDefault {
pub original: String,
}

impl AlwaysAutofixableViolation for DictGetWithNoneDefault {
impl Violation for DictGetWithNoneDefault {
#[derive_message_formats]
fn message(&self) -> String {
let DictGetWithNoneDefault { expected, original } = self;
format!("Use `{expected}` instead of `{original}`")
}

fn autofix_title(&self) -> String {
let DictGetWithNoneDefault { expected, original } = self;
format!("Replace `{original}` with `{expected}`")
}
}

/// SIM112
pub fn use_capital_environment_variables(checker: &mut Checker, expr: &Expr) {
// check `os.environ['foo']`
// Ex) `os.environ['foo']`
if let ExprKind::Subscript { .. } = &expr.node {
check_os_environ_subscript(checker, expr);
return;
}

// check `os.environ.get('foo')` and `os.getenv('foo')`.
// Ex) `os.environ.get('foo')`, `os.getenv('foo')`
let ExprKind::Call { func, args, .. } = &expr.node else {
return;
};
let Some(arg) = args.get(0) else {
return;
};
let ExprKind::Constant { value: Constant::Str(env_var), kind } = &arg.node else {
let ExprKind::Constant { value: Constant::Str(env_var), .. } = &arg.node else {
return;
};
if !checker
Expand All @@ -80,25 +75,13 @@ pub fn use_capital_environment_variables(checker: &mut Checker, expr: &Expr) {
return;
}

let mut diagnostic = Diagnostic::new(
checker.diagnostics.push(Diagnostic::new(
UncapitalizedEnvironmentVariables {
expected: capital_env_var.clone(),
expected: capital_env_var,
original: env_var.clone(),
},
Range::from(arg),
);
if checker.patch(diagnostic.kind.rule()) {
let new_env_var = create_expr(ExprKind::Constant {
value: capital_env_var.into(),
kind: kind.clone(),
});
diagnostic.set_fix(Edit::replacement(
unparse_expr(&new_env_var, checker.stylist),
arg.location,
arg.end_location.unwrap(),
));
}
checker.diagnostics.push(diagnostic);
));
}

fn check_os_environ_subscript(checker: &mut Checker, expr: &Expr) {
Expand Down
50 changes: 0 additions & 50 deletions ...ules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM112_SIM112.py.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,6 @@ SIM112.py:6:16: SIM112 [*] Use capitalized environment variable `FOO` instead of
|
= help: Replace `foo` with `FOO`

ℹ Suggested fix
3 3 | # Bad
4 4 | os.environ['foo']
5 5 |
6 |-os.environ.get('foo')
6 |+os.environ.get('FOO')
7 7 |
8 8 | os.environ.get('foo', 'bar')
9 9 |

SIM112.py:8:16: SIM112 [*] Use capitalized environment variable `FOO` instead of `foo`
|
8 | os.environ.get('foo')
Expand All @@ -53,16 +43,6 @@ SIM112.py:8:16: SIM112 [*] Use capitalized environment variable `FOO` instead of
|
= help: Replace `foo` with `FOO`

ℹ Suggested fix
5 5 |
6 6 | os.environ.get('foo')
7 7 |
8 |-os.environ.get('foo', 'bar')
8 |+os.environ.get('FOO', 'bar')
9 9 |
10 10 | os.getenv('foo')
11 11 |

SIM112.py:10:11: SIM112 [*] Use capitalized environment variable `FOO` instead of `foo`
|
10 | os.environ.get('foo', 'bar')
Expand All @@ -74,16 +54,6 @@ SIM112.py:10:11: SIM112 [*] Use capitalized environment variable `FOO` instead o
|
= help: Replace `foo` with `FOO`

ℹ Suggested fix
7 7 |
8 8 | os.environ.get('foo', 'bar')
9 9 |
10 |-os.getenv('foo')
10 |+os.getenv('FOO')
11 11 |
12 12 | env = os.environ.get('foo')
13 13 |

SIM112.py:12:22: SIM112 [*] Use capitalized environment variable `FOO` instead of `foo`
|
12 | os.getenv('foo')
Expand All @@ -95,16 +65,6 @@ SIM112.py:12:22: SIM112 [*] Use capitalized environment variable `FOO` instead o
|
= help: Replace `foo` with `FOO`

ℹ Suggested fix
9 9 |
10 10 | os.getenv('foo')
11 11 |
12 |-env = os.environ.get('foo')
12 |+env = os.environ.get('FOO')
13 13 |
14 14 | env = os.environ['foo']
15 15 |

SIM112.py:14:18: SIM112 [*] Use capitalized environment variable `FOO` instead of `foo`
|
14 | env = os.environ.get('foo')
Expand Down Expand Up @@ -136,16 +96,6 @@ SIM112.py:16:26: SIM112 [*] Use capitalized environment variable `FOO` instead o
|
= help: Replace `foo` with `FOO`

ℹ Suggested fix
13 13 |
14 14 | env = os.environ['foo']
15 15 |
16 |-if env := os.environ.get('foo'):
16 |+if env := os.environ.get('FOO'):
17 17 | pass
18 18 |
19 19 | if env := os.environ['foo']:

SIM112.py:19:22: SIM112 [*] Use capitalized environment variable `FOO` instead of `foo`
|
19 | pass
Expand Down
15 changes: 5 additions & 10 deletions ...ules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM910_SIM910.py.snap
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
---
source: crates/ruff/src/rules/flake8_simplify/mod.rs
---
SIM910.py:2:1: SIM910 [*] Use `{}.get(key)` instead of `{}.get(key, None)`
SIM910.py:2:1: SIM910 Use `{}.get(key)` instead of `{}.get(key, None)`
|
2 | # SIM910
3 | {}.get(key, None)
| ^^^^^^^^^^^^^^^^^ SIM910
4 |
5 | # SIM910
|
= help: Replace `{}.get(key, None)` with `{}.get(key)`

ℹ Suggested fix
1 1 | # SIM910
Expand All @@ -19,15 +18,14 @@ SIM910.py:2:1: SIM910 [*] Use `{}.get(key)` instead of `{}.get(key, None)`
4 4 | # SIM910
5 5 | {}.get("key", None)

SIM910.py:5:1: SIM910 [*] Use `{}.get("key")` instead of `{}.get("key", None)`
SIM910.py:5:1: SIM910 Use `{}.get("key")` instead of `{}.get("key", None)`
|
5 | # SIM910
6 | {}.get("key", None)
| ^^^^^^^^^^^^^^^^^^^ SIM910
7 |
8 | # OK
|
= help: Replace `{}.get("key", None)` with `{}.get("key")`

ℹ Suggested fix
2 2 | {}.get(key, None)
Expand All @@ -39,14 +37,13 @@ SIM910.py:5:1: SIM910 [*] Use `{}.get("key")` instead of `{}.get("key", None)`
7 7 | # OK
8 8 | {}.get(key)

SIM910.py:20:9: SIM910 [*] Use `{}.get(key)` instead of `{}.get(key, None)`
SIM910.py:20:9: SIM910 Use `{}.get(key)` instead of `{}.get(key, None)`
|
20 | # SIM910
21 | if a := {}.get(key, None):
| ^^^^^^^^^^^^^^^^^ SIM910
22 | pass
|
= help: Replace `{}.get(key, None)` with `{}.get(key)`

ℹ Suggested fix
17 17 | {}.get("key", False)
Expand All @@ -58,15 +55,14 @@ SIM910.py:20:9: SIM910 [*] Use `{}.get(key)` instead of `{}.get(key, None)`
22 22 |
23 23 | # SIM910

SIM910.py:24:5: SIM910 [*] Use `{}.get(key)` instead of `{}.get(key, None)`
SIM910.py:24:5: SIM910 Use `{}.get(key)` instead of `{}.get(key, None)`
|
24 | # SIM910
25 | a = {}.get(key, None)
| ^^^^^^^^^^^^^^^^^ SIM910
26 |
27 | # SIM910
|
= help: Replace `{}.get(key, None)` with `{}.get(key)`

ℹ Suggested fix
21 21 | pass
Expand All @@ -78,13 +74,12 @@ SIM910.py:24:5: SIM910 [*] Use `{}.get(key)` instead of `{}.get(key, None)`
26 26 | # SIM910
27 27 | ({}).get(key, None)

SIM910.py:27:1: SIM910 [*] Use `({}).get(key)` instead of `({}).get(key, None)`
SIM910.py:27:1: SIM910 Use `({}).get(key)` instead of `({}).get(key, None)`
|
27 | # SIM910
28 | ({}).get(key, None)
| ^^^^^^^^^^^^^^^^^^^ SIM910
|
= help: Replace `({}).get(key, None)` with `({}).get(key)`

ℹ Suggested fix
24 24 | a = {}.get(key, None)
Expand Down

0 comments on commit fc5aa58

Please sign in to comment.