gradle-grammar-kit-plugin icon indicating copy to clipboard operation
gradle-grammar-kit-plugin copied to clipboard

Code generation isn't equal with Grammar-Kit

Open sanore opened this issue 7 years ago • 29 comments

I am trying to run the generateParser task. But the generated code isn't the same than if I run the action within context menu.

Versions jflexRelease = '1.7.0' grammarKitRelease = '1.5.2' idea = '2017.2'

Gradle

task generateParser(type: GenerateParser) {
    source = "src/main/java/plugin/lang/grammar/PluginParser.bnf"
    targetRoot = 'src/main/gen/'
    pathToParser = '/plugin/lang/parser/PluginParser.java'
    pathToPsiRoot = '/plugin/lang/psi'
}

PluginParser.bnf File

{
    [..]
    // used to attach custom methods into the classes
    psiImplUtilClass            = "plugin.impl.PluginPsiImplUtil"
    psiTreeUtilClass            = "plugin.lang.parser.PluginTreeUtil"

    // if true, generated PsiElement has a get method to get the specified Token
    generateTokenAccessors      = true
    [..]
}

[..]
Block  ::= BodyBlock | CodeBlock {methods=[processDeclarations]}
[..]

Generated Block interface

// This is a generated file. Not intended for manual editing.
package plugin.lang.psi;

import java.util.List;
import org.jetbrains.annotations.*;
import com.intellij.psi.PsiElement;

public interface PluginBlock extends PluginCompositeElement {

  @Nullable
  PluginBlock getBlock();

  @Nullable
  PsiElement getEnd();

  //WARNING: processDeclarations(...) is skipped
  //matching processDeclarations(PluginBlock, ...)
  //methods are not found in PluginPsiImplUtil
}

Generated Block interface with Grammar-Kit

// This is a generated file. Not intended for manual editing.
package plugin.lang.psi;

import java.util.List;
import org.jetbrains.annotations.*;
import com.intellij.psi.PsiElement;
import com.intellij.psi.ResolveState;
import com.intellij.psi.scope.PsiScopeProcessor;

public interface PluginBlock extends PluginCompositeElement {

  @Nullable
  PluginBlock getBlock();

  @Nullable
  PsiElement getEnd();

  boolean processDeclarations(PsiScopeProcessor processor, ResolveState state, PsiElement lastParent, PsiElement place);

}

UtilClass

public static boolean processDeclarations(@NotNull PluginCompositeElement element,
                                              @NotNull PsiScopeProcessor processor,
                                              @NotNull ResolveState state,
                                              PsiElement lastParent,
                                              @NotNull PsiElement place) {
    return true;
}

sanore avatar Aug 25 '17 12:08 sanore

Ha, I ran into this recently as well. I know what the problem is but I dont know how to fix it. It is because PluginsPsiImplUtil is not on the classpath when gradle is running, so grammar-kit can't find the method. Essentially a chicken egg problem where grammar kit generates java but the gradle task depends on java and since they're in the same module the gradle task runs first.

In my own repo I just ended up using mixins instead of a util class because it does not have this problem.

AlecKazakova avatar Aug 25 '17 12:08 AlecKazakova

Also encountered this issue - this plugin doesn't work with the implement interface via method injection method in Grammar-Kit. You can't use psiImplUtilClass in the BNF.

breandan avatar Jan 09 '18 19:01 breandan

That's really a pity! I feel really bad when I see this I though I can use gradle+CI without uploading the generated files...

ice1000 avatar Feb 03 '18 10:02 ice1000

this looks like a fundumental shortcoming... basically you can't use this plugin if you want to use custom methods. And it seems like it can't be fixed? Or is there some hope?

fedork avatar May 08 '18 12:05 fedork

There is alway hope :)

hurricup avatar May 08 '18 12:05 hurricup

Wat? @hurricup you mean you're working on this and there's some progress?

ice1000 avatar May 08 '18 13:05 ice1000

Not exactly. I understand where is the problem. But not working on it now. Tldr: to make injected methods work, GK should have class files to analyze. But gradle plugin generates parsers before any compilation. This probably can work if you'll add generated code to the vcs and disable grammar files cleaning before generation. But this, probably, reduces plugin usefulness.

hurricup avatar May 08 '18 14:05 hurricup

@hurricup My guess is that to properly fix it parser generator should parse mixin sources instead of relying on compiled classes... But that's probably substantial rework

fedork avatar May 08 '18 15:05 fedork

I guess my best workaround for this is not to use "methods" and instead create an interface with those methods and use "implements". Seems cleaner too

fedork avatar May 08 '18 15:05 fedork

@fedork this is a question for @gregsh Probably it's easier to make gradle compile twice.

