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

Rule G004 doesn't work with inline instantiated logger #11031

Closed
Spenhouet opened this issue Apr 19, 2024 · 1 comment · Fixed by #11154
Closed

Rule G004 doesn't work with inline instantiated logger #11031

Spenhouet opened this issue Apr 19, 2024 · 1 comment · Fixed by #11154
Assignees
Labels
rule Implementing or modifying a lint rule

Comments

@Spenhouet
Copy link

Given the following code:

logger = Logger()
logger.warning(f"{database_name} database not available.")

the ruff linter will complain about G004 ("Logging statement uses f-string").

But given an inline instantiation of the logger, there is no such warning.

Logger().warning(f"{database_name} database not available.")

The "G" rule (flake8-logging-format) needs to be enabled.
My ruff version is 0.3.1.
I searched for G004 but none of the existing issues cover this. The answer might be similar to #6353 (comment) but I do think that this is something which could be covered.

@dhruvmanila dhruvmanila added the rule Implementing or modifying a lint rule label Apr 19, 2024
@dhruvmanila
Copy link
Member

Yeah, our heuristics doesn't check for a direct instantiation

/// 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.
///
/// It also matches direct `logging.error` calls when 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(
func: &Expr,
semantic: &SemanticModel,
logger_objects: &[String],
) -> bool {
let Expr::Attribute(ast::ExprAttribute { value, .. }) = func else {
return false;
};
// If the symbol was imported from another module, ensure that it's either a user-specified
// logger object, the `logging` module itself, or `flask.current_app.logger`.
if let Some(qualified_name) = semantic.resolve_qualified_name(value) {
if matches!(
qualified_name.segments(),
["logging"] | ["flask", "current_app", "logger"]
) {
return true;
}
if logger_objects
.iter()
.any(|logger| QualifiedName::from_dotted_name(logger) == qualified_name)
{
return true;
}
return false;
}
// Otherwise, if the symbol was defined in the current module, match against some common
// logger names.
if let Some(name) = UnqualifiedName::from_expr(value) {
if let Some(tail) = name.segments().last() {
if tail.starts_with("log")
|| tail.ends_with("logger")
|| tail.ends_with("logging")
|| tail.starts_with("LOG")
|| tail.ends_with("LOGGER")
|| tail.ends_with("LOGGING")
{
return true;
}
}
}
false
}

We could support this by updating the heuristic to include logging.Logger() calls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants