Skip to content

Commit

Permalink
Add support for .log(level, msg) calls in flake8-logging-format (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
dhruvmanila authored Mar 25, 2023
1 parent 8ac7584 commit 2659336
Show file tree
Hide file tree
Showing 9 changed files with 143 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import logging

logging.info("Hello %s" % "World!")
logging.log(logging.INFO, "Hello %s" % "World!")
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import logging

logging.info("Hello" + " " + "World!")
logging.log(logging.INFO, "Hello" + " " + "World!")
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@

name = "world"
logging.info(f"Hello {name}")
logging.log(logging.INFO, f"Hello {name}")
65 changes: 44 additions & 21 deletions crates/ruff/src/rules/flake8_logging_format/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,14 +125,31 @@ 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<Self> {
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) {
return;
}

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(
Expand All @@ -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()) {
Expand Down Expand Up @@ -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),
));
}
}
_ => {}
}
_ => {}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: ~

Original file line number Diff line number Diff line change
Expand Up @@ -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: ~

Original file line number Diff line number Diff line change
Expand Up @@ -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: ~

Original file line number Diff line number Diff line change
Expand Up @@ -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: ~

0 comments on commit 2659336

Please sign in to comment.