GCI1066 [Team TREE][2025] - For logging format, prefer using %s with kwargs instead of builtin formatter \"\".format() or f\"\""

[fabienhusseperso]

WIP

[cleophass]

Add corresponding header :
/*

• creedengo - Python language - Provides rules to reduce the environmental footprint of your Python programs
• Copyright © 2024 Green Code Initiative (https://green-code-initiative.org)
• This program is free software: you can redistribute it and/or modify
• it under the terms of the GNU General Public License as published by
• the Free Software Foundation, either version 3 of the License, or
• (at your option) any later version.
• This program is distributed in the hope that it will be useful,
• but WITHOUT ANY WARRANTY; without even the implied warranty of
• MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
• GNU General Public License for more details.
• You should have received a copy of the GNU General Public License
• along with this program. If not, see http://www.gnu.org/licenses/.
  */

[cleophass]

Add corresponding header :
/*

• creedengo - Python language - Provides rules to reduce the environmental footprint of your Python programs
• Copyright © 2024 Green Code Initiative (https://green-code-initiative.org)
• This program is free software: you can redistribute it and/or modify
• it under the terms of the GNU General Public License as published by
• the Free Software Foundation, either version 3 of the License, or
• (at your option) any later version.
• This program is distributed in the hope that it will be useful,
• but WITHOUT ANY WARRANTY; without even the implied warranty of
• MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
• GNU General Public License for more details.
• You should have received a copy of the GNU General Public License
• along with this program. If not, see http://www.gnu.org/licenses/.
  */

[cleophass]

I suggest making a version that is insensitive to the variable name because the user can define it, so decteion won't work:

here's what I suggest:
public class DetectBadLoggingFormatInterpolation extends PythonSubscriptionCheck {

protected static final String MESSAGE_RULE = "For logging format, prefer using %s with kwargs instead of builtin formatter \"\".format() or f\"\"";
private static final Pattern PATTERN_FORMAT = Pattern.compile("f['\"].*['\"]");
private static final Pattern PATTERN_LOGGING_METHOD = Pattern.compile("info|debug|error");
private static final String LOGGER_FUNCCTION = "logging.getLogger";
private final Set<String> loggerNames = new HashSet<>();

@Override
public void initialize(Context context) {
    context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, this::visitNodeString);
    context.registerSyntaxNodeConsumer(ASSIGNMENT_STMT, this::handleLoggerAssignment);
}

public void visitNodeString(SubscriptionContext ctx) {
    CallExpression callExpression = (CallExpression) ctx.syntaxNode();
    Expression callee = callExpression.callee();

    if (!(callee instanceof QualifiedExpression)) {
        return;
    }

    QualifiedExpression qualifiedExpression = (QualifiedExpression) callee;
    Expression qualifier = qualifiedExpression.qualifier();

    if (!(qualifier instanceof Name)) {
        return;
    }

    Name qualifierName = (Name) qualifier;

    if (PATTERN_LOGGING_METHOD.matcher(qualifiedExpression.name().name()).matches()
            && loggerNames.contains(qualifierName.name())
            && !callExpression.arguments().isEmpty()
            && callExpression.arguments().get(0) instanceof RegularArgument) {

        Expression argExpr = ((RegularArgument) callExpression.arguments().get(0)).expression();

        if (argExpr.firstToken() != null && PATTERN_FORMAT.matcher(argExpr.firstToken().value()).matches()) {
            ctx.addIssue(callExpression, MESSAGE_RULE);
        }
    }
}

public void handleLoggerAssignment(SubscriptionContext ctx) {
    var assignmentStmt = (AssignmentStatement) ctx.syntaxNode();
    var value = assignmentStmt.assignedValue();

    if (value.is(CALL_EXPR) && isLoggerCreation((CallExpression) value)) {
        String variableName = Utils.getVariableName(ctx);
        if (variableName != null) {
            loggerNames.add(variableName);
            System.out.println("Logger created with name: " + variableName);
        }
    }
}

private boolean isLoggerCreation(CallExpression callExpression) {
    return LOGGER_FUNCCTION.equals(Utils.getQualifiedName(callExpression));
}

}

[cleophass]

If you modify the variable name so that it is not modifiable, you will have to add the corresponding tests, for example :

my_logger = logging.getLogger()
user = "admin"
ip = "192.168.0.1"

my_logger.debug(f"User {user} attempted login") # Noncompliant {{For logging format, prefer using %s with kwargs instead of builtin formatter "".format() or f""}}
my_logger.error(f"Blocked IP: {ip}") # Noncompliant {{For logging format, prefer using %s with kwargs instead of builtin formatter "".format() or f""}}

my_logger.info("Static log with no variables")

my_logger.info("User %s connected", user)
my_logger.warning("IP %s is blocked", ip)
my_logger.debug("User %s attempted login", user)
my_logger.error("Blocked IP: %s", ip)
my_logger.critical("CRITICAL issue with user %s on IP %s", user, ip)

if True:
my_logger.debug(f"Conditional debug for {user}") # Noncompliant {{For logging format, prefer using %s with kwargs instead of builtin formatter "".format() or f""}}

with open("log.txt", "w") as f:
my_logger.info("Opened file for logging")

class MyService:
def init(self):
self.logger = logging.getLogger("service")

def report(self, user):
    self.logger.info("Reporting user %s", user)

[github-actions]

This PR has been automatically marked as stale because it has no activity for 30 days.
Please add a comment if you want to keep the issue open. Thank you for your contributions!

[github-actions]

This PR has been automatically marked as stale because it has no activity for 30 days.
Please add a comment if you want to keep the issue open. Thank you for your contributions!

[github-actions]

This PR has been automatically marked as stale because it has no activity for 30 days.
Please add a comment if you want to keep the issue open. Thank you for your contributions!

[dedece35]

Hi @fabienhusseperso and @cleophass,

sorry but the current PR doesn't deal with all other use cases and only check string values inside the code.

there are different ways to declare an import in python.
there are different method names.
there is another logging framework : loguru
etc ...

I created a new PR with your code and I upgrade it a lot : you can see it here https://github.com/green-code-initiative/creedengo-python/tree/PR_84_DDC

For these reasons, I close this PR and the work continues in the new PR.

Thank you @fabienhusseperso and @cleophass for your work !