Fix latest Scala v2.13 version lookup to avoid NullPointerException

majjhima commented 2 years ago

ScalaVersion latest_2_13_version and byVersion are not defined at the time that url is defined, so we get an NPE when ScalaLoader is initialized.

majjhima commented 2 years ago

When I start a Paper server v1.18.1-216 with the current ScalaLoader v0.17.13 (built from source or downloaded from repo.repsy.io), I get the following error:

[16:47:59 WARN]: [ScalaLoader] Error while trying to replace the standard JavaPluginLoader.                                                                                       
java.lang.ExceptionInInitializerError: null                                                                                                                                       
        at xyz.janboerman.scalaloader.plugin.description.DescriptionScanner$ScalaAnnotationVisitor.visitEnum(DescriptionScanner.java:244) ~[ScalaLoader-0.17.13-SNAPSHOT.jar:?]   
        at xyz.janboerman.scalaloader.libs.asm.ClassReader.readElementValue(ClassReader.java:3074) ~[ScalaLoader-0.17.13-SNAPSHOT.jar:?]                                          
        at xyz.janboerman.scalaloader.libs.asm.ClassReader.readElementValues(ClassReader.java:2993) ~[ScalaLoader-0.17.13-SNAPSHOT.jar:?]                                         
        at xyz.janboerman.scalaloader.libs.asm.ClassReader.accept(ClassReader.java:608) ~[ScalaLoader-0.17.13-SNAPSHOT.jar:?]                                                     
        at xyz.janboerman.scalaloader.libs.asm.ClassReader.accept(ClassReader.java:424) ~[ScalaLoader-0.17.13-SNAPSHOT.jar:?]                                                     
        at xyz.janboerman.scalaloader.plugin.description.DescriptionScanner.<init>(DescriptionScanner.java:70) ~[ScalaLoader-0.17.13-SNAPSHOT.jar:?]                              
        at xyz.janboerman.scalaloader.plugin.ScalaPluginLoader.scanJar(ScalaPluginLoader.java:235) ~[ScalaLoader-0.17.13-SNAPSHOT.jar:?]                                          
        at xyz.janboerman.scalaloader.plugin.ScalaPluginLoader.init(ScalaPluginLoader.java:125) ~[ScalaLoader-0.17.13-SNAPSHOT.jar:?]                                             
        at xyz.janboerman.scalaloader.plugin.ScalaPluginLoader.<init>(ScalaPluginLoader.java:87) ~[ScalaLoader-0.17.13-SNAPSHOT.jar:?]                                            
        at xyz.janboerman.scalaloader.ScalaLoader.<init>(ScalaLoader.java:79) ~[ScalaLoader-0.17.13-SNAPSHOT.jar:?]                                                               
        at jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) ~[?:?]                                                                                  
        at jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(Unknown Source) ~[?:?]                                                                                  
        at jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(Unknown Source) ~[?:?]                                                                              
        at java.lang.reflect.Constructor.newInstanceWithCaller(Unknown Source) ~[?:?]                                                                                             
        at java.lang.reflect.ReflectAccess.newInstance(Unknown Source) ~[?:?]            
        at jdk.internal.reflect.ReflectionFactory.newInstance(Unknown Source) ~[?:?]                                                                                              
        at java.lang.Class.newInstance(Unknown Source) ~[?:?]                                                                                                                     
        at org.bukkit.plugin.java.PluginClassLoader.<init>(PluginClassLoader.java:83) ~[paper-api-1.18.1-R0.1-SNAPSHOT.jar:?]                                                             at org.bukkit.plugin.java.JavaPluginLoader.loadPlugin(JavaPluginLoader.java:153) ~[paper-api-1.18.1-R0.1-SNAPSHOT.jar:?]                                                  
        at org.bukkit.plugin.SimplePluginManager.loadPlugin(SimplePluginManager.java:415) ~[paper-api-1.18.1-R0.1-SNAPSHOT.jar:?]                                                 
        at org.bukkit.plugin.SimplePluginManager.loadPlugins(SimplePluginManager.java:323) ~[paper-api-1.18.1-R0.1-SNAPSHOT.jar:?]                                                
        at org.bukkit.craftbukkit.v1_18_R1.CraftServer.loadPlugins(CraftServer.java:420) ~[paper-1.18.1.jar:git-Paper-216]                                                        
        at net.minecraft.server.dedicated.DedicatedServer.initServer(DedicatedServer.java:285) ~[paper-1.18.1.jar:git-Paper-216]                                                  
        at net.minecraft.server.MinecraftServer.runServer(MinecraftServer.java:1211) ~[paper-1.18.1.jar:git-Paper-216]                                                            
        at net.minecraft.server.MinecraftServer.lambda$spin$0(MinecraftServer.java:317) ~[paper-1.18.1.jar:git-Paper-216]                                                         
        at java.lang.Thread.run(Unknown Source) ~[?:?]                                                                                                                            
