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

Code generation isn't equal with Grammar-Kit #3

Open
sanore opened this issue Aug 25, 2017 · 29 comments
Open

Code generation isn't equal with Grammar-Kit #3

sanore opened this issue Aug 25, 2017 · 29 comments

Comments

@sanore
Copy link

sanore commented Aug 25, 2017

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;
}
@AlecKazakova
Copy link
Contributor

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.

@breandan
Copy link

breandan commented Jan 9, 2018

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.

@ice1000
Copy link

ice1000 commented Feb 3, 2018

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

@fedork
Copy link

fedork commented May 8, 2018

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?

@hurricup
Copy link
Contributor

hurricup commented May 8, 2018

There is alway hope :)

@ice1000
Copy link

ice1000 commented May 8, 2018

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

@hurricup
Copy link
Contributor

hurricup commented May 8, 2018

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.

@fedork
Copy link

fedork commented May 8, 2018

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

fedork commented May 8, 2018

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

@hurricup
Copy link
Contributor

hurricup commented May 8, 2018

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

@ice1000
Copy link

ice1000 commented May 8, 2018

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.

@breandan
Copy link

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

@BjoernAkAManf
Copy link

BjoernAkAManf commented Nov 10, 2018

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.

@samowen-kiwipower
Copy link

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

@BjoernAkAManf
Copy link

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.

@stefansjs
Copy link

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?).

@necauqua
Copy link

necauqua commented Feb 20, 2020

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

@xBlackCat
Copy link

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.

@jord1e
Copy link

jord1e commented Jan 4, 2021

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 JetBrains/Grammar-Kit#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 (#23 (comment)) seems like a good idea if it cleans up our builds.

JojOatXGME added a commit to JojOatXGME/nix-idea that referenced this issue Jan 25, 2021
The Gradle plugin for Grammar-Kit does not support the usage of Util
classes.

JetBrains/gradle-grammar-kit-plugin#23 and
JetBrains/gradle-grammar-kit-plugin#3.
JojOatXGME added a commit to JojOatXGME/nix-idea that referenced this issue Jan 25, 2021
The Gradle plugin for Grammar-Kit does not support the usage of Util
classes. See the following issues from the Gradle plugin.

JetBrains/gradle-grammar-kit-plugin#23
JetBrains/gradle-grammar-kit-plugin#3
@hsz
Copy link
Member

hsz commented Mar 17, 2022

Is this issue still the thing?

@PHPirates
Copy link

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

Motzkiste pushed a commit to EraTiem-Network/ZenScriptSupport that referenced this issue Jul 23, 2022
Commented out parser-build because off JetBrains/gradle-grammar-kit-plugin#3
@Danil42Russia
Copy link

Danil42Russia commented Aug 27, 2022

Is this issue still the thing?

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

@lppedd
Copy link

lppedd commented Mar 26, 2023

@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

@lerno
Copy link

lerno commented May 1, 2023

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

@ice1000
Copy link

ice1000 commented May 1, 2023

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

Yes, using mixin.

@lerno
Copy link

lerno commented May 1, 2023

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

lerno commented May 2, 2023

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: JetBrains/Grammar-Kit#316
Without looking into details it seems like it solves at least their use cases. Unfortunately it's seen little interest.

@vlad20012
Copy link

JetBrains/Grammar-Kit#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

@yorlov
Copy link
Contributor

yorlov commented Mar 20, 2024

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?

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