Bug (and fix) when processing string with grammar having optionally empty rule

[plaurent]

Bug description

When using an ANTLR4 generated JavaScript Visitor on a grammar with nested optionally empty rules, and when the empty rule is used (by omission), Tree.js throws a TypeError: Cannot read property 'accept' of null. (No error is reported in the grammar or in the string being parsed).

Version

Antlr4 JavaScript runtime 4.7.1

Fix

Patch antlr4/tree/Tree.js in the following way to return null if the context is null:

71c71,75
<               return ctx.accept(this);
---
>         if (ctx) {
>             return ctx.accept(this);
>         } else {
>             return null;
>         }

so that the code reads:

ParseTreeVisitor.prototype.visit = function(ctx) {
    if (Array.isArray(ctx)) {
        return ctx.map(function(child) {
            return child.accept(this);
        }, this);
    } else {   
        if (ctx) {
            return ctx.accept(this);
        } else {
            return null;
        }
    }
};

Steps to reproduce:

Given the following grammar:

$ cat > SIMPLE.txt <<EOF
grammar SIMPLE;
simple : icecream EOF;
icecream: ICECREAM icecreamPart toppingsPart EOF ;
icecreamPart: FLAVORS flavors+ ;
toppingsPart: TOPPINGS toppings syrups ;
flavors : vanilla | strawberry | chocolate ;
toppings : /* empty */
grammar SIMPLE;
simple : icecream EOF;
icecream: ICECREAM icecreamPart toppingsPart EOF ;
icecreamPart: FLAVORS flavors+ ;
toppingsPart: TOPPINGS toppings syrups ;
flavors : vanilla | strawberry | chocolate ;
toppings : /* empty */
         | sprinkles | marshmallows ;

syrups : /* empty */
         | chocolate | fudge ;

vanilla: VANILLA;
strawberry: STRAWBERRY;
chocolate: CHOCOLATE;
sprinkles: SPRINKLES;
marshmallows: MARSHMALLOWS;
fudge: FUDGE;

MARSHMALLOWS: M A R S H M A L L O W S;
FUDGE: F U D G E;
VANILLA: V A N I L L A ;
STRAWBERRY: S T R A W B E R R Y;
CHOCOLATE: C H O C O L A T E;
ICECREAM: I C E C R E A M;
SPRINKLES: S P R I N K L E S;
FLAVORS: F L A V O R S;
TOPPINGS: T O P P I N G S;
SYRUPS: S Y R U P S;

WS : [ \t\r\n]+ -> channel(HIDDEN);
fragment A : [aA];
fragment B : [bB];
fragment C : [cC];
fragment D : [dD];
fragment E : [eE];
fragment F : [fF];
fragment G : [gG];
fragment H : [hH];
fragment I : [iI];
fragment J : [jJ];
fragment K : [kK];
fragment L : [lL];
fragment M : [mM];
fragment N : [nN];
fragment O : [oO];
fragment P : [pP];
fragment Q : [qQ];
fragment R : [rR];
fragment S : [sS];
fragment T : [tT];
fragment U : [uU];
fragment V : [vV];
fragment W : [wW];
fragment X : [xX];
fragment Y : [yY];
fragment Z : [zZ];

Generate the target JavaScript classes, and set up a Node.JS project with the following main.js script:

$ antlr4 -visitor -Dlanguage=JavaScript -o TestDirectory SIMPLE.txt 
$ cd TestDirectory
$ npm init
$ npm install --save antlr4
$ cat > main.js <<"EOF"
var antlr4 = require('antlr4');
var SIMPLELexer = require('./SIMPLELexer').SIMPLELexer;
var SIMPLEParser = require('./SIMPLEParser').SIMPLEParser;
var SIMPLEListener = require('./SIMPLEListener').SIMPLEListener;
var SIMPLEVisitor = require('./SIMPLEVisitor').SIMPLEVisitor;

let input = `
Icecream
 Flavors
   Vanilla
 Toppings
   Sprinkles
`;

var chars = new antlr4.InputStream(input);
var lexer = new SIMPLELexer(chars);
var tokens  = new antlr4.CommonTokenStream(lexer);
var parser = new SIMPLEParser(tokens);
parser.buildParseTrees = true;
var tree = parser.icecream();
let visitor = new SIMPLEVisitor();
visitor.visitIcecream(tree);
EOF

Running node main.js demonstrates the bug.

$ node main.js
/Users/patryk.laurent/Desktop/TEST/REPRODUCE_BUG/TestDirectory/node_modules/antlr4/tree/Tree.js:71
		return ctx.accept(this);
		           ^

TypeError: Cannot read property 'accept' of null
    at SIMPLEVisitor.ParseTreeVisitor.visit (/Users/patryk.laurent/Desktop/TEST/REPRODUCE_BUG/TestDirectory/node_modules/antlr4/tree/Tree.js:71:14)
    at SIMPLEVisitor.ParseTreeVisitor.visitChildren (/Users/patryk.laurent/Desktop/TEST/REPRODUCE_BUG/TestDirectory/node_modules/antlr4/tree/Tree.js:76:15)
    at SIMPLEVisitor.visitSyrups (/Users/patryk.laurent/Desktop/TEST/REPRODUCE_BUG/TestDirectory/SIMPLEVisitor.js:53:15)
    at SyrupsContext.accept (/Users/patryk.laurent/Desktop/TEST/REPRODUCE_BUG/TestDirectory/SIMPLEParser.js:660:24)
    at SIMPLEVisitor.<anonymous> (/Users/patryk.laurent/Desktop/TEST/REPRODUCE_BUG/TestDirectory/node_modules/antlr4/tree/Tree.js:68:26)
    at Array.map (<anonymous>)
    at SIMPLEVisitor.ParseTreeVisitor.visit (/Users/patryk.laurent/Desktop/TEST/REPRODUCE_BUG/TestDirectory/node_modules/antlr4/tree/Tree.js:67:14)
    at SIMPLEVisitor.ParseTreeVisitor.visitChildren (/Users/patryk.laurent/Desktop/TEST/REPRODUCE_BUG/TestDirectory/node_modules/antlr4/tree/Tree.js:76:15)
    at SIMPLEVisitor.visitToppingsPart (/Users/patryk.laurent/Desktop/TEST/REPRODUCE_BUG/TestDirectory/SIMPLEVisitor.js:35:15)
    at ToppingsPartContext.accept (/Users/patryk.laurent/Desktop/TEST/REPRODUCE_BUG/TestDirectory/SIMPLEParser.js:402:24)

Note: If an item from "syrups" section is present in toppings section (e.g., append "fudge"), the bug does not manifest.

[ericvergnaud]

Hi,
seems to make sense.
Can you submit a PR?
(also add your name to contributors.txt please)

[ericvergnaud]

@plaurent would be great to get your fix in upcoming release, which is imminent :-)

[plaurent]

@ericvergnaud Fantastic news, should be able to get to that later today (in a few hours).

[plaurent]

@ericvergnaud Looks like this bug is already fixed in master in commit dd3af94 -- we look forward to the 4.7.2 release. Thank you.