Skip to content

Commit

Permalink
Expand DeprecatedLogWarn to check for Expr::Atrribute calls (#7677)
Browse files Browse the repository at this point in the history
## Summary

`PGH002`, which checks for use of deprecated `logging.warn` calls, did
not check for calls made on the attribute `warn` yet. Since
#7521 we check both cases for
similar rules wherever possible. To be consistent this PR expands PGH002
to do the same.

## Test Plan

Expanded existing fixtures with `logger.warn()` calls

## Issue links

Fixes final inconsistency mentioned in
#7502
  • Loading branch information
qdegraaf committed Sep 27, 2023
1 parent 34480c0 commit c8360a1
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,6 @@
warnings.warn("this is ok")
warn("by itself is also ok")
logging.warning("this is fine")

logger = logging.getLogger(__name__)
logger.warning("this is fine")
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,6 @@

logging.warn("this is not ok")
warn("not ok")

logger = logging.getLogger(__name__)
logger.warn("this is not ok")
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -746,7 +746,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
pygrep_hooks::rules::no_eval(checker, func);
}
if checker.enabled(Rule::DeprecatedLogWarn) {
pygrep_hooks::rules::deprecated_log_warn(checker, func);
pygrep_hooks::rules::deprecated_log_warn(checker, call);
}
if checker.enabled(Rule::UnnecessaryDirectLambdaCall) {
pylint::rules::unnecessary_direct_lambda_call(checker, expr, func);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use ruff_python_ast::Expr;
use ruff_python_ast::{self as ast, Expr, ExprCall};
use ruff_python_semantic::analyze::logging;

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_stdlib::logging::LoggingLevel;
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -43,14 +45,36 @@ impl Violation for DeprecatedLogWarn {
}

/// PGH002
pub(crate) fn deprecated_log_warn(checker: &mut Checker, func: &Expr) {
if checker
.semantic()
.resolve_call_path(func)
.is_some_and(|call_path| matches!(call_path.as_slice(), ["logging", "warn"]))
{
checker
.diagnostics
.push(Diagnostic::new(DeprecatedLogWarn, func.range()));
pub(crate) fn deprecated_log_warn(checker: &mut Checker, call: &ExprCall) {
match call.func.as_ref() {
Expr::Attribute(ast::ExprAttribute { attr, .. }) => {
if !logging::is_logger_candidate(
&call.func,
checker.semantic(),
&checker.settings.logger_objects,
) {
return;
}
if !matches!(
LoggingLevel::from_attribute(attr.as_str()),
Some(LoggingLevel::Warn)
) {
return;
}
}
Expr::Name(_) => {
if !checker
.semantic()
.resolve_call_path(call.func.as_ref())
.is_some_and(|call_path| matches!(call_path.as_slice(), ["logging", "warn"]))
{
return;
}
}
_ => return,
}

checker
.diagnostics
.push(Diagnostic::new(DeprecatedLogWarn, call.func.range()));
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,15 @@ PGH002_1.py:5:1: PGH002 `warn` is deprecated in favor of `warning`
4 | logging.warn("this is not ok")
5 | warn("not ok")
| ^^^^ PGH002
6 |
7 | logger = logging.getLogger(__name__)
|

PGH002_1.py:8:1: PGH002 `warn` is deprecated in favor of `warning`
|
7 | logger = logging.getLogger(__name__)
8 | logger.warn("this is not ok")
| ^^^^^^^^^^^ PGH002
|


0 comments on commit c8360a1

Please sign in to comment.