{{grammarName}}Parser Typescript Class Should Be After the rule contexts

[petermetz]

Hiya,

This tool is a godsend, thank you very much for it!

I was giving this grammar a spin when I found out that the decorators generated (@RuleVersion) depend on the parser class being declared below them in the .ts file (which is not the case).
https://github.com/mulesoft/salesforce-soql-parser/blob/antlr4/SOQL.g4

I could fix it manually by going in to the generated typescript file and putting the parser class at the very end, but I suck at this JST syntax and couldn't (yet) figure out how to reposition the class in the template (without the rest of the code around it in that template function block).

If I figure it out I'll be happy to submit a PR (in case it's endorsed as a good solution), otherwise what I wanted to do is just placing the below block at the end of the template:

<namedActions.beforeParser>

<parser>

<namedActions.afterParser>

Does this sound like a good idea at all or I went the wrong way in the first place?

[BurtHarris]

Ahh, thank you @petermetz. This sounds familiar, some other people have reported similar sounding problems I hadn't yet reproduced it or connected it to the decorators and order in the file. I'll have a look at this with that added information.

[petermetz]

@BurtHarris Awesome, thanks very much for checking on it! Once the template is sorted out I'll also submit a PR with this
You may or may not be interested, but I did manage to get the parser up and running in a browser environment (which was huge for me) and figured it should be given back to the community.

[BurtHarris]

@petermetz, to be clear, when you said

... the decorators generated (@ruleversion) depend on the parser class being declared below them in the .ts file (which is not the case).

Was this the problem message:

c:\try\soql\SOQLLexer.js:30
var _this = _super.call(this, input) || this;
^

TypeError: Class constructor Lexer cannot be invoked without 'new'
    at new SOQLLexer (c:\try\soql\SOQLLexer.js:30:28)
    at Object.<anonymous> (c:\try\soql\main.js:8:11)
    at Module._compile (module.js:569:30)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:503:32)
    at tryModuleLoad (module.js:466:12)
    at Function.Module._load (module.js:458:3)
    at Function.Module.runMain (module.js:605:10)
    at startup (bootstrap_node.js:158:16)
    at bootstrap_node.js:575:3

[petermetz]

@BurtHarris Nope, I was having issues with the Parser class not being defined at the point in the code that was creating/setting up the decorators.

[BurtHarris]

@petermetz Then exactly what is the problem? Is there an error message? From what tool, or is it at runtime? Can you give me step-by-step repro instructions?

The thing that's got me puzzled is there really shouldn't be an order dependency, and I still haven't got a clear isolated repro case. Seems related to #326, #310, #283, I know that in #326 @fdeitelhoff was also trying to get this working in a browser.

I'm afraid this may be related to Babel and our (current) use of native ES classes for the base classes both (Lexer and Parser.) I found https://stackoverflow.com/questions/36577683/babel-error-class-constructor-foo-cannot-be-invoked-without-new/

Due to the way ES6 classes work, you cannot extend a native class with a transpiled class. If your platform supports native classes, my recommendation would be, instead of using the preset es2015, use es2015-node5, assuming you're on Node 5. That will cause Babel to skip compiling of classes so that your code uses native classes, and native classes can extend other native classes.

Another way to approach this is to change our build so that it generates .js files for ES5 rather than ES2015. That change is requested in #311, but we are currently planning to postpone that till after we release a stable NodeJS version.

[petermetz]

@BurtHarris Thanks again for checking on this and spending so much effort to make sure its covered.

I pushed my local test bench here, hopefully you can reproduce it with this: https://github.com/petermetz/antlr-4-ts-test
just by simply running
npm install; npm run antlr; npm start

NodeJS 8 is being used on my machine at the moment.

Should crash with something like this below:

ReferenceError: Keywords_alias_allowedContext is not defined at Object.<anonymous> (.../antlr-4-ts-test/build/tsc/gen/typescript/soql-parser/src/main/g4/SOQLParser.js:4650:37)

[BurtHarris]

Great. I'll have a look at that this evening (pacific time)

[WaiSiuKei]

I meet the same problem

1. use this file to generate the parser
2. you will get this
   CalculatorParser.ts.zip
3. the code compiled by Typescript (Typescript 2.4.2, target: es6) will look like this
   CalculatorParser.zip

you can find the CalculatorContext class is used to describe the function return type metadata of the CalculatorParser class, while the CalculatorParser is defined before CalculatorContext

[BurtHarris]

OK, I was tied up on another project, I haven't gotten a chance to dig into it yet, but will soon.

[uwesimm]

same issue here, a fix or a workaround would be welcome

[chetmurphy]

I am able to work around the issue by manually moving the Parser class to the bottom of the generated Parser file. Kind of a pain since I keep forgetting to edit the file.

[Ciantic]

Manually moving a generated parser to last in the file works. I wonder if some are not getting this dependent on environment.

