From 2279366e0f995c30d5256921c4eba13230a6d158 Mon Sep 17 00:00:00 2001 From: Eric Milles Date: Wed, 5 Jan 2022 13:35:47 -0600 Subject: [PATCH] GROOVY-5106 --- .../core/tests/basic/GroovySimpleTests.java | 58 +++++++++++- .../codehaus/groovy/classgen/Verifier.java | 76 ++++++++++++++-- .../codehaus/groovy/classgen/Verifier.java | 76 ++++++++++++++-- .../codehaus/groovy/classgen/Verifier.java | 88 +++++++++++++++---- 4 files changed, 269 insertions(+), 29 deletions(-) diff --git a/base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/basic/GroovySimpleTests.java b/base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/basic/GroovySimpleTests.java index 13ba46f25f..9a3e6bedc5 100644 --- a/base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/basic/GroovySimpleTests.java +++ b/base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/basic/GroovySimpleTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2021 the original author or authors. + * Copyright 2009-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -4218,8 +4218,62 @@ public void testImplementingInterface6() { runConformTest(sources); } - @Test // GROOVY-5687 + @Test // GROOVY-5106 public void testImplementingInterface7() { + //@formatter:off + String[] sources = { + "I.groovy", + "interface I {\n" + + "}\n", + + "J.groovy", + "interface J extends I {\n" + + "}\n", + + "X.groovy", + "class X implements I, J {\n" + + "}\n", + }; + //@formatter:on + + runNegativeTest(sources, + "----------\n" + + "1. ERROR in X.groovy (at line 1)\n" + + "\tclass X implements I, J {\n" + + "\t^\n" + + "Groovy:The interface I cannot be implemented more than once with different arguments: I and I\n" + + "----------\n"); + } + + @Test // GROOVY-5106 + public void testImplementingInterface8() { + //@formatter:off + String[] sources = { + "I.groovy", + "interface I {\n" + + "}\n", + + "X.groovy", + "class X implements I {\n" + + "}\n", + + "Y.groovy", + "class Y extends X implements I {\n" + + "}\n", + }; + //@formatter:on + + runNegativeTest(sources, + "----------\n" + + "1. ERROR in Y.groovy (at line 1)\n" + + "\tclass Y extends X implements I {\n" + + "\t^\n" + + "Groovy:The interface I cannot be implemented more than once with different arguments: I and I\n" + + "----------\n"); + } + + @Test // GROOVY-5687 + public void testImplementingInterface9() { //@formatter:off String[] sources = { "Script.groovy", diff --git a/base/org.codehaus.groovy25/src/org/codehaus/groovy/classgen/Verifier.java b/base/org.codehaus.groovy25/src/org/codehaus/groovy/classgen/Verifier.java index 2387a9973b..8950a963df 100644 --- a/base/org.codehaus.groovy25/src/org/codehaus/groovy/classgen/Verifier.java +++ b/base/org.codehaus.groovy25/src/org/codehaus/groovy/classgen/Verifier.java @@ -124,6 +124,7 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.varX; import static org.codehaus.groovy.ast.tools.GenericsUtils.correctToGenericsSpec; import static org.codehaus.groovy.ast.tools.GenericsUtils.createGenericsSpec; +import static org.codehaus.groovy.ast.tools.GenericsUtils.parameterizeType; import static org.codehaus.groovy.ast.tools.PropertyNodeUtils.adjustPropertyModifiersForMethod; /** @@ -239,9 +240,11 @@ private static FieldNode getMetaClassField(ClassNode node) { */ public void visitClass(final ClassNode node) { this.classNode = node; - - if (Traits.isTrait(node) // maybe possible to have this true in joint compilation mode - || classNode.isInterface()) { + // GRECLIPSE add -- GROOVY-5106 + checkForDuplicateInterfaces(node); + // GRECLIPSE end + if (classNode.isInterface() + || Traits.isTrait(node)) { // maybe possible to have this true in joint compilation mode //interfaces have no constructors, but this code expects one, //so create a dummy and don't add it to the class node ConstructorNode dummy = new ConstructorNode(0, null); @@ -252,7 +255,7 @@ public void visitClass(final ClassNode node) { } return; } - + /* GRECLIPSE edit -- GROOVY-5106 ClassNode[] classNodes = classNode.getInterfaces(); List interfaces = new ArrayList(); for (ClassNode classNode : classNodes) { @@ -262,7 +265,7 @@ public void visitClass(final ClassNode node) { if (interfaceSet.size() != interfaces.size()) { throw new RuntimeParserException("Duplicate interfaces in implements list: " + interfaces, classNode); } - + */ addDefaultParameterMethods(node); addDefaultParameterConstructors(node); @@ -318,6 +321,69 @@ public void variableNotAlwaysInitialized(final VariableExpression var) { }; } + // GRECLIPSE add + private static void addAllInterfaces(final Set result, final ClassNode source) { + for (ClassNode in : source.getInterfaces()) { + in = parameterizeType(source, in); + if (result.add(in)) + addAllInterfaces(result, in); + } + ClassNode sc = source.redirect().getUnresolvedSuperClass(false); + if (sc != null && !sc.equals(ClassHelper.OBJECT_TYPE)) { + addAllInterfaces(result, parameterizeType(source, sc)); + } + } + + private static Set getAllInterfaces(final ClassNode cn) { + Set result = new HashSet<>(); + if (cn.isInterface()) result.add(cn); + addAllInterfaces(result, cn); + return result; + } + + private static void checkForDuplicateInterfaces(final ClassNode cn) { + ClassNode[] interfaces = cn.getInterfaces(); + int nInterfaces = interfaces.length; + if (nInterfaces == 0) return; + + if (nInterfaces > 1) { + List interfaceNames = new ArrayList<>(nInterfaces); + for (ClassNode in : interfaces) interfaceNames.add(in.getName()); + if (interfaceNames.size() != new HashSet<>(interfaceNames).size()) { + throw new RuntimeParserException("Duplicate interfaces in implements list: " + interfaceNames, cn); + } + } + + // GROOVY-5106: check for same interface with different type argument(s) + List< Set > allInterfaces = new ArrayList<>(nInterfaces + 1); + for (ClassNode in : interfaces) allInterfaces.add(getAllInterfaces(in)); + allInterfaces.add(getAllInterfaces(cn.getUnresolvedSuperClass())); + if (nInterfaces == 1 && allInterfaces.get(1).isEmpty()) + return; // no peer interface(s) to verify + + for (int i = 0; i < nInterfaces; i += 1) { + for (ClassNode in : allInterfaces.get(i)) { + if (in.redirect().getGenericsTypes() != null) { + for (int j = i + 1; j < nInterfaces + 1; j += 1) { + Set set = allInterfaces.get(j); + if (set.contains(in)) { + for (ClassNode t : set) { // find match and check generics + if (t.equals(in)) { + String one = in.toString(false), two = t.toString(false); + if (!one.equals(two)) + throw new RuntimeParserException("The interface " + in.getNameWithoutPackage() + + " cannot be implemented more than once with different arguments: " + one + " and " + two, cn); + break; + } + } + } + } + } + } + } + } + // GRECLIPSE end + private static void checkForDuplicateMethods(ClassNode cn) { Set descriptors = new HashSet(); for (MethodNode mn : cn.getMethods()) { diff --git a/base/org.codehaus.groovy30/src/org/codehaus/groovy/classgen/Verifier.java b/base/org.codehaus.groovy30/src/org/codehaus/groovy/classgen/Verifier.java index 62c81cbff6..51e61e165e 100644 --- a/base/org.codehaus.groovy30/src/org/codehaus/groovy/classgen/Verifier.java +++ b/base/org.codehaus.groovy30/src/org/codehaus/groovy/classgen/Verifier.java @@ -133,6 +133,7 @@ import static org.codehaus.groovy.ast.tools.GenericsUtils.addMethodGenerics; import static org.codehaus.groovy.ast.tools.GenericsUtils.correctToGenericsSpec; import static org.codehaus.groovy.ast.tools.GenericsUtils.createGenericsSpec; +import static org.codehaus.groovy.ast.tools.GenericsUtils.parameterizeType; import static org.codehaus.groovy.ast.tools.PropertyNodeUtils.adjustPropertyModifiersForMethod; /** @@ -230,9 +231,11 @@ private static FieldNode getMetaClassField(final ClassNode node) { @Override public void visitClass(final ClassNode node) { this.classNode = node; - - if (Traits.isTrait(node) // maybe possible to have this true in joint compilation mode - || classNode.isInterface()) { + // GRECLIPSE add -- GROOVY-5106 + checkForDuplicateInterfaces(node); + // GRECLIPSE end + if (classNode.isInterface() + || Traits.isTrait(node)) { // maybe possible to have this true in joint compilation mode //interfaces have no constructors, but this code expects one, //so create a dummy and don't add it to the class node ConstructorNode dummy = new ConstructorNode(0, null); @@ -243,7 +246,7 @@ public void visitClass(final ClassNode node) { } return; } - + /* GRECLIPSE edit -- GROOVY-5106 ClassNode[] classNodes = classNode.getInterfaces(); List interfaces = new ArrayList<>(); for (ClassNode classNode : classNodes) { @@ -253,7 +256,7 @@ public void visitClass(final ClassNode node) { if (interfaceSet.size() != interfaces.size()) { throw new RuntimeParserException("Duplicate interfaces in implements list: " + interfaces, classNode); } - + */ addDefaultParameterMethods(node); addDefaultParameterConstructors(node); @@ -308,6 +311,69 @@ public void variableNotAlwaysInitialized(final VariableExpression var) { }; } + // GRECLIPSE add + private static void addAllInterfaces(final Set result, final ClassNode source) { + for (ClassNode in : source.getInterfaces()) { + in = parameterizeType(source, in); + if (result.add(in)) + addAllInterfaces(result, in); + } + ClassNode sc = source.redirect().getUnresolvedSuperClass(false); + if (sc != null && !sc.equals(ClassHelper.OBJECT_TYPE)) { + addAllInterfaces(result, parameterizeType(source, sc)); + } + } + + private static Set getAllInterfaces(final ClassNode cn) { + Set result = new HashSet<>(); + if (cn.isInterface()) result.add(cn); + addAllInterfaces(result, cn); + return result; + } + + private static void checkForDuplicateInterfaces(final ClassNode cn) { + ClassNode[] interfaces = cn.getInterfaces(); + int nInterfaces = interfaces.length; + if (nInterfaces == 0) return; + + if (nInterfaces > 1) { + List interfaceNames = new ArrayList<>(nInterfaces); + for (ClassNode in : interfaces) interfaceNames.add(in.getName()); + if (interfaceNames.size() != new HashSet<>(interfaceNames).size()) { + throw new RuntimeParserException("Duplicate interfaces in implements list: " + interfaceNames, cn); + } + } + + // GROOVY-5106: check for same interface with different type argument(s) + List< Set > allInterfaces = new ArrayList<>(nInterfaces + 1); + for (ClassNode in : interfaces) allInterfaces.add(getAllInterfaces(in)); + allInterfaces.add(getAllInterfaces(cn.getUnresolvedSuperClass())); + if (nInterfaces == 1 && allInterfaces.get(1).isEmpty()) + return; // no peer interface(s) to verify + + for (int i = 0; i < nInterfaces; i += 1) { + for (ClassNode in : allInterfaces.get(i)) { + if (in.redirect().getGenericsTypes() != null) { + for (int j = i + 1; j < nInterfaces + 1; j += 1) { + Set set = allInterfaces.get(j); + if (set.contains(in)) { + for (ClassNode t : set) { // find match and check generics + if (t.equals(in)) { + String one = in.toString(false), two = t.toString(false); + if (!one.equals(two)) + throw new RuntimeParserException("The interface " + in.getNameWithoutPackage() + + " cannot be implemented more than once with different arguments: " + one + " and " + two, cn); + break; + } + } + } + } + } + } + } + } + // GRECLIPSE end + private static void checkForDuplicateMethods(final ClassNode cn) { Set descriptors = new HashSet<>(); for (MethodNode mn : cn.getMethods()) { diff --git a/base/org.codehaus.groovy40/src/org/codehaus/groovy/classgen/Verifier.java b/base/org.codehaus.groovy40/src/org/codehaus/groovy/classgen/Verifier.java index 1b475e059e..64ec3719fb 100644 --- a/base/org.codehaus.groovy40/src/org/codehaus/groovy/classgen/Verifier.java +++ b/base/org.codehaus.groovy40/src/org/codehaus/groovy/classgen/Verifier.java @@ -146,6 +146,7 @@ import static org.codehaus.groovy.ast.tools.GenericsUtils.addMethodGenerics; import static org.codehaus.groovy.ast.tools.GenericsUtils.correctToGenericsSpec; import static org.codehaus.groovy.ast.tools.GenericsUtils.createGenericsSpec; +import static org.codehaus.groovy.ast.tools.GenericsUtils.parameterizeType; import static org.codehaus.groovy.ast.tools.PropertyNodeUtils.adjustPropertyModifiersForMethod; /** @@ -246,8 +247,6 @@ private static FieldNode getMetaClassField(final ClassNode node) { @Override public void visitClass(final ClassNode node) { - boolean skipGroovify = !node.getAnnotations(ClassHelper.make(CompileStatic.class)).isEmpty() && - !node.getAnnotations(ClassHelper.make(POJO.class)).isEmpty(); this.classNode = node; if (node.isRecord()) { @@ -256,8 +255,11 @@ public void visitClass(final ClassNode node) { throw new RuntimeParserException("Record '" + classNode.getNameWithoutPackage() + "' must not be abstract", classNode); } } - if (Traits.isTrait(node) // maybe possible to have this true in joint compilation mode - || classNode.isInterface()) { + + checkForDuplicateInterfaces(node); + + if (classNode.isInterface() + || Traits.isTrait(node)) { // maybe possible to have this true in joint compilation mode //interfaces have no constructors, but this code expects one, //so create a dummy and don't add it to the class node ConstructorNode dummy = new ConstructorNode(0, null); @@ -270,21 +272,12 @@ public void visitClass(final ClassNode node) { return; } - ClassNode[] classNodes = classNode.getInterfaces(); - List interfaces = new ArrayList<>(); - for (ClassNode classNode : classNodes) { - interfaces.add(classNode.getName()); - } - Set interfaceSet = new HashSet<>(interfaces); - if (interfaceSet.size() != interfaces.size()) { - throw new RuntimeParserException("Duplicate interfaces in implements list: " + interfaces, classNode); - } - addDefaultParameterMethods(node); addDefaultParameterConstructors(node); final String classInternalName = BytecodeHelper.getClassInternalName(node); - + boolean skipGroovify = !node.getAnnotations(ClassHelper.make(POJO.class)).isEmpty() + && !node.getAnnotations(ClassHelper.make(CompileStatic.class)).isEmpty(); if (!skipGroovify) { addStaticMetaClassField(node, classInternalName); @@ -312,11 +305,11 @@ public void visitClass(final ClassNode node) { checkFinalVariables(node); } - private static final List invalidNames = Arrays.asList("clone", "finalize", "getClass", "hashCode", "notify", "notifyAll", "toString", "wait"); + private static final String[] INVALID_COMPONENTS = {"clone", "finalize", "getClass", "hashCode", "notify", "notifyAll", "toString", "wait"}; private void detectInvalidRecordComponentNames(ClassNode node) { for (FieldNode fn : node.getFields()) { - if (invalidNames.contains(fn.getName())) { + if (Arrays.binarySearch(INVALID_COMPONENTS, fn.getName()) >= 0) { throw new RuntimeParserException("Illegal record component name '" + fn.getName() + "'", fn); } } @@ -390,6 +383,67 @@ public void variableNotAlwaysInitialized(final VariableExpression var) { }; } + private static void addAllInterfaces(final Set result, final ClassNode source) { + for (ClassNode in : source.getInterfaces()) { + in = parameterizeType(source, in); + if (result.add(in)) + addAllInterfaces(result, in); + } + ClassNode sc = source.redirect().getUnresolvedSuperClass(false); + if (sc != null && !isObjectType(sc)) { + addAllInterfaces(result, parameterizeType(source, sc)); + } + } + + private static Set getAllInterfaces(final ClassNode cn) { + Set result = new HashSet<>(); + if (cn.isInterface()) result.add(cn); + addAllInterfaces(result, cn); + return result; + } + + private static void checkForDuplicateInterfaces(final ClassNode cn) { + ClassNode[] interfaces = cn.getInterfaces(); + int nInterfaces = interfaces.length; + if (nInterfaces == 0) return; + + if (nInterfaces > 1) { + List interfaceNames = new ArrayList<>(nInterfaces); + for (ClassNode in : interfaces) interfaceNames.add(in.getName()); + if (interfaceNames.size() != new HashSet<>(interfaceNames).size()) { + throw new RuntimeParserException("Duplicate interfaces in implements list: " + interfaceNames, cn); + } + } + + // GROOVY-5106: check for same interface with different type argument(s) + List< Set > allInterfaces = new ArrayList<>(nInterfaces + 1); + for (ClassNode in : interfaces) allInterfaces.add(getAllInterfaces(in)); + allInterfaces.add(getAllInterfaces(cn.getUnresolvedSuperClass())); + if (nInterfaces == 1 && allInterfaces.get(1).isEmpty()) + return; // no peer interface(s) to verify + + for (int i = 0; i < nInterfaces; i += 1) { + for (ClassNode in : allInterfaces.get(i)) { + if (in.redirect().getGenericsTypes() != null) { + for (int j = i + 1; j < nInterfaces + 1; j += 1) { + Set set = allInterfaces.get(j); + if (set.contains(in)) { + for (ClassNode t : set) { // find match and check generics + if (t.equals(in)) { + String one = in.toString(false), two = t.toString(false); + if (!one.equals(two)) + throw new RuntimeParserException("The interface " + in.getNameWithoutPackage() + + " cannot be implemented more than once with different arguments: " + one + " and " + two, cn); + break; + } + } + } + } + } + } + } + } + private static void checkForDuplicateMethods(final ClassNode cn) { Set descriptors = new HashSet<>(); for (MethodNode mn : cn.getMethods()) {