Enable suppression of magic values by type (#1987)

Closes #1949.

README.md

@@ -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)

resources/test/fixtures/pylint/magic_value_comparison.py

@@ -1,5 +1,4 @@
-"""Check that magic values are not used in comparisons"""
+"""Check that magic values are not used in comparisons."""
-import cmath
 
 user_input = 10
 
@@ -68,4 +67,3 @@ if user_input == b"something": # [magic-value-comparison]
 
 if user_input == HELLO_WORLD:  # correct
     pass
-

ruff.schema.json

@@ -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": [
@@ -461,6 +472,20 @@
       },
       "additionalProperties": false
     },
+    "ConstantType": {
+      "type": "string",
+      "enum": [
+        "bool",
+        "bytes",
+        "complex",
+        "ellipsis",
+        "float",
+        "int",
+        "none",
+        "str",
+        "tuple"
+      ]
+    },
     "Convention": {
       "oneOf": [
         {
@@ -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": [

src/flake8_to_ruff/converter.rs

@@ -454,6 +454,7 @@ mod tests {
             pep8_naming: None,
             pycodestyle: None,
             pydocstyle: None,
+            pylint: None,
             pyupgrade: None,
         });
         assert_eq!(actual, expected);
@@ -520,6 +521,7 @@ mod tests {
             pep8_naming: None,
             pycodestyle: None,
             pydocstyle: None,
+            pylint: None,
             pyupgrade: None,
         });
         assert_eq!(actual, expected);
@@ -586,6 +588,7 @@ mod tests {
             pep8_naming: None,
             pycodestyle: None,
             pydocstyle: None,
+            pylint: None,
             pyupgrade: None,
         });
         assert_eq!(actual, expected);
@@ -652,6 +655,7 @@ mod tests {
             pep8_naming: None,
             pycodestyle: None,
             pydocstyle: None,
+            pylint: None,
             pyupgrade: None,
         });
         assert_eq!(actual, expected);
@@ -723,6 +727,7 @@ mod tests {
             pep8_naming: None,
             pycodestyle: None,
             pydocstyle: None,
+            pylint: None,
             pyupgrade: None,
         });
         assert_eq!(actual, expected);
@@ -795,6 +800,7 @@ mod tests {
             pydocstyle: Some(pydocstyle::settings::Options {
                 convention: Some(Convention::Numpy),
             }),
+            pylint: None,
             pyupgrade: None,
         });
         assert_eq!(actual, expected);
@@ -867,6 +873,7 @@ mod tests {
             pep8_naming: None,
             pycodestyle: None,
             pydocstyle: None,
+            pylint: None,
             pyupgrade: None,
         });
         assert_eq!(actual, expected);

src/rules/pylint/mod.rs

@@ -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 {
@@ -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")]
@@ -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(())
+    }
 }

src/rules/pylint/rules/magic_value_comparison.rs

@@ -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,
     }
 }
 
@@ -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) {
+            if is_magic_value(value, &checker.settings.pylint.allow_magic_value_types) {
-                let diagnostic = Diagnostic::new(
+                checker.diagnostics.push(Diagnostic::new(
-                    violations::MagicValueComparison(value.to_string()),
+                    violations::MagicValueComparison {
+                        value: value.to_string(),
+                    },
                     Range::from_located(comparison_expr),
-                );
+                ));
-
-                checker.diagnostics.push(diagnostic);
             }
         }
     }

src/rules/pylint/settings.rs

