Sealed types support

[brokenhappy]

Sealed types support

I think it would be a very powerful addition if the grammar allows to define sealed types, which would correspond to sealed types in modern languages. This stronger leveraging of the type system has all the benefits of sealed types and most of the extends attribute.

Problem statement

Given the following bnf:

foo ::= bar | baz
bar ::= 'barbs' {extends=foo}
baz ::= 'bazbs' {extends=foo}

In this case, the generated Psi hierarchy has an FooImpl and it's associated IElementType, even though it is not instantiable. Also, compilers and IDE's don't know that Foo can only have Bar and Baz as subtypes.

Proposed solution

As a solution I'll pitch a sealed modifier that is only valid when the rule contains a choice of only references to public (sealed) rules.

So you could define:

// { generate=[java=">=15"] }
sealed foo ::= bar | baz
bar ::= 'barbs'
baz ::= 'bazbs'

Now interface Foo will be a sealed interface without FooImpl and FOO IElementType, and interfaces Bar and Baz will be non-sealed. In most other regards, this will have similar semantics to the extends attribute.

Detailed expected behavior

From the bnf in the proposed solution, this pitch proposes generating the following Psi (code is simplified to keep it shorter):

// ---- GeneratedTypes.java -----------------
public interface GeneratedTypes {
  // No FOO!
  IElementType BAR = new IElementType("BAR", null);
  IElementType BAZ = new IElementType("BAZ", null);

  class Factory {
    // just generation of bar and baz
  }
}
// ---- Bar.java -----------------
public non-sealed interface Bar extends Foo {

}
// ---- Baz.java -----------------
public non-sealed interface Baz extends Foo {

}
// ---- Foo.java -----------------
public sealed interface Foo extends PsiElement permits Bar, Baz {

}
// ---- BarImpl.java -----------------
// All normal
public class BarImpl extends ASTWrapperPsiElement implements Bar {
  public BarImpl(@NotNull ASTNode node) {
    super(node);
  }

  public void accept(@NotNull Visitor visitor) {
    visitor.visitBar(this);
  }

  @Override
  public void accept(@NotNull PsiElementVisitor visitor) {
    if (visitor instanceof Visitor) accept((Visitor)visitor);
    else super.accept(visitor);
  }
}
// ---- BazImpl.java -----------------
public class BazImpl extends ASTWrapperPsiElement implements Baz {
  // Similar to BarImpl, nothing unusual
}
// ---- Parser.java -----------------
// ...
// is public but does not have a marker
// bar | baz
public static boolean foo(PsiBuilder builder_, int level_) {
  if (!recursion_guard_(builder_, level_, "foo")) return false;
  boolean result_;
  result_ = bar(builder_, level_ + 1);
  if (!result_) result_ = baz(builder_, level_ + 1);
  return result_;
}
// ...

[hurricup]

Doesn't this looks like https://github.com/JetBrains/Grammar-Kit/blob/master/resources/messages/attributeDescriptions/extends.html ?

[hfhbd]

Do you have a sample of what would be needed? If we could introduce the sealed behavior by ourselves without a new release, this would be very nice! Especially with the Kotlin interop of consuming sealed Java interfaces.

[brokenhappy]

@hurricup You are right, thanks for the tip. I have updated the proposal accordingly

[JojOatXGME]

Do we actually have to extend the Grammar-Kit language for that? Couldn't we introduce an option which seals all interfaces generated by Grammar-Kit?

{
  generate=[java="17"]
  sealed="yes"
}

foo ::= bar | baz
bar ::= 'barbs' {extends=foo}
baz ::= 'bazbs' {extends=foo}

This would automatically make the interfaces Foo, Bar and Baz sealed. If you only want to seal or “unseal” a specific interface, you may add the option to the individual element:

foo ::= bar | baz { sealed="yes|no" }
bar ::= 'barbs' {extends=foo}
baz ::= 'bazbs' {extends=foo}

Also note that all non-sealed interfaces and types generated by Grammar-Kit should explicitly declare non-sealed. I just had the problem that I wanted to declare an interface outside Grammar-Kit where an interface from Grammar-Kit was a permitted subclass.

foo ::= ... { implements="my.own.SealedInterface" }

Unfortunately, permitted subtypes must be final, sealed or explicitly non-sealed.

[brokenhappy]

Yes you are right @JojOatXGME when it comes to the required modifiers. In case you are interested, in the pull request you can see that I declare the correct modifiers in the generated interfaces.

Regarding the sealed by default setting, that's easy to implement, let's tweak the expectations.

Regarding whether it should be a modifier or a rule's property.. I don't have too strong of an opinion yet, I personally think modifiers feel more natural here, but I'm open to hear your arguments.

Regarding forcing forcing subtypes to have explicit {extends=foo}:

