missing XFactory interning in test driver

[GrimmiMeloni]

I wanted to share something I noticed last night while I was creating a testcase for one of my detectors.

I have a detector that is making frequent use of the X-Class hierarchy provided in findbugs 2.x (not sure if it was present already in 1.3.x).
So I have lines like XFactory.createXMethod(), and stuff like that.

What I noticed is, that the current findbugs "bootstrapping" in the test-driver does not seem to trigger this area to be properly initialized.

I checked the code of FindBugs2.java as well as FindBugs.java. What actually is missing from the test-drivers init is what currently happens in

FindBugs2.analyzeApplication() (lines 1056 onwards in the 2.0.1 codebase):

            XFactory factory = AnalysisContext.currentXFactory();
            Collection<ClassDescriptor> badClasses = new LinkedList<ClassDescriptor>();
            for (ClassDescriptor desc : referencedClassSet) {
                try {
                    XClass info = Global.getAnalysisCache().getClassAnalysis(XClass.class, desc);
                    factory.intern(info);
                } catch (CheckedAnalysisException e) {
                    AnalysisContext.logError("Couldn't get class info for " + desc, e);
                    badClasses.add(desc);
                } catch (RuntimeException e) {
                    AnalysisContext.logError("Couldn't get class info for " + desc, e);
                    badClasses.add(desc);
                }
            }

The factory.intern(info) statement is the key for this. It is the step where FindBugs iterates the applications classes and then "intern"s every one of it into the factory.

Without this step, all XFactory output is of type UnresolvedField and/or UnresolvedMethod, instead of MethodInfo/FieldInfo.

I worked around this, by triggering this step myself, i.e.

    private void internClass(final ClassDescriptor classDesc) {
        XFactory currentXFactory = AnalysisContext.currentXFactory();
          XClass xClass = currentXFactory.getXClass(classDesc);
          currentXFactory.intern(xClass);
    }

classDesc in the method above is a ClassDescriptor for a given class under analysis.

But it took me some time to figure out what was wrong in the first place.
Maybe there's some chance to "properly" init the XFactory before the detector is called.
I could imagine something like the above happening in

DetectorRunner.runDetectorOnClass(Detector2, Class<?>, BugReporter)

The problem with my suggested solution would be, that this however would only work for the class being handed in. Super- and SubTypes could still work by tweaking the above internClass() helper method to take this into account. But what about other types this class refers to?

I think this quickly becomes quite complex, and the "original" findbugs bootstrapping has this solved already.
Maybe it is possible to lend the setup logic from findbugs 2.x for this as well?

[Grundlefleck]

Ack :)

Will take a look, no ETA at the moment though.

[Grundlefleck]

Hi @GrimmiMeloni I've taken a stab at surgically removing the necessary parts of FindBugs2.analyzeApplication. I have written a test which has a detector using XFactory to create a method. I did observe this being returned as an UnresolvedMethod, then change to MethodInfo once I made the appropriate interning calls.

Could you build a jar from HEAD to test?

I couldn't reproduce the problem with accessing super/subtypes. That's probably because I am less familiar with how to use XFactory in this way -- it's not what I tend to in custom detectors. There's a test case here: https://github.com/youdevise/test-driven-detectors4findbugs/blob/master/src/test/java/com/youdevise/fbplugins/tdd4fb/DetectorRunnerTest.java#L97 . Would you be able to tell me what I'm doing wrong, or any cases that I should cover? If you could share some of your example uses of XClass that would also be really handy.

[GrimmiMeloni]

Hi @Grundlefleck !

I pulled your change. It solves the problem for the class under test, as well as it's supertypes.
The problem that still remains, is that all classes touched by the one under test are not interned into the factory.

I have pushed an additional test into my fork. The commit is:

GrimmiMeloni@c6398cb#diff-a8816d1f4768a47ae5230a0defa5839cR120

I think what we are really lacking here, is the possibility to add more classes to the classpath. Right now, when we use the DetectorAssert, the call includes only the Class under test itself. This is contrary to the way the classpath is setup when findbugs is executed "for real", because I there I can provide a whole set of classes.

Basically the situation we now have, is the one that findbugs typically reports as "missing classfiles".

I could imagine several solutions.
Maybe it is already sufficient to setup the Aux-Classpath. I am not sure if this is already possible with the test harness? (cough documentation cough :) )

If that is not working (or possible), than the other option could be to add a possibility to add more than just the class under test to the classpath, maybe in form of a Collection of classes.

Third option could be to automatically resolve all classes referenced by the class under test. I am not sure if this is feasable though.

[Grundlefleck]

The way I intended it to be setup was that you ran your unit test with a classpath that contained all the classes you referenced. It gets all the files to analyse by looking up the java.class.path SystemProperty. So it's not intended just to put the class under test onto the classpath, if you run unit tests under Maven/Eclipse/etc, you should have your entire project and it's dependencies on the classpath. There may be a mistake in how that's achieved, or I may be misremembering, but that was the goal anyway -- you just pass your class under test to DetectorAssert, and it should Just Work.

Maybe it is already sufficient to setup the Aux-Classpath. I am not sure if this is already possible with the test harness? (cough documentation cough :) )

No, that's not available, I wanted to shield users from having to worry about it. Not adverse to allowing extra config where necessary, just want to get as close as possible to working out-of-the-box, so that option is open.

I'm not sure why it's not currently working. You can see the aux code base paths being added from the current classpath in this code. Maybe there is a mismatch between the classes added to the aux code base paths, and those that are used for interning. Thanks for the test case, will have a look :)