You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2020/02/04 00:48:37 UTC
[groovy] 02/02: Tweak unbalanced parens
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 81fbbb45d9fd59f2d2ba8b00fdf26c30031175d0
Author: Daniel Sun <su...@apache.org>
AuthorDate: Tue Feb 4 08:28:49 2020 +0800
Tweak unbalanced parens
---
src/antlr/GroovyLexer.g4 | 37 +++++++++++------
.../org/apache/groovy/groovysh/Groovysh.groovy | 18 +++++++--
.../org/apache/groovy/groovysh/GroovyshTest.groovy | 14 +++++++
.../groovy/parser/antlr4/SyntaxErrorTest.groovy | 47 ++++++++++++++++++++++
4 files changed, 99 insertions(+), 17 deletions(-)
diff --git a/src/antlr/GroovyLexer.g4 b/src/antlr/GroovyLexer.g4
index 016824f..6456af1 100644
--- a/src/antlr/GroovyLexer.g4
+++ b/src/antlr/GroovyLexer.g4
@@ -47,11 +47,15 @@ options {
import java.util.Collections;
import java.util.Arrays;
import java.util.stream.IntStream;
+ import java.util.logging.Logger;
+ import java.util.logging.Level;
+ import java.util.EmptyStackException;
import org.apache.groovy.util.Maps;
import static org.apache.groovy.parser.antlr4.SemanticPredicates.*;
}
@members {
+ private static final Logger LOGGER = Logger.getLogger(GroovyLexer.class.getName());
private long tokenIndex = 0;
private int lastTokenType = 0;
private int invalidDigitCount = 0;
@@ -140,13 +144,6 @@ options {
}
}
- private static final Map<String, String> PAREN_MAP =
- Maps.of(
- "(", ")",
- "[", "]",
- "{", "}"
- );
-
protected void enterParenCallback(String text) {}
protected void exitParenCallback(String text) {}
@@ -156,18 +153,16 @@ options {
private void enterParen() {
String text = getText();
enterParenCallback(text);
+
parenStack.push(new Paren(text, this.lastTokenType, getLine(), getCharPositionInLine()));
}
private void exitParen() {
- Paren paren = parenStack.peek();
String text = getText();
-
- require(null != paren, "Too many '" + text + "'");
- require(text.equals(PAREN_MAP.get(paren.getText())),
- "'" + paren.getText() + "'" + new PositionInfo(paren.getLine(), paren.getColumn()) + " can not match '" + text + "'", -1);
-
exitParenCallback(text);
+
+ Paren paren = parenStack.peek();
+ if (null == paren) return;
parenStack.pop();
}
private boolean isInsideParens() {
@@ -178,6 +173,7 @@ options {
if (null == paren) {
return false;
}
+
return ("(".equals(paren.getText()) && TRY != paren.getLastTokenType()) // we don't treat try-paren(i.e. try (....)) as parenthesis
|| "[".equals(paren.getText());
}
@@ -210,6 +206,19 @@ options {
public int getErrorColumn() {
return getCharPositionInLine() + 1;
}
+
+ @Override
+ public int popMode() {
+ try {
+ return super.popMode();
+ } catch (EmptyStackException ignored) { // raised when parens are unmatched: too many ), ], or }
+ if (LOGGER.isLoggable(Level.FINEST)) {
+ LOGGER.finest(org.codehaus.groovy.runtime.DefaultGroovyMethods.asString(ignored));
+ }
+ }
+
+ return Integer.MIN_VALUE;
+ }
}
@@ -806,8 +815,10 @@ NOT_IN : '!in' { isFollowedBy(_input, ' ', '\t', '\r', '\n'
LPAREN : '(' { this.enterParen(); } -> pushMode(DEFAULT_MODE);
RPAREN : ')' { this.exitParen(); } -> popMode;
+
LBRACE : '{' { this.enterParen(); } -> pushMode(DEFAULT_MODE);
RBRACE : '}' { this.exitParen(); } -> popMode;
+
LBRACK : '[' { this.enterParen(); } -> pushMode(DEFAULT_MODE);
RBRACK : ']' { this.exitParen(); } -> popMode;
diff --git a/subprojects/groovy-groovysh/src/main/groovy/org/apache/groovy/groovysh/Groovysh.groovy b/subprojects/groovy-groovysh/src/main/groovy/org/apache/groovy/groovysh/Groovysh.groovy
index 414a092..77c7fcf 100644
--- a/subprojects/groovy-groovysh/src/main/groovy/org/apache/groovy/groovysh/Groovysh.groovy
+++ b/subprojects/groovy-groovysh/src/main/groovy/org/apache/groovy/groovysh/Groovysh.groovy
@@ -202,8 +202,7 @@ class Groovysh extends Shell {
try {
setLastResult(result = interp.evaluate(buff))
} catch(MultipleCompilationErrorsException t) {
- // TODO antlr4 parser errors pop out here - can we rework to be like antlr2?
- if (t.message.contains('Unexpected input:') || t.message.contains("Missing ')'")) {
+ if (isIncompleteCaseOfAntlr4(t)) {
// treat like INCOMPLETE case
buffers.updateSelected(current)
break
@@ -215,8 +214,7 @@ class Groovysh extends Shell {
try {
result = evaluateWithStoredBoundVars(importsSpec, current)
} catch(MultipleCompilationErrorsException t) {
- // TODO antlr4 parser errors pop out here - can we rework to be like antlr2?
- if (t.message.contains('Unexpected input:') || t.message.contains("Missing ')'")) {
+ if (isIncompleteCaseOfAntlr4(t)) {
// treat like INCOMPLETE case
buffers.updateSelected(current)
break
@@ -244,6 +242,18 @@ class Groovysh extends Shell {
return result
}
+ @CompileStatic
+ private boolean isIncompleteCaseOfAntlr4(MultipleCompilationErrorsException t) {
+ // TODO antlr4 parser errors pop out here - can we rework to be like antlr2?
+ (
+ t.message.contains('Unexpected input:') && !(
+ t.message.contains('Unexpected input: \'}\';')
+ || t.message.contains('Unexpected input: \')\';')
+ || t.message.contains('Unexpected input: \']\';')
+ )
+ ) || t.message.contains("Missing ')'")
+ }
+
/**
* return true if the buffer can be recognized as a type declaration statement
* @param strings
diff --git a/subprojects/groovy-groovysh/src/test/groovy/org/apache/groovy/groovysh/GroovyshTest.groovy b/subprojects/groovy-groovysh/src/test/groovy/org/apache/groovy/groovysh/GroovyshTest.groovy
index ab9541d..74a5be3 100644
--- a/subprojects/groovy-groovysh/src/test/groovy/org/apache/groovy/groovysh/GroovyshTest.groovy
+++ b/subprojects/groovy-groovysh/src/test/groovy/org/apache/groovy/groovysh/GroovyshTest.groovy
@@ -96,6 +96,20 @@ class GroovyshTest extends GroovyTestCase {
} catch (MultipleCompilationErrorsException e) {
assert '' == mockOut.toString()
}
+
+ try {
+ groovysh.execute('x)')
+ fail('expected MultipleCompilationErrorsException ')
+ } catch (MultipleCompilationErrorsException e) {
+ assert '' == mockOut.toString()
+ }
+
+ try {
+ groovysh.execute('x]')
+ fail('expected MultipleCompilationErrorsException ')
+ } catch (MultipleCompilationErrorsException e) {
+ assert '' == mockOut.toString()
+ }
}
void testIncompleteBracketMultilineExpr() {
diff --git a/subprojects/parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/SyntaxErrorTest.groovy b/subprojects/parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/SyntaxErrorTest.groovy
index 86bc59d..98d2a1c 100644
--- a/subprojects/parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/SyntaxErrorTest.groovy
+++ b/subprojects/parser-antlr4/src/test/groovy/org/apache/groovy/parser/antlr4/SyntaxErrorTest.groovy
@@ -21,6 +21,8 @@ package org.apache.groovy.parser.antlr4
import groovy.test.GroovyTestCase
import groovy.test.NotYetImplemented
import org.apache.groovy.parser.antlr4.util.ASTComparatorCategory
+import org.codehaus.groovy.control.CompilationUnit
+import org.codehaus.groovy.control.Phases
/**
* Some syntax error test cases for the new parser.
@@ -249,7 +251,52 @@ final class SyntaxErrorTest extends GroovyTestCase {
TestUtils.doRunAndShouldFail('fail/Array_02x.groovy')
}
+
+ void "test error alternative - Missing ')' 1"() {
+ def err = expectFail '''\
+println ((int 123)
+ '''
+
+ assert err == '''\
+startup failed:
+test.groovy: 1: Missing ')' @ line 1, column 14.
+ println ((int 123)
+ ^
+
+1 error
+'''
+ }
+
+ void "test error alternative - Missing ')' 2"() {
+ def err = expectFail '''\
+def x() {
+ println((int) 123
+}
+ '''
+
+ assert err == '''\
+startup failed:
+test.groovy: 2: Missing ')' @ line 2, column 22.
+ println((int) 123
+ ^
+
+1 error
+'''
+ }
+
//--------------------------------------------------------------------------
+ def expectFail(String code) {
+ try {
+ def ast = new CompilationUnit().tap {
+ addSource 'test.groovy', code
+ compile Phases.CONVERSION
+ }.getAST()
+
+ fail("should fail")
+ } catch (e) {
+ return e.message.replaceAll('\r\n', '\n')
+ }
+ }
private static unzipScriptAndShouldFail(String entryName, List ignoreClazzList, Map<String, String> replacementsMap=[:], boolean toCheckNewParserOnly = false) {
ignoreClazzList.addAll(TestUtils.COMMON_IGNORE_CLASS_LIST)