kscripting / kscript

Scripting enhancements for Kotlin
MIT License
2.08k stars 126 forks source link

4.X Regression: Invalid script does not report error on second run #349

Closed holgerbrandl closed 2 years ago

holgerbrandl commented 2 years ago

I've discovered an imho major regression.

brandl@ubuntu:kscript$ kscript invalid_script.kts 
[kscript] Resolving log4j:log4j:1.2.14...
[kscript] [ERROR] compilation of scriplet failed
[kscript] [ERROR] Command     : bash -c kotlinc  -classpath /home/brandl/.m2/repository/log4j/log4j/1.2.14/log4j-1.2.14.jar -d '/home/brandl/.kscript/cache/jar_dc3cc43e50a91471262373fddebc753b/scriplet.jar' /home/brandl/.kscript/cache/jar_dc3cc43e50a91471262373fddebc753b/Invalid_script.kts /home/brandl/.kscript/cache/jar_dc3cc43e50a91471262373fddebc753b/Main_Invalid_script.kt
[kscript] [ERROR] Exit Code   : 1   
[kscript] [ERROR] Stdout      : 
[kscript] [ERROR] Stderr      : 
[kscript] [ERROR] /home/brandl/.kscript/cache/jar_dc3cc43e50a91471262373fddebc753b/Invalid_script.kts:14:43: error: expecting ')'
[kscript] [ERROR] println("kscript with annotations rocks!")
[kscript] [ERROR]                                           ^
[kscript] [ERROR] /home/brandl/.kscript/cache/jar_dc3cc43e50a91471262373fddebc753b/Invalid_script.kts:14:1: error: too many arguments for public open fun getRootLogger(): Logger! defined in org.apache.log4j.Logger
[kscript] [ERROR] println("kscript with annotations rocks!")
[kscript] [ERROR] ^
[kscript] [ERROR] .stderr
brandl@ubuntu:kscript$ kscript invalid_script.kts 
[kscript] [ERROR] /home/brandl/.kscript/cache/jar_dc3cc43e50a91471262373fddebc753b/jarArtifact.descriptor

Now the same with 3.1

brandl@ubuntu:kscript$ sdk default kscript 3.1.0
Default kscript version set to 3.1.0
brandl@ubuntu:kscript$ rm -rf .m2/repository/log4j/log4j/1.2.14/; kscript --clear-cache
Cleaning up cache...
brandl@ubuntu:kscript$ kscript invalid_script.kts 
[kscript] Resolving dependencies...
[kscript] Resolving log4j:log4j:1.2.14...
[kscript] Resolving com.github.holgerbrandl:kscript-annotations:1.4...
[kscript] Dependencies resolved
[kscript] [ERROR] compilation of 'invalid_script.kts' failed
invalid_script.kts:15:43: error: expecting ')'
println("kscript with annotations rocks!")
                                          ^
invalid_script.kts:15:1: error: too many arguments for public open fun getRootLogger(): Logger! defined in org.apache.log4j.Logger
println("kscript with annotations rocks!")
^

brandl@ubuntu:kscript$ kscript invalid_script.kts 
[kscript] [ERROR] compilation of 'invalid_script.kts' failed
invalid_script.kts:15:43: error: expecting ')'
println("kscript with annotations rocks!")
                                          ^
invalid_script.kts:15:1: error: too many arguments for public open fun getRootLogger(): Logger! defined in org.apache.log4j.Logger
println("kscript with annotations rocks!")
^

So it seems that it's using some stale cache even if the script compilation failed. The correct behavior would be to show the same compiler error in the second run as well.

There was/is no test coverage for this process, so I guess have to blame myself my missing it in the first place. Anyway, @aartiPl would you have and idea and time to look into this problem? I still have to understand the revised impl better first.

aartiPl commented 2 years ago

Yes, I will look at all regressions.

aartiPl commented 2 years ago

The root cause of this issue is easy to fix. I will still have to work on the test case, which is not that easy.

I am putting here partial PR, which can be merged if the fix is required quickly: https://github.com/holgerbrandl/kscript/pull/350

aartiPl commented 2 years ago

Issue resolved - code fixed and test case added.