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

Parse error: Muliple annotations with line comment #33

Open
jschneider opened this issue Oct 5, 2019 · 12 comments
Open

Parse error: Muliple annotations with line comment #33

jschneider opened this issue Oct 5, 2019 · 12 comments

Comments

@jschneider
Copy link

jschneider commented Oct 5, 2019

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)

`

@cretz
Copy link
Owner

cretz commented Oct 7, 2019

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.

@martinflorek
Copy link

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
Copy link

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")
    }

@drieks
Copy link
Contributor

drieks commented Oct 15, 2019

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: kotlinx/ast@d4da88a

  • 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 :-)

@martinflorek
Copy link

@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?

@drieks
Copy link
Contributor

drieks commented Oct 16, 2019

@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")

@martinflorek
Copy link

@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

@drieks
Copy link
Contributor

drieks commented Oct 16, 2019

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 (Strumenta/antlr-kotlin#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").

@martinflorek
Copy link

@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.

@drieks
Copy link
Contributor

drieks commented Oct 17, 2019

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. kotlinx/ast@67b2331

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

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

@martinflorek
Copy link

@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 kotlinx/ast#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.

@cretz
Copy link
Owner

cretz commented Oct 18, 2019

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.

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

No branches or pull requests

4 participants