Caused by: java.lang.NullPointerException: Cannot invoke "xyz.janboerman.scalaloader.plugin.description.ScalaVersion.getVersion()" because "xyz.janboerman.scalaloader.plugin.desc
ription.ScalaVersion.latest_2_13" is null                                                                                                                                         
        at xyz.janboerman.scalaloader.plugin.description.ScalaVersion.urls(ScalaVersion.java:90) ~[ScalaLoader-0.17.13-SNAPSHOT.jar:?]                                            
        at xyz.janboerman.scalaloader.plugin.description.ScalaVersion.<init>(ScalaVersion.java:73) ~[ScalaLoader-0.17.13-SNAPSHOT.jar:?]                                          
        at xyz.janboerman.scalaloader.plugin.description.ScalaVersion.<clinit>(ScalaVersion.java:44) ~[ScalaLoader-0.17.13-SNAPSHOT.jar:?]                                        
        ... 26 more                                                                                                                                                               
[16:47:59 INFO]: [ScalaLoader] Did not manage to override the default .jar file association. Plugins may not load in the expected order.

If I build the v0.17.12 tag from source, then ScalaLoader does load OK and the examples all work. However, I can't find the jar files on jitpack.io (as documented in the README) or repo.repsy.io in order to configure a direct dependency from my own plugin pom.xml file. I don't see any jar files listed as artifacts in https://jitpack.io/com/github/Jannyboy11/ScalaPluginLoader/v0.17.12/build.log

I can download the v0.17.11 jar from spigotmc.org, but there is no tag for this version in this git repo to checkout the examples at.

So, I put together this pull request which seems to fix the issue for v0.17.13.

Jannyboy11 commented 2 years ago

Hi, apologies for the inactivity and thank you for the report! Essentially what this is PR is doing is to re-introduce the hardcodedness of the latest version in the urls() method. I created that latest_2_13 field and that loop so that I wouldn't have to think about changing the urls() method every time a new version of Scala 2.13 was added..

As you pointed out, the initialization order of fields is now borked, so I can think of two solutions:

change it back so that the urls() method uses ScalaVersion.2_13_8 (this functionally is equivalent to what your PR does)
Fix initialization order issue by introducing yet another class that hosts a few constants ands gets initialized before any of the ScalaVersion fields are accessed.

ScalaVersion is a public-facing api, and I do not wish to publish a magic constant that I have to keep up-to-date myself every time a new version of Scala is added.

I'll get onto this tonight.

Jannyboy11 commented 2 years ago

I fixed it by making the urls map generate lazily. Will create a test, and push in a bit.

Jannyboy11 commented 2 years ago

I added a unit test to prevent this from happening in the future: https://github.com/Jannyboy11/ScalaPluginLoader/blob/master/ScalaLoader/src/test/java/xyz/janboerman/scalaloader/plugin/description/ScalaVersionTest.java

Jannyboy11 / ScalaPluginLoader

Fix latest Scala v2.13 version lookup to avoid NullPointerException #15