kastree icon indicating copy to clipboard operation
kastree copied to clipboard

Parse error: Muliple annotations with line comment

Open jschneider opened this issue 5 years ago • 12 comments

That code can not be parsed:

@Suppress("unused") //
@JsName("daName")
fun myname(){
....

If the both annotations are switched, it works just fine.

--> The exception occurs if:

  • there are two annotations
  • there is a line comment between these two annotations

Stack trace: ` ava.lang.IllegalStateException: Unrecognized modifier: // public API

at kastree.ast.psi.Converter$convertModifiers$1.invoke(Converter.kt:367)
at kastree.ast.psi.Converter$convertModifiers$1.invoke(Converter.kt:16)
at kotlin.sequences.TransformingSequence$iterator$1.next(Sequences.kt:172)
at kotlin.sequences.FilteringSequence$iterator$1.calcNext(Sequences.kt:132)
at kotlin.sequences.FilteringSequence$iterator$1.hasNext(Sequences.kt:156)
at kotlin.sequences.SequencesKt___SequencesKt.toCollection(_Sequences.kt:702)
at kotlin.sequences.SequencesKt___SequencesKt.toMutableList(_Sequences.kt:732)
at kotlin.sequences.SequencesKt___SequencesKt.toList(_Sequences.kt:723)
at kastree.ast.psi.Converter.convertModifiers(Converter.kt:371)
at kastree.ast.psi.Converter.convertModifiers(Converter.kt:350)
at kastree.ast.psi.Converter.convertFunc(Converter.kt:304)
at kastree.ast.psi.Converter.convertDecl(Converter.kt:195)
at kastree.ast.psi.Converter.convertFile(Converter.kt:293)
at kastree.ast.psi.Parser.parseFile(Parser.kt:23)
at kastree.ast.psi.Parser.parseFile$default(Parser.kt:23)

`

jschneider avatar Oct 05 '19 20:10 jschneider

I am not really doing much to this library these days (my day job has taken me away fro Kotlin mostly). It appears this might be a bug in the Kotlin PSI libraries, but admittedly I probably use an old version and this may have been fixed in the meantime.

cretz avatar Oct 07 '19 14:10 cretz

I have hit this same issue and have found another case when the comment crashes the parser, when the line comment is between the lines with annotations:

@Suppress("unused")
// ...
@JsName("daName")

martinflorek avatar Oct 10 '19 08:10 martinflorek

I have changed Kotlin to the latest 1.3.50, this includes the latest PSI libraries, I think, and it still crashes.

The crash is because of the line 367 in Converter.kt. Returning null instead of error("Unrecognized modifier: ${node.text}") ignores "unknown" cases. I do not think this is correct solution because I do not know what exactly what other cases I am breaking...

Unit tests fail for this case because Writer.write(node).trim() will write the original source code without the comments:


    @Test
    fun testCommentBetweenAnnotations() {
        assertParseAndWriteExact("""
            @Annotation1("abc")
            // line comment between annotations crashes parser
            @Annotation2("123")
            fun parse() {}
        """.trimIndent())
    }


    fun assertParseAndWriteExact(code: String) {

        val node = Parser.parseFile(code)
        val identityNode = MutableVisitor.preVisit(node) { v, _ -> v }

        assertEquals(
            code.trim(),
            Writer.write(node).trim(),
            "Parse -> Write for $code, not equal")

        assertEquals(
            code.trim(),
            Writer.write(identityNode).trim(),
            "Parse -> Identity Transform -> Write for $code, failed")
    }

martinflorek avatar Oct 15 '19 11:10 martinflorek

Hi @martinflorek,

maybe my Kotlin parsing library can help you: https://github.com/kotlinx/ast

It is a generic AST parsing library, Kotlin is currently the only supported language. The library is designed that other languages can be easily added. kotlinx.ast does not use the kotlin compiler for paring, it is using ANTLR (the kotlin variant https://github.com/Strumenta/antlr-kotlin) using the official kotlin grammer (https://kotlinlang.org/docs/reference/grammar.html)

One Component is "Klass", a collection of language independent data classes used to represent and easily access the AST.

Your example code will be parsed fine: https://github.com/kotlinx/ast/commit/d4da88af8bdbcc9d4b5f2a125ee0b985301905e0

  • MultiAnnotationsWithLineComment.kt.txt contains the source of your example
  • MultiAnnotationsWithLineComment.raw.txt contains the raw AST
  • MultiAnnotationsWithLineComment.summary.txt contains the "Klass" Representation you can use:
PackageHeader()
importList
KlassDeclaration(fun parse)
  KlassAnnotation(Annotation1)
    KlassArgument()
      KlassString
        "abc"
  KlassAnnotation(Annotation2)
    KlassArgument()
      KlassString
        "123"

using kotlin code, this is:

listOf(
  PackageHeader(),
  DefaultAstNode("importList"),
  KlassDeclaration(
    keyword = "fun",
    identifier = KlassIdentifier("parse"),
    annotations = listOf(
      KlassAnnotation(KlassIdentifier("Annotation1")),
      KlassAnnotation(KlassIdentifier("Annotation2"))
    )
)

There are some parts missing, for example the importList is not converted into an easy-to-use data class. But I'm already using the library for AST parsing. You can just open the project in IDEA or run ./gradlew clean check publishToMavenLocal if you want to use it locally.

Please let me know if you have any questions, or just open an issue :-)

drieks avatar Oct 15 '19 20:10 drieks

@drieks I tried it but it complains "Could not find com.strumenta.antlr-kotlin:antlr-kotlin-gradle-plugin:0.0.5". I added the jitpack repo into repositories (maven("https://jitpack.io")), based on their examples for versions > 0.0.4, but it is still cannot find it. Any ides?

martinflorek avatar Oct 16 '19 08:10 martinflorek

@martinflorek : Oh sorry, please try to replace version "0.0.5" of antlr-kotlin with "2e25fd550f" (the latest revision listed here: https://jitpack.io/#com.strumenta/antlr-kotlin when clicking on "Commits")

drieks avatar Oct 16 '19 10:10 drieks

@drieks there are more and more issues...

> Could not resolve all files for configuration ':common:jvmCompileClasspath'.
   > Could not find com.strumenta.antlr-kotlin:antlr-kotlin-runtime-jvm:0.0.5.
     Searched in the following locations:
       - https://repo.maven.apache.org/maven2/com/strumenta/antlr-kotlin/antlr-kotlin-runtime-jvm/0.0.5/antlr-kotlin-runtime-jvm-0.0.5.module
       - https://repo.maven.apache.org/maven2/com/strumenta/antlr-kotlin/antlr-kotlin-runtime-jvm/0.0.5/antlr-kotlin-runtime-jvm-0.0.5.pom
       - https://jcenter.bintray.com/com/strumenta/antlr-kotlin/antlr-kotlin-runtime-jvm/0.0.5/antlr-kotlin-runtime-jvm-0.0.5.module
       - https://jcenter.bintray.com/com/strumenta/antlr-kotlin/antlr-kotlin-runtime-jvm/0.0.5/antlr-kotlin-runtime-jvm-0.0.5.pom
       - https://jitpack.io/com/strumenta/antlr-kotlin/antlr-kotlin-runtime-jvm/0.0.5/antlr-kotlin-runtime-jvm-0.0.5.module
       - https://jitpack.io/com/strumenta/antlr-kotlin/antlr-kotlin-runtime-jvm/0.0.5/antlr-kotlin-runtime-jvm-0.0.5.pom
     Required by:
         project :common > com.strumenta.antlr-kotlin:antlr-kotlin-runtime:2e25fd550f

martinflorek avatar Oct 16 '19 13:10 martinflorek

Hi @martinflorek,

it seems that I have always used the locally build version of antlr-kotlin.

I fixed the bug in antlr-kotlin that forced the usage of version 0.0.5 (https://github.com/Strumenta/antlr-kotlin/issues/29)

I added kotlinx.ast to jitpack, you can include it now in your project without building it locally:

In settings.gradle.kts add this line: (otherwise, the jars are not resolved):

// Enables KotlinMultiplatform publication and resolving (in dependencies)
enableFeaturePreview("GRADLE_METADATA")

You have to add jitpack to build.gradle.kts:

repositories {
  maven("https://jitpack.io")
}

Now, you can add the dependency to kotlinx.ast:

kotlin {
    jvm()

    sourceSets {
        val commonMain by getting {
            dependencies {
                api("com.github.kotlinx.ast:kotlin:0e92ad018e")
            }
        }
    }
}

This should add kotlinx.ast.kotlin with transitive dependencies to kotlinx.ast.common and antlr-kotlin.

(currently, for multiplatform only common and jvm is supported, but native and js support should be possible without too much work)

If you don't use kotlin-multiplatform, please try to add implementation("com.github.kotlinx.ast:kotlin:0e92ad018e") or implementation("com.github.kotlinx.ast:kotlin-jvm:0e92ad018e").

drieks avatar Oct 16 '19 21:10 drieks

@drieks it works now, thank you. But it crashes on parsing annotation values with \$ e.g. @Value("\${abc}") :( Which is a much bigger problem, for me, than the line comments.

martinflorek avatar Oct 17 '19 12:10 martinflorek

Hi @martinflorek,

fixed, please try Version 67b2331974.

I added StringComponentEscape for escaped characters in Strings. When I find some time, I will add a test that tries to parse a large kotlin codebase to find such errors. https://github.com/kotlinx/ast/commit/67b23319747afb8479c3b25b3683db8288294798

@Value("\${abc}") will now be parsed as:

KlassAnnotation(Value)
  KlassArgument()
    KlassString
      Escape("\$")
      "{abc}"

drieks avatar Oct 17 '19 18:10 drieks

@drieks Now the parsing of "escaped" values in annotations work, but I have found another issue. It does not belong here, so I created a new issue in your repo https://github.com/kotlinx/ast/issues/1

@cretz I still prefer your parser, especially its speed, and it handles everything I have thrown at it except for the comments after annotations. Thank you for this library.

martinflorek avatar Oct 18 '19 11:10 martinflorek

Thanks (it's the Kotlin core code that I use, I didn't write the parser)...I apologize as I have left the JVM ecosystem for the time being and don't have time to work on this or any of my other Kotlin libs.

cretz avatar Oct 18 '19 15:10 cretz