From 2659336ed1fd678745bb790b27f1a2862bfe0c64 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Sat, 25 Mar 2023 21:25:53 +0530 Subject: [PATCH] Add support for `.log(level, msg)` calls in `flake8-logging-format` (#3726) --- .../fixtures/flake8_logging_format/G001.py | 4 ++ .../fixtures/flake8_logging_format/G002.py | 1 + .../fixtures/flake8_logging_format/G003.py | 1 + .../fixtures/flake8_logging_format/G004.py | 1 + .../src/rules/flake8_logging_format/rules.rs | 65 +++++++++++++------ ...flake8_logging_format__tests__G001.py.snap | 54 ++++++++++++++- ...flake8_logging_format__tests__G002.py.snap | 13 ++++ ...flake8_logging_format__tests__G003.py.snap | 13 ++++ ...flake8_logging_format__tests__G004.py.snap | 13 ++++ 9 files changed, 143 insertions(+), 22 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/flake8_logging_format/G001.py b/crates/ruff/resources/test/fixtures/flake8_logging_format/G001.py index 016afa5fcdfe2..de1424f3a254a 100644 --- a/crates/ruff/resources/test/fixtures/flake8_logging_format/G001.py +++ b/crates/ruff/resources/test/fixtures/flake8_logging_format/G001.py @@ -2,4 +2,8 @@ import logging as foo logging.info("Hello {}".format("World!")) +logging.log(logging.INFO, "Hello {}".format("World!")) foo.info("Hello {}".format("World!")) +logging.log(logging.INFO, msg="Hello {}".format("World!")) +logging.log(level=logging.INFO, msg="Hello {}".format("World!")) +logging.log(msg="Hello {}".format("World!"), level=logging.INFO) diff --git a/crates/ruff/resources/test/fixtures/flake8_logging_format/G002.py b/crates/ruff/resources/test/fixtures/flake8_logging_format/G002.py index 0d2cbb7c61c0c..11b61a1cb8e7a 100644 --- a/crates/ruff/resources/test/fixtures/flake8_logging_format/G002.py +++ b/crates/ruff/resources/test/fixtures/flake8_logging_format/G002.py @@ -1,3 +1,4 @@ import logging logging.info("Hello %s" % "World!") +logging.log(logging.INFO, "Hello %s" % "World!") diff --git a/crates/ruff/resources/test/fixtures/flake8_logging_format/G003.py b/crates/ruff/resources/test/fixtures/flake8_logging_format/G003.py index 7a1c6ae82122a..4e20dc6d9b285 100644 --- a/crates/ruff/resources/test/fixtures/flake8_logging_format/G003.py +++ b/crates/ruff/resources/test/fixtures/flake8_logging_format/G003.py @@ -1,3 +1,4 @@ import logging logging.info("Hello" + " " + "World!") +logging.log(logging.INFO, "Hello" + " " + "World!") diff --git a/crates/ruff/resources/test/fixtures/flake8_logging_format/G004.py b/crates/ruff/resources/test/fixtures/flake8_logging_format/G004.py index e8f45eb72d11e..abbbe8b7d04ca 100644 --- a/crates/ruff/resources/test/fixtures/flake8_logging_format/G004.py +++ b/crates/ruff/resources/test/fixtures/flake8_logging_format/G004.py @@ -2,3 +2,4 @@ name = "world" logging.info(f"Hello {name}") +logging.log(logging.INFO, f"Hello {name}") diff --git a/crates/ruff/src/rules/flake8_logging_format/rules.rs b/crates/ruff/src/rules/flake8_logging_format/rules.rs index c8e99afaf8e45..b4dedf580f5af 100644 --- a/crates/ruff/src/rules/flake8_logging_format/rules.rs +++ b/crates/ruff/src/rules/flake8_logging_format/rules.rs @@ -125,6 +125,23 @@ fn check_log_record_attr_clash(checker: &mut Checker, extra: &Keyword) { } } +enum LoggingCallType { + /// Logging call with a level method, e.g., `logging.info`. + LevelCall(LoggingLevel), + /// Logging call with an integer level as an argument, e.g., `logger.log(level, ...)`. + LogCall, +} + +impl LoggingCallType { + fn from_attribute(attr: &str) -> Option { + if attr == "log" { + Some(LoggingCallType::LogCall) + } else { + LoggingLevel::from_attribute(attr).map(LoggingCallType::LevelCall) + } + } +} + /// Check logging calls for violations. pub fn logging_call(checker: &mut Checker, func: &Expr, args: &[Expr], keywords: &[Keyword]) { if !is_logger_candidate(&checker.ctx, func) { @@ -132,7 +149,7 @@ pub fn logging_call(checker: &mut Checker, func: &Expr, args: &[Expr], keywords: } if let ExprKind::Attribute { value, attr, .. } = &func.node { - if let Some(logging_level) = LoggingLevel::from_attribute(attr.as_str()) { + if let Some(logging_call_type) = LoggingCallType::from_attribute(attr.as_str()) { let call_args = SimpleCallArgs::new(args, keywords); let level_call_range = Range::new( Location::new( @@ -146,13 +163,17 @@ pub fn logging_call(checker: &mut Checker, func: &Expr, args: &[Expr], keywords: ); // G001 - G004 - if let Some(format_arg) = call_args.argument("msg", 0) { + let msg_pos = usize::from(matches!(logging_call_type, LoggingCallType::LogCall)); + if let Some(format_arg) = call_args.argument("msg", msg_pos) { check_msg(checker, format_arg); } // G010 if checker.settings.rules.enabled(Rule::LoggingWarn) - && matches!(logging_level, LoggingLevel::Warn) + && matches!( + logging_call_type, + LoggingCallType::LevelCall(LoggingLevel::Warn) + ) { let mut diagnostic = Diagnostic::new(LoggingWarn, level_call_range); if checker.patch(diagnostic.kind.rule()) { @@ -204,27 +225,29 @@ pub fn logging_call(checker: &mut Checker, func: &Expr, args: &[Expr], keywords: return; } - match logging_level { - LoggingLevel::Error => { - if checker.settings.rules.enabled(Rule::LoggingExcInfo) { - checker - .diagnostics - .push(Diagnostic::new(LoggingExcInfo, level_call_range)); + if let LoggingCallType::LevelCall(logging_level) = logging_call_type { + match logging_level { + LoggingLevel::Error => { + if checker.settings.rules.enabled(Rule::LoggingExcInfo) { + checker + .diagnostics + .push(Diagnostic::new(LoggingExcInfo, level_call_range)); + } } - } - LoggingLevel::Exception => { - if checker - .settings - .rules - .enabled(Rule::LoggingRedundantExcInfo) - { - checker.diagnostics.push(Diagnostic::new( - LoggingRedundantExcInfo, - Range::from(exc_info), - )); + LoggingLevel::Exception => { + if checker + .settings + .rules + .enabled(Rule::LoggingRedundantExcInfo) + { + checker.diagnostics.push(Diagnostic::new( + LoggingRedundantExcInfo, + Range::from(exc_info), + )); + } } + _ => {} } - _ => {} } } } diff --git a/crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G001.py.snap b/crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G001.py.snap index 6661949b98c71..39fe94410443c 100644 --- a/crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G001.py.snap +++ b/crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G001.py.snap @@ -22,10 +22,62 @@ expression: diagnostics fixable: false location: row: 5 - column: 9 + column: 26 end_location: row: 5 + column: 53 + fix: ~ + parent: ~ +- kind: + name: LoggingStringFormat + body: "Logging statement uses `string.format()`" + suggestion: ~ + fixable: false + location: + row: 6 + column: 9 + end_location: + row: 6 column: 36 fix: ~ parent: ~ +- kind: + name: LoggingStringFormat + body: "Logging statement uses `string.format()`" + suggestion: ~ + fixable: false + location: + row: 7 + column: 30 + end_location: + row: 7 + column: 57 + fix: ~ + parent: ~ +- kind: + name: LoggingStringFormat + body: "Logging statement uses `string.format()`" + suggestion: ~ + fixable: false + location: + row: 8 + column: 36 + end_location: + row: 8 + column: 63 + fix: ~ + parent: ~ +- kind: + name: LoggingStringFormat + body: "Logging statement uses `string.format()`" + suggestion: ~ + fixable: false + location: + row: 9 + column: 16 + end_location: + row: 9 + column: 43 + fix: ~ + parent: ~ diff --git a/crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G002.py.snap b/crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G002.py.snap index 6f0fea8958a7e..ea4bc943842f9 100644 --- a/crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G002.py.snap +++ b/crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G002.py.snap @@ -15,4 +15,17 @@ expression: diagnostics column: 34 fix: ~ parent: ~ +- kind: + name: LoggingPercentFormat + body: "Logging statement uses `%`" + suggestion: ~ + fixable: false + location: + row: 4 + column: 26 + end_location: + row: 4 + column: 47 + fix: ~ + parent: ~ diff --git a/crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G003.py.snap b/crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G003.py.snap index 6f54ca0d5a4b6..4ce0e9cf6f64a 100644 --- a/crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G003.py.snap +++ b/crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G003.py.snap @@ -15,4 +15,17 @@ expression: diagnostics column: 37 fix: ~ parent: ~ +- kind: + name: LoggingStringConcat + body: "Logging statement uses `+`" + suggestion: ~ + fixable: false + location: + row: 4 + column: 26 + end_location: + row: 4 + column: 50 + fix: ~ + parent: ~ diff --git a/crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G004.py.snap b/crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G004.py.snap index 3ee12ae84e8fa..6d3a7ff1d6304 100644 --- a/crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G004.py.snap +++ b/crates/ruff/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G004.py.snap @@ -15,4 +15,17 @@ expression: diagnostics column: 28 fix: ~ parent: ~ +- kind: + name: LoggingFString + body: Logging statement uses f-string + suggestion: ~ + fixable: false + location: + row: 5 + column: 26 + end_location: + row: 5 + column: 41 + fix: ~ + parent: ~