CoatiSoftware / Sourcetrail

Sourcetrail - free and open-source interactive source explorer
https://www.sourcetrail.com/
GNU General Public License v3.0
14.8k stars 1.39k forks source link

Add better support for method overloading? #441

Closed LouisStAmour closed 7 years ago

LouisStAmour commented 7 years ago

Edit: I've changed the title of this bug to better reflect later discussion.

Java 8, and IntelliJ, have no problem with older class files, but I'm finding that Sourcetrail can't resolve symbols within Java 5.0 (bytecode 49.0) class files in jars.

I tried both with the maven and standalone Java source group. Unfortunately, as the project I'm working on is confidential, I can't share sources or jars here. But I'd be happy to post log files or run another copy of Sourcetrail.

I'm running a just-downloaded copy of Sourcetrail 2017.2.0. Also annoying, Sourcetrail didn't like the use of private sun.* classes, which, while indeed shouldn't be used, are sometimes present in legacy code -- which I might then want to read/analyze. :)

mlangkabel commented 7 years ago

Hi @LouisStAmour. Thanks for reaching out to us. The first question that comes to my mind regarding the issue with Java5 jars is: Is Sourcetrail unable to solve any symbol declared inside those jars? Or are there just some of the symbols declared in those jars unsolved while others are working fine?

I'm asking because in our 2017.2 release we still had a lot of issues when trying to solve types and methods declared in jars, but I guess that you'll be glad when I tell you that we are planning to release Sourcetrail 2017.3 TODAY! And there have been a lot of fixes regarding type solving in jar files :)

Is it possible for you to provide a small code snippet that illustrates the issue with private sun.* classes? That would help us a lot in reproducing the issue.

LouisStAmour commented 7 years ago

Partial indexing is certainly possible. I’ll see what I can do re the sun.* example. Looking forward to trying 2017.3!

mlangkabel commented 7 years ago

Does this code snippet resemble your use case?

package sun.nio.ch;
import java.io.FileDescriptor;
import java.nio.channels.spi.SelectorProvider;

class MySourceChannel extends SourceChannelImpl {
  public MySourceChannel(SelectorProvider sp, FileDescriptor fd) {
    super(sp, fd);
  }
}

public class Main {
  public static void main(String[] args) {
    new MySourceChannel(null, FileDescriptor.in);
  }
}

I've just tried to index that code with Sourcetrail 2017.2 and there have been a couple of unsolved symbols. With 2017.3 they were gone :)

Partial indexing sounds great. That means that Java5 jars do not create an issue on their own. Instead the issues that you are experiencing are well known. Most of them will be fixed with 2017.3 and we will be working hard on fixing the rest!

egraether commented 7 years ago

Release 2017.3 got delayed until tomorrow. :(

LouisStAmour commented 7 years ago

Okay, so I downloaded 2017.3.22 and the good news is that all the errors I had before with missing imports and sun.* classes are gone.

But I have a very high number of unsolved-symbol mentions. Same as before.

If I simplify one, it's:

package something.else;

import org.apache.http.impl.client.DefaultHttpClient;
//... followed by, in a private function:
DefaultHttpClient httpClient = new DefaultHttpClient();

In the above example, new DefaultHttpClient() is highlighted in grey as unsolved.

Another strange one, from the same file, is configuring a org.jboss.resteasy.client.ClientRequest object (Java 6, so perhaps the bytecode version doesn't matter...) where calling ClientRequest header(String headerName, Object value) is solved but calling public ClientRequest accept(String accept) the next line down, is not. I do see that there's two versions of the accept method in that file, one takes a MediaType, and the other below it, takes a String. It's the String one it doesn't recognize.

Actually ... the same is true of the DefaultHttpClient. The class has 4 constructors, and here we're using the last of the 4, and it's unrecognized.

Okay, maybe I should edit the title of the bug to "Add better support for method overloading"? Is that all it is now?

Another example -- this kind of code: https://github.com/xabe/JAX-RS/blob/411dcb3a2f075b3954fb988925cdf0e010f52612/JbossRestClient/src/main/java/es/xabe/arquitectura/Client.java#L20 Note the use of generics in request.get(Acta.class), I'm seeing unsolved references for similar code, namely getEntity() and getResponseStatus(). While there are 6 different versions of getEntity, there's only one getResponseStatus so ... that can't be everything. :)

I wonder if a good fuzz test would be to download a few random, active Java projects from Github and see how many unresolved symbols you get? ;-)

mlangkabel commented 7 years ago

Thanks for all those hints! It looks like the symbol solver mostly gets stuck when trying to resolve an ambiguous method call. We are already testing on active Java projects from Github, as you recommended :) With this release we got the rate of unsolved-symbols from 6.5% down to 1.7%. Fixing these remaining 1.7% will probably resolve some of your issues as well. After getting it to 0% we will expanding our scope of tested source code to further projects!

mlangkabel commented 7 years ago

@LouisStAmour please try again with Sourcetrail 2017.3.48. Those unsolved symbols should be gone by now.

LouisStAmour commented 7 years ago

Yep, they're all resolved, just some are non-indexed now, different issue. Ideally you guys would integrate Fernflower ( https://github.com/JetBrains/intellij-community/tree/master/plugins/java-decompiler/engine ) or JavaDoc to better analyze third-party JAR method calls, though decompilation might be restricted in license agreements if the JARs are commercial. Attaching source or doc JARs might also be an option.

mlangkabel commented 7 years ago

Thanks for the feedback! Non-indexed symbols are nothing to be afraid of. As you probably already figured out, those are symbols that aren't defined inside the indexed source code. They probably come from one of those JAR dependencies. Right now Sourcetrail can only index Java source code files. Adding support for indexing source JARs is certainly a good idea.

Also: Thanks for the hint to Fernflower!