Re: [Cp-tools-discuss] bug + possible fix (patch) in texidoclet: TreeNode.java

[Julian Scheid]

Hi Alex,

great to see that you give it a try!

> I found another apparent bug in texidoclet when attempting to test
> texidoclet with gjdoc.  This is a strange one because using the same
> doclet (texidoclet) with both Sun's javadoc and gjdoc gives different
> errors, but the bug itself seems to be in texidoclet, not gjdoc.

As gjdoc doesn't currently deliver exactly the same information as
javadoc via the Doclet API, this may of course reveal bugs in
TexiDoclet that go unnoticed when using javadoc.

> This can easily be traced to a logic problem in TreeNode.java, in
> which it appears that following a test for null on a variable, the
> subsequent statement is an operation on that variable.
>
> This is the offending piece of code:
>
>  ClassEntry ifaceEntry = 
> (ClassEntry)interfaceEntries.get(implementedInterfaces[i]);
>  if (ifaceEntry==null) {
>      String ifaceName = implementedInterfaces[i];
>      int lastPoint=ifaceName.lastIndexOf('.');
>      if (lastPoint>=0) {
>          String ifaceNode = Toolkit.classNameToNodeName(ifaceName); 
> //"("+ifaceName.substring(0,lastPoint)+") "+ifaceName.substring(lastPoint+1);
>
>          ifaceEntry = new ClassEntry(ifaceName, ifaceNode); // FIXME! invalid 
> target node
>          interfaceEntries.put(ifaceName,ifaceEntry);
>      }
>  }
>  ifaceEntry.add(classEntry);
>
> It looks like it's possibly intended to be inside an "else" statement,
> or otherwise trapped (i.e. is it supposed to be something that is
> "set" after the null check, or is it supposed to be an "alternative"
> to the null check?).  If you look at the context can see why
> "ifaceEntry" won't necessarily be non-null, because the (lastPoint>=0)
> test may not be true...
>
> I'm not sure what the intended behaviour should be, which is why I
> didn't fix in CVS.  If it is, here's a patch that implements the
> "else".  If it's correct, let me know and I'll apply it:

It seems like my point was to create a new ClassEntry if a ClassEntry
corresponding to implementedInterfaces[i] is not already contained in
interfaceEntries. What is missing here is dealing with the case
(ifaceEntry==null && lastPoint<0), i.e. creating a new ClassEntry
given a non-qualified class name. So what needs be added is an else
statement for the inner if-construct, not the outer.

I expect the proper solution is to use the package name of the
current class (the "context" class) and append the non-qualified
class name to this, using the result for deducing the TexiDoclet
node name. However we need to be careful here, w.r.t things like
Inner Classes etc.

It could be that the interface class name supplied by gjdoc ought
to be fully qualified but isn't, which is why this problem doesn't
occur when using javadoc. In any case, this is a bug because you
also have to be able to generate docs for classes in the unnamed
package, whose names don't contain a period by definition (if it
is not an Inner class.)

Perhaps we should test TexiDoclet using javadoc and a bunch of
classes in the unnamed package to see how it behaves in this
situation. In this test scenario, we also should include a class
which implements an interface which is located in the unnamed
package.

I'll have a further look at this in the afternoon.

Julian