Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ClassNotFoundException with google-java-format 1.8 #562

Closed
benkard opened this issue May 2, 2020 · 4 comments · Fixed by #563
Closed

ClassNotFoundException with google-java-format 1.8 #562

benkard opened this issue May 2, 2020 · 4 comments · Fixed by #563
Labels

Comments

@benkard
Copy link
Contributor

benkard commented May 2, 2020

google-java-format 1.8 removes the feature to remove Javadoc-only imports (google/google-java-format@73c522a), but Spotless still tries to access the feature (

Class<?> removeJavadocOnlyClass = classLoader.loadClass(REMOVE_UNUSED_IMPORT_JavadocOnlyImports);
). This causes an exception to be thrown when I attempt to use the Maven plugin with 1.8 as the google-java-format version:

com.google.googlejavaformat.java.RemoveUnusedImports$JavadocOnlyImports
java.lang.ClassNotFoundException: com.google.googlejavaformat.java.RemoveUnusedImports$JavadocOnlyImports
	at java.base/java.net.URLClassLoader.findClass(URLClassLoader.java:435)
	at com.diffplug.spotless.FeatureClassLoader.findClass(FeatureClassLoader.java:76)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:589)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:522)
	at com.diffplug.spotless.java.GoogleJavaFormatStep$State.createFormat(GoogleJavaFormatStep.java:128)
	at com.diffplug.spotless.FormatterStepImpl$Standard.format(FormatterStepImpl.java:76)
	at com.diffplug.spotless.FormatterStep$Strict.format(FormatterStep.java:76)
	at com.diffplug.spotless.Formatter.compute(Formatter.java:230)
	at com.diffplug.spotless.Formatter.applyToAndReturnResultIfDirty(Formatter.java:192)
	at com.diffplug.spotless.Formatter.applyTo(Formatter.java:175)
	at com.diffplug.spotless.maven.SpotlessApplyMojo.process(SpotlessApplyMojo.java:45)
	at com.diffplug.spotless.maven.AbstractSpotlessMojo.execute(AbstractSpotlessMojo.java:127)
	at com.diffplug.spotless.maven.AbstractSpotlessMojo.execute(AbstractSpotlessMojo.java:120)
	at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:137)
	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:210)
	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:156)
	at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:148)
	at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:117)
	at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:81)
	at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build(SingleThreadedBuilder.java:56)
	at org.apache.maven.lifecycle.internal.LifecycleStarter.execute(LifecycleStarter.java:128)
	at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:305)
	at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:192)
	at org.apache.maven.DefaultMaven.execute(DefaultMaven.java:105)
	at org.apache.maven.cli.MavenCli.execute(MavenCli.java:957)
	at org.apache.maven.cli.MavenCli.doMain(MavenCli.java:289)
	at org.apache.maven.cli.MavenCli.main(MavenCli.java:193)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:564)
	at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced(Launcher.java:282)
	at org.codehaus.plexus.classworlds.launcher.Launcher.launch(Launcher.java:225)
	at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode(Launcher.java:406)
	at org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:347)

Configuration:

        <plugin>
          <groupId>com.diffplug.spotless</groupId>
          <artifactId>spotless-maven-plugin</artifactId>
          <version>1.30.0</version>
          <configuration>
            <java>
              <googleJavaFormat>
                <version>1.8</version>
                <style>GOOGLE</style>
              </googleJavaFormat>
            </java>
          </configuration>
        </plugin>
@nedtwigg nedtwigg added the bug label May 2, 2020
@nedtwigg
Copy link
Member

nedtwigg commented May 2, 2020

Thanks for bug report! We've already got 1.1-specific code, we'll just need to add post-1.8 code.

Looks to me like we'll need to change these two places, this one:

Class<?> removeJavadocOnlyClass = classLoader.loadClass(REMOVE_UNUSED_IMPORT_JavadocOnlyImports);
Object removeJavadocConstant = Enum.valueOf((Class<Enum>) removeJavadocOnlyClass, REMOVE_UNUSED_IMPORT_JavadocOnlyImports_Keep);
Method removeUnusedMethod = removeUnusedClass.getMethod(REMOVE_UNUSED_METHOD, String.class, removeJavadocOnlyClass);

and this one:

Class<?> removeJavadocOnlyClass = classLoader.loadClass(REMOVE_UNUSED_IMPORT_JavadocOnlyImports);
Object removeJavadocConstant = Enum.valueOf((Class<Enum>) removeJavadocOnlyClass, REMOVE_UNUSED_IMPORT_JavadocOnlyImports_Keep);
Method removeUnusedMethod = removeUnusedClass.getMethod(REMOVE_UNUSED_METHOD, String.class, removeJavadocOnlyClass);

As for parsing that we're on a post-1.8 version, we've used simple regexes for that in the past:

Matcher matcher = VERSION_MATCHER.matcher(version);

Matcher versionMatcher = VERSION_PRE_2_0.matcher(version);
if (versionMatcher.matches()) {
mavenCoordinate = MAVEN_COORDINATE_PRE_2_0;
} else {
mavenCoordinate = MAVEN_COORDINATE;
}

Happy to take a PR for this. If the reflection is too messy, happy to take a PR for #524, which would make fixing this easier.

benkard added a commit to benkard/spotless that referenced this issue May 2, 2020
In google-java-format 1.8, the signature of
RemoveUnusedImports#removeUnusedImports changed and the
RemoveUnusedImports$JavadocOnlyImports class was removed entirely.
With this patch, GoogleJavaFormatStep detects which version of the
method is available and calls the right one.

Fixes diffplug#562.
@benkard
Copy link
Contributor Author

benkard commented May 2, 2020

Created PR #563. Is the approach taken there OK?

@nedtwigg
Copy link
Member

nedtwigg commented May 2, 2020

Thanks very much, code looks great. I tried adding a test, but google-java-format requires Java 11. The next eclipse formatter is going to require 11 as well. I'm gonna dabble a bit in this PR to figure out a way to cleanly get our CI going on both 8 and 11 for a while. We'll have your PR merged and shipped by Monday, ideally with CI, but if I don't finish in time we'll still get it out the door.

@benkard
Copy link
Contributor Author

benkard commented May 3, 2020

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants