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 2019/12/12 23:45:17 UTC

[groovy] branch GROOVY_3_0_X updated (028a31a -> 06c7652)

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 028a31a  Add the missing checks for lambda to align with closure
     new 648fca2  minor edits
     new 93d8c7e  output wildcard generics as <? super T> not <java.lang.Object super T>
     new 9b3b11d  minor edits
     new 06c7652  GROOVY-9329: OOME raised while `parseClass(scriptText)` (#1117)

The 4 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/main/java/groovy/lang/GroovyClassLoader.java   |  10 +-
 .../codehaus/groovy/antlr/AntlrParserPlugin.java   |   1 -
 .../java/org/codehaus/groovy/ast/GenericsType.java | 327 +++++++++++----------
 .../codehaus/groovy/ast/tools/GenericsUtils.java   |   1 -
 .../org/codehaus/groovy/classgen/EnumVisitor.java  | 104 +++----
 .../vmplugin/v5/PluginDefaultGroovyMethods.java    |  34 +--
 .../bugs/{Groovy9294.groovy => Groovy9329.groovy}  |  23 +-
 .../groovy/transform/stc/GenericsSTCTest.groovy    |   6 +-
 .../apache/groovy/parser/antlr4/AstBuilder.java    |   1 -
 9 files changed, 257 insertions(+), 250 deletions(-)
 copy src/test/groovy/bugs/{Groovy9294.groovy => Groovy9329.groovy} (73%)


[groovy] 03/04: minor edits

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 9b3b11d1238b64e14b114160c6c5d2c09dc656e3
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Thu Dec 12 15:49:56 2019 -0600

    minor edits
    
    (cherry picked from commit 9b32406111235493fc873510a86575053fe87128)
---
 .../org/codehaus/groovy/classgen/EnumVisitor.java  | 104 ++++++++++-----------
 .../vmplugin/v5/PluginDefaultGroovyMethods.java    |  34 +++----
 2 files changed, 67 insertions(+), 71 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java b/src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java
index 86d1a29..793ab82 100644
--- a/src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java
+++ b/src/main/java/org/codehaus/groovy/classgen/EnumVisitor.java
@@ -56,69 +56,70 @@ import org.codehaus.groovy.control.messages.SyntaxErrorMessage;
 import org.codehaus.groovy.syntax.SyntaxException;
 import org.codehaus.groovy.syntax.Token;
 import org.codehaus.groovy.syntax.Types;
-import org.objectweb.asm.Opcodes;
 
 import java.util.ArrayList;
 import java.util.List;
 
 import static org.codehaus.groovy.ast.tools.GeneralUtils.localVarX;
+import static org.objectweb.asm.Opcodes.ACC_ABSTRACT;
+import static org.objectweb.asm.Opcodes.ACC_FINAL;
+import static org.objectweb.asm.Opcodes.ACC_PRIVATE;
+import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
+import static org.objectweb.asm.Opcodes.ACC_STATIC;
+import static org.objectweb.asm.Opcodes.ACC_SYNTHETIC;
 
 public class EnumVisitor extends ClassCodeVisitorSupport {
-    // some constants for modifiers
-    private static final int FS = Opcodes.ACC_FINAL | Opcodes.ACC_STATIC;
-    private static final int PS = Opcodes.ACC_PUBLIC | Opcodes.ACC_STATIC;
-    private static final int PUBLIC_FS = Opcodes.ACC_PUBLIC | FS;
-    private static final int PRIVATE_FS = Opcodes.ACC_PRIVATE | FS;
 
     private final SourceUnit sourceUnit;
 
-    public EnumVisitor(CompilationUnit cu, SourceUnit su) {
+    public EnumVisitor(final CompilationUnit cu, final SourceUnit su) {
         sourceUnit = su;
     }
 
-    public void visitClass(ClassNode node) {
-        if (!node.isEnum()) return;
-        completeEnum(node);
-    }
-
+    @Override
     protected SourceUnit getSourceUnit() {
         return sourceUnit;
     }
 
-    private void completeEnum(ClassNode enumClass) {
-        boolean isAic = isAnonymousInnerClass(enumClass);
-        // create MIN_VALUE and MAX_VALUE fields
+    @Override
+    public void visitClass(final ClassNode node) {
+        if (!node.isEnum()) return;
+        completeEnum(node);
+    }
+
+    private void completeEnum(final ClassNode enumClass) {
         FieldNode minValue = null, maxValue = null, values = null;
 
+        boolean isAic = isAnonymousInnerClass(enumClass);
         if (!isAic) {
             ClassNode enumRef = enumClass.getPlainNodeReference();
 
-            // create values field
-            values = new FieldNode("$VALUES", PRIVATE_FS | Opcodes.ACC_SYNTHETIC, enumRef.makeArray(), enumClass, null);
+            // create $VALUES field
+            values = new FieldNode("$VALUES", ACC_FINAL | ACC_PRIVATE | ACC_STATIC | ACC_SYNTHETIC, enumRef.makeArray(), enumClass, null);
             values.setSynthetic(true);
 
             addMethods(enumClass, values);
             checkForAbstractMethods(enumClass);
 
             // create MIN_VALUE and MAX_VALUE fields
-            minValue = new FieldNode("MIN_VALUE", PUBLIC_FS, enumRef, enumClass, null);
-            maxValue = new FieldNode("MAX_VALUE", PUBLIC_FS, enumRef, enumClass, null);
+            minValue = new FieldNode("MIN_VALUE", ACC_FINAL | ACC_PUBLIC | ACC_STATIC, enumRef, enumClass, null);
+            maxValue = new FieldNode("MAX_VALUE", ACC_FINAL | ACC_PUBLIC | ACC_STATIC, enumRef, enumClass, null);
         }
+
         addInit(enumClass, minValue, maxValue, values, isAic);
     }
 
-    private static void checkForAbstractMethods(ClassNode enumClass) {
-        List<MethodNode> methods = enumClass.getMethods();
-        for (MethodNode m : methods) {
-            if (m.isAbstract()) {
-                // make the class abstract also see Effective Java p.152
-                enumClass.setModifiers(enumClass.getModifiers() | Opcodes.ACC_ABSTRACT);
+    private static void checkForAbstractMethods(final ClassNode enumClass) {
+        for (MethodNode method : enumClass.getMethods()) {
+            if (method.isAbstract()) {
+                // make the class abstract also; see Effective Java p.152
+                enumClass.setModifiers(enumClass.getModifiers() | ACC_ABSTRACT);
                 break;
             }
         }
     }
 
-    private static void addMethods(ClassNode enumClass, FieldNode values) {
+    private static void addMethods(final ClassNode enumClass, final FieldNode values) {
         List<MethodNode> methods = enumClass.getMethods();
 
         boolean hasNext = false;
@@ -133,7 +134,7 @@ public class EnumVisitor extends ClassCodeVisitorSupport {
 
         {
             // create values() method
-            MethodNode valuesMethod = new MethodNode("values", PUBLIC_FS, enumRef.makeArray(), Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null);
+            MethodNode valuesMethod = new MethodNode("values", ACC_FINAL | ACC_PUBLIC | ACC_STATIC, enumRef.makeArray(), Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null);
             valuesMethod.setSynthetic(true);
             BlockStatement code = new BlockStatement();
             MethodCallExpression cloneCall = new MethodCallExpression(new FieldExpression(values), "clone", MethodCallExpression.NO_ARGUMENTS);
@@ -152,7 +153,7 @@ public class EnumVisitor extends ClassCodeVisitorSupport {
             //     }
             Token assign = Token.newSymbol(Types.ASSIGN, -1, -1);
             Token ge = Token.newSymbol(Types.COMPARE_GREATER_THAN_EQUAL, -1, -1);
-            MethodNode nextMethod = new MethodNode("next", Opcodes.ACC_PUBLIC | Opcodes.ACC_SYNTHETIC, enumRef, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null);
+            MethodNode nextMethod = new MethodNode("next", ACC_PUBLIC | ACC_SYNTHETIC, enumRef, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null);
             nextMethod.setSynthetic(true);
             BlockStatement code = new BlockStatement();
             BlockStatement ifStatement = new BlockStatement();
@@ -211,8 +212,8 @@ public class EnumVisitor extends ClassCodeVisitorSupport {
             //    }
             Token assign = Token.newSymbol(Types.ASSIGN, -1, -1);
             Token lt = Token.newSymbol(Types.COMPARE_LESS_THAN, -1, -1);
-            MethodNode nextMethod = new MethodNode("previous", Opcodes.ACC_PUBLIC | Opcodes.ACC_SYNTHETIC, enumRef, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null);
-            nextMethod.setSynthetic(true);
+            MethodNode prevMethod = new MethodNode("previous", ACC_PUBLIC | ACC_SYNTHETIC, enumRef, Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, null);
+            prevMethod.setSynthetic(true);
             BlockStatement code = new BlockStatement();
             BlockStatement ifStatement = new BlockStatement();
             ifStatement.addStatement(
@@ -263,14 +264,14 @@ public class EnumVisitor extends ClassCodeVisitorSupport {
                             new MethodCallExpression(new FieldExpression(values), "getAt", new VariableExpression("ordinal"))
                     )
             );
-            nextMethod.setCode(code);
-            enumClass.addMethod(nextMethod);
+            prevMethod.setCode(code);
+            enumClass.addMethod(prevMethod);
         }
 
         {
             // create valueOf
             Parameter stringParameter = new Parameter(ClassHelper.STRING_TYPE, "name");
-            MethodNode valueOfMethod = new MethodNode("valueOf", PS, enumRef, new Parameter[]{stringParameter}, ClassNode.EMPTY_ARRAY, null);
+            MethodNode valueOfMethod = new MethodNode("valueOf", ACC_PUBLIC | ACC_STATIC, enumRef, new Parameter[]{stringParameter}, ClassNode.EMPTY_ARRAY, null);
             ArgumentListExpression callArguments = new ArgumentListExpression();
             callArguments.addExpression(new ClassExpression(enumClass));
             callArguments.addExpression(new VariableExpression("name"));
@@ -287,15 +288,13 @@ public class EnumVisitor extends ClassCodeVisitorSupport {
         }
     }
 
-    private void addInit(ClassNode enumClass, FieldNode minValue,
-                         FieldNode maxValue, FieldNode values,
-                         boolean isAic) {
+    private void addInit(final ClassNode enumClass, final FieldNode minValue, final FieldNode maxValue, final FieldNode values, final boolean isAic) {
         // constructor helper
         // This method is used instead of calling the constructor as
         // calling the constructor may require a table with MetaClass
         // selecting the constructor for each enum value. So instead we
         // use this method to have a central point for constructor selection
-        // and only one table. The whole construction is needed because 
+        // and only one table. The whole construction is needed because
         // Reflection forbids access to the enum constructor.
         // code:
         // def $INIT(Object[] para) {
@@ -303,7 +302,7 @@ public class EnumVisitor extends ClassCodeVisitorSupport {
         // }
         ClassNode enumRef = enumClass.getPlainNodeReference();
         Parameter[] parameter = new Parameter[]{new Parameter(ClassHelper.OBJECT_TYPE.makeArray(), "para")};
-        MethodNode initMethod = new MethodNode("$INIT", PUBLIC_FS | Opcodes.ACC_SYNTHETIC, enumRef, parameter, ClassNode.EMPTY_ARRAY, null);
+        MethodNode initMethod = new MethodNode("$INIT", ACC_FINAL | ACC_PUBLIC | ACC_STATIC | ACC_SYNTHETIC, enumRef, parameter, ClassNode.EMPTY_ARRAY, null);
         initMethod.setSynthetic(true);
         ConstructorCallExpression cce = new ConstructorCallExpression(
                 ClassNode.THIS,
@@ -318,15 +317,15 @@ public class EnumVisitor extends ClassCodeVisitorSupport {
 
         // static init
         List<FieldNode> fields = enumClass.getFields();
-        List<Expression> arrayInit = new ArrayList<Expression>();
+        List<Expression> arrayInit = new ArrayList<>();
         int value = -1;
         Token assign = Token.newSymbol(Types.ASSIGN, -1, -1);
-        List<Statement> block = new ArrayList<Statement>();
+        List<Statement> block = new ArrayList<>();
         FieldNode tempMin = null;
         FieldNode tempMax = null;
         for (FieldNode field : fields) {
-            if ((field.getModifiers() & Opcodes.ACC_ENUM) == 0) continue;
-            value++;
+            if (!field.isEnum()) continue;
+            value += 1;
             if (tempMin == null) tempMin = field;
             tempMax = field;
 
@@ -335,13 +334,13 @@ public class EnumVisitor extends ClassCodeVisitorSupport {
             args.addExpression(new ConstantExpression(field.getName()));
             args.addExpression(new ConstantExpression(value));
             if (field.getInitialExpression() == null) {
-                if ((enumClass.getModifiers() & Opcodes.ACC_ABSTRACT) != 0) {
+                if (enumClass.isAbstract()) {
                     addError(field, "The enum constant " + field.getName() + " must override abstract methods from " + enumBase.getName() + ".");
                     continue;
                 }
             } else {
                 ListExpression oldArgs = (ListExpression) field.getInitialExpression();
-                List<MapEntryExpression> savedMapEntries = new ArrayList<MapEntryExpression>();
+                List<MapEntryExpression> savedMapEntries = new ArrayList<>();
                 for (Expression exp : oldArgs.getExpressions()) {
                     if (exp instanceof MapEntryExpression) {
                         savedMapEntries.add((MapEntryExpression) exp);
@@ -361,7 +360,7 @@ public class EnumVisitor extends ClassCodeVisitorSupport {
                         for (MethodNode methodNode : baseMethods) {
                             if (!methodNode.isAbstract()) continue;
                             MethodNode enumConstMethod = inner.getMethod(methodNode.getName(), methodNode.getParameters());
-                            if (enumConstMethod == null || (enumConstMethod.getModifiers() & Opcodes.ACC_ABSTRACT) != 0) {
+                            if (enumConstMethod == null || enumConstMethod.isAbstract()) {
                                 addError(field, "Can't have an abstract method in enum constant " + field.getName() + ". Implement method '" + methodNode.getTypeDescriptor() + "'.");
                             }
                         }
@@ -372,7 +371,7 @@ public class EnumVisitor extends ClassCodeVisitorSupport {
                              * so that subclasses of enum generated for enum constants (aic) can provide
                              * their own $INIT method
                              */
-                            initMethod.setModifiers(initMethod.getModifiers() & ~Opcodes.ACC_FINAL);
+                            initMethod.setModifiers(initMethod.getModifiers() & ~ACC_FINAL);
                             continue;
                         }
                     }
@@ -429,16 +428,17 @@ public class EnumVisitor extends ClassCodeVisitorSupport {
         enumClass.addStaticInitializerStatements(block, true);
     }
 
-    private void addError(AnnotatedNode exp, String msg) {
-        sourceUnit.getErrorCollector().addErrorAndContinue(
+    private void addError(final AnnotatedNode exp, final String msg) {
+        getSourceUnit().getErrorCollector().addErrorAndContinue(
                 new SyntaxErrorMessage(
-                        new SyntaxException(msg + '\n', exp.getLineNumber(), exp.getColumnNumber(), exp.getLastLineNumber(), exp.getLastColumnNumber()), sourceUnit)
+                        new SyntaxException(msg + '\n', exp),
+                        getSourceUnit()
+                )
         );
     }
 
-    private static boolean isAnonymousInnerClass(ClassNode enumClass) {
+    private static boolean isAnonymousInnerClass(final ClassNode enumClass) {
         if (!(enumClass instanceof EnumConstantClassNode)) return false;
-        InnerClassNode ic = (InnerClassNode) enumClass;
-        return ic.getVariableScope() == null;
+        return (((EnumConstantClassNode) enumClass).getVariableScope() == null);
     }
 }
diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v5/PluginDefaultGroovyMethods.java b/src/main/java/org/codehaus/groovy/vmplugin/v5/PluginDefaultGroovyMethods.java
index f30bcb2..04b8041 100644
--- a/src/main/java/org/codehaus/groovy/vmplugin/v5/PluginDefaultGroovyMethods.java
+++ b/src/main/java/org/codehaus/groovy/vmplugin/v5/PluginDefaultGroovyMethods.java
@@ -34,7 +34,6 @@ import java.util.Arrays;
  * first parameter the destination class.
  */
 public class PluginDefaultGroovyMethods extends DefaultGroovyMethodsSupport {
-    private static final Object[] NO_ARGS = new Object[0];
 
     /**
      * This method is called by the ++ operator for enums. It will invoke
@@ -44,14 +43,13 @@ public class PluginDefaultGroovyMethods extends DefaultGroovyMethodsSupport {
      * @param self an Enum
      * @return the next defined enum from the enum class
      */
-    public static Object next(Enum self) {
-        final Method[] methods = self.getClass().getMethods();
-        for (Method method : methods) {
-            if (method.getName().equals("next") && method.getParameterTypes().length == 0) {
-                return InvokerHelper.invokeMethod(self, "next", NO_ARGS);
+    public static Object next(final Enum self) {
+        for (Method method : self.getClass().getMethods()) {
+            if (method.getName().equals("next") && method.getParameterCount() == 0) {
+                return InvokerHelper.invokeMethod(self, "next", InvokerHelper.EMPTY_ARGS);
             }
         }
-        Object[] values = (Object[]) InvokerHelper.invokeStaticMethod(self.getClass(), "values", NO_ARGS);
+        Object[] values = (Object[]) InvokerHelper.invokeStaticMethod(self.getClass(), "values", InvokerHelper.EMPTY_ARGS);
         int index = Arrays.asList(values).indexOf(self);
         return values[index < values.length - 1 ? index + 1 : 0];
     }
@@ -64,14 +62,13 @@ public class PluginDefaultGroovyMethods extends DefaultGroovyMethodsSupport {
      * @param self an Enum
      * @return the previous defined enum from the enum class
      */
-    public static Object previous(Enum self) {
-        final Method[] methods = self.getClass().getMethods();
-        for (Method method : methods) {
-            if (method.getName().equals("previous") && method.getParameterTypes().length == 0) {
-                return InvokerHelper.invokeMethod(self, "previous", NO_ARGS);
+    public static Object previous(final Enum self) {
+        for (Method method : self.getClass().getMethods()) {
+            if (method.getName().equals("previous") && method.getParameterCount() == 0) {
+                return InvokerHelper.invokeMethod(self, "previous", InvokerHelper.EMPTY_ARGS);
             }
         }
-        Object[] values = (Object[]) InvokerHelper.invokeStaticMethod(self.getClass(), "values", NO_ARGS);
+        Object[] values = (Object[]) InvokerHelper.invokeStaticMethod(self.getClass(), "values", InvokerHelper.EMPTY_ARGS);
         int index = Arrays.asList(values).indexOf(self);
         return values[index > 0 ? index - 1 : values.length - 1];
     }
@@ -82,7 +79,7 @@ public class PluginDefaultGroovyMethods extends DefaultGroovyMethodsSupport {
      * @param builder a StringBuilder
      * @return the length of the StringBuilder
      */
-    public static int size(StringBuilder builder) {
+    public static int size(final StringBuilder builder) {
         return builder.length();
     }
 
@@ -94,7 +91,7 @@ public class PluginDefaultGroovyMethods extends DefaultGroovyMethodsSupport {
      * @param value a value to append
      * @return the StringBuilder on which this operation was invoked
      */
-    public static StringBuilder leftShift(StringBuilder self, Object value) {
+    public static StringBuilder leftShift(final StringBuilder self, final Object value) {
         if (value instanceof GString) {
             // Force the conversion of the GString to string now, or appending
             // is going to be extremely expensive, due to calls to GString#charAt,
@@ -115,7 +112,7 @@ public class PluginDefaultGroovyMethods extends DefaultGroovyMethodsSupport {
      * @param range a Range
      * @param value the object that's toString() will be inserted
      */
-    public static void putAt(StringBuilder self, IntRange range, Object value) {
+    public static void putAt(final StringBuilder self, final IntRange range, final Object value) {
         RangeInfo info = subListBorders(self.length(), range);
         self.replace(info.from, info.to, value.toString());
     }
@@ -127,7 +124,7 @@ public class PluginDefaultGroovyMethods extends DefaultGroovyMethodsSupport {
      * @param range a Range
      * @param value the object that's toString() will be inserted
      */
-    public static void putAt(StringBuilder self, EmptyRange range, Object value) {
+    public static void putAt(final StringBuilder self, final EmptyRange range, final Object value) {
         RangeInfo info = subListBorders(self.length(), range);
         self.replace(info.from, info.to, value.toString());
     }
@@ -139,8 +136,7 @@ public class PluginDefaultGroovyMethods extends DefaultGroovyMethodsSupport {
      * @param value a String
      * @return a String
      */
-    public static String plus(StringBuilder self, String value) {
+    public static String plus(final StringBuilder self, final String value) {
         return self + value;
     }
-
 }


[groovy] 04/04: GROOVY-9329: OOME raised while `parseClass(scriptText)` (#1117)

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 06c7652a24e66541880ef4d403ee1637528faba0
Author: Daniel.Sun <su...@apache.org>
AuthorDate: Fri Dec 13 07:35:32 2019 +0800

    GROOVY-9329: OOME raised while `parseClass(scriptText)` (#1117)
    
    (cherry picked from commit 635039e03e71b361931e77e7328baff9481d85cd)
---
 src/main/java/groovy/lang/GroovyClassLoader.java | 10 ++++--
 src/test/groovy/bugs/Groovy9329.groovy           | 42 ++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/src/main/java/groovy/lang/GroovyClassLoader.java b/src/main/java/groovy/lang/GroovyClassLoader.java
index cd44d74..abc6da0 100644
--- a/src/main/java/groovy/lang/GroovyClassLoader.java
+++ b/src/main/java/groovy/lang/GroovyClassLoader.java
@@ -26,6 +26,7 @@
 package groovy.lang;
 
 import groovy.util.CharsetToolkit;
+import org.codehaus.groovy.GroovyBugError;
 import org.codehaus.groovy.ast.ClassHelper;
 import org.codehaus.groovy.ast.ClassNode;
 import org.codehaus.groovy.ast.FieldNode;
@@ -115,7 +116,7 @@ public class GroovyClassLoader extends URLClassLoader {
                         URL ret = getSourceFile(filename, extension);
                         if (ret != null)
                             return ret;
-                    } catch (Throwable t) { //
+                    } catch (Throwable t) {
                     }
                 }
                 return null;
@@ -263,8 +264,11 @@ public class GroovyClassLoader extends URLClassLoader {
      * @return the main class defined in the given script
      */
     public Class parseClass(String text) throws CompilationFailedException {
-        return parseClass(text, "script" + System.currentTimeMillis() +
-                Math.abs((long) text.hashCode()) + ".groovy");
+        try {
+            return parseClass(text, "Script_" + EncodingGroovyMethods.md5(text) + ".groovy");
+        } catch (NoSuchAlgorithmException e) {
+            throw new GroovyBugError("Failed to generate md5", e); // should never happen
+        }
     }
 
     public synchronized String generateScriptName() {
diff --git a/src/test/groovy/bugs/Groovy9329.groovy b/src/test/groovy/bugs/Groovy9329.groovy
new file mode 100644
index 0000000..00cdce7
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy9329.groovy
@@ -0,0 +1,42 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package groovy.bugs
+
+import groovy.transform.CompileStatic
+import org.junit.Test
+
+import static groovy.test.GroovyAssert.assertScript
+
+@CompileStatic
+final class Groovy9329 {
+    @Test
+    void test() {
+        assertScript '''
+            def gcl = new GroovyClassLoader()
+            def scriptText = 'def x = 1'
+            gcl.parseClass(scriptText)
+            
+            def begin = gcl.@classCache.size()
+            3.times { gcl.parseClass(scriptText) }
+            def end = gcl.@classCache.size()
+            
+            assert end == begin
+        '''
+    }
+}


[groovy] 02/04: output wildcard generics as not

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 93d8c7ee39e16ffd396fbe41c61a168444f85aa5
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Thu Dec 12 05:28:02 2019 -0600

    output wildcard generics as <? super T> not <java.lang.Object super T>
    
    (cherry picked from commit f27c8ae1fb4dd1b6a4b312a18076ffcc5d8be043)
---
 src/main/java/org/codehaus/groovy/antlr/AntlrParserPlugin.java      | 1 -
 src/main/java/org/codehaus/groovy/ast/GenericsType.java             | 4 ++--
 src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java      | 1 -
 src/test/groovy/transform/stc/GenericsSTCTest.groovy                | 6 +++---
 .../src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java   | 1 -
 5 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/antlr/AntlrParserPlugin.java b/src/main/java/org/codehaus/groovy/antlr/AntlrParserPlugin.java
index bf722ef..cd96348 100644
--- a/src/main/java/org/codehaus/groovy/antlr/AntlrParserPlugin.java
+++ b/src/main/java/org/codehaus/groovy/antlr/AntlrParserPlugin.java
@@ -2917,7 +2917,6 @@ public class AntlrParserPlugin extends ASTHelper implements ParserPlugin, Groovy
             } else {
                 gt = new GenericsType(base, null, null);
             }
-            gt.setName("?");
             gt.setWildcard(true);
         } else {
             ClassNode argument = makeTypeWithArguments(rootNode);
diff --git a/src/main/java/org/codehaus/groovy/ast/GenericsType.java b/src/main/java/org/codehaus/groovy/ast/GenericsType.java
index d8f68b8..9fc58ba 100644
--- a/src/main/java/org/codehaus/groovy/ast/GenericsType.java
+++ b/src/main/java/org/codehaus/groovy/ast/GenericsType.java
@@ -74,7 +74,7 @@ public class GenericsType extends ASTNode {
 
         if (placeholder) visited.add(gt.getName());
 
-        StringBuilder ret = new StringBuilder(wildcard ? "?" : placeholder ? gt.getName() : genericsBounds(type, visited));
+        StringBuilder ret = new StringBuilder(wildcard || placeholder ? gt.getName() : genericsBounds(type, visited));
         if (lowerBound != null) {
             ret.append(" super ").append(genericsBounds(lowerBound, visited));
         } else if (upperBounds != null
@@ -145,7 +145,7 @@ public class GenericsType extends ASTNode {
     }
 
     public String getName() {
-        return name;
+        return (isWildcard() ? "?" : name);
     }
 
     public void setName(final String name) {
diff --git a/src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java b/src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java
index cd3b166..1fc1b9f 100644
--- a/src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java
+++ b/src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java
@@ -392,7 +392,6 @@ public class GenericsUtils {
                         lower = correctToGenericsSpecRecurse(genericsSpec, oldLower, exclusions);
                     }
                     GenericsType fixed = new GenericsType(oldgType.getType(), upper, lower);
-                    fixed.setName(oldgType.getName());
                     fixed.setWildcard(true);
                     newgTypes[i] = fixed;
                 } else if (oldgType.isPlaceholder()) {
diff --git a/src/test/groovy/transform/stc/GenericsSTCTest.groovy b/src/test/groovy/transform/stc/GenericsSTCTest.groovy
index bed5c5d..db80a7e 100644
--- a/src/test/groovy/transform/stc/GenericsSTCTest.groovy
+++ b/src/test/groovy/transform/stc/GenericsSTCTest.groovy
@@ -205,7 +205,7 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
     void testLinkedListWithListArgumentAndWrongElementTypes() {
         shouldFailWithMessages '''
             List<String> list = new LinkedList<String>([1,2,3])
-        ''', 'Cannot call java.util.LinkedList <String>#<init>(java.util.Collection <java.lang.Object extends java.lang.String>) with arguments [java.util.List <java.lang.Integer>]'
+        ''', 'Cannot call java.util.LinkedList <String>#<init>(java.util.Collection <? extends java.lang.String>) with arguments [java.util.List <java.lang.Integer>]'
     }
 
     void testCompatibleGenericAssignmentWithInference() {
@@ -534,7 +534,7 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
             List<String> list = ['a','b','c']
             Collection<Integer> e = (Collection<Integer>) [1,2,3]
             boolean r = list.addAll(e)
-        ''', 'Cannot call java.util.List <java.lang.String>#addAll(java.util.Collection <java.lang.Object extends java.lang.String>) with arguments [java.util.Collection <Integer>]'
+        ''', 'Cannot call java.util.List <java.lang.String>#addAll(java.util.Collection <? extends java.lang.String>) with arguments [java.util.Collection <Integer>]'
     }
 
     // GROOVY-5528
@@ -1370,7 +1370,7 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
             List<Object> l = new ArrayList<>()
             assert foo(l) == 1
         ''',
-        '#foo(java.util.List <A extends A>) with arguments [java.util.ArrayList <java.lang.Object>]'
+        '#foo(java.util.List <? extends A>) with arguments [java.util.ArrayList <java.lang.Object>]'
     }
 
     void testMethodLevelGenericsForMethodCall() {
diff --git a/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java b/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
index 6987fa6..0eb22ed 100644
--- a/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
+++ b/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
@@ -3862,7 +3862,6 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> implements Groov
 
             GenericsType genericsType = new GenericsType(baseType, upperBounds, lowerBound);
             genericsType.setWildcard(true);
-            genericsType.setName(QUESTION_STR);
 
             return configureAST(genericsType, ctx);
         } else if (asBoolean(ctx.type())) {


[groovy] 01/04: minor edits

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 648fca2e4e489eaa273e666f95f2c6aac4e0d665
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Thu Dec 12 04:32:08 2019 -0600

    minor edits
    
    (cherry picked from commit 9d715844689e5bd859943fad48a48ba0316b5bdb)
---
 .../java/org/codehaus/groovy/ast/GenericsType.java | 325 +++++++++++----------
 1 file changed, 168 insertions(+), 157 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/ast/GenericsType.java b/src/main/java/org/codehaus/groovy/ast/GenericsType.java
index 59e3168..d8f68b8 100644
--- a/src/main/java/org/codehaus/groovy/ast/GenericsType.java
+++ b/src/main/java/org/codehaus/groovy/ast/GenericsType.java
@@ -27,8 +27,6 @@ import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
 
-import static org.codehaus.groovy.ast.ClassHelper.GROOVY_OBJECT_TYPE;
-
 /**
  * This class is used to describe generic type signatures for ClassNodes.
  *
@@ -37,24 +35,21 @@ import static org.codehaus.groovy.ast.ClassHelper.GROOVY_OBJECT_TYPE;
 public class GenericsType extends ASTNode {
     public static final GenericsType[] EMPTY_ARRAY = new GenericsType[0];
 
-    private final ClassNode[] upperBounds;
-    private final ClassNode lowerBound;
-    private ClassNode type;
     private String name;
-    private boolean placeholder;
-    private boolean resolved;
-    private boolean wildcard;
+    private ClassNode type;
+    private final ClassNode lowerBound;
+    private final ClassNode[] upperBounds;
+    private boolean placeholder, resolved, wildcard;
 
-    public GenericsType(ClassNode type, ClassNode[] upperBounds, ClassNode lowerBound) {
-        this.type = type;
-        this.name = type.isGenericsPlaceHolder() ? type.getUnresolvedName() : type.getName();
-        this.upperBounds = upperBounds;
+    public GenericsType(final ClassNode type, final ClassNode[] upperBounds, final ClassNode lowerBound) {
+        setType(type);
         this.lowerBound = lowerBound;
-        placeholder = type.isGenericsPlaceHolder();
-        resolved = false;
+        this.upperBounds = upperBounds;
+        this.placeholder = type.isGenericsPlaceHolder();
+        setName(placeholder ? type.getUnresolvedName() : type.getName());
     }
 
-    public GenericsType(ClassNode basicType) {
+    public GenericsType(final ClassNode basicType) {
         this(basicType, null, null);
     }
 
@@ -62,35 +57,39 @@ public class GenericsType extends ASTNode {
         return type;
     }
 
-    public void setType(ClassNode type) {
-        this.type = type;
+    public void setType(final ClassNode type) {
+        this.type = Objects.requireNonNull(type);
     }
 
     public String toString() {
-        Set<String> visited = new HashSet<String>();
-        return toString(visited);
+        return toString(this, new HashSet<>());
     }
 
-    private String toString(Set<String> visited) {
-        if (placeholder) visited.add(name);
-        StringBuilder ret = new StringBuilder(wildcard ? "?" : ((type == null || placeholder) ? name : genericsBounds(type, visited)));
-        if (upperBounds != null) {
-            if (placeholder && upperBounds.length==1 && !upperBounds[0].isGenericsPlaceHolder() && upperBounds[0].getName().equals("java.lang.Object")) {
+    private static String toString(final GenericsType gt, final Set<String> visited) {
+        ClassNode type = gt.getType();
+        boolean wildcard = gt.isWildcard();
+        boolean placeholder = gt.isPlaceholder();
+        ClassNode lowerBound = gt.getLowerBound();
+        ClassNode[] upperBounds = gt.getUpperBounds();
+
+        if (placeholder) visited.add(gt.getName());
+
+        StringBuilder ret = new StringBuilder(wildcard ? "?" : placeholder ? gt.getName() : genericsBounds(type, visited));
+        if (lowerBound != null) {
+            ret.append(" super ").append(genericsBounds(lowerBound, visited));
+        } else if (upperBounds != null
                 // T extends Object should just be printed as T
-            } else {
-                ret.append(" extends ");
-                for (int i = 0; i < upperBounds.length; i++) {
-                    ret.append(genericsBounds(upperBounds[i], visited));
-                    if (i + 1 < upperBounds.length) ret.append(" & ");
-                }
+                && !(placeholder && upperBounds.length == 1 && !upperBounds[0].isGenericsPlaceHolder() && upperBounds[0].getName().equals("java.lang.Object"))) {
+            ret.append(" extends ");
+            for (int i = 0, n = upperBounds.length; i < n; i += 1) {
+                if (i != 0) ret.append(" & ");
+                ret.append(genericsBounds(upperBounds[i], visited));
             }
-        } else if (lowerBound != null) {
-            ret.append(" super ").append(genericsBounds(lowerBound, visited));
         }
         return ret.toString();
     }
 
-    private String nameOf(ClassNode theType) {
+    private static String nameOf(final ClassNode theType) {
         StringBuilder ret = new StringBuilder();
         if (theType.isArray()) {
             ret.append(nameOf(theType.getComponentType()));
@@ -101,87 +100,80 @@ public class GenericsType extends ASTNode {
         return ret.toString();
     }
 
-    private String genericsBounds(ClassNode theType, Set<String> visited) {
-
+    private static String genericsBounds(final ClassNode theType, final Set<String> visited) {
         StringBuilder ret = new StringBuilder();
 
         if (theType.isArray()) {
             ret.append(nameOf(theType));
-        } else if (theType.redirect() instanceof InnerClassNode) {
-            InnerClassNode innerClassNode = (InnerClassNode) theType.redirect();
-            String parentClassNodeName = innerClassNode.getOuterClass().getName();
-            if (Modifier.isStatic(innerClassNode.getModifiers()) || innerClassNode.isInterface()) {
-                ret.append(innerClassNode.getOuterClass().getName());
+        } else if (theType.getOuterClass() != null) {
+            String parentClassNodeName = theType.getOuterClass().getName();
+            if (Modifier.isStatic(theType.getModifiers()) || theType.isInterface()) {
+                ret.append(parentClassNodeName);
             } else {
-                ret.append(genericsBounds(innerClassNode.getOuterClass(), new HashSet<String>()));
+                ret.append(genericsBounds(theType.getOuterClass(), new HashSet<>()));
             }
-            ret.append(".");
-            String typeName = theType.getName();
-            ret.append(typeName.substring(parentClassNodeName.length() + 1));
+            ret.append('.');
+            ret.append(theType.getName().substring(parentClassNodeName.length() + 1));
         } else {
             ret.append(theType.getName());
         }
 
         GenericsType[] genericsTypes = theType.getGenericsTypes();
-        if (genericsTypes == null || genericsTypes.length == 0)
+        if (genericsTypes == null || genericsTypes.length == 0) {
             return ret.toString();
+        }
 
-        // TODO instead of catching Object<T> here stop it from being placed into type in first place
+        // TODO: instead of catching Object<T> here stop it from being placed into type in first place
         if (genericsTypes.length == 1 && genericsTypes[0].isPlaceholder() && theType.getName().equals("java.lang.Object")) {
             return genericsTypes[0].getName();
         }
 
-        ret.append("<");
-        for (int i = 0; i < genericsTypes.length; i++) {
+        ret.append('<');
+        for (int i = 0, n = genericsTypes.length; i < n; i += 1) {
             if (i != 0) ret.append(", ");
 
             GenericsType type = genericsTypes[i];
             if (type.isPlaceholder() && visited.contains(type.getName())) {
                 ret.append(type.getName());
-            }
-            else {
-                ret.append(type.toString(visited));
+            } else {
+                ret.append(toString(type, visited));
             }
         }
-        ret.append(">");
+        ret.append('>');
 
         return ret.toString();
     }
 
-    public ClassNode[] getUpperBounds() {
-        return upperBounds;
-    }
-
     public String getName() {
         return name;
     }
 
-    public boolean isPlaceholder() {
-        return placeholder;
+    public void setName(final String name) {
+        this.name = Objects.requireNonNull(name);
     }
 
-    public void setPlaceholder(boolean placeholder) {
-        this.placeholder = placeholder;
-        type.setGenericsPlaceHolder(placeholder);
+    public boolean isResolved() {
+        return (resolved || isPlaceholder());
     }
 
-    public boolean isResolved() {
-        return resolved || placeholder;
+    public void setResolved(final boolean resolved) {
+        this.resolved = resolved;
     }
 
-    public void setResolved(boolean res) {
-        resolved = res;
+    public boolean isPlaceholder() {
+        return placeholder;
     }
 
-    public void setName(String name) {
-        this.name = name;
+    public void setPlaceholder(final boolean placeholder) {
+        this.placeholder = placeholder;
+        getType().setGenericsPlaceHolder(placeholder);
     }
 
     public boolean isWildcard() {
         return wildcard;
     }
 
-    public void setWildcard(boolean wildcard) {
+    public void setWildcard(final boolean wildcard) {
         this.wildcard = wildcard;
     }
 
@@ -189,12 +181,56 @@ public class GenericsType extends ASTNode {
         return lowerBound;
     }
 
+    public ClassNode[] getUpperBounds() {
+        return upperBounds;
+    }
+
+    /**
+     * If you have a class which extends a class using generics, returns the superclass with parameterized types. For
+     * example, if you have:
+     * <code>class MyList&lt;T&gt; extends LinkedList&lt;T&gt;
+     * def list = new MyList&lt;String&gt;
+     * </code>
+     * then the parameterized superclass for MyList&lt;String&gt; is LinkedList&lt;String&gt;
+     * @param classNode the class for which we want to return the parameterized superclass
+     * @return the parameterized superclass
+     */
+    private static ClassNode getParameterizedSuperClass(final ClassNode classNode) {
+        if (ClassHelper.OBJECT_TYPE.equals(classNode)) return null;
+        ClassNode superClass = classNode.getUnresolvedSuperClass();
+        if (superClass == null) return ClassHelper.OBJECT_TYPE;
+
+        if (!classNode.isUsingGenerics() || !superClass.isUsingGenerics()) {
+            return superClass;
+        }
+
+        GenericsType[] genericsTypes = classNode.getGenericsTypes();
+        GenericsType[] redirectGenericTypes = classNode.redirect().getGenericsTypes();
+        superClass = superClass.getPlainNodeReference();
+        if (genericsTypes == null || redirectGenericTypes == null || superClass.getGenericsTypes() == null) {
+            return superClass;
+        }
+        for (int i = 0, genericsTypesLength = genericsTypes.length; i < genericsTypesLength; i += 1) {
+            if (redirectGenericTypes[i].isPlaceholder()) {
+                GenericsType genericsType = genericsTypes[i];
+                GenericsType[] superGenericTypes = superClass.getGenericsTypes();
+                for (int j = 0, superGenericTypesLength = superGenericTypes.length; j < superGenericTypesLength; j += 1) {
+                    final GenericsType superGenericType = superGenericTypes[j];
+                    if (superGenericType.isPlaceholder() && superGenericType.getName().equals(redirectGenericTypes[i].getName())) {
+                        superGenericTypes[j] = genericsType;
+                    }
+                }
+            }
+        }
+        return superClass;
+    }
+
     /**
      * Tells if the provided class node is compatible with this generic type definition
      * @param classNode the class node to be checked
      * @return true if the class node is compatible with this generics type definition
      */
-    public boolean isCompatibleWith(ClassNode classNode) {
+    public boolean isCompatibleWith(final ClassNode classNode) {
         return new GenericsTypeMatcher().matches(classNode);
     }
 
@@ -203,14 +239,14 @@ public class GenericsType extends ASTNode {
      */
     private class GenericsTypeMatcher {
 
-        public boolean implementsInterfaceOrIsSubclassOf(ClassNode type, ClassNode superOrInterface) {
+        public boolean implementsInterfaceOrIsSubclassOf(final ClassNode type, final ClassNode superOrInterface) {
             boolean result = type.equals(superOrInterface)
                     || type.isDerivedFrom(superOrInterface)
                     || type.implementsInterface(superOrInterface);
             if (result) {
                 return true;
             }
-            if (GROOVY_OBJECT_TYPE.equals(superOrInterface) && type.getCompileUnit()!=null) {
+            if (ClassHelper.GROOVY_OBJECT_TYPE.equals(superOrInterface) && type.getCompileUnit() != null) {
                 // type is being compiled so it will implement GroovyObject later
                 return true;
             }
@@ -219,7 +255,7 @@ public class GenericsType extends ASTNode {
                 result = implementsInterfaceOrIsSubclassOf(type, cn.getSuperClass());
                 if (result) {
                     for (ClassNode interfaceNode : cn.getInterfaces()) {
-                        result = implementsInterfaceOrIsSubclassOf(type,interfaceNode);
+                        result = implementsInterfaceOrIsSubclassOf(type, interfaceNode);
                         if (!result) break;
                     }
                 }
@@ -238,60 +274,64 @@ public class GenericsType extends ASTNode {
          * @param classNode the classnode to be checked
          * @return true iff the classnode is compatible with this generics specification
          */
-        public boolean matches(ClassNode classNode) {
+        public boolean matches(final ClassNode classNode) {
             GenericsType[] genericsTypes = classNode.getGenericsTypes();
             // diamond always matches
-            if (genericsTypes!=null && genericsTypes.length==0) return true;
+            if (genericsTypes != null && genericsTypes.length == 0) {
+                return true;
+            }
             if (classNode.isGenericsPlaceHolder()) {
                 // if the classnode we compare to is a generics placeholder (like <E>) then we
                 // only need to check that the names are equal
-                if (genericsTypes==null) return true;
+                if (genericsTypes == null) {
+                    return true;
+                }
                 if (isWildcard()) {
-                    if (lowerBound!=null) return genericsTypes[0].getName().equals(lowerBound.getUnresolvedName());
-                    if (upperBounds!=null) {
-                        for (ClassNode upperBound : upperBounds) {
+                    if (getLowerBound() != null) {
+                        return genericsTypes[0].getName().equals(getLowerBound().getUnresolvedName());
+                    }
+                    if (getUpperBounds() != null) {
+                        for (ClassNode upperBound : getUpperBounds()) {
                             String name = upperBound.getGenericsTypes()[0].getName();
-                            if (genericsTypes[0].getName().equals(name)) return true;
+                            if (genericsTypes[0].getName().equals(name)) {
+                                return true;
+                            }
                         }
                         return false;
                     }
                 }
-                return genericsTypes[0].getName().equals(name);
+                return genericsTypes[0].getName().equals(getName());
             }
-            if (wildcard || placeholder) {
+            if (isWildcard() || isPlaceholder()) {
+                ClassNode lowerBound = getLowerBound();
+                ClassNode[] upperBounds = getUpperBounds();
                 // if the current generics spec is a wildcard spec or a placeholder spec
                 // then we must check upper and lower bounds
                 if (upperBounds != null) {
                     // check that the provided classnode is a subclass of all provided upper bounds
                     boolean upIsOk = true;
-                    for (int i = 0, upperBoundsLength = upperBounds.length; i < upperBoundsLength && upIsOk; i++) {
-                        final ClassNode upperBound = upperBounds[i];
-                        upIsOk = implementsInterfaceOrIsSubclassOf(classNode, upperBound);
+                    for (int i = 0, n = upperBounds.length; i < n && upIsOk; i += 1) {
+                        upIsOk = implementsInterfaceOrIsSubclassOf(classNode, upperBounds[i]);
                     }
                     // if the provided classnode is a subclass of the upper bound
                     // then check that the generic types supplied by the class node are compatible with
                     // this generics specification
                     // for example, we could have the spec saying List<String> but provided classnode
                     // saying List<Integer>
-                    upIsOk = upIsOk && checkGenerics(classNode);
-                    return upIsOk;
+                    return (upIsOk && checkGenerics(classNode));
                 }
                 if (lowerBound != null) {
                     // if a lower bound is declared, then we must perform the same checks that for an upper bound
                     // but with reversed arguments
-                    return implementsInterfaceOrIsSubclassOf(lowerBound, classNode) && checkGenerics(classNode);
+                    return (implementsInterfaceOrIsSubclassOf(lowerBound, classNode) && checkGenerics(classNode));
                 }
                 // If there are no bounds, the generic type is basically Object, and everything is compatible.
                 return true;
             }
-            // if this is not a generics placeholder, first compare that types represent the same type
-            if ((type!=null && !type.equals(classNode))) {
-                return false;
-            }
             // last, we could have the spec saying List<String> and a classnode saying List<Integer> so
             // we must check that generics are compatible.
             // The null check is normally not required but done to prevent from NPEs
-            return type == null || compareGenericsWithBound(classNode, type);
+            return getType().equals(classNode) && compareGenericsWithBound(classNode, type);
         }
 
         /**
@@ -302,12 +342,16 @@ public class GenericsType extends ASTNode {
          * @return true if generics from bounds are compatible
          */
         private boolean checkGenerics(final ClassNode classNode) {
-            if (upperBounds!=null) {
+            ClassNode lowerBound = getLowerBound();
+            ClassNode[] upperBounds = getUpperBounds();
+            if (upperBounds != null) {
                 for (ClassNode upperBound : upperBounds) {
-                    if (!compareGenericsWithBound(classNode, upperBound)) return false;
+                    if (!compareGenericsWithBound(classNode, upperBound)) {
+                        return false;
+                    }
                 }
             }
-            if (lowerBound!=null) {
+            if (lowerBound != null) {
                 if (!lowerBound.redirect().isUsingGenerics()) {
                     return compareGenericsWithBound(classNode, lowerBound);
                 }
@@ -323,8 +367,10 @@ public class GenericsType extends ASTNode {
          * @return true if generics are compatible
          */
         private boolean compareGenericsWithBound(final ClassNode classNode, final ClassNode bound) {
-            if (classNode==null) return false;
-            if (!bound.isUsingGenerics() || (classNode.getGenericsTypes()==null && classNode.redirect().getGenericsTypes()!=null)) {
+            if (classNode == null) {
+                return false;
+            }
+            if (!bound.isUsingGenerics() || (classNode.getGenericsTypes() == null && classNode.redirect().getGenericsTypes() != null)) {
                 // if the bound is not using generics, there's nothing to compare with
                 return true;
             }
@@ -365,8 +411,10 @@ public class GenericsType extends ASTNode {
                 return compareGenericsWithBound(getParameterizedSuperClass(classNode), bound);
             }
             GenericsType[] cnTypes = classNode.getGenericsTypes();
-            if (cnTypes==null && classNode.isRedirectNode()) cnTypes=classNode.redirect().getGenericsTypes();
-            if (cnTypes==null) {
+            if (cnTypes == null && classNode.isRedirectNode()) {
+                cnTypes = classNode.redirect().getGenericsTypes();
+            }
+            if (cnTypes == null) {
                 // may happen if generic type is Foo<T extends Foo> and classnode is Foo -> Foo
                 return true;
             }
@@ -374,7 +422,7 @@ public class GenericsType extends ASTNode {
             Map<GenericsTypeName, GenericsType> classNodePlaceholders = GenericsUtils.extractPlaceholders(classNode);
             Map<GenericsTypeName, GenericsType> boundPlaceHolders = GenericsUtils.extractPlaceholders(bound);
             boolean match = true;
-            for (int i = 0; redirectBoundGenericTypes!=null && i < redirectBoundGenericTypes.length && match; i++) {
+            for (int i = 0; redirectBoundGenericTypes != null && i < redirectBoundGenericTypes.length && match; i += 1) {
                 GenericsType redirectBoundType = redirectBoundGenericTypes[i];
                 GenericsType classNodeType = cnTypes[i];
                 if (classNodeType.isPlaceholder()) {
@@ -385,15 +433,15 @@ public class GenericsType extends ASTNode {
                         if (!match) {
                             GenericsType genericsType = boundPlaceHolders.get(gtn);
                             match = false;
-                            if (genericsType!=null) {
+                            if (genericsType != null) {
                                 if (genericsType.isPlaceholder()) {
                                     match = true;
                                 } else if (genericsType.isWildcard()) {
-                                    if (genericsType.getUpperBounds()!=null) {
-                                        for (ClassNode up : genericsType.getUpperBounds()) {
-                                            match |= redirectBoundType.isCompatibleWith(up);
+                                    if (genericsType.getUpperBounds() != null) {
+                                        for (ClassNode ub : genericsType.getUpperBounds()) {
+                                            match |= redirectBoundType.isCompatibleWith(ub);
                                         }
-                                        if (genericsType.getLowerBound()!=null) {
+                                        if (genericsType.getLowerBound() != null) {
                                             match |= redirectBoundType.isCompatibleWith(genericsType.getLowerBound());
                                         }
                                     }
@@ -401,7 +449,8 @@ public class GenericsType extends ASTNode {
                             }
                         }
                     } else {
-                        if (classNodePlaceholders.containsKey(name)) classNodeType=classNodePlaceholders.get(name);
+                        if (classNodePlaceholders.containsKey(name))
+                            classNodeType = classNodePlaceholders.get(name);
                         match = classNodeType.isCompatibleWith(redirectBoundType.getType());
                     }
                 } else {
@@ -416,11 +465,10 @@ public class GenericsType extends ASTNode {
                                 boolean placeholder = redirectBoundType.isPlaceholder();
                                 if (placeholder || wildcard) {
                                     // placeholder aliases, like Map<U,V> -> Map<K,V>
-//                                    redirectBoundType = classNodePlaceholders.get(name);
                                     if (wildcard) {
                                         // ex: Comparable<Integer> <=> Comparable<? super T>
-                                        if (redirectBoundType.lowerBound!=null) {
-                                            GenericsType gt = new GenericsType(redirectBoundType.lowerBound);
+                                        if (redirectBoundType.getLowerBound() != null) {
+                                            GenericsType gt = new GenericsType(redirectBoundType.getLowerBound());
                                             if (gt.isPlaceholder()) {
                                                 // check for recursive generic typedef, like in
                                                 // <T extends Comparable<? super T>>
@@ -431,8 +479,8 @@ public class GenericsType extends ASTNode {
                                             }
                                             match = implementsInterfaceOrIsSubclassOf(gt.getType(), classNodeType.getType());
                                         }
-                                        if (match && redirectBoundType.upperBounds!=null) {
-                                            for (ClassNode upperBound : redirectBoundType.upperBounds) {
+                                        if (match && redirectBoundType.getUpperBounds() != null) {
+                                            for (ClassNode upperBound : redirectBoundType.getUpperBounds()) {
                                                 GenericsType gt = new GenericsType(upperBound);
                                                 if (gt.isPlaceholder()) {
                                                     // check for recursive generic typedef, like in
@@ -456,7 +504,7 @@ public class GenericsType extends ASTNode {
                             match = redirectBoundType.isCompatibleWith(classNodeType.getType());
                         }
                     } else {
-                        // todo: the check for isWildcard should be replaced with a more complete check
+                        // TODO: the check for isWildcard should be replaced with a more complete check
                         match = redirectBoundType.isWildcard() || classNodeType.isCompatibleWith(redirectBoundType.getType());
                     }
                 }
@@ -466,42 +514,6 @@ public class GenericsType extends ASTNode {
     }
 
     /**
-     * If you have a class which extends a class using generics, returns the superclass with parameterized types. For
-     * example, if you have:
-     * <code>class MyList&lt;T&gt; extends LinkedList&lt;T&gt;
-     * def list = new MyList&lt;String&gt;
-     * </code>
-     * then the parameterized superclass for MyList&lt;String&gt; is LinkedList&lt;String&gt;
-     * @param classNode the class for which we want to return the parameterized superclass
-     * @return the parameterized superclass
-     */
-    private static ClassNode getParameterizedSuperClass(ClassNode classNode) {
-        if (ClassHelper.OBJECT_TYPE.equals(classNode)) return null;
-        ClassNode superClass = classNode.getUnresolvedSuperClass();
-        if (superClass==null) {
-            return ClassHelper.OBJECT_TYPE;
-        }
-        if (!classNode.isUsingGenerics() || !superClass.isUsingGenerics()) return superClass;
-        GenericsType[] genericsTypes = classNode.getGenericsTypes();
-        GenericsType[] redirectGenericTypes = classNode.redirect().getGenericsTypes();
-        superClass = superClass.getPlainNodeReference();
-        if (genericsTypes==null || redirectGenericTypes==null || superClass.getGenericsTypes()==null) return superClass;
-        for (int i = 0, genericsTypesLength = genericsTypes.length; i < genericsTypesLength; i++) {
-            if (redirectGenericTypes[i].isPlaceholder()) {
-                final GenericsType genericsType = genericsTypes[i];
-                GenericsType[] superGenericTypes = superClass.getGenericsTypes();
-                for (int j = 0, superGenericTypesLength = superGenericTypes.length; j < superGenericTypesLength; j++) {
-                    final GenericsType superGenericType = superGenericTypes[j];
-                    if (superGenericType.isPlaceholder() && superGenericType.getName().equals(redirectGenericTypes[i].getName())) {
-                        superGenericTypes[j] = genericsType;
-                    }
-                }
-            }
-        }
-        return superClass;
-    }
-
-    /**
      * Represents GenericsType name
      * TODO In order to distinguish GenericsType with same name(See GROOVY-8409), we should add a property to keep the declaring class.
      *
@@ -516,8 +528,8 @@ public class GenericsType extends ASTNode {
     public static class GenericsTypeName {
         private String name;
 
-        public GenericsTypeName(String name) {
-            this.name = name;
+        public GenericsTypeName(final String name) {
+            this.name = Objects.requireNonNull(name);
         }
 
         public String getName() {
@@ -525,21 +537,20 @@ public class GenericsType extends ASTNode {
         }
 
         @Override
-        public boolean equals(Object o) {
-            if (this == o) return true;
-            if (o == null || getClass() != o.getClass()) return false;
-            GenericsTypeName that = (GenericsTypeName) o;
-            return Objects.equals(name, that.name);
+        public boolean equals(Object that) {
+            if (this == that) return true;
+            if (!(that instanceof GenericsTypeName)) return false;
+            return getName().equals(((GenericsTypeName) that).getName());
         }
 
         @Override
         public int hashCode() {
-            return Objects.hash(name);
+            return getName().hashCode();
         }
 
         @Override
         public String toString() {
-            return name;
+            return getName();
         }
     }
 }