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 01:10:49 UTC
[groovy] branch GROOVY_3_0_X updated (10f40ab -> d897034)
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a change to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git.
from 10f40ab minor refactoring
new 8b76135 SC: check for getter/setter on implicit-this expression in closure
new 46f4466 Tweak `variableDeclaration` rule
new d897034 Tweak unbalanced parens
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
src/antlr/GroovyLexer.g4 | 36 +++++++++++------
src/antlr/GroovyParser.g4 | 2 -
.../transform/stc/StaticTypeCheckingVisitor.java | 2 +-
.../asm/sc/ClosuresStaticCompileTest.groovy | 35 +++++++++++++++-
.../org/apache/groovy/groovysh/Groovysh.groovy | 45 ++++++++++++---------
.../org/apache/groovy/groovysh/GroovyshTest.groovy | 14 +++++++
.../groovy/parser/antlr4/SyntaxErrorTest.groovy | 47 ++++++++++++++++++++++
7 files changed, 147 insertions(+), 34 deletions(-)
[groovy] 01/03: SC: check for getter/setter on implicit-this
expression in closure
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 8b76135a848c6a27e54a59287587e6a4208dd08e
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sun Feb 2 13:46:28 2020 -0600
SC: check for getter/setter on implicit-this expression in closure
@groovy.transform.CompileStatic
void meth(Pogo pogo) {
pogo.with {
string += 'x' // LHS could be Pogo#setString(String)
}
}
(cherry picked from commit 67509d9bb8ff8c18b07755fc58130e3082d7eedf)
---
.../transform/stc/StaticTypeCheckingVisitor.java | 2 +-
.../asm/sc/ClosuresStaticCompileTest.groovy | 35 +++++++++++++++++++++-
2 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index f72aaa2..86e49f5 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -1523,7 +1523,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
PropertyNode property = current.getProperty(propertyName);
property = allowStaticAccessToMember(property, staticOnly);
// prefer explicit getter or setter over property if receiver is not 'this'
- if (property == null || !enclosingTypes.contains(objectExpressionType)) {
+ if (property == null || !enclosingTypes.contains(testClass)) {
if (readMode) {
if (getter != null) {
ClassNode returnType = inferReturnTypeGenerics(current, getter, ArgumentListExpression.EMPTY_ARGUMENTS);
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/ClosuresStaticCompileTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/ClosuresStaticCompileTest.groovy
index e25d0fb..ce7a420 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/ClosuresStaticCompileTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/ClosuresStaticCompileTest.groovy
@@ -108,5 +108,38 @@ sample.doStuff('John', 'Doe') { String errors, String formattedFname, String for
println "${formattedFname}.${formattedLname}"
}'''
}
-}
+ void testMethodVersusPropertyOnCompoundAssignmentTarget() {
+ assertScript '''
+ import org.codehaus.groovy.ast.expr.*
+ import org.codehaus.groovy.ast.stmt.*
+
+ class Pogo {
+ String string = ''
+ void setString(String string) {
+ this.string = string
+ }
+ }
+
+ @ASTTest(phase=INSTRUCTION_SELECTION, value={
+ node = node.code.statements[0].expression
+ assert node instanceof MethodCallExpression
+
+ node = node.arguments.expressions[0].code.statements[0].expression
+ assert node instanceof BinaryExpression
+
+ assert node.leftExpression instanceof PropertyExpression
+ def target = node.leftExpression.getNodeMetaData(DIRECT_METHOD_CALL_TARGET)
+
+ assert target != null && target.name == 'setString'
+ })
+ void test(Pogo pogo) {
+ pogo.with {
+ string += 'x'
+ }
+ }
+
+ test(new Pogo())
+ '''
+ }
+}
[groovy] 02/03: Tweak `variableDeclaration` rule
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 46f446696cdbb943ce0a5cb20e28873b3f815bda
Author: Sam Harwell <sa...@tunnelvisionlabs.com>
AuthorDate: Tue Feb 4 05:01:43 2020 +0800
Tweak `variableDeclaration` rule
```
classifiedModifiers[$t]? type variableDeclarators
```
can be written as:
```
( classifiedModifiers[$t] type variableDeclarators
| type variableDeclarators
)
```
(cherry picked from commit bc84072f29d48977b5951d9777f47201ece834b1)
---
src/antlr/GroovyParser.g4 | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/antlr/GroovyParser.g4 b/src/antlr/GroovyParser.g4
index 93d4662..d77c581 100644
--- a/src/antlr/GroovyParser.g4
+++ b/src/antlr/GroovyParser.g4
@@ -592,13 +592,11 @@ classifiedModifiers[int t]
* t 0: local variable declaration; 1: field declaration
*/
variableDeclaration[int t]
-@leftfactor { classifiedModifiers }
: classifiedModifiers[$t]
( type? variableDeclarators
| typeNamePairs nls ASSIGN nls variableInitializer
)
|
- classifiedModifiers[$t]?
type variableDeclarators
;
[groovy] 03/03: Tweak unbalanced parens
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit d8970348d17e5433030c2d4ef633bb2ae3ed968d
Author: Daniel Sun <su...@apache.org>
AuthorDate: Tue Feb 4 08:28:49 2020 +0800
Tweak unbalanced parens
(cherry picked from commit 81fbbb45d9fd59f2d2ba8b00fdf26c30031175d0)
---
src/antlr/GroovyLexer.g4 | 36 +++++++++++------
.../org/apache/groovy/groovysh/Groovysh.groovy | 45 ++++++++++++---------
.../org/apache/groovy/groovysh/GroovyshTest.groovy | 14 +++++++
.../groovy/parser/antlr4/SyntaxErrorTest.groovy | 47 ++++++++++++++++++++++
4 files changed, 112 insertions(+), 30 deletions(-)
diff --git a/src/antlr/GroovyLexer.g4 b/src/antlr/GroovyLexer.g4
index f11df22..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,19 +153,18 @@ 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() {
Paren paren = parenStack.peek();
@@ -177,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());
}
@@ -209,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;
+ }
}
@@ -805,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 7e3a95c..99f7c79 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
@@ -55,7 +55,6 @@ import java.util.regex.Pattern
*/
class Groovysh extends Shell {
-
private static final MessageSource messages = new MessageSource(Groovysh)
private static final Pattern TYPEDEF_PATTERN = ~'^\\s*((?:public|protected|private|static|abstract|final)\\s+)*(?:class|enum|interface).*'
@@ -116,7 +115,7 @@ class Groovysh extends Shell {
}
private static Closure createDefaultRegistrar(final ClassLoader classLoader) {
- return { Groovysh shell ->
+ return {Groovysh shell ->
URL xmlCommandResource = getClass().getResource('commands.xml')
if (xmlCommandResource != null) {
def r = new XmlCommandRegistrar(shell, classLoader)
@@ -200,12 +199,11 @@ class Groovysh extends Shell {
if (!Boolean.valueOf(getPreference(INTERPRETER_MODE_PREFERENCE_KEY, 'false')) || isTypeOrMethodDeclaration(current)) {
// Evaluate the current buffer w/imports and dummy statement
- List buff = [importsSpec] + ['true'] + current
+ List buff = [importsSpec] + [ 'true' ] + current
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 ')'")) {
+ } catch(MultipleCompilationErrorsException t) {
+ if (isIncompleteCaseOfAntlr4(t)) {
// treat like INCOMPLETE case
buffers.updateSelected(current)
break
@@ -216,9 +214,8 @@ class Groovysh extends Shell {
// Evaluate Buffer wrapped with code storing bounded vars
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 ')'")) {
+ } catch(MultipleCompilationErrorsException t) {
+ if (isIncompleteCaseOfAntlr4(t)) {
// treat like INCOMPLETE case
buffers.updateSelected(current)
break
@@ -246,6 +243,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
@@ -260,7 +269,6 @@ class Groovysh extends Shell {
* to simulate an interpreter mode, this method wraps the statements into a try/finally block that
* stores bound variables like unbound variables
*/
-
private Object evaluateWithStoredBoundVars(String importsSpec, final List<String> current) {
Object result
String variableBlocks = null
@@ -278,7 +286,7 @@ try {$COLLECTED_BOUND_VARS_MAP_VARNAME[\"$varname\"] = $varname;
})
}
// Evaluate the current buffer w/imports and dummy statement
- List<String> buff;
+ List<String> buff
if (variableBlocks) {
buff = [importsSpec] + ['try {', 'true'] + current + ['} finally {' + variableBlocks + '}']
} else {
@@ -295,6 +303,7 @@ try {$COLLECTED_BOUND_VARS_MAP_VARNAME[\"$varname\"] = $varname;
}
+
protected Object executeCommand(final String line) {
return super.execute(line)
}
@@ -313,7 +322,7 @@ try {$COLLECTED_BOUND_VARS_MAP_VARNAME[\"$varname\"] = $varname;
}
String getImportStatements() {
- return this.imports.collect({ String it -> "import $it;" }).join('')
+ return this.imports.collect({String it -> "import $it;"}).join('')
}
//
@@ -331,7 +340,6 @@ try {$COLLECTED_BOUND_VARS_MAP_VARNAME[\"$varname\"] = $varname;
The code will always assume you want the line number in the prompt. To implement differently overhead the render
prompt variable.
*/
-
private String buildPrompt() {
def lineNum = formatLineNumber(buffers.current().size())
@@ -341,7 +349,7 @@ try {$COLLECTED_BOUND_VARS_MAP_VARNAME[\"$varname\"] = $varname;
}
def groovyshellEnv = System.getenv('GROOVYSH_PROMPT')
if (groovyshellEnv) {
- return "@|bold ${groovyshellEnv}:|@${lineNum}@|bold >|@ "
+ return "@|bold ${groovyshellEnv}:|@${lineNum}@|bold >|@ "
}
return "@|bold groovy:|@${lineNum}@|bold >|@ "
@@ -427,7 +435,7 @@ try {$COLLECTED_BOUND_VARS_MAP_VARNAME[\"$varname\"] = $varname;
// Disable the result hook for profile scripts
def previousHook = resultHook
- resultHook = { result -> /* nothing */ }
+ resultHook = { result -> /* nothing */}
try {
command.load(file.toURI().toURL())
@@ -530,7 +538,8 @@ try {$COLLECTED_BOUND_VARS_MAP_VARNAME[\"$varname\"] = $varname;
if (log.debug) {
// If we have debug enabled then skip the fancy bits below
log.debug(cause)
- } else {
+ }
+ else {
boolean sanitize = getPreference(SANITIZE_PREFERENCE_KEY, 'false')
// Sanitize the stack trace unless we are in verbose mode, or the user has request otherwise
@@ -604,7 +613,7 @@ try {$COLLECTED_BOUND_VARS_MAP_VARNAME[\"$varname\"] = $varname;
startCommands.add(evalString)
}
if (filenames != null && filenames.size() > 0) {
- startCommands.addAll(filenames.collect({ String it -> "${LoadCommand.COMMAND_NAME} $it" }))
+ startCommands.addAll(filenames.collect({String it -> "${LoadCommand.COMMAND_NAME} $it"}))
}
return run(startCommands.join('\n'))
}
@@ -667,7 +676,7 @@ try {$COLLECTED_BOUND_VARS_MAP_VARNAME[\"$varname\"] = $varname;
* maybe displays log information and a welcome message
* @param term
*/
- public void displayWelcomeBanner(InteractiveShellRunner runner) {
+ void displayWelcomeBanner(InteractiveShellRunner runner) {
if (!log.debug && io.quiet) {
// nothing to do here
return
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 7d3ebeb..614c13a 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)