hurricup avatar May 08 '18 16:05 hurricup

What about writing the full-qualified method signature in the .bnf file? I think in this way GK can do a very very simple parsing and generate the methods.

ice1000 avatar May 08 '18 17:05 ice1000

@hurricup Any progress? It would be great to have a solution to this issue, even if that is supporting only a subset of the GrammarKit syntax.

breandan avatar Oct 17 '18 22:10 breandan

Just encountered the same issue. I worked around it by invoking parser GenerateParser twice. First one is a dependend of compileJava Second one depends on compileJava which also needed the following statements to work in my setup:

outputs.upToDateWhen { false }
doFirst {
    sourceSets.main.runtimeClasspath
}

Edit: You'd also need a second compilation step.

BjoernAkAManf avatar Nov 10 '18 18:11 BjoernAkAManf

Just encountered the same issue. I worked around it by invoking parser GenerateParser twice. First one is a dependend of compileJava Second one depends on compileJava which also needed the following statements to work in my setup:

outputs.upToDateWhen { false }
doFirst {
    sourceSets.main.runtimeClasspath
}

Edit: You'd also need a second compilation step.

Did you get this to work inside a gradle build? I've worked around for now by creating a bash script to do the 2 compile dance:

#!/bin/bash
rm -rf gen
java -cp "tools/grammar-kit.jar" org.intellij.grammar.Main gen <the bnf>

./gradlew clean compileJava

java -cp 'tools/*' jflex.Main --skel idea/tools/lexer/idea-flex.skeleton --nobak <the lex file> -d gen/...

java -cp "build/classes/java/main:tools/grammar-kit.jar" org.intellij.grammar.Main gen <the bnf>
./gradlew clean buildPlugin

samowen-kiwipower avatar Nov 26 '18 15:11 samowen-kiwipower

Yeah i am using gradle for it. While not a perfect solution, it works. A significant disadvantage is, that i'm required to maintain two projects, because referencing utility classes will not work in the same project that is used for generation.

BjoernAkAManf avatar Nov 26 '18 17:11 BjoernAkAManf

So, there should be no additional chicken/egg issues with running gradle in the JVM compared to using the grammarkit plugin. How does grammarkit solve this problem? It's also a .jar also running in the JVM right? There must be some solution (I suspect just specifying -classpath or something?).

stefansjs avatar Feb 20 '19 21:02 stefansjs

Sorry to bring it up exactly a year later, but this is still unresolved and I want to share how I got it to work more or less okay for me.

The key point is that it works if you write your plugin in Kotlin, so you have an additional compilation step without dealing with multiprojects.

It is accomplished by a really small snipped in the following gradle build script (click on it) :

build.gradle.kts
// You may also want to add `outputs.upToDateWhen { false }`
// to the common parser configs if your changes to the BNF
// break stuff and you have to manually remove the 'gen' folder to fix those.
fun generateParserTask(suffix: String = "", config: GenerateParser.() -> Unit = {}) =
        task<GenerateParser>("generateParser${suffix.capitalize()}") {
            source = "src/main/grammars/grammar.bnf"
            // ... other common parser configs
            purgeOldFiles = true
            config()
        }

val generateParserInitial = generateParserTask("initial")

val compileKotlin = tasks.named("compileKotlin") {
    dependsOn(generateLexer, generateParserInitial)
}

val generateParser = generateParserTask {
    dependsOn(compileKotlin)
    classpath(compileKotlin.get().outputs)
}

tasks.named("compileJava") {
    dependsOn(generateParser)
}

The compilation process then should go this way:

  • generate the parser sources ignoring warnings about it not finding the methods - this would make broken generated Java sources, because it did not actually implement the methods that migh be required by the interfaces you've added
  • compile the Kotlin code, which works, as it sees the generated Java sources and the fact that the classes implement the interfaces, and apparently it ignores the fact that Java sources actually do not yet implement the interface methods
  • regenerate the parser sources, but add Kotlin compile output to the generator task classpath - now the sources are fixed
  • compile the Java code, which is our new fixed parser sources

necauqua avatar Feb 20 '20 10:02 necauqua

What if grammar-kit-plugin generates Lexer after compile task?

The whole idea is: A plugin project can be split on two modules:

  1. psi-structure (bnf and Psi util java classes for proper generation code from bnf)
  2. Remain logic of the plugin (depends on 1.)

Custom plugin project will have following dependency tree: my-language-plugin +---- my-language-plugin-psi

And gradle will compile module 1 (as dependency) and then compile module 2 in regular build process.

I think the idea is better than nothing.

xBlackCat avatar Jun 03 '20 19:06 xBlackCat