@@ -0,0 +1,87 @@
+//! Settings for the `pylint` plugin.
+
+use ruff_macros::ConfigurationOptions;
+use rustpython_ast::Constant;
+use schemars::JsonSchema;
+use serde::{Deserialize, Serialize};
+
+#[derive(Debug, Copy, Clone, PartialEq, Eq, Serialize, Deserialize, Hash, JsonSchema)]
+#[serde(deny_unknown_fields, rename_all = "kebab-case")]
+pub enum ConstantType {
+    Bool,
+    Bytes,
+    Complex,
+    Ellipsis,
+    Float,
+    Int,
+    None,
+    Str,
+    Tuple,
+}
+
+impl From<&Constant> for ConstantType {
+    fn from(value: &Constant) -> Self {
+        match value {
+            Constant::Bool(..) => Self::Bool,
+            Constant::Bytes(..) => Self::Bytes,
+            Constant::Complex { .. } => Self::Complex,
+            Constant::Ellipsis => Self::Ellipsis,
+            Constant::Float(..) => Self::Float,
+            Constant::Int(..) => Self::Int,
+            Constant::None => Self::None,
+            Constant::Str(..) => Self::Str,
+            Constant::Tuple(..) => Self::Tuple,
+        }
+    }
+}
+
+#[derive(
+    Debug, PartialEq, Eq, Serialize, Deserialize, Default, ConfigurationOptions, JsonSchema,
+)]
+#[serde(
+    deny_unknown_fields,
+    rename_all = "kebab-case",
+    rename = "PylintOptions"
+)]
+pub struct Options {
+    #[option(
+        default = r#"["str"]"#,
+        value_type = "Vec<ConstantType>",
+        example = r#"
+            allow-magic-value-types = ["int"]
+        "#
+    )]
+    /// Constant types to ignore when used as "magic values".
+    pub allow_magic_value_types: Option<Vec<ConstantType>>,
+}
+
+#[derive(Debug, Hash)]
+pub struct Settings {
+    pub allow_magic_value_types: Vec<ConstantType>,
+}
+
+impl Default for Settings {
+    fn default() -> Self {
+        Self {
+            allow_magic_value_types: vec![ConstantType::Str],
+        }
+    }
+}
+
+impl From<Options> for Settings {
+    fn from(options: Options) -> Self {
+        Self {
+            allow_magic_value_types: options
+                .allow_magic_value_types
+                .unwrap_or_else(|| vec![ConstantType::Str]),
+        }
+    }
+}
+
+impl From<Settings> for Options {
+    fn from(settings: Settings) -> Self {
+        Self {
+            allow_magic_value_types: Some(settings.allow_magic_value_types),
+        }
+    }
+}

src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR2004_magic_value_comparison.py.snap

@@ -3,52 +3,46 @@ source: src/rules/pylint/mod.rs
 expression: diagnostics
 ---
 - kind:
-    MagicValueComparison: "10"
+    MagicValueComparison:
+      value: "10"
   location:
-    row: 6
+    row: 5
     column: 3
   end_location:
-    row: 6
+    row: 5
     column: 5
   fix: ~
   parent: ~
 - kind:
-    MagicValueComparison: "2"
+    MagicValueComparison:
+      value: "2"
   location:
-    row: 36
+    row: 35
     column: 11
   end_location:
-    row: 36
+    row: 35
     column: 12
   fix: ~
   parent: ~
 - kind:
-    MagicValueComparison: "'Hunter2'"
+    MagicValueComparison:
+      value: "3.141592653589793"
   location:
-    row: 51
+    row: 56
-    column: 21
-  end_location:
-    row: 51
-    column: 30
-  fix: ~
-  parent: ~
-- kind:
-    MagicValueComparison: "3.141592653589793"
-  location:
-    row: 57
     column: 20
   end_location:
-    row: 57
+    row: 56
     column: 40
   fix: ~
   parent: ~
 - kind:
-    MagicValueComparison: "b'something'"
+    MagicValueComparison:
+      value: "b'something'"
   location:
-    row: 66
+    row: 65
     column: 17
   end_location:
-    row: 66
+    row: 65
     column: 29
   fix: ~
   parent: ~

src/rules/pylint/snapshots/ruff__rules__pylint__tests__allow_magic_value_types.snap

@@ -0,0 +1,38 @@
+---
+source: src/rules/pylint/mod.rs
+expression: diagnostics
+---
+- kind:
+    MagicValueComparison:
+      value: "'Hunter2'"
+  location:
+    row: 50
+    column: 21
+  end_location:
+    row: 50
+    column: 30
+  fix: ~
+  parent: ~
+- kind:
+    MagicValueComparison:
+      value: "3.141592653589793"
+  location:
+    row: 56
+    column: 20
+  end_location:
+    row: 56
+    column: 40
+  fix: ~
+  parent: ~
+- kind:
+    MagicValueComparison:
+      value: "b'something'"
+  location:
+    row: 65
+    column: 17
+  end_location:
+    row: 65
+    column: 29
+  fix: ~
+  parent: ~
+

src/settings/configuration.rs

