mike-lischke / antlr4ng

Next Generation TypeScript runtime for ANTLR4
Other
65 stars 11 forks source link

Generated code for listener creates properties, not function bodies to be implemented. #42

Closed lairdresearch closed 4 months ago

lairdresearch commented 4 months ago

When building a listener, antlr4ng creates the following code (here the language is called FormulaLang and the start node is "formula", so the visitor function is visitFormula):

export class FormulaLangVisitor<Result> extends AbstractParseTreeVisitor<Result> {
    /**
     * Visit a parse tree produced by `FormulaLangParser.formula`.
     * @param ctx the parse tree
     * @return the visitor result
     */
    visitFormula?: (ctx: FormulaContext) => Result;

Note that the visitFormula is a property holding a function, not a function itself. When I follow the instructions in the readme, it indicates I should be building a function body:

class FormulaEvalVisitor extends FormulaLangVisitor<dfd.Series> {
    visitFormula(ctx: FormulaContext) : dfd.Series {
        if (ctx.where_list() != null) this.visit(ctx.where_list());
        return this.visit(ctx.expr());
     };
}

However, typescript throws an error saying that FormulaLangVisitor defines a property and I'm trying to create a function. Instead it wants something like:

class FormulaEvalVisitor extends FormulaLangVisitor<dfd.Series> {
    declare visitFormula = (ctx: FormulaContext) => { <code> }

Which should I be doing? Is the documentation incorrect or the generator? Or (shudder) is it me?

Swappea commented 4 months ago

Yes, I am also facing this exact issue.

Class 'ABCGrammarVisitor<void>' defines instance member property 'visitStart', but extended class 'MyQueryVisitor' defines it as instance member function.
mike-lischke commented 4 months ago

Yes, this way of generation makes it possible to implement only those visitor or listener methods you really want. If they all were defined as normal instance methods (abstract then) you would have to override all of them, regardless of what you actually need.

Optional methods can only be defined using the properties syntax, so you have to write the actual implementation by assiging a closure to that function property. There's no need to use the declare keyword (which is only useful for ambient declarations). Instead just write:

class FormulaEvalVisitor extends FormulaLangVisitor<dfd.Series> {
    public visitFormula = (ctx: FormulaContext) => { <code> }
mike-lischke commented 4 months ago

I forgot to mention that sometimes declare is useful also outside of ambient declarations. TS supports it to narrow inherited types, which is kinda "override" for properties. See for example class LexerNoViableAltException which redeclares member input: CharStream | null, inherited from RecognitionException.

Other than that, is there anything else to discuss here? If not we can close this issue.

lairdresearch commented 4 months ago

I think the issue is that the documentation is incorrect in that is shows the developer implementing a function, not assigning a function to a property. With your guidance above I got it working though.

mike-lischke commented 4 months ago

OK, good. Meanwhile I also updated the documentation to avoid confusion.