INRIA / spoon

Spoon is a metaprogramming library to analyze and transform Java source code. :spoon: is made with :heart:, :beers: and :sparkles:. It parses source files to build a well-designed AST with powerful analysis and transformation API.
http://spoon.gforge.inria.fr/
Other
1.76k stars 352 forks source link

`CtModel#getAllTypes` returns a class which is not part of the AST #3010

Open msteinbeck opened 5 years ago

msteinbeck commented 5 years ago

Steps to reproduce:

  1. Clone this repository: https://github.com/lets-blade/blade
  2. Check out this revision: 926913f3eaa2bc44b4bb8703e7d61e773cccc4d0
  3. Run the following code snippet (replace /path/to/repo):
    Launcher launcher = new Launcher();
    launcher.addInputResource("/path/to/repo/src/main/java");
    List<CtType> types = launcher.buildModel().getAllTypes().stream()
        .filter(t -> !t.getPosition().isValidPosition())
    .collect(Collectors.toList());
    System.out.println(types);

Result:

[class HttpVersion {}]

If I use grep to search for HttpVersion:

grep -r HttpVersion *

I get the following output:

src/main/java/com/blade/server/netty/HttpServerHandler.java:import static io.netty.handler.codec.http.HttpVersion.HTTP_1_1;
src/main/java/com/blade/server/netty/StaticFileHandler.java:import static io.netty.handler.codec.http.HttpVersion.HTTP_1_1;
src/main/java/com/blade/server/netty/StaticFileHandler.java:        HttpResponse httpResponse = new DefaultHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK);
src/main/java/com/blade/server/netty/StaticFileHandler.java:        FullHttpResponse httpResponse = new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, OK, staticInputStream.asByteBuf());
src/main/java/com/blade/server/netty/RouteMethodHandler.java:import static io.netty.handler.codec.http.HttpVersion.HTTP_1_1;
src/main/java/com/blade/mvc/wrapper/OutputStreamWrapper.java:            HttpResponse httpResponse = new DefaultHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK);
src/main/java/com/blade/mvc/handler/DefaultExceptionHandler.java:import io.netty.handler.codec.http.HttpVersion;
src/main/java/com/blade/mvc/handler/DefaultExceptionHandler.java:            FullHttpResponse fullHttpResponse = new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.valueOf(response.statusCode()), buffer);

So HttpVersion is not part of my input source. I tested version 7.4.0 and 7.5.0-beta-21.

nharrand commented 5 years ago

Hello @msteinbeck ,

Thanks for the report. From what I can read here, in noClasspath mode, when importing a static field, we create a class holding the field to be imported. So it seems to be intentional. But I get that it is weird that these types are listed byt getAllTypes() I am not sure what behavior would be less misleading for spoon users. We could mark these types and filter them out in getAllTypes() but then it might lead to some other weird discrepancies.

WDYT @monperrus ?

monperrus commented 5 years ago

Indeed, there is an ambiguity in the spec: "returns all top-level types of the model".

Two options to resolve this ambiguity:

I prefer the latter, and we can also add a method "getAllTypesInclShadowTypes()"

msteinbeck commented 5 years ago

I think excluding shadow types (those built from binary classes) will break incremental builds, because previously compiled files are added to the classpath while creating a new increment. In the example stated above, a class which neither is created from a .class file nor has been added to the input source is returned by getAllTypes.

monperrus commented 5 years ago

OK.

Would it work if we change the incremental builder to call getAllTypesInclShadowTypes instead of getAllTypes (new version)?

monperrus commented 5 years ago

@msteinbeck WDYT?