Sorry to bring it up exactly a year later, but this is still unresolved and I want to share how I got it to work more or less okay for me.

The key point is that it works if you write your plugin in Kotlin, so you have an additional compilation step without dealing with multiprojects.

It is accomplished by a really small snipped in the following gradle build script (click on it) : build.gradle.kts

The compilation process then should go this way:

* generate the parser sources ignoring warnings about it not finding the methods - this would make broken generated Java sources, because it did not actually implement the methods that migh be required by the interfaces you've added

* compile the Kotlin code, which works, as it sees the generated Java sources and the fact that the classes implement the interfaces, and apparently it ignores the fact that Java sources actually do not yet implement the interface methods

* regenerate the parser sources, but add Kotlin compile output to the generator task classpath - now the sources are fixed

* compile the Java code, which is our new fixed parser sources

This worked for the Gradle Kotlin DSL, thank you.

It really shouldn't be this hard, #23 is practically the same issue (also see https://github.com/JetBrains/Grammar-Kit/issues/35). It's kind of frustating that we either have to do two passes or manually generate the parser. It's been almost three years. I don't know how hard it would be to fix but the solution using "hidden" annotation processors (https://github.com/JetBrains/gradle-grammar-kit-plugin/issues/23#issuecomment-721111310) seems like a good idea if it cleans up our builds.

jord1e avatar Jan 04 '21 23:01 jord1e

Is this issue still the thing?

hsz avatar Mar 17 '22 08:03 hsz

@hsz Yes, this issue is why we cannot use this for our plugin and we have to resort to committing all generated files to git.

PHPirates avatar Mar 17 '22 16:03 PHPirates

Is this issue still the thing?

Yes, this problem is still present and prevents the development of plugins:(

Danil42Russia avatar Aug 27 '22 13:08 Danil42Russia

@hsz is this issue still relevant? If it is, is it documented somewhere?

Edit: there is a note on the Grammar-Kit repository

Otherwise use gradle-grammar-kit-plugin if the following limitations are not critical:

  • Method mixins are not supported (two-pass generation is not implemented)
  • Generic signatures and annotations may not be correct

lppedd avatar Mar 26 '23 23:03 lppedd

I just ran into this issue myself :( Is there some useful workaround when using Java?

lerno avatar May 01 '23 21:05 lerno

I just ran into this issue myself :( Is there some useful workaround when using Java?

Yes, using mixin.

ice1000 avatar May 01 '23 21:05 ice1000

I was more looking for a good way to make gradle call java compilation twice. I haven't used gradle before this so having this as the way to learn the build tool hasn't been ideal... I am intrigued to know how @BjoernAkAManf solved this. I tried to fix it myself, creating a second project inside of the same build file.

This seems to do what the Rust plugin does: https://github.com/intellij-rust/intellij-rust/blob/master/build.gradle.kts but it also does a lot of other things and trying to copy it quickly makes things go south. So ideally some cut-and-paste:able solution would be ideal.

lerno avatar May 01 '23 23:05 lerno

Reading more abut the Rust solution, it seems like it stores some fake PSI code first. I'd like to avoid that solution. The more I read about this, the less amenable to a fix it seems to be.

The rust plugin team did file this pull request: https://github.com/JetBrains/Grammar-Kit/pull/316 Without looking into details it seems like it solves at least their use cases. Unfortunately it's seen little interest.

lerno avatar May 02 '23 07:05 lerno

https://github.com/JetBrains/Grammar-Kit/pull/316 is absolutely not a solution for this issue. Furthermore, I'm not sure this is an issue at all. You can just use mixin classes to add any methods you want without providing any .class files to the Grammar-Kit

vlad20012 avatar May 02 '23 08:05 vlad20012

This may sound like a simple question, but why can't the gradle-plugin utilize the same logic as grammar-kit?

As I understand it, the issue arises here.

After going through the source code, I've realised that the problem lies in the org.intellij.grammar.java.JavaHelper#findClassMethods method.

Within grammar-kit, the org.intellij.grammar.java.JavaHelper.PsiHelper#findClassMethods is used, while within gradle-plugin, org.intellij.grammar.java.JavaHelper.AsmHelper#findClassMethods is employed.

Technically, we should be able to do:

project.registerService(JavaHelper.class, new JavaHelper.PsiHelper(project));

in exchange for:

project.registerService(JavaHelper.class, new JavaHelper.AsmHelper());

after modifying the visibility of the PsiHelper class, shouldn't we?

Or simply, will org.intellij.grammar.java.JavaHelper.PsiHelper not function properly due to its internal dependencies?

yorlov avatar Mar 20 '24 06:03 yorlov