2

I have ThreadLocal instance which was initialized with overridden initValue method. Also I have annotated it with @edu.umd.cs.findbugs.annotations.SuppressWarnings("SIC_INNER_SHOULD_BE_STATIC_ANON") as follows.

@edu.umd.cs.findbugs.annotations.SuppressWarnings("SIC_INNER_SHOULD_BE_STATIC_ANON")
private ThreadLocal<Integer> dbSwitchCount=new ThreadLocal<Integer>() {
    @Override 
    protected Integer initialValue() {
        return 0;
    }
};

Still Sonar report complains about "Performance - Could be re factored into a named static inner class". How can I make sure the above complain being ignored or what best way I can avoid that complain.

Tagir Valeev
  • 97,161
  • 19
  • 222
  • 334
Brinal
  • 172
  • 2
  • 18

2 Answers2

2

Do what Sonar suggests "Could be re factored into a named static inner class".

Named static inner class:

class MyClass {
    static class MyThreadLocal extends ThreadLocal<Integer> {
        @Override 
        protected Integer initialValue() {
            return 0;
        }
    }
    private ThreadLocal<Integer> dbSwitchCount = new MyThreadLocal();
}

I think the reason Sonar thinks that's a "Performance" improvement is because the anonymous class is non-static, and making it static improves memory management.

Andreas
  • 154,647
  • 11
  • 152
  • 247
1

The annotation does not work, because you are annotating the dbSwitchCount field, not the anonymous class and the bug report is not bound to the field. Here's how bug report looks like in XML:

<BugInstance type="SIC_INNER_SHOULD_BE_STATIC_ANON" priority="3" rank="20" abbrev="SIC" 
             category="PERFORMANCE" first="1">
  <Class classname="MyClass$1">
    <SourceLine classname="MyClass$1" start="1" end="10" 
                sourcefile="MyClass.java" sourcepath="MyClass.java"/>
  </Class>
  <SourceLine classname="MyClass$1" start="7" end="7" startBytecode="0" endBytecode="0" 
              sourcefile="MyClass.java" sourcepath="MyClass.java"/>
</BugInstance>

See, nothing about the dbSwitchCount field. Thus when suppress-filter works, it does not know that dbSwitchCount annotation is somehow connected with this bug report. And unfortunately I see no way to annotate the anonymous class itself. The only thing you can do to suppress this warning without changing the actual code is to annotate the outer class instead:

@SuppressFBWarnings("SIC_INNER_SHOULD_BE_STATIC_ANON")
public class MyClass {

    private final ThreadLocal<Integer> dbSwitchCount=new ThreadLocal<Integer>() {
        @Override 
        protected Integer initialValue() {
            return 0;
        }
    };
}

This way the warning disappears (btw it's recommended to use @SuppressFBWarnings annotation instead).

In general instance-bound (non-static) thread-locals are suspicious. See, for example, this question. Thus probably the original problem is that dbSwitchCount should be declared as static (this way the SIC_INNER_SHOULD_BE_STATIC_ANON warning will disappear as well).

UPDATE I examined the FindBugs code for this detector. Looks like it's possible to add the missing bug annotation, to make it possible to suppress warning annotating the field or the enclosing method. I've created a ticket in our bug tracker.

UPDATE-2 Fixed in FindBugs trunk.

Community
  • 1
  • 1
Tagir Valeev
  • 97,161
  • 19
  • 222
  • 334