Skip to content

Commit

Permalink
Enable suppression of magic values by type
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Jan 19, 2023
1 parent 34412a0 commit 7cd7683
Show file tree
Hide file tree
Showing 16 changed files with 287 additions and 62 deletions.
19 changes: 19 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3407,6 +3407,25 @@ convention = "google"

---

### `pylint`

#### [`allow-magic-value-types`](#allow-magic-value-types)

Constant types to ignore when used as "magic values".

**Default value**: `["str"]`

**Type**: `Vec<ConstantType>`

**Example usage**:

```toml
[tool.ruff.pylint]
allow-magic-value-types = ["int"]
```

---

### `pyupgrade`

#### [`keep-runtime-typing`](#keep-runtime-typing)
Expand Down
38 changes: 18 additions & 20 deletions resources/test/fixtures/pylint/magic_value_comparison.py
Original file line number Diff line number Diff line change
@@ -1,71 +1,69 @@
"""Check that magic values are not used in comparisons"""
import cmath
"""Check that magic values are not used in comparisons."""

user_input = 10

if 10 > user_input: # [magic-value-comparison]
if 10 > user_input: # [magic-value-comparison]
pass

if 10 == 100: # [comparison-of-constants] R0133
if 10 == 100: # [comparison-of-constants] R0133
pass

if 1 == 3: # [comparison-of-constants] R0133
if 1 == 3: # [comparison-of-constants] R0133
pass

x = 0
if 4 == 3 == x: # [comparison-of-constants] R0133
if 4 == 3 == x: # [comparison-of-constants] R0133
pass

time_delta = 7224
ONE_HOUR = 3600

if time_delta > ONE_HOUR: # correct
if time_delta > ONE_HOUR: # correct
pass

argc = 1

if argc != -1: # correct
if argc != -1: # correct
pass

if argc != 0: # correct
if argc != 0: # correct
pass

if argc != 1: # correct
if argc != 1: # correct
pass

if argc != 2: # [magic-value-comparison]
if argc != 2: # [magic-value-comparison]
pass

if __name__ == "__main__": # correct
if __name__ == "__main__": # correct
pass

ADMIN_PASSWORD = "SUPERSECRET"
input_password = "password"

if input_password == "": # correct
if input_password == "": # correct
pass

if input_password == ADMIN_PASSWORD: # correct
if input_password == ADMIN_PASSWORD: # correct
pass

if input_password == "Hunter2": # [magic-value-comparison]
if input_password == "Hunter2": # [magic-value-comparison]
pass

PI = 3.141592653589793238
pi_estimation = 3.14

if pi_estimation == 3.141592653589793238: # [magic-value-comparison]
if pi_estimation == 3.141592653589793238: # [magic-value-comparison]
pass

if pi_estimation == PI: # correct
if pi_estimation == PI: # correct
pass

HELLO_WORLD = b"Hello, World!"
user_input = b"Hello, There!"

if user_input == b"something": # [magic-value-comparison]
if user_input == b"something": # [magic-value-comparison]
pass

if user_input == HELLO_WORLD: # correct
if user_input == HELLO_WORLD: # correct
pass