@@ -17,7 +17,7 @@ use crate::registry::RuleCodePrefix;
 use crate::rules::{
     flake8_annotations, flake8_bandit, flake8_bugbear, flake8_errmsg, flake8_import_conventions,
     flake8_pytest_style, flake8_quotes, flake8_tidy_imports, flake8_unused_arguments, isort,
-    mccabe, pep8_naming, pycodestyle, pydocstyle, pyupgrade,
+    mccabe, pep8_naming, pycodestyle, pydocstyle, pylint, pyupgrade,
 };
 use crate::settings::options::Options;
 use crate::settings::pyproject::load_options;
@@ -72,6 +72,7 @@ pub struct Configuration {
     pub pep8_naming: Option<pep8_naming::settings::Options>,
     pub pycodestyle: Option<pycodestyle::settings::Options>,
     pub pydocstyle: Option<pydocstyle::settings::Options>,
+    pub pylint: Option<pylint::settings::Options>,
     pub pyupgrade: Option<pyupgrade::settings::Options>,
 }
 
@@ -178,6 +179,7 @@ impl Configuration {
             pep8_naming: options.pep8_naming,
             pycodestyle: options.pycodestyle,
             pydocstyle: options.pydocstyle,
+            pylint: options.pylint,
             pyupgrade: options.pyupgrade,
         })
     }
@@ -248,6 +250,7 @@ impl Configuration {
             pep8_naming: self.pep8_naming.or(config.pep8_naming),
             pycodestyle: self.pycodestyle.or(config.pycodestyle),
             pydocstyle: self.pydocstyle.or(config.pydocstyle),
+            pylint: self.pylint.or(config.pylint),
             pyupgrade: self.pyupgrade.or(config.pyupgrade),
         }
     }

src/settings/defaults.rs

@@ -10,7 +10,7 @@ use crate::registry::RuleCodePrefix;
 use crate::rules::{
     flake8_annotations, flake8_bandit, flake8_bugbear, flake8_errmsg, flake8_import_conventions,
     flake8_pytest_style, flake8_quotes, flake8_tidy_imports, flake8_unused_arguments, isort,
-    mccabe, pep8_naming, pycodestyle, pydocstyle, pyupgrade,
+    mccabe, pep8_naming, pycodestyle, pydocstyle, pylint, pyupgrade,
 };
 
 pub const PREFIXES: &[RuleCodePrefix] = &[RuleCodePrefix::E, RuleCodePrefix::F];
@@ -84,6 +84,7 @@ impl Default for Settings {
             pep8_naming: pep8_naming::settings::Settings::default(),
             pycodestyle: pycodestyle::settings::Settings::default(),
             pydocstyle: pydocstyle::settings::Settings::default(),
+            pylint: pylint::settings::Settings::default(),
             pyupgrade: pyupgrade::settings::Settings::default(),
         }
     }

src/settings/hashable.rs

@@ -12,7 +12,7 @@ use super::types::FilePattern;
 pub struct HashableRegex(Regex);
 
 impl Hash for HashableRegex {
-    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+    fn hash<H: Hasher>(&self, state: &mut H) {
         self.0.as_str().hash(state);
     }
 }
@@ -49,7 +49,7 @@ impl Deref for HashableGlobMatcher {
 }
 
 impl Hash for HashableGlobMatcher {
-    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+    fn hash<H: Hasher>(&self, state: &mut H) {
         self.0.glob().hash(state);
     }
 }
