Add Expr::Name checks to rules which use is_logger_candidate (#7521)

## Summary

Expands several rules to also check for `Expr::Name` values. As they
would previously not consider:
```python
from logging import error

error("foo")
```
as potential violations
```python
import logging

logging.error("foo")
```
as potential violations leading to inconsistent behaviour. 

The rules impacted are:

- `BLE001`
- `TRY400`
- `TRY401`
- `PLE1205`
- `PLE1206`
- `LOG007`
- `G001`-`G004`
- `G101`
- `G201`
- `G202`

## Test Plan

Fixtures for all impacted rules expanded. 

## Issue Link

Refers: https://github.com/astral-sh/ruff/issues/7502
This commit is contained in:
qdegraaf 2023-09-27 02:21:22 +02:00 committed by GitHub
parent 528f386131
commit 2aef46cb6f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
37 changed files with 826 additions and 110 deletions

View file

@ -92,3 +92,35 @@ try:
pass
except Exception:
logging.error("...", exc_info=True)
from logging import error, exception
try:
pass
except Exception:
error("...")
try:
pass
except Exception:
error("...", exc_info=False)
try:
pass
except Exception:
error("...", exc_info=None)
try:
pass
except Exception:
exception("...")
try:
pass
except Exception:
error("...", exc_info=True)

View file

@ -16,3 +16,9 @@ from logging import exception
exception("foo", exc_info=False) # LOG007
exception("foo", exc_info=True) # OK
exception = lambda *args, **kwargs: None
exception("foo", exc_info=False) # OK
exception("foo", exc_info=True) # OK

View file

@ -16,3 +16,12 @@ from flask import current_app as app
flask.current_app.logger.info("Hello {}".format("World!"))
current_app.logger.info("Hello {}".format("World!"))
app.logger.log(logging.INFO, "Hello {}".format("World!"))
from logging import info, log
info("Hello {}".format("World!"))
log(logging.INFO, "Hello {}".format("World!"))
info("Hello {}".format("World!"))
log(logging.INFO, msg="Hello {}".format("World!"))
log(level=logging.INFO, msg="Hello {}".format("World!"))
log(msg="Hello {}".format("World!"), level=logging.INFO)

View file

@ -2,3 +2,8 @@ import logging
logging.info("Hello %s" % "World!")
logging.log(logging.INFO, "Hello %s" % "World!")
from logging import info, log
info("Hello %s" % "World!")
log(logging.INFO, "Hello %s" % "World!")

View file

@ -2,3 +2,8 @@ import logging
logging.info("Hello" + " " + "World!")
logging.log(logging.INFO, "Hello" + " " + "World!")
from logging import info, log
info("Hello" + " " + "World!")
log(logging.INFO, "Hello" + " " + "World!")

View file

@ -6,3 +6,7 @@ logging.log(logging.INFO, f"Hello {name}")
_LOGGER = logging.getLogger()
_LOGGER.info(f"{__name__}")
from logging import info
info(f"{name}")
info(f"{__name__}")

View file

@ -8,3 +8,8 @@ log.warn("Hello world!") # This shouldn't be considered as a logger candidate
logger.warn("Hello world!")
logging . warn("Hello World!")
from logging import warn, warning, exception
warn("foo")
warning("foo")
exception("foo")

View file

@ -6,3 +6,12 @@ logging.info(
"name": "foobar",
},
)
from logging import info
info(
"Hello world!",
extra={
"name": "foobar",
},
)

View file

@ -6,3 +6,12 @@ logging.info(
name="foobar",
),
)
from logging import info
info(
"Hello world!",
extra=dict(
name="foobar",
),
)

View file

@ -19,3 +19,24 @@ except:
logging.error("Hello World", exc_info=False)
logging.error("Hello World", exc_info=sys.exc_info())
# G201
from logging import error
try:
pass
except:
error("Hello World", exc_info=True)
try:
pass
except:
error("Hello World", exc_info=sys.exc_info())
# OK
try:
pass
except:
error("Hello World", exc_info=False)
error("Hello World", exc_info=sys.exc_info())

View file

@ -19,3 +19,23 @@ except:
logging.exception("Hello World", exc_info=False)
logging.exception("Hello World", exc_info=True)
# G202
from logging import exception
try:
pass
except:
exception("Hello World", exc_info=True)
try:
pass
except:
exception("Hello World", exc_info=sys.exc_info())
# OK
try:
pass
except:
exception("Hello World", exc_info=False)
exception("Hello World", exc_info=True)

