Allow aliased logging module as a logger candidate (#3718)

This commit is contained in:
Dhruv Manilawala 2023-03-25 02:49:09 +05:30 committed by GitHub
parent 7af83460ce
commit 63adf9f5e8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 47 additions and 11 deletions

View file

@ -1,3 +1,5 @@
import logging
import logging as foo
logging.info("Hello {}".format("World!"))
foo.info("Hello {}".format("World!"))

View file

@ -59,7 +59,7 @@ pub fn blind_except(
if body.iter().any(|stmt| {
if let StmtKind::Expr { value } = &stmt.node {
if let ExprKind::Call { func, keywords, .. } = &value.node {
if helpers::is_logger_candidate(func) {
if helpers::is_logger_candidate(&checker.ctx, func) {
if let ExprKind::Attribute { attr, .. } = &func.node {
if attr == "exception" {
return true;

View file

@ -127,7 +127,7 @@ fn check_log_record_attr_clash(checker: &mut Checker, extra: &Keyword) {
/// Check logging calls for violations.
pub fn logging_call(checker: &mut Checker, func: &Expr, args: &[Expr], keywords: &[Keyword]) {
if !is_logger_candidate(func) {
if !is_logger_candidate(&checker.ctx, func) {
return;
}

View file

@ -8,11 +8,24 @@ expression: diagnostics
suggestion: ~
fixable: false
location:
row: 3
row: 4
column: 13
end_location:
row: 3
row: 4
column: 40
fix: ~
parent: ~
- kind:
name: LoggingStringFormat
body: "Logging statement uses `string.format()`"
suggestion: ~
fixable: false
location:
row: 5
column: 9
end_location:
row: 5
column: 36
fix: ~
parent: ~

View file

@ -100,7 +100,7 @@ pub fn logging_call(checker: &mut Checker, func: &Expr, args: &[Expr], keywords:
return;
}
if !is_logger_candidate(func) {
if !is_logger_candidate(&checker.ctx, func) {
return;
}

View file

@ -1,22 +1,32 @@
use rustpython_parser::ast::{Expr, ExprKind};
use ruff_python_ast::context::Context;
use ruff_python_ast::helpers::is_logger_candidate;
use ruff_python_ast::visitor;
use ruff_python_ast::visitor::Visitor;
#[derive(Default)]
/// Collect `logging`-like calls from an AST.
pub struct LoggerCandidateVisitor<'a> {
context: &'a Context<'a>,
pub calls: Vec<(&'a Expr, &'a Expr)>,
}
impl<'a> LoggerCandidateVisitor<'a> {
pub fn new(context: &'a Context<'a>) -> Self {
LoggerCandidateVisitor {
context,
calls: Vec::new(),
}
}
}
impl<'a, 'b> Visitor<'b> for LoggerCandidateVisitor<'a>
where
'b: 'a,
{
fn visit_expr(&mut self, expr: &'b Expr) {
if let ExprKind::Call { func, .. } = &expr.node {
if is_logger_candidate(func) {
if is_logger_candidate(self.context, func) {
self.calls.push((expr, func));
}
}

View file

@ -23,7 +23,7 @@ pub fn error_instead_of_exception(checker: &mut Checker, handlers: &[Excepthandl
for handler in handlers {
let ExcepthandlerKind::ExceptHandler { body, .. } = &handler.node;
let calls = {
let mut visitor = LoggerCandidateVisitor::default();
let mut visitor = LoggerCandidateVisitor::new(&checker.ctx);
visitor.visit_body(body);
visitor.calls
};

View file

@ -73,7 +73,7 @@ pub fn verbose_log_message(checker: &mut Checker, handlers: &[Excepthandler]) {
// Find all calls to `logging.exception`.
let calls = {
let mut visitor = LoggerCandidateVisitor::default();
let mut visitor = LoggerCandidateVisitor::new(&checker.ctx);
visitor.visit_body(body);
visitor.calls
};

View file

@ -1288,9 +1288,20 @@ impl<'a> SimpleCallArgs<'a> {
/// Return `true` if the given `Expr` is a potential logging call. Matches
/// `logging.error`, `logger.error`, `self.logger.error`, etc., but not
/// arbitrary `foo.error` calls.
pub fn is_logger_candidate(func: &Expr) -> bool {
///
/// It even matches direct `logging.error` calls even if the `logging` module
/// is aliased. Example:
/// ```python
/// import logging as bar
///
/// # This is detected to be a logger candidate
/// bar.error()
/// ```
pub fn is_logger_candidate(context: &Context, func: &Expr) -> bool {
if let ExprKind::Attribute { value, .. } = &func.node {
let call_path = collect_call_path(value);
let call_path = context
.resolve_call_path(value)
.unwrap_or_else(|| collect_call_path(value));
if let Some(tail) = call_path.last() {
if tail.starts_with("log") || tail.ends_with("logger") || tail.ends_with("logging") {
return true;