@@ -87,7 +87,7 @@ impl Deref for HashableGlobSet {
 }
 
 impl Hash for HashableGlobSet {
-    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+    fn hash<H: Hasher>(&self, state: &mut H) {
         for pattern in self.patterns.iter().sorted() {
             pattern.hash(state);
         }

src/settings/mod.rs

@@ -18,7 +18,7 @@ use crate::registry::{RuleCode, RuleCodePrefix, SuffixLength, CATEGORIES, INCOMP
 use crate::rules::{
     flake8_annotations, flake8_bandit, flake8_bugbear, flake8_errmsg, flake8_import_conventions,
     flake8_pytest_style, flake8_quotes, flake8_tidy_imports, flake8_unused_arguments, isort,
-    mccabe, pep8_naming, pycodestyle, pydocstyle, pyupgrade,
+    mccabe, pep8_naming, pycodestyle, pydocstyle, pylint, pyupgrade,
 };
 use crate::settings::configuration::Configuration;
 use crate::settings::types::{PerFileIgnore, PythonVersion, SerializationFormat, Version};
@@ -117,6 +117,7 @@ pub struct Settings {
     pub pep8_naming: pep8_naming::settings::Settings,
     pub pycodestyle: pycodestyle::settings::Settings,
     pub pydocstyle: pydocstyle::settings::Settings,
+    pub pylint: pylint::settings::Settings,
     pub pyupgrade: pyupgrade::settings::Settings,
 }
 
@@ -200,6 +201,7 @@ impl Settings {
             pep8_naming: config.pep8_naming.map(Into::into).unwrap_or_default(),
             pycodestyle: config.pycodestyle.map(Into::into).unwrap_or_default(),
             pydocstyle: config.pydocstyle.map(Into::into).unwrap_or_default(),
+            pylint: config.pylint.map(Into::into).unwrap_or_default(),
             pyupgrade: config.pyupgrade.map(Into::into).unwrap_or_default(),
         })
     }

src/settings/options.rs

@@ -9,7 +9,7 @@ use crate::registry::RuleCodePrefix;
 use crate::rules::{
     flake8_annotations, flake8_bandit, flake8_bugbear, flake8_errmsg, flake8_import_conventions,
     flake8_pytest_style, flake8_quotes, flake8_tidy_imports, flake8_unused_arguments, isort,
-    mccabe, pep8_naming, pycodestyle, pydocstyle, pyupgrade,
+    mccabe, pep8_naming, pycodestyle, pydocstyle, pylint, pyupgrade,
 };
 use crate::settings::types::{PythonVersion, SerializationFormat, Version};
 
@@ -462,6 +462,9 @@ pub struct Options {
     /// Options for the `pydocstyle` plugin.
     pub pydocstyle: Option<pydocstyle::settings::Options>,
     #[option_group]
+    /// Options for the `pylint` plugin.
+    pub pylint: Option<pylint::settings::Options>,
+    #[option_group]
     /// Options for the `pyupgrade` plugin.
     pub pyupgrade: Option<pyupgrade::settings::Options>,
     // Tables are required to go last.

src/settings/pyproject.rs

@@ -208,6 +208,7 @@ mod tests {
                     pep8_naming: None,
                     pycodestyle: None,
                     pydocstyle: None,
+                    pylint: None,
                     pyupgrade: None,
                 })
             })
@@ -268,6 +269,7 @@ line-length = 79
                     pep8_naming: None,
                     pycodestyle: None,
                     pydocstyle: None,
+                    pylint: None,
                     pyupgrade: None,
                 })
             })
@@ -328,6 +330,7 @@ exclude = ["foo.py"]
                     pep8_naming: None,
                     pycodestyle: None,
                     pydocstyle: None,
+                    pylint: None,
                     pyupgrade: None,
                 })
             })
@@ -388,6 +391,7 @@ select = ["E501"]
                     pep8_naming: None,
                     pycodestyle: None,
                     pydocstyle: None,
+                    pylint: None,
                     pyupgrade: None,
                 })
             })
@@ -449,6 +453,7 @@ ignore = ["E501"]
                     pep8_naming: None,
                     pycodestyle: None,
                     pydocstyle: None,
+                    pylint: None,
                     pyupgrade: None,
                 })
             })
@@ -630,6 +635,7 @@ other-attribute = 1
                 }),
                 pycodestyle: None,
                 pydocstyle: None,
+                pylint: None,
                 pyupgrade: None,
             }
         );

src/violations.rs

@@ -1251,18 +1251,22 @@ impl AlwaysAutofixableViolation for UseSysExit {
 }
 
 define_violation!(
-    pub struct MagicValueComparison(pub String);
+    pub struct MagicValueComparison {
+        pub value: String,
+    }
 );
 impl Violation for MagicValueComparison {
     fn message(&self) -> String {
-        let MagicValueComparison(value) = self;
+        let MagicValueComparison { value } = self;
         format!(
             "Magic value used in comparison, consider replacing {value} with a constant variable"
         )
     }
 
     fn placeholder() -> Self {
-        MagicValueComparison("magic".to_string())
+        MagicValueComparison {
+            value: "magic".to_string(),
+        }
     }
 }