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

[java] GuardLogStatement reports violation for Log4j parameterized logs #4703

Closed
ThorodanBrom opened this issue Oct 3, 2023 · 1 comment
Closed
Labels
a:false-positive PMD flags a piece of code that is not problematic
Milestone

Comments

@ThorodanBrom
Copy link

Affects PMD Version: 6.53.0, 6.55.0

Rule: GuardLogStatement

Description:

PMD reports a violation when Log4j parameterized logs are used. We are in the process of converting all logs from concatenated strings to parameterized logs, using PMD to find offending logs. But PMD reports the same logs even after updating.

Code Sample demonstrating the issue:

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

class Test {
    private static final Logger LOGGER = LogManager.getLogger(Test.class);

    public static void main() {
        LOGGER.info("Info : {} : Request received", LOGGER.getName());
        String s = "Hello World";
        LOGGER.debug("Logging string {}", s);
    }
}

Expected outcome:

PMD reports a violation at lines 8 and 10, but that's wrong. That's a false positive.

Running PMD through: Maven


P.S. I saw someone else report the same false-positive on another issue - #3144 (comment)

@ThorodanBrom ThorodanBrom added the a:false-positive PMD flags a piece of code that is not problematic label Oct 3, 2023
@jsotuyod jsotuyod added the needs:pmd7-revalidation The issue hasn't yet been retested vs PMD 7 and may be stale label Mar 17, 2024
@jsotuyod jsotuyod removed the needs:pmd7-revalidation The issue hasn't yet been retested vs PMD 7 and may be stale label Mar 25, 2024
@jsotuyod
Copy link
Member

Under PMD 7.0.0 the second line will not be reported.

The first one, however, will.

LOGGER.info("Info : {} : Request received", LOGGER.getName());

this line will always perform a call to LOGGER.getName() regardless of the logging level. As any method calls done here can be potentially expensive, they still get flagged. The correct approach would be to use a Callable here through a lambda such as:

LOGGER.info("Info : {} : Request received", () -> LOGGER.getName());

@jsotuyod jsotuyod added this to the 7.0.0 milestone Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:false-positive PMD flags a piece of code that is not problematic
Projects
None yet
Development

No branches or pull requests

2 participants