[pylint] Implement E1205 and E106 (#3084)

crates/ruff/resources/test/fixtures/pylint/logging_too_few_args.py

@@ -0,0 +1,12 @@
+import logging
+
+logging.warning("Hello %s %s", "World!")  # [logging-too-few-args]
+
+# do not handle calls with kwargs (like pylint)
+logging.warning("Hello %s", "World!", "again", something="else")
+
+logging.warning("Hello %s", "World!")
+
+import warning
+
+warning.warning("Hello %s %s", "World!")

crates/ruff/resources/test/fixtures/pylint/logging_too_many_args.py

@@ -0,0 +1,12 @@
+import logging
+
+logging.warning("Hello %s", "World!", "again")  # [logging-too-many-args]
+
+# do not handle calls with kwargs (like pylint)
+logging.warning("Hello %s", "World!", "again", something="else")
+
+logging.warning("Hello %s", "World!")
+
+import warning
+
+warning.warning("Hello %s", "World!", "again")

crates/ruff/src/ast/logging.rs

@@ -0,0 +1,24 @@
+pub enum LoggingLevel {
+    Debug,
+    Critical,
+    Error,
+    Exception,
+    Info,
+    Warn,
+    Warning,
+}
+
+impl LoggingLevel {
+    pub fn from_str(level: &str) -> Option<Self> {
+        match level {
+            "debug" => Some(LoggingLevel::Debug),
+            "critical" => Some(LoggingLevel::Critical),
+            "error" => Some(LoggingLevel::Error),
+            "exception" => Some(LoggingLevel::Exception),
+            "info" => Some(LoggingLevel::Info),
+            "warn" => Some(LoggingLevel::Warn),
+            "warning" => Some(LoggingLevel::Warning),
+            _ => None,
+        }
+    }
+}

crates/ruff/src/ast/mod.rs

@@ -4,6 +4,7 @@ pub mod comparable;
 pub mod function_type;
 pub mod hashable;
 pub mod helpers;
+pub mod logging;
 pub mod operations;
 pub mod relocate;
 pub mod types;

crates/ruff/src/checkers/ast.rs

@@ -2918,6 +2918,13 @@ where
                 {
                     flake8_logging_format::rules::logging_call(self, func, args, keywords);
                 }
+
+                // pylint logging checker
+                if self.settings.rules.enabled(&Rule::LoggingTooFewArgs)
+                    || self.settings.rules.enabled(&Rule::LoggingTooManyArgs)
+                {
+                    pylint::rules::logging_call(self, func, args, keywords);
+                }
             }
             ExprKind::Dict { keys, values } => {
                 if self

crates/ruff/src/codes.rs

@@ -126,6 +126,8 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
         (Pylint, "E0101") => Rule::ReturnInInit,
         (Pylint, "E0604") => Rule::InvalidAllObject,
         (Pylint, "E0605") => Rule::InvalidAllFormat,
+        (Pylint, "E1205") => Rule::LoggingTooManyArgs,
+        (Pylint, "E1206") => Rule::LoggingTooFewArgs,
         (Pylint, "E1307") => Rule::BadStringFormatType,
         (Pylint, "E2502") => Rule::BidirectionalUnicode,
         (Pylint, "E1310") => Rule::BadStrStripCall,

crates/ruff/src/registry.rs

@@ -150,6 +150,8 @@ ruff_macros::register_rules!(
     rules::pylint::rules::TooManyBranches,
     rules::pylint::rules::TooManyStatements,
     rules::pylint::rules::RedefinedLoopName,
+    rules::pylint::rules::LoggingTooFewArgs,
+    rules::pylint::rules::LoggingTooManyArgs,
     // flake8-builtins
     rules::flake8_builtins::rules::BuiltinVariableShadowing,
     rules::flake8_builtins::rules::BuiltinArgumentShadowing,

crates/ruff/src/rules/flake8_logging_format/rules.rs

@@ -1,6 +1,7 @@
 use rustpython_parser::ast::{Constant, Expr, ExprKind, Keyword, Location, Operator};
 
 use crate::ast::helpers::{find_keyword, is_logger_candidate, SimpleCallArgs};
+use crate::ast::logging::LoggingLevel;
 use crate::ast::types::Range;
 use crate::checkers::ast::Checker;
 use crate::fix::Fix;
@@ -10,31 +11,6 @@ use crate::rules::flake8_logging_format::violations::{
     LoggingRedundantExcInfo, LoggingStringConcat, LoggingStringFormat, LoggingWarn,
 };
 
-enum LoggingLevel {
-    Debug,
-    Critical,
-    Error,
-    Exception,
-    Info,
-    Warn,
-    Warning,
-}
-
-impl LoggingLevel {
-    fn from_str(level: &str) -> Option<Self> {
-        match level {
-            "debug" => Some(LoggingLevel::Debug),
-            "critical" => Some(LoggingLevel::Critical),
-            "error" => Some(LoggingLevel::Error),
-            "exception" => Some(LoggingLevel::Exception),
-            "info" => Some(LoggingLevel::Info),
-            "warn" => Some(LoggingLevel::Warn),
-            "warning" => Some(LoggingLevel::Warning),
-            _ => None,
-        }
-    }
-}
-
 const RESERVED_ATTRS: &[&str; 22] = &[
     "args",
     "asctime",

crates/ruff/src/rules/pylint/mod.rs

@@ -17,6 +17,8 @@ mod tests {
     use crate::settings::Settings;
     use crate::test::test_path;
 
+    #[test_case(Rule::LoggingTooManyArgs, Path::new("logging_too_many_args.py"); "PLE1205")]
+    #[test_case(Rule::LoggingTooFewArgs, Path::new("logging_too_few_args.py"); "PLE1206")]
     #[test_case(Rule::ReturnInInit, Path::new("return_in_init.py"); "PLE0101")]
     #[test_case(Rule::UselessImportAlias, Path::new("import_aliasing.py"); "PLC0414")]
     #[test_case(Rule::UnnecessaryDirectLambdaCall, Path::new("unnecessary_direct_lambda_call.py"); "PLC3002")]

crates/ruff/src/rules/pylint/rules/logging.rs

@@ -0,0 +1,139 @@
+use rustpython_parser::ast::{Constant, Expr, ExprKind, Keyword};
+
+use ruff_macros::{define_violation, derive_message_formats};
+
+use crate::ast::helpers::{is_logger_candidate, SimpleCallArgs};
+use crate::ast::logging::LoggingLevel;
+use crate::ast::types::Range;
+use crate::checkers::ast::Checker;
+use crate::registry::{Diagnostic, Rule};
+use crate::rules::pyflakes::cformat::CFormatSummary;
+use crate::violation::Violation;
+
+define_violation!(
+    /// ## What it does
+    /// Checks for too few positional arguments for a `logging` format string.
+    ///
+    /// ## Why is this bad?
+    /// A `TypeError` will be raised if the statement is run.
+    ///
+    /// ## Example
+    /// ```python
+    /// import logging
+    ///
+    /// try:
+    ///     function()
+    /// except Exception as e:
+    ///     logging.error('%s error occurred: %s', e)  # [logging-too-few-args]
+    ///     raise
+    /// ```
+    ///
+    /// Use instead:
+    /// ```python
+    /// import logging
+    ///
+    /// try:
+    ///     function()
+    /// except Exception as e:
+    ///     logging.error('%s error occurred: %s', type(e), e)
+    ///     raise
+    /// ```
+    pub struct LoggingTooFewArgs;
+);
+impl Violation for LoggingTooFewArgs {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("Not enough arguments for `logging` format string")
+    }
+}
+
+define_violation!(
+    /// ## What it does
+    /// Checks for too many positional arguments for a `logging` format string.
+    ///
+    /// ## Why is this bad?
+    /// A `TypeError` will be raised if the statement is run.
+    ///
+    /// ## Example
+    /// ```python
+    /// import logging
+    ///
+    /// try:
+    ///     function()
+    /// except Exception as e:
+    ///     logging.error('Error occurred: %s', type(e), e)  # [logging-too-many-args]
+    ///     raise
+    /// ```
+    ///
+    /// Use instead:
+    /// ```python
+    /// import logging
+    ///
+    /// try:
+    ///     function()
+    /// except Exception as e:
+    ///     logging.error('%s error occurred: %s', type(e), e)
+    ///     raise
+    /// ```
+    pub struct LoggingTooManyArgs;
+);
+impl Violation for LoggingTooManyArgs {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        format!("Too many arguments for `logging` format string")
+    }
+}
+
+/// Check logging calls for violations.
+pub fn logging_call(checker: &mut Checker, func: &Expr, args: &[Expr], keywords: &[Keyword]) {
+    if !is_logger_candidate(func) {
+        return;
+    }
+
+    if let ExprKind::Attribute { attr, .. } = &func.node {
+        if LoggingLevel::from_str(attr.as_str()).is_some() {
+            let call_args = SimpleCallArgs::new(args, keywords);
+
+            // E1205 - E1206
+            if let Some(msg) = call_args.get_argument("msg", Some(0)) {
+                if let ExprKind::Constant {
+                    value: Constant::Str(value),
+                    ..
+                } = &msg.node
+                {
+                    if let Ok(summary) = CFormatSummary::try_from(value.as_str()) {
+                        if summary.starred {
+                            return;
+                        }
+
+                        if !call_args.kwargs.is_empty() {
+                            // Keyword checking on logging strings is complicated by
+                            // special keywords - out of scope.
+                            return;
+                        }
+
+                        let message_args = call_args.args.len() - 1;
+
+                        if checker.settings.rules.enabled(&Rule::LoggingTooManyArgs)
+                            && summary.num_positional < message_args
+                        {
+                            checker.diagnostics.push(Diagnostic::new(
+                                LoggingTooManyArgs,
+                                Range::from_located(func),
+                            ));
+                        }
+
+                        if checker.settings.rules.enabled(&Rule::LoggingTooFewArgs)
+                            && summary.num_positional > message_args
+                        {
+                            checker.diagnostics.push(Diagnostic::new(
+                                LoggingTooFewArgs,
+                                Range::from_located(func),
+                            ));
+                        }
+                    }
+                }
+            }
+        }
+    }
+}

crates/ruff/src/rules/pylint/rules/mod.rs

@@ -7,6 +7,7 @@ pub use consider_using_sys_exit::{consider_using_sys_exit, ConsiderUsingSysExit}
 pub use global_variable_not_assigned::GlobalVariableNotAssigned;
 pub use invalid_all_format::{invalid_all_format, InvalidAllFormat};
 pub use invalid_all_object::{invalid_all_object, InvalidAllObject};
+pub use logging::{logging_call, LoggingTooFewArgs, LoggingTooManyArgs};
 pub use magic_value_comparison::{magic_value_comparison, MagicValueComparison};
 pub use merge_isinstance::{merge_isinstance, ConsiderMergingIsinstance};
 pub use nonlocal_without_binding::NonlocalWithoutBinding;
@@ -37,6 +38,7 @@ mod consider_using_sys_exit;
 mod global_variable_not_assigned;
 mod invalid_all_format;
 mod invalid_all_object;
+mod logging;
 mod magic_value_comparison;
 mod merge_isinstance;
 mod nonlocal_without_binding;

crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLE1205_logging_too_many_args.py.snap

@@ -0,0 +1,15 @@
+---
+source: crates/ruff/src/rules/pylint/mod.rs
+expression: diagnostics
+---
+- kind:
+    LoggingTooManyArgs: ~
+  location:
+    row: 3
+    column: 0
+  end_location:
+    row: 3
+    column: 15
+  fix: ~
+  parent: ~
+

crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLE1206_logging_too_few_args.py.snap

@@ -0,0 +1,15 @@
+---
+source: crates/ruff/src/rules/pylint/mod.rs
+expression: diagnostics
+---
+- kind:
+    LoggingTooFewArgs: ~
+  location:
+    row: 3
+    column: 0
+  end_location:
+    row: 3
+    column: 15
+  fix: ~
+  parent: ~
+

ruff.schema.json

@@ -1793,6 +1793,10 @@
         "PLE11",
         "PLE114",
         "PLE1142",
+        "PLE12",
+        "PLE120",
+        "PLE1205",
+        "PLE1206",
         "PLE13",
         "PLE130",
         "PLE1307",