Painless: Fix Semicolon Regression (#33212)

Trailers (statements following something like an if statement) that don't use brackets currently require a semicolon even if they're the last statement. This is a regression caused by (#29566) and noted by (#33193). This change fixes the regression and adds a test for the broken case.
This commit is contained in:
Jack Conradson 2018-08-28 12:40:41 -07:00 committed by GitHub
parent e9b0807c67
commit a381749aac
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 667 additions and 692 deletions

View File

@ -22,7 +22,7 @@ parser grammar PainlessParser;
options { tokenVocab=PainlessLexer; } options { tokenVocab=PainlessLexer; }
source source
: function* statement* dstatement? EOF : function* statement* EOF
; ;
function function
@ -35,7 +35,7 @@ parameters
statement statement
: rstatement : rstatement
| dstatement SEMICOLON | dstatement ( SEMICOLON | EOF )
; ;
// Note we use a predicate on the if/else case here to prevent the // Note we use a predicate on the if/else case here to prevent the

View File

@ -1,17 +1,13 @@
// ANTLR GENERATED CODE: DO NOT EDIT // ANTLR GENERATED CODE: DO NOT EDIT
package org.elasticsearch.painless.antlr; package org.elasticsearch.painless.antlr;
import org.antlr.v4.runtime.CharStream;
import org.antlr.v4.runtime.Lexer; import org.antlr.v4.runtime.Lexer;
import org.antlr.v4.runtime.RuleContext; import org.antlr.v4.runtime.CharStream;
import org.antlr.v4.runtime.RuntimeMetaData; import org.antlr.v4.runtime.Token;
import org.antlr.v4.runtime.Vocabulary; import org.antlr.v4.runtime.TokenStream;
import org.antlr.v4.runtime.VocabularyImpl; import org.antlr.v4.runtime.*;
import org.antlr.v4.runtime.atn.ATN; import org.antlr.v4.runtime.atn.*;
import org.antlr.v4.runtime.atn.ATNDeserializer;
import org.antlr.v4.runtime.atn.LexerATNSimulator;
import org.antlr.v4.runtime.atn.PredictionContextCache;
import org.antlr.v4.runtime.dfa.DFA; import org.antlr.v4.runtime.dfa.DFA;
import org.antlr.v4.runtime.misc.*;
@SuppressWarnings({"all", "warnings", "unchecked", "unused", "cast"}) @SuppressWarnings({"all", "warnings", "unchecked", "unused", "cast"})
abstract class PainlessLexer extends Lexer { abstract class PainlessLexer extends Lexer {

View File

@ -261,10 +261,6 @@ public final class Walker extends PainlessParserBaseVisitor<ANode> {
statements.add((AStatement)visit(statement)); statements.add((AStatement)visit(statement));
} }
if (ctx.dstatement() != null) {
statements.add((AStatement)visit(ctx.dstatement()));
}
return new SSource(scriptClassInfo, settings, sourceName, debugStream, (MainMethodReserved)reserved.pop(), return new SSource(scriptClassInfo, settings, sourceName, debugStream, (MainMethodReserved)reserved.pop(),
location(ctx), functions, globals, statements); location(ctx), functions, globals, statements);
} }

View File

@ -129,4 +129,8 @@ public class BasicAPITests extends ScriptTestCase {
assertEquals(5, exec("org.elasticsearch.painless.FeatureTest ft = new org.elasticsearch.painless.FeatureTest();" + assertEquals(5, exec("org.elasticsearch.painless.FeatureTest ft = new org.elasticsearch.painless.FeatureTest();" +
"ft.z = 5; return ft.z;")); "ft.z = 5; return ft.z;"));
} }
public void testNoSemicolon() {
assertEquals(true, exec("def x = true; if (x) return x"));
}
} }

View File

@ -278,6 +278,6 @@ public class RegexTests extends ScriptTestCase {
IllegalArgumentException e = expectScriptThrows(IllegalArgumentException.class, () -> { IllegalArgumentException e = expectScriptThrows(IllegalArgumentException.class, () -> {
exec("/asdf/b", false); // Not picky so we get a non-assertion error exec("/asdf/b", false); // Not picky so we get a non-assertion error
}); });
assertEquals("invalid sequence of tokens near ['b'].", e.getMessage()); assertEquals("unexpected token ['b'] was expecting one of [{<EOF>, ';'}].", e.getMessage());
} }
} }

View File

@ -255,7 +255,7 @@ public class WhenThingsGoWrongTests extends ScriptTestCase {
// We don't want PICKY here so we get the normal error message // We don't want PICKY here so we get the normal error message
exec("def i = 1} return 1", emptyMap(), emptyMap(), null, false); exec("def i = 1} return 1", emptyMap(), emptyMap(), null, false);
}); });
assertEquals("invalid sequence of tokens near ['}'].", e.getMessage()); assertEquals("unexpected token ['}'] was expecting one of [{<EOF>, ';'}].", e.getMessage());
} }
public void testBadBoxingCast() { public void testBadBoxingCast() {