I'm using a webpack.

PS. If I put this to false: tsconfig.json emitDecoratorMetadata : false it works without reordering.

@petermetz you can try that also, looking at your project you have emitDecoratorMetadata as true.

[BurtHarris]

Interesting observation about emitDecoratorMetadata : false avoiding this issue. That seems significant. I wonder if this has anything to do with the issue flagged by TypeScript 2.6 in issue #345.

[BurtHarris]

@Ciantic, I'm guessing that the fix of disabling emitDecoratorMetadata applies when building the generated code for your own grammar rather than the runtime. Can you attach or reference at a grammar file that reproduces this or point me at a version of your project that generates the message?

[emazv72]

@Ciantic @BurtHarris Thanks for pointing me to the right direction, I'm having the same issue ( typescript 2.6.2):

Running a simple script like this

    let inputStream = new ANTLRInputStream(buffer);
    let lexer = new gyooLexer(inputStream);
    let tokenStream = new CommonTokenStream(lexer);
    let parser = new gyooParser(tokenStream);
    let result = parser.program();
    console.log(result.toStringTree(parser));

I get

ReferenceError: ProgramContext is not defined
    at Object.<anonymous> (src\gen\gyooParser.ts:79:20)

Setting emitDecoratorMetadata to false ( only for the second line of the script) fixed the issue:

antlr4ts gyoo.g4 -visitor -o src/gen"
ts-node src/ts/gyRunner.ts

I created a sample project Here

[pedroteixeira]

Had the same issue. Manually moving the class definitions before they are referenced seems to fix the compiler issue.

[pedroteixeira]

I ended up running a fix script that re-order the file, posting here in case might be usefull for someone else (you'd have to replace some of it):

post-antlrts.sh

#!/usr/bin/env bash
FILE='<Parser File Path>.ts'
START_LINE=`grep -n 'export class <your first context class here> extends ParserRuleContext' ${FILE} | cut -d: -f 1`

TMPFILE='tmpfile.ts'

echo '// tslint: disabled' > $TMPFILE
tail -n+${START_LINE} ${FILE} >> $TMPFILE
head -n$(($START_LINE - 1)) ${FILE} >> $TMPFILE

mv $TMPFILE $FILE

[pedroteixeira]

btw, I created this issue in the Babel repo for something that affects this library right now: babel/babel#7736

i.e. currently antlr4ts does not work in @babel/typescript (Babel 7)

[BurtHarris]

@pedroteixeira I'm not sure exactly what the babel/babel#7736 has to do with this. I've not gotten into babel at all. Is this something a fix in this codebase could help with? Does it deserve a separate bug?

[sharwell]

💭 Need to figure out if this is still an issue with #361 merged.

[TheBoz]

Just came upon this tool while exploring ANTLR, looks very promising. I've got the latest copy of the tool (0.4.0-alpha.4) working with Angular 6, and came across this issue. Manually re-ordering the file does fix it, but I want to make my generation dynamic. and automatic. Has there been any progress on solving it?

[johnholliday]

This is still an issue as of version 6.4.1. The fix for me was also to set 'emitDecoratorMetadata' to false.

[davidhockey22]

@Ciantic, I'm guessing that the fix of disabling emitDecoratorMetadata applies when building the generated code for your own grammar rather than the runtime. Can you attach or reference at a grammar file that reproduces this or point me at a version of your project that generates the message?

Generating the files with emitDecoratorMetadata set to false does not fix the issue when the files are moved into a project that has emitDecoratorMetadata set to true. In my case I'm moving the files into an AG6 project and have to reorder it.

[zakjan]

After TS parser is generated, I use this Shell function to reorder the file content:

reorderParser() {
    FILE="$1"
    TMPFILE=`mktemp`

    # reorder classes in the generated parser
    # see https://github.com/tunnelvisionlabs/antlr4ts/issues/341
    PARSER_START_LINE=`grep -n "extends Parser " "$FILE" | head -n1 | cut -d: -f 1`
    RULES_START_LINE=`grep -n "extends ParserRuleContext " "$FILE" | head -n1 | cut -d: -f 1`

    # 0 .. PARSER_START_LINE
    head -n$(($PARSER_START_LINE - 1)) "$FILE" >> "$TMPFILE"
    # RULES_START_LINE .. end
    tail -n+$RULES_START_LINE "$FILE" >> "$TMPFILE"
    # PARSER_START_LINE .. RULES_START_LINE
    tail -n+$PARSER_START_LINE "$FILE" | head -n$(($RULES_START_LINE - $PARSER_START_LINE - 1)) >> "$TMPFILE"

    mv "$TMPFILE" "$FILE"
}

reorderParser "xxxParser.ts"

[sharwell]

@johnholliday @davidhockey22 @zakjan Can one of you create a small sample project demonstrating the issue so I can better understand it, and hopefully be able to add a regression test when it's fixed?