antlr4ts icon indicating copy to clipboard operation
antlr4ts copied to clipboard

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

Open petermetz opened this issue 7 years ago • 25 comments

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?

petermetz avatar Jul 19 '17 23:07 petermetz

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.

BurtHarris avatar Jul 27 '17 00:07 BurtHarris

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

petermetz avatar Jul 27 '17 00:07 petermetz

@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

BurtHarris avatar Jul 27 '17 01:07 BurtHarris

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

petermetz avatar Jul 27 '17 01:07 petermetz

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

BurtHarris avatar Jul 27 '17 02:07 BurtHarris

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

petermetz avatar Jul 27 '17 02:07 petermetz

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

BurtHarris avatar Jul 27 '17 23:07 BurtHarris

I meet the same problem screen shot 2017-07-29 at 10 21 28 am

  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

WaiSiuKei avatar Jul 29 '17 02:07 WaiSiuKei

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

BurtHarris avatar Jul 29 '17 03:07 BurtHarris

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

uwesimm avatar Aug 16 '17 09:08 uwesimm

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.

chetmurphy avatar Oct 28 '17 04:10 chetmurphy

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.

Ciantic avatar Dec 20 '17 18:12 Ciantic

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 avatar Dec 22 '17 00:12 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?

BurtHarris avatar Dec 23 '17 18:12 BurtHarris

@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

emazv72 avatar Jan 09 '18 14:01 emazv72

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

pedroteixeira avatar Mar 16 '18 21:03 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 avatar Mar 17 '18 13:03 pedroteixeira

btw, I created this issue in the Babel repo for something that affects this library right now: https://github.com/babel/babel/issues/7736

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

pedroteixeira avatar Apr 15 '18 16:04 pedroteixeira

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

BurtHarris avatar Apr 23 '18 00:04 BurtHarris

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

sharwell avatar May 21 '18 16:05 sharwell

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?

TheBoz avatar Aug 06 '18 16:08 TheBoz

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

johnholliday avatar Oct 05 '18 18:10 johnholliday

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

davidhockey22 avatar Dec 07 '18 16:12 davidhockey22

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"

zakjan avatar Dec 09 '18 12:12 zakjan

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

sharwell avatar Dec 09 '18 21:12 sharwell