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!