41 changes: 41 additions & 0 deletions ruff.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,17 @@
}
]
},
"pylint": {
"description": "Options for the `pylint` plugin.",
"anyOf": [
{
"$ref": "#/definitions/PylintOptions"
},
{
"type": "null"
}
]
},
"pyupgrade": {
"description": "Options for the `pyupgrade` plugin.",
"anyOf": [
Expand Down Expand Up @@ -461,6 +472,20 @@
},
"additionalProperties": false
},
"ConstantType": {
"type": "string",
"enum": [
"bool",
"bytes",
"complex",
"ellipsis",
"float",
"int",
"none",
"str",
"tuple"
]
},
"Convention": {
"oneOf": [
{
Expand Down Expand Up @@ -1046,6 +1071,22 @@
},
"additionalProperties": false
},
"PylintOptions": {
"type": "object",
"properties": {
"allow-magic-value-types": {
"description": "Constant types to ignore when used as \"magic values\".",
"type": [
"array",
"null"
],
"items": {
"$ref": "#/definitions/ConstantType"
}
}
},
"additionalProperties": false
},
"PythonVersion": {
"type": "string",
"enum": [
Expand Down
7 changes: 7 additions & 0 deletions src/flake8_to_ruff/converter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,7 @@ mod tests {
pep8_naming: None,
pycodestyle: None,
pydocstyle: None,
pylint: None,
pyupgrade: None,
});
assert_eq!(actual, expected);
Expand Down Expand Up @@ -520,6 +521,7 @@ mod tests {
pep8_naming: None,
pycodestyle: None,
pydocstyle: None,
pylint: None,
pyupgrade: None,
});
assert_eq!(actual, expected);
Expand Down Expand Up @@ -586,6 +588,7 @@ mod tests {
pep8_naming: None,
pycodestyle: None,
pydocstyle: None,
pylint: None,
pyupgrade: None,
});
assert_eq!(actual, expected);
Expand Down Expand Up @@ -652,6 +655,7 @@ mod tests {
pep8_naming: None,
pycodestyle: None,
pydocstyle: None,
pylint: None,
pyupgrade: None,
});
assert_eq!(actual, expected);
Expand Down Expand Up @@ -723,6 +727,7 @@ mod tests {
pep8_naming: None,
pycodestyle: None,
pydocstyle: None,
pylint: None,
pyupgrade: None,
});
assert_eq!(actual, expected);
Expand Down Expand Up @@ -795,6 +800,7 @@ mod tests {
pydocstyle: Some(pydocstyle::settings::Options {
convention: Some(Convention::Numpy),
}),
pylint: None,
pyupgrade: None,
});
assert_eq!(actual, expected);
Expand Down Expand Up @@ -867,6 +873,7 @@ mod tests {
pep8_naming: None,
pycodestyle: None,
pydocstyle: None,
pylint: None,
pyupgrade: None,
});
assert_eq!(actual, expected);
Expand Down
17 changes: 17 additions & 0 deletions src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! Rules from [Pylint](https://pypi.org/project/pylint/2.15.7/).
pub(crate) mod rules;
pub mod settings;

#[cfg(test)]
mod tests {
Expand All @@ -10,6 +11,7 @@ mod tests {

use crate::linter::test_path;
use crate::registry::RuleCode;
use crate::rules::pylint;
use crate::settings::Settings;

#[test_case(RuleCode::PLC0414, Path::new("import_aliasing.py"); "PLC0414")]
Expand Down Expand Up @@ -42,4 +44,19 @@ mod tests {
insta::assert_yaml_snapshot!(snapshot, diagnostics);
Ok(())
}

#[test]
fn allow_magic_value_types() -> Result<()> {
let diagnostics = test_path(
Path::new("./resources/test/fixtures/pylint/magic_value_comparison.py"),
&Settings {
pylint: pylint::settings::Settings {
allow_magic_value_types: vec![pylint::settings::ConstantType::Int],
},
..Settings::for_rules(vec![RuleCode::PLR2004])
},
)?;
insta::assert_yaml_snapshot!(diagnostics);
Ok(())
}
}
25 changes: 15 additions & 10 deletions src/rules/pylint/rules/magic_value_comparison.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,25 @@ use rustpython_ast::{Constant, Expr, ExprKind};
use crate::ast::types::Range;
use crate::checkers::ast::Checker;
use crate::registry::Diagnostic;
use crate::rules::pylint::settings::ConstantType;
use crate::violations;

fn is_magic_value(constant: &Constant) -> bool {
fn is_magic_value(constant: &Constant, allowed_types: &[ConstantType]) -> bool {
if allowed_types.contains(&constant.into()) {
return false;
}
match constant {
// Ignore `None`, `Bool`, and `Ellipsis` constants.
Constant::None => false,
// E712 `if True == do_something():`
Constant::Bool(_) => false,
Constant::Ellipsis => false,
// Otherwise, special-case some common string and integer types.
Constant::Str(value) => !matches!(value.as_str(), "" | "__main__"),
Constant::Bytes(_) => true,
Constant::Int(value) => !matches!(value.try_into(), Ok(-1 | 0 | 1)),
Constant::Bytes(_) => true,
Constant::Tuple(_) => true,
Constant::Float(_) => true,
Constant::Complex { .. } => true,
Constant::Ellipsis => true,
}
}

Expand All @@ -38,13 +43,13 @@ pub fn magic_value_comparison(checker: &mut Checker, left: &Expr, comparators: &

for comparison_expr in std::iter::once(left).chain(comparators.iter()) {
if let ExprKind::Constant { value, .. } = &comparison_expr.node {
if is_magic_value(value) {
let diagnostic = Diagnostic::new(
violations::MagicValueComparison(value.to_string()),
if is_magic_value(value, &checker.settings.pylint.allow_magic_value_types) {
checker.diagnostics.push(Diagnostic::new(
violations::MagicValueComparison {
value: value.to_string(),
},
Range::from_located(comparison_expr),
);

checker.diagnostics.push(diagnostic);
));
}
}
}
Expand Down
Loading

0 comments on commit 7cd7683

Please sign in to comment.