View file

@ -26,3 +26,29 @@ logging.info(msg="Hello %s %s")
import warning
warning.warning("Hello %s %s", "World!")
from logging import error, info, warning
warning("Hello %s %s", "World!") # [logging-too-few-args]
# do not handle calls with kwargs (like pylint)
warning("Hello %s", "World!", "again", something="else")
warning("Hello %s", "World!")
# do not handle calls without any args
info("100% dynamic")
# do not handle calls with *args
error("Example log %s, %s", "foo", "bar", "baz", *args)
# do not handle calls with **kwargs
error("Example log %s, %s", "foo", "bar", "baz", **kwargs)
# do not handle keyword arguments
error("%(objects)d modifications: %(modifications)d errors: %(errors)d")
info(msg="Hello %s")
info(msg="Hello %s %s")

View file

@ -22,3 +22,25 @@ logging.info(msg="Hello", something="else")
import warning
warning.warning("Hello %s", "World!", "again")
from logging import info, error, warning
warning("Hello %s", "World!", "again") # [logging-too-many-args]
warning("Hello %s", "World!", "again", something="else")
warning("Hello %s", "World!")
# do not handle calls with *args
error("Example log %s, %s", "foo", "bar", "baz", *args)
# do not handle calls with **kwargs
error("Example log %s, %s", "foo", "bar", "baz", **kwargs)
# do not handle keyword arguments
error("%(objects)d modifications: %(modifications)d errors: %(errors)d", {"objects": 1, "modifications": 1, "errors": 1})
info(msg="Hello")
info(msg="Hello", something="else")

View file

@ -75,3 +75,37 @@ def fine():
a = 1
except Exception:
logger.error("Context message here", exc_info=sys.exc_info())
from logging import error, exception
def bad():
try:
a = 1
except Exception:
error("Context message here")
if True:
error("Context message here")
def good():
try:
a = 1
except Exception:
exception("Context message here")
def fine():
try:
a = 1
except Exception:
error("Context message here", exc_info=True)
def fine():
try:
a = 1
except Exception:
error("Context message here", exc_info=sys.exc_info())

View file

@ -62,3 +62,67 @@ def main_function():
except Exception as ex:
logger.exception(f"Found an error: {er}")
logger.exception(f"Found an error: {ex.field}")
from logging import error, exception
def main_function():
try:
process()
handle()
finish()
except Exception as ex:
exception(f"Found an error: {ex}") # TRY401
def main_function():
try:
process()
handle()
finish()
except ValueError as bad:
if True is False:
for i in range(10):
exception(f"Found an error: {bad} {good}") # TRY401
except IndexError as bad:
exception(f"Found an error: {bad} {bad}") # TRY401
except Exception as bad:
exception(f"Found an error: {bad}") # TRY401
exception(f"Found an error: {bad}") # TRY401
if True:
exception(f"Found an error: {bad}") # TRY401
def func_fstr():
try:
...
except Exception as ex:
exception(f"Logging an error: {ex}") # TRY401
def func_concat():
try:
...
except Exception as ex:
exception("Logging an error: " + str(ex)) # TRY401
def func_comma():
try:
...
except Exception as ex:
exception("Logging an error:", ex) # TRY401
# OK
def main_function():
try:
process()
handle()
finish()
except Exception as ex:
exception(f"Found an error: {er}")
exception(f"Found an error: {ex.field}")

View file

