You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2018/04/03 13:54:21 UTC
groovy git commit: GROOVY-8523: also handle !instanceof operator
Repository: groovy
Updated Branches:
refs/heads/master e79f837e6 -> 263393d77
GROOVY-8523: also handle !instanceof operator
Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/263393d7
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/263393d7
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/263393d7
Branch: refs/heads/master
Commit: 263393d77034cef8585d335d8d0564b3bf5a4755
Parents: e79f837
Author: Paul King <pa...@asert.com.au>
Authored: Tue Apr 3 23:54:12 2018 +1000
Committer: Paul King <pa...@asert.com.au>
Committed: Tue Apr 3 23:54:12 2018 +1000
----------------------------------------------------------------------
gradle/pomconfigurer.gradle | 3 +
.../stc/StaticTypeCheckingVisitor.java | 59 ++++++++++++++++----
src/test/groovy/bugs/Groovy8523Bug.groovy | 56 ++++++++++++++-----
3 files changed, 95 insertions(+), 23 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/groovy/blob/263393d7/gradle/pomconfigurer.gradle
----------------------------------------------------------------------
diff --git a/gradle/pomconfigurer.gradle b/gradle/pomconfigurer.gradle
index de35cdf..9438dc1 100644
--- a/gradle/pomconfigurer.gradle
+++ b/gradle/pomconfigurer.gradle
@@ -562,6 +562,9 @@ project.ext.pomConfigureClosureWithoutTweaks = {
name 'Mike Spille'
}
contributor {
+ name 'Nikolay Chugunov'
+ }
+ contributor {
name 'Paolo Di Tommaso'
}
contributor {
http://git-wip-us.apache.org/repos/asf/groovy/blob/263393d7/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
----------------------------------------------------------------------
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 bd647fc..c8e79bb 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -3470,7 +3470,9 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
BinaryExpression instanceOfExpression = findInstanceOfNotReturnExpression(ifElse);
if (instanceOfExpression == null) {
- } else {
+ instanceOfExpression = findNotInstanceOfReturnExpression(ifElse);
+ }
+ if (instanceOfExpression != null) {
if(typeCheckingContext.enclosingBlocks.size()>0) {
visitInstanceofNot(instanceOfExpression);
}
@@ -3478,7 +3480,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
- public void visitInstanceofNot(BinaryExpression be) {
+ protected void visitInstanceofNot(BinaryExpression be) {
final BlockStatement currentBlock = typeCheckingContext.enclosingBlocks.getFirst();
assert currentBlock != null;
if (typeCheckingContext.blockStatements2Types.containsKey(currentBlock)) {
@@ -3517,15 +3519,15 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
/**
* Check IfStatement matched pattern :
* Object var1;
- * if (!(var1 instanceOf Runnable)){
- * return
+ * if (!(var1 instanceOf Runnable)) {
+ * return
* }
* // Here var1 instance of Runnable
*
* Return expression , which contains instanceOf (without not)
* Return null, if not found
*/
- public BinaryExpression findInstanceOfNotReturnExpression(IfStatement ifElse) {
+ protected BinaryExpression findInstanceOfNotReturnExpression(IfStatement ifElse) {
Statement elseBlock = ifElse.getElseBlock();
if (!(elseBlock instanceof EmptyStatement)) {
return null;
@@ -3544,19 +3546,56 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
if (op != Types.KEYWORD_INSTANCEOF) {
return null;
}
- Statement block = ifElse.getIfBlock();
- if (!(block instanceof BlockStatement)) {
+ if (notReturningBlock(ifElse.getIfBlock())) {
+ return null;
+ }
+ return instanceOfExpression;
+ }
+
+ /**
+ * Check IfStatement matched pattern :
+ * Object var1;
+ * if (var1 !instanceOf Runnable) {
+ * return
+ * }
+ * // Here var1 instance of Runnable
+ *
+ * Return expression , which contains instanceOf (without not)
+ * Return null, if not found
+ */
+ protected BinaryExpression findNotInstanceOfReturnExpression(IfStatement ifElse) {
+ Statement elseBlock = ifElse.getElseBlock();
+ if (!(elseBlock instanceof EmptyStatement)) {
return null;
}
+ Expression conditionExpression = ifElse.getBooleanExpression().getExpression();
+ if (!(conditionExpression instanceof BinaryExpression)) {
+ return null;
+ }
+ BinaryExpression instanceOfExpression = (BinaryExpression) conditionExpression;
+ int op = instanceOfExpression.getOperation().getType();
+ if (op != Types.COMPARE_NOT_INSTANCEOF) {
+ return null;
+ }
+ if (notReturningBlock(ifElse.getIfBlock())) {
+ return null;
+ }
+ return instanceOfExpression;
+ }
+
+ private boolean notReturningBlock(Statement block) {
+ if (!(block instanceof BlockStatement)) {
+ return true;
+ }
BlockStatement bs = (BlockStatement) block;
if (bs.getStatements().size() == 0) {
- return null;
+ return true;
}
Statement last = DefaultGroovyMethods.last(bs.getStatements());
if (!(last instanceof ReturnStatement)) {
- return null;
+ return true;
}
- return instanceOfExpression;
+ return false;
}
@Override
http://git-wip-us.apache.org/repos/asf/groovy/blob/263393d7/src/test/groovy/bugs/Groovy8523Bug.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/bugs/Groovy8523Bug.groovy b/src/test/groovy/bugs/Groovy8523Bug.groovy
index ac59854..d3c3165 100755
--- a/src/test/groovy/bugs/Groovy8523Bug.groovy
+++ b/src/test/groovy/bugs/Groovy8523Bug.groovy
@@ -27,9 +27,9 @@ class Groovy8523Bug extends GroovyTestCase {
static int checkRes = 0
static void f1(Object var1) {
- if(!(var1 instanceof Runnable)){
+ if (!(var1 instanceof Runnable)){
checkRes = 3
- return;
+ return
}
f2(var1)
}
@@ -41,7 +41,37 @@ class Groovy8523Bug extends GroovyTestCase {
Runnable r = {}
Test1.f1(r)
- assert Test1.checkRes == 4;
+ assert Test1.checkRes == 4
+ Test1.f1(42)
+ assert Test1.checkRes == 3
+ '''
+ }
+
+ void testNotInstanceof1() {
+ assertScript '''
+ import groovy.transform.CompileStatic
+ @CompileStatic
+ class Test1 {
+ static int checkRes = 0
+
+ static void f1(Object var1) {
+ if (var1 !instanceof Runnable){
+ checkRes = 3
+ return
+ }
+ f2(var1)
+ }
+
+ static void f2(Runnable var2) {
+ checkRes = 4
+ }
+ }
+
+ Runnable r = {}
+ Test1.f1(r)
+ assert Test1.checkRes == 4
+ Test1.f1(42)
+ assert Test1.checkRes == 3
'''
}
@@ -54,13 +84,13 @@ class Groovy8523Bug extends GroovyTestCase {
static int checkRes = 0
static void f1(Object var1) {
- if(!(var1 instanceof Runnable)){
+ if (!(var1 instanceof Runnable)){
checkRes = 3
- return;
+ return
}
- if(!(var1 instanceof List)){
+ if (!(var1 instanceof List)){
checkRes = 5
- return;
+ return
}
f2(var1)
}
@@ -72,7 +102,7 @@ class Groovy8523Bug extends GroovyTestCase {
Runnable r = {}
Test1.f1(r)
- assert Test1.checkRes == 5;
+ assert Test1.checkRes == 5
'''
}
@@ -85,13 +115,13 @@ class Groovy8523Bug extends GroovyTestCase {
static int checkRes = 0
static void f1(Object var1) {
- if(!(var1 instanceof Runnable)){
+ if (!(var1 instanceof Runnable)){
checkRes = 3
- return;
+ return
}
- if(!(var1 instanceof Thread)){
+ if (!(var1 instanceof Thread)){
checkRes = 5
- return;
+ return
}
f2(var1)
}
@@ -103,7 +133,7 @@ class Groovy8523Bug extends GroovyTestCase {
Runnable r = {}
Test1.f1(r)
- assert Test1.checkRes == 5;
+ assert Test1.checkRes == 5
'''
}