• I don't think that's valuable to the user. First of all, there is no FooImpl, so extends sounds off where in this case it's only conformance.
• In the current implementation, one rule can be the sub-rule of multiple sealed rules. I'd argue that it's expected. Your suggestion would make this impossible (you can only extend one type). Although you can work around it by making a separate public rule for each case in your implementation. (Similar to how it's done in algebraic data types in most languages).
• Additionally, it's always clear what subtypes are just from reading the sealed type. It must be all the choices of a sealed rule. If foo = bar | baz, then bar and baz must always extend foo. Otherwise the contract is broken. So if they always must extend their supertype. If you read resources/messages/GrammarKitBundle.properties in my pull request, than you can see the requirements and the quick fixes for them. I'd say that's easy and intuitive to work with through IDE support.

I'd suggest dropping the {extends=...} requirement for sub-rules of sealed rules.

[JojOatXGME]

@brokenhappy Disclaimer: I am just expressing my own view. I am NOT a maintainer or the project.

I'd suggest dropping the {extends=...} requirement for sub-rules of sealed rules.

I had not realized that you wanted to implicitly implement the interface of the sealed rules in all sub-rules. I guess this also means that sealed rules can only contain a disjunction of other rules? This also means the fake rule string from the following example could not be sealed?

std_string ::= STRING_OPEN string_part* STRING_CLOSE { pin=1 }
ind_string ::= IND_STRING_OPEN string_part* IND_STRING_CLOSE { pin=1 }
fake string ::= string_part* { methods=[ string_parts="string_part" ] }

(Note that the fake rule needs to specify string_part* to make the method available.)

I am skeptical if we should introduce a new behavior for sealed rules. I would rather keep the extends property independent of the use of sealed interfaces.

Regarding whether it should be a modifier or a rule's property.. I don't have too strong of an opinion yet, I personally think modifiers feel more natural here, but I'm open to hear your arguments.

If the behavior of the BNF is changed as you have suggested, I guess I would use a modifier as well. Otherwise, it seems odd to me to introduce a modifier into the BNF syntax.

I already wrote the following two paragraphs before I realized that you indented to make sealed rules work differently:

Whether generated classes are sealed would be unrelated to the syntax defined by the BNF. A BNF modifier which has no semantic meaning for the language defined by the BNF seems off to me. Whether a class is sealed would be a detail about the class generation, which I think is currently always handled using properties.

That sealed is a modifier in Java does not mean that it should be a modifier in .bnf files. The private modifier in .bnf files does also not translate to a private classes. Modifiers in .bnf files are in a different domain and therefore have a naturally different meaning compared to Java.

Regarding forcing forcing subtypes to have explicit {extends=foo}:

I did not suggest changing the behavior regarding this property. I used the extends property to construct an example which uses inheritance. I assumed the meaning of extends as it working in current release. As discussed above, I had not realized that you wanted to change the behavior of sealed rules compared to other rules.

[brokenhappy]

This also means the fake rule string from the following example could not be sealed?

Yes, I'd argue that it would not make sense to combine these, please let me know if you see a use case!

I am skeptical if we should introduce a new behavior for sealed rules. I would rather keep the extends property independent of the use of sealed interfaces.

I'd agree, in which sense do you think they are dependent?

I did not suggest changing the behavior regarding this property. I used the extends property to construct an example which uses inheritance. I assumed the meaning of extends as it working in current release. As discussed above, I had not realized that you wanted to change the behavior of sealed rules compared to other rules.

Okay, then we seem to agree here :)

It seems like we're getting closer to agreeing! let's discuss expected semantics of enabling sealed as top level setting:

I think IDE support becomes harder if sealed rules become implicit. What about cases that are almost a correct sealed rule (and the user likely intends for it to be so)? eg. a rule of choices, but one of the choices is a private rule. Should we silently ignore them and risk causing confusion, should we forbid this, or should we require some kind of a non-sealed modifer?

[JojOatXGME]

It seems like we're getting closer to agreeing!

There was some misunderstanding which has been resolved. However, I am unfortunately skeptical about the proposed approach.

I am skeptical if we should introduce a new behavior for sealed rules. I would rather keep the extends property independent of the use of sealed interfaces.

I'd agree, in which sense do you think they are dependent?

What I mean is that your proposal changes the class hierarchies based on the presence of the sealed modifier. Classes which would otherwise use composition will use inheritance as soon as you add the sealed modifier. The modifier is not affecting the extends property itself, but it affects where you have to add it. I am wondering if a simple option to generate sealed classes might be better than such a more elaborate feature which interacts with other functionalities of Grammar-Kit.

This also means the fake rule string from the following example could not be sealed?

Yes, I'd argue that it would not make sense to combine these, please let me know if you see a use case!

Can you explain why you don't think that it makes sense to use sealed classes in such a case? I mean, the example contained two types of strings and one super-type for both types of strings. What makes this case different from the other cases you suggested?

PS: I acknowledge that the new semantic you want to introduce can ease up some scenarios. You no-longer have to specify the extends property for all the elements below a certain rule. It might also be less error-prone in such scenarios. However, I am not convinced that it makes sense to bind this new behavior to sealed interfaces. I think this new behavior can be useful without sealed interfaces, and I think sealed interfaces can be useful without this new behavior.