False positive - S109 Magic numbers should not be used - final local variable

[Alix Lourme]

Hi,

Currently, sonar-java v3.4 reports S109 issue on this snippet :

    private void test() {
        // S109 issue on following line
        final int foo = 42;
    }

Some other implementation (checkstyle) considers this usage as correct => could be considered as false positive or not ?

Thanks in advance.
Best regards.

[Freddy Mallet]

Hi Alix, 

I should miss something here because I don't understand why this should be considered as a false-positive ?

Thanks

Freddy MALLET | SonarSource

Product Director & Co-Founder
http://sonarsource.com

--
You received this message because you are subscribed to the Google Groups "SonarQube" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sonarqube+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/80cc068b-c412-43ba-8a87-f5d08cb6203e%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Alix Lourme]

Hi Freddy,

A Magic Number sould declared as constant.
In Java, the best way for constant declaration is static final : static for instantiation and final for unchanged value.
Beyond implementation / memory optimisation, a constant is a "unchanged value" => final "could represent" a constant (lexically).

For field declaration, a only final field (like following snippet) has 2 violations : S109 'MagicNumber' and S1170 'Public constants and fields initialized at declaration should be "static final" rather than merely "final"'

public class Clazz {

    /** This is a constant (use static is better) */
    private final int foo = 42;
}

I agree with S1170, this is not the perfect way for constant declaration ... but this is a constant ; so S109 is severe.

And when you change a rule implementation (from checkstyle to squid) ... this difference could be a little disturbing for users :-(.

Best regards.

[Freddy Mallet]

I tend to agree with you @Alix, I let the @Java team taking the final decision.

Thanks 

Freddy MALLET | SonarSource

Product Director & Co-Founder
http://sonarsource.com

To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/7808ebbc-7ca7-4255-8554-8a490e91b490%40googlegroups.com.

[Michael Gumowski]

Hey Alix,

Sorry for the very long period between your initial message and this response...

I agree with you, numbers used as initializers of final fields should not be considered as magic numbers (while S1170 will still raise an issue).

I created the ticket to handle the issue: https://jira.sonarsource.com/browse/SONARJAVA-1235

Regards,

Michael GUMOWSKI | SonarSource

Software Developer @ Language Team
http://sonarsource.com

To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/CACfeEofEydE8_o1oZa6yHnKnEmqF9A941tvLF6qt6TOg3N-0yQ%40mail.gmail.com.

[Alix Lourme]

Hi Michael,

Sorry for the very long period between your initial message and this response...

I created the ticket to handle the issue: https://jira.sonarsource.com/browse/SONARJAVA-1235

No problem, holiday time for everyone ;-). Thanks for the ticket.

Just a complement ... the previous demonstration is about field initialization and explicit ; but I initially (first post of this thread) raised the issue on variable initialization in a method :

private void test() {
    // S109 issue on following line
    final int foo = 42;
}

The demonstration could be similar : final is a constant => no magic number

=> Agree with that or not ? (SONARJAVA-1235 doesn't speak about this case).

Best regards.

[Michael Gumowski]

Hello Alix,

Agreed, JIRA ticket updated!

Regards,

Michael GUMOWSKI | SonarSource

Software Developer @ Language Team
http://sonarsource.com

To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/fb792b99-fdfa-4365-b1c1-42b260ffaf04%40googlegroups.com.