google / guava

Google core libraries for Java
Apache License 2.0
50.2k stars 10.91k forks source link

ClassPath.getTopLevelClasses() does not return a top-level class with $ character #3349

Open suztomo opened 5 years ago

suztomo commented 5 years ago

@netdpb and I found that ClassPath.getTopLevelClasses() does not return top-level class with $ character in its name. Example class: com.google.cloud.bigquery.$AutoValue_Labels in google-cloud-bigquery-1.56.0.jar.

Test case

  @Test
  public void testGuavaTopLevelClass() throws Exception {
    // google-cloud-bigquery-1.56.0.jar contains com.google.cloud.bigquery.$AutoValue_Labels
    URL jarFileUrl =
        new URL(
            "file:///home/suztomo/.m2/repository/com/google/cloud/google-cloud-bigquery/1.56.0/google-cloud-bigquery-1.56.0.jar");
    URL[] jarFileUrls = new URL[] {jarFileUrl};

    URLClassLoader classLoaderFromJar = new URLClassLoader(jarFileUrls, null);

    com.google.common.reflect.ClassPath classPath =
        com.google.common.reflect.ClassPath.from(classLoaderFromJar);

    ImmutableList<String> allClassNames =
        classPath.getAllClasses().stream()
            .map(classInfo -> classInfo.getName())
            .collect(toImmutableList());

    // This code does not return class com.google.cloud.bigquery.$AutoValue_Labels
    // Guava only checks the existence of '$'
    // https://github.com/google/guava/blob/master/guava/src/com/google/common/reflect/ClassPath.java#L85
    ImmutableList<String> topLevelClassNames =
        classPath.getTopLevelClasses().stream()
            .map(classInfo -> classInfo.getName())
            .collect(toImmutableList());

    String class$AutoValue_Labels = "com.google.cloud.bigquery.$AutoValue_Labels";
    System.out.println("In all classes? " + allClassNames.contains(class$AutoValue_Labels)); // true
    System.out.println(
        "In top-level classes? " + topLevelClassNames.contains(class$AutoValue_Labels)); // false. It should be true.
  }

JLS 3.8 states that dollar sign ('$') is a valid Java letter.

AutoValue has @Memoized annotation, which creates class $AutoValue_XXXX and AutoValue_XXXX extends $AutoValue_XXXX in the same package.

eamonnmcmanus commented 5 years ago

The issue of course is that ClassPath.getTopLevelClasses() assumes that a class is nested if it has $ in its name. If a class Bar is nested inside com.example.Foo then its full name will be com.example.Foo$Bar, but it is perfectly legal to define a class whose actual name is Foo$Bar. So the heuristic that getTopLevelClasses() uses cannot work in general. I believe the only way to be sure would be to look for the InnerClasses attribute inside the .class file, and that implies that this method would become quite a bit slower and more complicated. Alternatively, we could disregard sequences of $ characters at the beginning and end of a class name when determining nestedness. That would cover the case at hand, while still not being a general solution.

suztomo commented 5 years ago

Thank you for response. Our project (cloud-opensource-java) will use getAllClasses instead. So I don't expect Guava to implement something specific to Autovalue.

Maaartinus commented 5 years ago

@eamonnmcmanus

Alternatively, we could disregard sequences of $ characters at the beginning and end of a class name when determining nestedness.

I guess, doing this would be a clear improvement for a minimum cost.

You could also use a heuristics like "when there's no Foo, then Foo$Bar is not a nested class" (which also takes care of the leading dollar). This feels pretty hacky, but solves cases like "sometool" generating sometool$SomeClass (I can't recall where I saw it and if it really looked like this).