@ -98,24 +98,49 @@ pub(crate) fn blind_except(
func, arguments, ..
}) = value.as_ref()
{
if logging::is_logger_candidate(
func,
checker.semantic(),
&checker.settings.logger_objects,
) {
if let Some(attribute) = func.as_attribute_expr() {
let attr = attribute.attr.as_str();
if attr == "exception" {
return true;
}
if attr == "error" {
if let Some(keyword) = arguments.find_keyword("exc_info") {
if is_const_true(&keyword.value) {
return true;
match func.as_ref() {
Expr::Attribute(ast::ExprAttribute { attr, .. }) => {
if logging::is_logger_candidate(
func,
checker.semantic(),
&checker.settings.logger_objects,
) {
match attr.as_str() {
"exception" => return true,
"error" => {
if let Some(keyword) = arguments.find_keyword("exc_info") {
if is_const_true(&keyword.value) {
return true;
}
}
}
_ => {}
}
}
}
Expr::Name(ast::ExprName { .. }) => {
if checker
.semantic()
.resolve_call_path(func.as_ref())
.is_some_and(|call_path| match call_path.as_slice() {
["logging", "exception"] => true,
["logging", "error"] => {
if let Some(keyword) = arguments.find_keyword("exc_info") {
if is_const_true(&keyword.value) {
return true;
}
}
false
}
_ => false,
})
{
return true;
}
}
_ => {
return false;
}
}
}
}

View file

@ -94,4 +94,31 @@ BLE.py:81:8: BLE001 Do not catch blind exception: `Exception`
82 | logging.error("...", exc_info=None)
|
BLE.py:101:8: BLE001 Do not catch blind exception: `Exception`
|
99 | try:
100 | pass
101 | except Exception:
| ^^^^^^^^^ BLE001
102 | error("...")
|
BLE.py:107:8: BLE001 Do not catch blind exception: `Exception`
|
105 | try:
106 | pass
107 | except Exception:
| ^^^^^^^^^ BLE001
108 | error("...", exc_info=False)
|
BLE.py:113:8: BLE001 Do not catch blind exception: `Exception`
|
111 | try:
112 | pass
113 | except Exception:
| ^^^^^^^^^ BLE001
114 | error("...", exc_info=None)
|

View file

@ -1,8 +1,7 @@
use ruff_python_ast::{self as ast, Expr, ExprCall};
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::Truthiness;
use ruff_python_ast::{self as ast, Expr, ExprCall};
use ruff_python_semantic::analyze::logging;
use ruff_python_stdlib::logging::LoggingLevel;
use ruff_text_size::Ranged;
@ -43,35 +42,47 @@ impl Violation for ExceptionWithoutExcInfo {
/// LOG007
pub(crate) fn exception_without_exc_info(checker: &mut Checker, call: &ExprCall) {
let Expr::Attribute(ast::ExprAttribute { value: _, attr, .. }) = call.func.as_ref() else {
return;
};
match call.func.as_ref() {
Expr::Attribute(ast::ExprAttribute { attr, .. }) => {
if !matches!(
LoggingLevel::from_attribute(attr.as_str()),
Some(LoggingLevel::Exception)
) {
return;
}
if !matches!(
LoggingLevel::from_attribute(attr.as_str()),
Some(LoggingLevel::Exception)
) {
return;
if !logging::is_logger_candidate(
&call.func,
checker.semantic(),
&checker.settings.logger_objects,
) {
return;
}
}
Expr::Name(_) => {
if !checker
.semantic()
.resolve_call_path(call.func.as_ref())
.is_some_and(|call_path| matches!(call_path.as_slice(), ["logging", "exception"]))
{
return;
}
}
_ => return,
}
if !logging::is_logger_candidate(
&call.func,
checker.semantic(),
&checker.settings.logger_objects,
) {
return;
}
if call
.arguments
.find_keyword("exc_info")
.map(|keyword| &keyword.value)
.is_some_and(|value| {
Truthiness::from_expr(value, |id| checker.semantic().is_builtin(id)).is_falsey()
})
{
if exc_info_arg_is_falsey(call, checker) {
checker
.diagnostics
.push(Diagnostic::new(ExceptionWithoutExcInfo, call.range()));
}
}
fn exc_info_arg_is_falsey(call: &ExprCall, checker: &mut Checker) -> bool {
call.arguments
.find_keyword("exc_info")
.map(|keyword| &keyword.value)
.is_some_and(|value| {
Truthiness::from_expr(value, |id| checker.semantic().is_builtin(id)).is_falsey()
})
}

View file

@ -40,4 +40,13 @@ LOG007.py:10:1: LOG007 Use of `logging.exception` with falsy `exc_info`
12 | logger.info("foo", exc_info=False) # OK
|
LOG007.py:17:1: LOG007 Use of `logging.exception` with falsy `exc_info`
|
15 | from logging import exception
16 |
17 | exception("foo", exc_info=False) # LOG007
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ LOG007
18 | exception("foo", exc_info=True) # OK
|

View file

@ -153,22 +153,36 @@ impl LoggingCallType {
/// Check logging calls for violations.
pub(crate) fn logging_call(checker: &mut Checker, call: &ast::ExprCall) {
let Expr::Attribute(ast::ExprAttribute { value: _, attr, .. }) = call.func.as_ref() else {
return;
// Determine the call type (e.g., `info` vs. `exception`) and the range of the attribute.
let (logging_call_type, range) = match call.func.as_ref() {
Expr::Attribute(ast::ExprAttribute { value: _, attr, .. }) => {
let Some(call_type) = LoggingCallType::from_attribute(attr.as_str()) else {
return;
};
if !logging::is_logger_candidate(
&call.func,
checker.semantic(),
&checker.settings.logger_objects,
) {
return;
}
(call_type, attr.range())
}
Expr::Name(_) => {
let Some(call_path) = checker.semantic().resolve_call_path(call.func.as_ref()) else {
return;
};
let ["logging", attribute] = call_path.as_slice() else {
return;
};
let Some(call_type) = LoggingCallType::from_attribute(attribute) else {
return;
};
(call_type, call.func.range())
}
_ => return,
};
let Some(logging_call_type) = LoggingCallType::from_attribute(attr.as_str()) else {
return;
};
if !logging::is_logger_candidate(
&call.func,
checker.semantic(),
&checker.settings.logger_objects,
) {
return;
}
// G001 - G004
let msg_pos = usize::from(matches!(logging_call_type, LoggingCallType::LogCall));
if let Some(format_arg) = call.arguments.find_argument("msg", msg_pos) {
@ -181,11 +195,11 @@ pub(crate) fn logging_call(checker: &mut Checker, call: &ast::ExprCall) {
logging_call_type,
LoggingCallType::LevelCall(LoggingLevel::Warn)
) {
let mut diagnostic = Diagnostic::new(LoggingWarn, attr.range());
let mut diagnostic = Diagnostic::new(LoggingWarn, range);
if checker.patch(diagnostic.kind.rule()) {
diagnostic.set_fix(Fix::automatic(Edit::range_replacement(
"warning".to_string(),
attr.range(),
range,
)));
}
checker.diagnostics.push(diagnostic);
@ -213,7 +227,7 @@ pub(crate) fn logging_call(checker: &mut Checker, call: &ast::ExprCall) {
if checker.enabled(Rule::LoggingExcInfo) {
checker
.diagnostics
.push(Diagnostic::new(LoggingExcInfo, attr.range()));
.push(Diagnostic::new(LoggingExcInfo, range));
}
}
LoggingLevel::Exception => {

View file

@ -83,6 +83,64 @@ G001.py:18:30: G001 Logging statement uses `str.format`
17 | current_app.logger.info("Hello {}".format("World!"))
18 | app.logger.log(logging.INFO, "Hello {}".format("World!"))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ G001
19 |
20 | from logging import info, log
|
G001.py:22:6: G001 Logging statement uses `str.format`
|
20 | from logging import info, log
21 |
22 | info("Hello {}".format("World!"))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ G001
23 | log(logging.INFO, "Hello {}".format("World!"))
24 | info("Hello {}".format("World!"))
|
G001.py:23:19: G001 Logging statement uses `str.format`
|
22 | info("Hello {}".format("World!"))
23 | log(logging.INFO, "Hello {}".format("World!"))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ G001
24 | info("Hello {}".format("World!"))
25 | log(logging.INFO, msg="Hello {}".format("World!"))
|
G001.py:24:6: G001 Logging statement uses `str.format`
|
22 | info("Hello {}".format("World!"))
23 | log(logging.INFO, "Hello {}".format("World!"))
24 | info("Hello {}".format("World!"))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ G001
25 | log(logging.INFO, msg="Hello {}".format("World!"))
26 | log(level=logging.INFO, msg="Hello {}".format("World!"))
|
G001.py:25:23: G001 Logging statement uses `str.format`
|
23 | log(logging.INFO, "Hello {}".format("World!"))
24 | info("Hello {}".format("World!"))
25 | log(logging.INFO, msg="Hello {}".format("World!"))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ G001
26 | log(level=logging.INFO, msg="Hello {}".format("World!"))
27 | log(msg="Hello {}".format("World!"), level=logging.INFO)
|
G001.py:26:29: G001 Logging statement uses `str.format`
|
24 | info("Hello {}".format("World!"))
25 | log(logging.INFO, msg="Hello {}".format("World!"))
26 | log(level=logging.INFO, msg="Hello {}".format("World!"))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ G001
27 | log(msg="Hello {}".format("World!"), level=logging.INFO)
|
G001.py:27:9: G001 Logging statement uses `str.format`
|
25 | log(logging.INFO, msg="Hello {}".format("World!"))
26 | log(level=logging.INFO, msg="Hello {}".format("World!"))
27 | log(msg="Hello {}".format("World!"), level=logging.INFO)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ G001
|

View file

@ -15,6 +15,24 @@ G002.py:4:27: G002 Logging statement uses `%`
3 | logging.info("Hello %s" % "World!")
4 | logging.log(logging.INFO, "Hello %s" % "World!")
| ^^^^^^^^^^^^^^^^^^^^^ G002
5 |
6 | from logging import info, log
|
G002.py:8:6: G002 Logging statement uses `%`
|
6 | from logging import info, log
7 |
8 | info("Hello %s" % "World!")
| ^^^^^^^^^^^^^^^^^^^^^ G002
9 | log(logging.INFO, "Hello %s" % "World!")
|
G002.py:9:19: G002 Logging statement uses `%`
|
8 | info("Hello %s" % "World!")
9 | log(logging.INFO, "Hello %s" % "World!")
| ^^^^^^^^^^^^^^^^^^^^^ G002
|

View file

@ -15,6 +15,24 @@ G003.py:4:27: G003 Logging statement uses `+`
3 | logging.info("Hello" + " " + "World!")
4 | logging.log(logging.INFO, "Hello" + " " + "World!")
| ^^^^^^^^^^^^^^^^^^^^^^^^ G003
5 |
6 | from logging import info, log
|
G003.py:8:6: G003 Logging statement uses `+`
|
6 | from logging import info, log
7 |
8 | info("Hello" + " " + "World!")
| ^^^^^^^^^^^^^^^^^^^^^^^^ G003
9 | log(logging.INFO, "Hello" + " " + "World!")
|
G003.py:9:19: G003 Logging statement uses `+`
|
8 | info("Hello" + " " + "World!")
9 | log(logging.INFO, "Hello" + " " + "World!")
| ^^^^^^^^^^^^^^^^^^^^^^^^ G003
|

View file

@ -20,10 +20,28 @@ G004.py:5:27: G004 Logging statement uses f-string
|
G004.py:8:14: G004 Logging statement uses f-string
|
7 | _LOGGER = logging.getLogger()
8 | _LOGGER.info(f"{__name__}")
| ^^^^^^^^^^^^^ G004
|
|
7 | _LOGGER = logging.getLogger()
8 | _LOGGER.info(f"{__name__}")
| ^^^^^^^^^^^^^ G004
9 |
10 | from logging import info
|
G004.py:11:6: G004 Logging statement uses f-string
|
10 | from logging import info
11 | info(f"{name}")
| ^^^^^^^^^ G004
12 | info(f"{__name__}")
|
G004.py:12:6: G004 Logging statement uses f-string
|
10 | from logging import info
11 | info(f"{name}")
12 | info(f"{__name__}")
| ^^^^^^^^^^^^^ G004
|

View file

@ -41,6 +41,7 @@ G010.py:8:8: G010 [*] Logging statement uses `warn` instead of `warning`
8 |+logger.warning("Hello world!")
9 9 |
10 10 | logging . warn("Hello World!")
11 11 |
G010.py:10:11: G010 [*] Logging statement uses `warn` instead of `warning`
|
@ -48,6 +49,8 @@ G010.py:10:11: G010 [*] Logging statement uses `warn` instead of `warning`
9 |
10 | logging . warn("Hello World!")
| ^^^^ G010
11 |
12 | from logging import warn, warning, exception
|
= help: Convert to `warn`
@ -57,5 +60,27 @@ G010.py:10:11: G010 [*] Logging statement uses `warn` instead of `warning`
9 9 |
10 |-logging . warn("Hello World!")
10 |+logging . warning("Hello World!")
11 11 |
12 12 | from logging import warn, warning, exception
13 13 | warn("foo")
G010.py:13:1: G010 [*] Logging statement uses `warn` instead of `warning`
|
12 | from logging import warn, warning, exception
13 | warn("foo")
| ^^^^ G010
14 | warning("foo")
15 | exception("foo")
|
= help: Convert to `warn`
Fix
10 10 | logging . warn("Hello World!")
11 11 |
12 12 | from logging import warn, warning, exception
13 |-warn("foo")
14 13 | warning("foo")
14 |+warning("foo")
15 15 | exception("foo")

View file

@ -11,4 +11,14 @@ G101_1.py:6:9: G101 Logging statement uses an `extra` field that clashes with a
8 | )
|
G101_1.py:15:9: G101 Logging statement uses an `extra` field that clashes with a `LogRecord` field: `name`
|
13 | "Hello world!",
14 | extra={
15 | "name": "foobar",
| ^^^^^^ G101
16 | },
17 | )
|

View file

@ -11,4 +11,14 @@ G101_2.py:6:9: G101 Logging statement uses an `extra` field that clashes with a
8 | )
|
G101_2.py:15:9: G101 Logging statement uses an `extra` field that clashes with a `LogRecord` field: `name`
|
13 | "Hello world!",
14 | extra=dict(
15 | name="foobar",
| ^^^^^^^^^^^^^ G101
16 | ),
17 | )
|

View file

@ -21,4 +21,24 @@ G201.py:13:13: G201 Logging `.exception(...)` should be used instead of `.error(
15 | # OK
|
G201.py:28:5: G201 Logging `.exception(...)` should be used instead of `.error(..., exc_info=True)`
|
26 | pass
27 | except:
28 | error("Hello World", exc_info=True)
| ^^^^^ G201
29 |
30 | try:
|
G201.py:33:5: G201 Logging `.exception(...)` should be used instead of `.error(..., exc_info=True)`
|
31 | pass
32 | except:
33 | error("Hello World", exc_info=sys.exc_info())
| ^^^^^ G201
34 |
35 | # OK
|

View file

@ -21,4 +21,24 @@ G202.py:13:38: G202 Logging statement has redundant `exc_info`
15 | # OK
|
G202.py:28:30: G202 Logging statement has redundant `exc_info`
|
26 | pass
27 | except:
28 | exception("Hello World", exc_info=True)
| ^^^^^^^^^^^^^ G202
29 |
30 | try:
|
G202.py:33:30: G202 Logging statement has redundant `exc_info`
|
31 | pass
32 | except:
33 | exception("Hello World", exc_info=sys.exc_info())
| ^^^^^^^^^^^^^^^^^^^^^^^ G202
34 |
35 | # OK
|

View file

@ -101,14 +101,33 @@ pub(crate) fn logging_call(checker: &mut Checker, call: &ast::ExprCall) {
return;
}
let Expr::Attribute(ast::ExprAttribute { attr, .. }) = call.func.as_ref() else {
return;
match call.func.as_ref() {
Expr::Attribute(ast::ExprAttribute { attr, .. }) => {
if LoggingLevel::from_attribute(attr).is_none() {
return;
};
if !logging::is_logger_candidate(
&call.func,
checker.semantic(),
&checker.settings.logger_objects,
) {
return;
}
}
Expr::Name(_) => {
let Some(call_path) = checker.semantic().resolve_call_path(call.func.as_ref()) else {
return;
};
let ["logging", attribute] = call_path.as_slice() else {
return;
};
if LoggingLevel::from_attribute(attribute).is_none() {
return;
};
}
_ => return,
};
if LoggingLevel::from_attribute(attr.as_str()).is_none() {
return;
}
let Some(Expr::Constant(ast::ExprConstant {
value: Constant::Str(ast::StringConstant { value, .. }),
..
@ -117,14 +136,6 @@ pub(crate) fn logging_call(checker: &mut Checker, call: &ast::ExprCall) {
return;
};
if !logging::is_logger_candidate(
&call.func,
checker.semantic(),
&checker.settings.logger_objects,
) {
return;
}
let Ok(summary) = CFormatSummary::try_from(value.as_str()) else {
return;
};

View file

@ -21,4 +21,24 @@ logging_too_many_args.py:5:1: PLE1205 Too many arguments for `logging` format st
7 | logging.warning("Hello %s", "World!")
|
logging_too_many_args.py:29:1: PLE1205 Too many arguments for `logging` format string
|
27 | from logging import info, error, warning
28 |
29 | warning("Hello %s", "World!", "again") # [logging-too-many-args]
| ^^^^^^^ PLE1205
30 |
31 | warning("Hello %s", "World!", "again", something="else")
|
logging_too_many_args.py:31:1: PLE1205 Too many arguments for `logging` format string
|
29 | warning("Hello %s", "World!", "again") # [logging-too-many-args]
30 |
31 | warning("Hello %s", "World!", "again", something="else")
| ^^^^^^^ PLE1205
32 |
33 | warning("Hello %s", "World!")
|

View file

@ -11,4 +11,14 @@ logging_too_few_args.py:3:1: PLE1206 Not enough arguments for `logging` format s
5 | # do not handle calls with kwargs (like pylint)
|
logging_too_few_args.py:33:1: PLE1206 Not enough arguments for `logging` format string
|
31 | from logging import error, info, warning
32 |
33 | warning("Hello %s %s", "World!") # [logging-too-few-args]
| ^^^^^^^ PLE1206
34 |
35 | # do not handle calls with kwargs (like pylint)
|

View file

@ -1,15 +1,15 @@
use ruff_python_ast::{self as ast, Expr};
use ruff_python_ast::visitor;
use ruff_python_ast::visitor::Visitor;
use ruff_python_ast::{self as ast, Expr};
use ruff_python_semantic::analyze::logging;
use ruff_python_semantic::SemanticModel;
use ruff_python_stdlib::logging::LoggingLevel;
/// Collect `logging`-like calls from an AST.
pub(super) struct LoggerCandidateVisitor<'a, 'b> {
semantic: &'a SemanticModel<'b>,
logger_objects: &'a [String],
pub(super) calls: Vec<&'b ast::ExprCall>,
pub(super) calls: Vec<(&'b ast::ExprCall, LoggingLevel)>,
}
impl<'a, 'b> LoggerCandidateVisitor<'a, 'b> {
@ -25,8 +25,27 @@ impl<'a, 'b> LoggerCandidateVisitor<'a, 'b> {
impl<'a, 'b> Visitor<'b> for LoggerCandidateVisitor<'a, 'b> {
fn visit_expr(&mut self, expr: &'b Expr) {
if let Expr::Call(call) = expr {
if logging::is_logger_candidate(&call.func, self.semantic, self.logger_objects) {
self.calls.push(call);
match call.func.as_ref() {
Expr::Attribute(ast::ExprAttribute { attr, .. }) => {
if logging::is_logger_candidate(&call.func, self.semantic, self.logger_objects)
{
if let Some(logging_level) = LoggingLevel::from_attribute(attr) {
self.calls.push((call, logging_level));
};
}
}
Expr::Name(_) => {
if let Some(call_path) = self.semantic.resolve_call_path(call.func.as_ref()) {
if let ["logging", attribute] = call_path.as_slice() {
if let Some(logging_level) = LoggingLevel::from_attribute(attribute) {
{
self.calls.push((call, logging_level));
}
}
}
}
}
_ => {}
}
}
visitor::walk_expr(self, expr);

View file

@ -1,9 +1,9 @@
use ruff_python_ast::{self as ast, ExceptHandler, Expr};
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::visitor::Visitor;
use ruff_python_ast::{self as ast, ExceptHandler};
use ruff_python_semantic::analyze::logging::exc_info;
use ruff_python_stdlib::logging::LoggingLevel;
use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;
@ -64,14 +64,12 @@ pub(crate) fn error_instead_of_exception(checker: &mut Checker, handlers: &[Exce
visitor.visit_body(body);
visitor.calls
};
for expr in calls {
if let Expr::Attribute(ast::ExprAttribute { attr, .. }) = expr.func.as_ref() {
if attr == "error" {
if exc_info(&expr.arguments, checker.semantic()).is_none() {
checker
.diagnostics
.push(Diagnostic::new(ErrorInsteadOfException, expr.range()));
}
for (expr, logging_level) in calls {
if matches!(logging_level, LoggingLevel::Error) {
if exc_info(&expr.arguments, checker.semantic()).is_none() {
checker
.diagnostics
.push(Diagnostic::new(ErrorInsteadOfException, expr.range()));
}
}
}

View file

@ -4,6 +4,7 @@ use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::visitor;
use ruff_python_ast::visitor::Visitor;
use ruff_python_stdlib::logging::LoggingLevel;
use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;
@ -74,25 +75,23 @@ pub(crate) fn verbose_log_message(checker: &mut Checker, handlers: &[ExceptHandl
visitor.calls
};
for expr in calls {
if let Expr::Attribute(ast::ExprAttribute { attr, .. }) = expr.func.as_ref() {
if attr == "exception" {
// Collect all referenced names in the `logging.exception` call.
let names: Vec<&ast::ExprName> = {
let mut names = Vec::new();
for arg in &expr.arguments.args {
let mut visitor = NameVisitor::default();
visitor.visit_expr(arg);
names.extend(visitor.names);
}
names
};
for expr in names {
if expr.id == target.as_str() {
checker
.diagnostics
.push(Diagnostic::new(VerboseLogMessage, expr.range()));
}
for (expr, logging_level) in calls {
if matches!(logging_level, LoggingLevel::Exception) {
// Collect all referenced names in the `logging.exception` call.
let names: Vec<&ast::ExprName> = {
let mut names = Vec::new();
for arg in &expr.arguments.args {
let mut visitor = NameVisitor::default();
visitor.visit_expr(arg);
names.extend(visitor.names);
}
names
};
for expr in names {
if expr.id == target.as_str() {
checker
.diagnostics
.push(Diagnostic::new(VerboseLogMessage, expr.range()));
}
}
}

View file

@ -69,4 +69,21 @@ TRY400.py:49:13: TRY400 Use `logging.exception` instead of `logging.error`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TRY400
|
TRY400.py:87:9: TRY400 Use `logging.exception` instead of `logging.error`
|
85 | a = 1
86 | except Exception:
87 | error("Context message here")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TRY400
88 |
89 | if True:
|
TRY400.py:90:13: TRY400 Use `logging.exception` instead of `logging.error`
|
89 | if True:
90 | error("Context message here")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ TRY400
|

View file

@ -89,4 +89,92 @@ TRY401.py:53:47: TRY401 Redundant exception object included in `logging.exceptio
| ^^ TRY401
|
TRY401.py:77:38: TRY401 Redundant exception object included in `logging.exception` call
|
75 | finish()
76 | except Exception as ex:
77 | exception(f"Found an error: {ex}") # TRY401
| ^^ TRY401
|
TRY401.py:88:46: TRY401 Redundant exception object included in `logging.exception` call
|
86 | if True is False:
87 | for i in range(10):
88 | exception(f"Found an error: {bad} {good}") # TRY401
| ^^^ TRY401
89 | except IndexError as bad:
90 | exception(f"Found an error: {bad} {bad}") # TRY401
|
TRY401.py:90:38: TRY401 Redundant exception object included in `logging.exception` call
|
88 | exception(f"Found an error: {bad} {good}") # TRY401
89 | except IndexError as bad:
90 | exception(f"Found an error: {bad} {bad}") # TRY401
| ^^^ TRY401
91 | except Exception as bad:
92 | exception(f"Found an error: {bad}") # TRY401
|
TRY401.py:90:44: TRY401 Redundant exception object included in `logging.exception` call
|
88 | exception(f"Found an error: {bad} {good}") # TRY401
89 | except IndexError as bad:
90 | exception(f"Found an error: {bad} {bad}") # TRY401
| ^^^ TRY401
91 | except Exception as bad:
92 | exception(f"Found an error: {bad}") # TRY401
|
TRY401.py:92:38: TRY401 Redundant exception object included in `logging.exception` call
|
90 | exception(f"Found an error: {bad} {bad}") # TRY401
91 | except Exception as bad:
92 | exception(f"Found an error: {bad}") # TRY401
| ^^^ TRY401
93 | exception(f"Found an error: {bad}") # TRY401
|
TRY401.py:93:38: TRY401 Redundant exception object included in `logging.exception` call
|
91 | except Exception as bad:
92 | exception(f"Found an error: {bad}") # TRY401
93 | exception(f"Found an error: {bad}") # TRY401
| ^^^ TRY401
94 |
95 | if True:
|
TRY401.py:96:42: TRY401 Redundant exception object included in `logging.exception` call
|
95 | if True:
96 | exception(f"Found an error: {bad}") # TRY401
| ^^^ TRY401
|
TRY401.py:103:40: TRY401 Redundant exception object included in `logging.exception` call
|
101 | ...
102 | except Exception as ex:
103 | exception(f"Logging an error: {ex}") # TRY401
| ^^ TRY401
|
TRY401.py:110:46: TRY401 Redundant exception object included in `logging.exception` call
|
108 | ...
109 | except Exception as ex:
110 | exception("Logging an error: " + str(ex)) # TRY401
| ^^ TRY401
|
TRY401.py:117:40: TRY401 Redundant exception object included in `logging.exception` call
|
115 | ...
116 | except Exception as ex:
117 | exception("Logging an error:", ex) # TRY401
| ^^ TRY401
|