JetBrains / Grammar-Kit

Grammar files support & parser/PSI generation for IntelliJ IDEA
Other
715 stars 125 forks source link

Fleet-compatible generation for Grammar-Kit #359

Closed KrylovBoris closed 1 week ago

KrylovBoris commented 5 months ago

What is this PR for?

This PR aims to introduce additional functionality to the Grammar-Kit plugin that allows users to generate a parser and adjacent code that is compatible with Fleet's API.

Why were these changes made?

Currently, for plugin developers to port their IDEA plugins over to Fleet, they need to edit generated parser files or write them from scratch. To reduce the workload and allow for a more streamlined development process we introduce a way to generate Fleet-compatible code.

What is Fleet-compatible code?

For the parser to be compatible with Fleet, it needs to import packages from Fleet's own parsing core and not have PSI-related features. This could be achieved by tweaking imports and stopping Grammar-Kit from generating PSI.

What has been done?

For Fleet-specific generation logic, we created a separate FleetParserGenerator, a subclass of ParserGenerator. To avoid copying large chunks of code from the parent, we introduced a bunch of small methods that are responsible for adjusting original namespaces and generating small parts of the parser code. We also introduced FleetGenOptions, a subclass of GenOptions, to control file generation.

To interact with Fleet-specific parser, we added 2 actions to the plugin:

  1. Generate Parser Code For Fleet;
  2. Generate JFlex Lexer For Fleet;

To generate lexers we use a new template file, which includes Fleet-specific imports.

We also introduced 2 Fleet-only attributes to BNF files:

  1. adjustPackagesForFleet allows users to suppress moving generated files to "fleet/" directory. By default, when we generate files for Fleet, they are moved to "fleet/" directory, and "fleet." prefix is added to their respective package names. This is done to mimic Fleet's parser code layout and this option allows users to turn it off.
  2. generateFileTypeForFleet was added by request of the Fleet team. Since almost all IFileType implementations in Fleet are the same, this attribute was added to reduce the amount of boilerplate code, plugin developers have to write to make their parsers compilable. This attribute holds all the parameters to generate IFileType implementation for Fleet.

Finally, we added an optional parameter [-f] to the main function to switch from ParserGenerator to FleetParserGenerator

How have the other parts of the plugin been affected?

Since all Fleet-specific generation is stored in a separate class, it should not affect other parts of the program.

What's your plans for the future?

In the future we plan to add support of introduced functionality in grammar-kit-gradle-plugin.

gregsh commented 4 months ago

Please avoid changing any configuration files, including libraries xmls, idea/*.xml, and build scripts.

YannCebron commented 4 months ago

please add entry in CHANGELOG.md

gregsh commented 2 months ago

Overall

We don't want to have a separate FleetParserGenerator. We wish to parametrize ParsersGenerator and BNF PSI to simultaneously solve IntelliJ and Fleet tasks.

gregsh commented 1 month ago

I like that much more. And the FleetBnfFileWrapper trick is cool

Comments:

  1. Some args miss nullability annos
  2. RuleInfo belongs to ParserGenerator. Let's move it back
  3. ParserConstantSet -> IntelliJPlatformContants ?
  4. Let's have NameShortener back in ParserGeneratorUtil
  5. adjustPackagesForFleet must not be publicly visible - let's encapsulate it more
  6. I'd move all fleet-related classes in one flat package - org.intellij.grammar.fleet
  7. GenerateAction#getParserClass seems unnecessary - we already adjust all BNF attributes
  8. org.intellij.grammar.Main "Usage" string misses some spaces
gregsh commented 1 month ago

More Notes:

  1. Let's map files directly in run -> doGenerate instead of BnfRunJFlexAction#getFileIterator (and indent the code nearby
  2. Avoid copy-pasting json.bnf from IntelliJ repo
  3. Inline GeneratorBase.getFile() and use myFile as before. The method is never overridden and must no be overridden
  4. Reformat org.intellij.grammar.BnfGeneratorAbstractTest#newTestGenerator to avoid excessive indents and parens
  5. Discuss org.intellij.grammar.Main arguments more
zajac commented 2 weeks ago

Any chance to get this merged already?

gregsh commented 2 weeks ago

One last change, please, and we are good to go!

  1. Use the same naming: GeneratorBase.file -> GeneratorBase.myFile (lots of unnecessary changes in ParserGenerator)