Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 14 commits into from Sep 27, 2023
Merged
Expand Up @@ -92,3 +92,35 @@
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)
Expand Up @@ -16,3 +16,9 @@

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
Expand Up @@ -16,3 +16,12 @@
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)
Expand Up @@ -2,3 +2,8 @@

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!")
Expand Up @@ -2,3 +2,8 @@

logging.info("Hello" + " " + "World!")
logging.log(logging.INFO, "Hello" + " " + "World!")

from logging import info, log

info("Hello" + " " + "World!")
log(logging.INFO, "Hello" + " " + "World!")
Expand Up @@ -6,3 +6,7 @@

_LOGGER = logging.getLogger()
_LOGGER.info(f"{__name__}")

from logging import info
info(f"{name}")
info(f"{__name__}")
Expand Up @@ -8,3 +8,8 @@
logger.warn("Hello world!")

logging . warn("Hello World!")

from logging import warn, warning, exception
warn("foo")
warning("foo")
exception("foo")
Expand Up @@ -6,3 +6,12 @@
"name": "foobar",
},
)

from logging import info

info(
"Hello world!",
extra={
"name": "foobar",
},
)
Expand Up @@ -6,3 +6,12 @@
name="foobar",
),
)

from logging import info

info(
"Hello world!",
extra=dict(
name="foobar",
),
)
Expand Up @@ -19,3 +19,24 @@
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())

Expand Up @@ -19,3 +19,23 @@
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)
Expand Up @@ -26,3 +26,29 @@
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")
Expand Up @@ -22,3 +22,25 @@
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")
34 changes: 34 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/tryceratops/TRY400.py
Expand Up @@ -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())
64 changes: 64 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/tryceratops/TRY401.py
Expand Up @@ -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}")
Expand Up @@ -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;
}
}
}
}
Expand Down