From 5f5755d8ed8f85a1d2b184f9d7ba641a7938f0f5 Mon Sep 17 00:00:00 2001 From: Eric Milles Date: Thu, 13 Sep 2018 09:53:45 -0500 Subject: [PATCH] Fix for #714: defer initialization/resolution of inner types --- .../org/codehaus/groovy/ast/ClassNode.java | 15 +----- .../org/codehaus/groovy/ast/ClassNode.java | 15 +----- .../org/codehaus/groovy/ast/ClassNode.java | 15 +----- .../internal/compiler/ast/JDTClassNode.java | 49 ++++++++++++++----- .../groovy/internal/compiler/ast/JDTNode.java | 11 +++-- .../ConvertGroovyLocalToFieldRefactoring.java | 6 +-- .../refactoring/core/utils/ASTTools.java | 13 +++-- .../lookup/ParameterizedTypeBinding.java | 22 +++++++-- .../lookup/ParameterizedTypeBinding.java | 20 ++++++-- .../lookup/ParameterizedTypeBinding.java | 22 +++++++-- 10 files changed, 108 insertions(+), 80 deletions(-) diff --git a/base/org.codehaus.groovy24/src/org/codehaus/groovy/ast/ClassNode.java b/base/org.codehaus.groovy24/src/org/codehaus/groovy/ast/ClassNode.java index 3c78b27611..d52d3c7604 100644 --- a/base/org.codehaus.groovy24/src/org/codehaus/groovy/ast/ClassNode.java +++ b/base/org.codehaus.groovy24/src/org/codehaus/groovy/ast/ClassNode.java @@ -1662,12 +1662,6 @@ public Iterator getInnerClasses() { } // GRECLIPSE add - public void forgetInnerClass(InnerClassNode icn) { - if (innerClasses != null) { - innerClasses.remove(icn); // GRECLIPSE-1167 - } - } - public String getClassInternalName() { return (isRedirectNode() ? redirect().getClassInternalName() : null); } @@ -1680,15 +1674,8 @@ public boolean isPrimitive() { return (clazz != null && clazz.isPrimitive()); } - /** - * @return true if this classnode might have inners, conservatively it says yes if it is unsure. - */ public boolean mightHaveInners() { - ClassNode r = redirect(); - if (r.hasClass()) { - return true; - } - return (r.innerClasses != null && !r.innerClasses.isEmpty()); + return (hasClass() ? true : getInnerClasses().hasNext()); } // GRECLIPSE end diff --git a/base/org.codehaus.groovy25/src/org/codehaus/groovy/ast/ClassNode.java b/base/org.codehaus.groovy25/src/org/codehaus/groovy/ast/ClassNode.java index 7f5cf02562..26124b93c3 100644 --- a/base/org.codehaus.groovy25/src/org/codehaus/groovy/ast/ClassNode.java +++ b/base/org.codehaus.groovy25/src/org/codehaus/groovy/ast/ClassNode.java @@ -1604,12 +1604,6 @@ public Iterator getInnerClasses() { } // GRECLIPSE add - public void forgetInnerClass(InnerClassNode icn) { - if (innerClasses != null) { - innerClasses.remove(icn); // GRECLIPSE-1167 - } - } - public String getClassInternalName() { return (isRedirectNode() ? redirect().getClassInternalName() : null); } @@ -1622,15 +1616,8 @@ public boolean isPrimitive() { return (clazz != null && clazz.isPrimitive()); } - /** - * @return true if this classnode might have inners, conservatively it says yes if it is unsure. - */ public boolean mightHaveInners() { - ClassNode r = redirect(); - if (r.hasClass()) { - return true; - } - return (r.innerClasses != null && !r.innerClasses.isEmpty()); + return (hasClass() ? true : getInnerClasses().hasNext()); } // GRECLIPSE end diff --git a/base/org.codehaus.groovy26/src/org/codehaus/groovy/ast/ClassNode.java b/base/org.codehaus.groovy26/src/org/codehaus/groovy/ast/ClassNode.java index c839dad550..8f03dc7987 100644 --- a/base/org.codehaus.groovy26/src/org/codehaus/groovy/ast/ClassNode.java +++ b/base/org.codehaus.groovy26/src/org/codehaus/groovy/ast/ClassNode.java @@ -1639,12 +1639,6 @@ public Iterator getInnerClasses() { } // GRECLIPSE add - public void forgetInnerClass(InnerClassNode icn) { - if (innerClasses != null) { - innerClasses.remove(icn); // GRECLIPSE-1167 - } - } - public String getClassInternalName() { return (isRedirectNode() ? redirect().getClassInternalName() : null); } @@ -1657,15 +1651,8 @@ public boolean isPrimitive() { return (clazz != null && clazz.isPrimitive()); } - /** - * @return true if this classnode might have inners, conservatively it says yes if it is unsure. - */ public boolean mightHaveInners() { - ClassNode r = redirect(); - if (r.hasClass()) { - return true; - } - return (r.innerClasses != null && !r.innerClasses.isEmpty()); + return (hasClass() ? true : getInnerClasses().hasNext()); } // GRECLIPSE end diff --git a/base/org.eclipse.jdt.groovy.core/src/org/codehaus/jdt/groovy/internal/compiler/ast/JDTClassNode.java b/base/org.eclipse.jdt.groovy.core/src/org/codehaus/jdt/groovy/internal/compiler/ast/JDTClassNode.java index 99809831dc..04906733bd 100644 --- a/base/org.eclipse.jdt.groovy.core/src/org/codehaus/jdt/groovy/internal/compiler/ast/JDTClassNode.java +++ b/base/org.eclipse.jdt.groovy.core/src/org/codehaus/jdt/groovy/internal/compiler/ast/JDTClassNode.java @@ -19,6 +19,7 @@ import java.util.Collections; import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Set; import java.util.stream.Stream; @@ -42,6 +43,8 @@ import org.codehaus.jdt.groovy.internal.compiler.ast.GroovyCompilationUnitDeclaration.FieldDeclarationWithInitializer; import org.eclipse.jdt.core.Flags; import org.eclipse.jdt.core.compiler.CharOperation; +import org.eclipse.jdt.groovy.core.util.ArrayUtils; +import org.eclipse.jdt.groovy.core.util.ReflectionUtils; import org.eclipse.jdt.groovy.search.AccessorSupport; import org.eclipse.jdt.internal.compiler.ast.FieldDeclaration; import org.eclipse.jdt.internal.compiler.ast.TypeDeclaration; @@ -295,16 +298,6 @@ private void initializeMembers() { addField(fNode); } } - - if (mightHaveInners()) { - Stream.of(jdtBinding.memberTypes()).map(resolver::convertToClassNode).forEach(cn -> { - @SuppressWarnings("unused") // InnerClassNode constructor adds reference to this.innerClasses - ClassNode icn = new InnerClassNode(this, cn.getName(), cn.getModifiers(), cn.getSuperClass()) {{ - isPrimaryNode = false; - setRedirect(cn); - }}; - }); - } } catch (AbortCompilation e) { throw e; } catch (RuntimeException e) { @@ -476,7 +469,7 @@ public List getAnnotations() { long tagBits = ((SourceTypeBinding) jdtBinding).getAnnotationTagBits(); } for (AnnotationBinding annotationBinding : jdtBinding.getAnnotations()) { - this.addAnnotation(new JDTAnnotationNode(annotationBinding, resolver)); + addAnnotation(new JDTAnnotationNode(annotationBinding, resolver)); } bits |= ANNOTATIONS_INITIALIZED; } @@ -585,6 +578,38 @@ public List getProperties() { return Collections.unmodifiableList(super.getProperties()); } + @Override + public Iterator getInnerClasses() { + if ((bits & INNER_TYPES_INITIALIZED) == 0) { + synchronized (this) { + if ((bits & INNER_TYPES_INITIALIZED) == 0) { + bits |= INNER_TYPES_INITIALIZED; + if (mightHaveInners()) { + // workaround for https://github.com/groovy/groovy-eclipse/issues/714 + if (jdtBinding instanceof BinaryTypeBinding && jdtBinding == jdtBinding.prototype() && Traits.isTrait(this)) { + ReferenceBinding[] memberTypes = ReflectionUtils.getPrivateField(BinaryTypeBinding.class, "memberTypes", jdtBinding); + for (int i = 0; i < memberTypes.length; i += 1) { + if (String.valueOf(memberTypes[i].sourceName).endsWith("$Trait$FieldHelper$1")) { + memberTypes = (ReferenceBinding[]) ArrayUtils.remove(memberTypes, i--); + } + } + ReflectionUtils.setPrivateField(BinaryTypeBinding.class, "memberTypes", jdtBinding, memberTypes); + } + // workaround end + Stream.of(jdtBinding.memberTypes()).map(resolver::convertToClassNode).forEach(cn -> { + @SuppressWarnings("unused") // InnerClassNode constructor adds itself to this.innerClasses + ClassNode icn = new InnerClassNode(this, cn.getName(), cn.getModifiers(), cn.getSuperClass()) {{ + isPrimaryNode = false; + setRedirect(cn); + }}; + }); + } + } + } + } + return (innerClasses == null ? Collections.EMPTY_LIST : Collections.unmodifiableList(innerClasses)).iterator(); + } + @Override public ClassNode getOuterClass() { if (jdtBinding.isNestedType()) { @@ -643,6 +668,6 @@ public boolean isResolved() { @Override public boolean mightHaveInners() { - return (jdtBinding.memberTypes().length > 0); + return jdtBinding.hasMemberTypes(); } } diff --git a/base/org.eclipse.jdt.groovy.core/src/org/codehaus/jdt/groovy/internal/compiler/ast/JDTNode.java b/base/org.eclipse.jdt.groovy.core/src/org/codehaus/jdt/groovy/internal/compiler/ast/JDTNode.java index 98bd92ac99..b360e2cabe 100644 --- a/base/org.eclipse.jdt.groovy.core/src/org/codehaus/jdt/groovy/internal/compiler/ast/JDTNode.java +++ b/base/org.eclipse.jdt.groovy.core/src/org/codehaus/jdt/groovy/internal/compiler/ast/JDTNode.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2017 the original author or authors. + * Copyright 2009-2018 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. @@ -22,18 +22,19 @@ import org.eclipse.jdt.internal.compiler.lookup.Binding; /** - * Common interface for Groovy ASTNodes backed by a JDT binding + * Common interface for Groovy {@code ASTNode}s backed by a JDT binding. */ public interface JDTNode { int ANNOTATIONS_INITIALIZED = 0x0001; - int PROPERTIES_INITIALIZED = 0x0002; + int INNER_TYPES_INITIALIZED = 0x0002; + int PROPERTIES_INITIALIZED = 0x0004; - JDTResolver getResolver(); + List getAnnotations(ClassNode type); List getAnnotations(); - List getAnnotations(ClassNode type); + JDTResolver getResolver(); Binding getJdtBinding(); diff --git a/ide/org.codehaus.groovy.eclipse.refactoring/src/org/codehaus/groovy/eclipse/refactoring/core/extract/ConvertGroovyLocalToFieldRefactoring.java b/ide/org.codehaus.groovy.eclipse.refactoring/src/org/codehaus/groovy/eclipse/refactoring/core/extract/ConvertGroovyLocalToFieldRefactoring.java index 5c209b9b67..7d0f8090bb 100644 --- a/ide/org.codehaus.groovy.eclipse.refactoring/src/org/codehaus/groovy/eclipse/refactoring/core/extract/ConvertGroovyLocalToFieldRefactoring.java +++ b/ide/org.codehaus.groovy.eclipse.refactoring/src/org/codehaus/groovy/eclipse/refactoring/core/extract/ConvertGroovyLocalToFieldRefactoring.java @@ -401,9 +401,9 @@ public void visitVariableExpression(VariableExpression variableExpression) { }; referencesVisitor.visitClass(getContainingClassNode()); - Iterator innerClasses = getContainingClassNode().getInnerClasses(); - while (innerClasses != null && innerClasses.hasNext()) { - ClassNode innerClass = innerClasses.next(); + + for (Iterator it = getContainingClassNode().getInnerClasses(); it.hasNext();) { + InnerClassNode innerClass = it.next(); referencesVisitor.visitClass(innerClass); } diff --git a/ide/org.codehaus.groovy.eclipse.refactoring/src/org/codehaus/groovy/eclipse/refactoring/core/utils/ASTTools.java b/ide/org.codehaus.groovy.eclipse.refactoring/src/org/codehaus/groovy/eclipse/refactoring/core/utils/ASTTools.java index 426dba3d8f..1c6f5a9c27 100644 --- a/ide/org.codehaus.groovy.eclipse.refactoring/src/org/codehaus/groovy/eclipse/refactoring/core/utils/ASTTools.java +++ b/ide/org.codehaus.groovy.eclipse.refactoring/src/org/codehaus/groovy/eclipse/refactoring/core/utils/ASTTools.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2017 the original author or authors. + * Copyright 2009-2018 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. @@ -284,12 +284,11 @@ public static ClassNode getContainingClassNode(ModuleNode moduleNode, int offset } } else { // look for inner classes - Iterator innerClasses = containingClassNode.getInnerClasses(); - while (innerClasses != null && innerClasses.hasNext()) { - InnerClassNode inner = innerClasses.next(); - if (inner.getStart() <= offset && inner.getEnd() >= offset) { - containingClassNode = inner; - innerClasses = inner.getInnerClasses(); + for (Iterator it = containingClassNode.getInnerClasses(); it.hasNext();) { + InnerClassNode innerClass = it.next(); + if (innerClass.getStart() <= offset && innerClass.getEnd() >= offset) { + containingClassNode = innerClass; + it = innerClass.getInnerClasses(); } } } diff --git a/jdt-patch/e47/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ParameterizedTypeBinding.java b/jdt-patch/e47/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ParameterizedTypeBinding.java index 8f41fe5a74..5ce4949716 100644 --- a/jdt-patch/e47/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ParameterizedTypeBinding.java +++ b/jdt-patch/e47/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ParameterizedTypeBinding.java @@ -1,3 +1,4 @@ +// GROOVY PATCHED /******************************************************************************* * Copyright (c) 2005, 2018 IBM Corporation and others. * All rights reserved. This program and the accompanying materials @@ -39,7 +40,7 @@ * Bug 435805 - [1.8][compiler][null] Java 8 compiler does not recognize declaration style null annotations * Bug 456508 - Unexpected RHS PolyTypeBinding for: * Bug 390064 - [compiler][resource] Resource leak warning missing when extending parameterized class - * Jesper S Møller - Contributions for bug 381345 : [1.8] Take care of the Java 8 major version + * Jesper S M�ller - Contributions for bug 381345 : [1.8] Take care of the Java 8 major version * Bug 527554 - [18.3] Compiler support for JEP 286 Local-Variable Type *******************************************************************************/ package org.eclipse.jdt.internal.compiler.lookup; @@ -748,18 +749,31 @@ public MethodBinding getExactMethod(char[] selector, TypeBinding[] argumentTypes return null; } - /** + /** * @see org.eclipse.jdt.internal.compiler.lookup.ReferenceBinding#getField(char[], boolean) */ public FieldBinding getField(char[] fieldName, boolean needResolve) { fields(); // ensure fields have been initialized... must create all at once unlike methods return ReferenceBinding.binarySearch(fieldName, this.fields); } - - /** + + /** * @see org.eclipse.jdt.internal.compiler.lookup.ReferenceBinding#getMemberType(char[]) */ public ReferenceBinding getMemberType(char[] typeName) { + // GROOVY add + if (this.memberTypes == null && this.type.hasMemberTypes() && + this.type instanceof BinaryTypeBinding && this.type == this.type.prototype()) { //$IDENTITY-COMPARISON$ + boolean found = false; + for (ReferenceBinding memberType : ((BinaryTypeBinding) this.type).memberTypes) { + if (CharOperation.fragmentEquals(typeName, memberType.sourceName, CharOperation.lastIndexOf('$', memberType.sourceName) + 1, true)) { + found = true; + break; + } + } + if (!found) return null; + } + // GROOVY end memberTypes(); // ensure memberTypes have been initialized... must create all at once unlike methods int typeLength = typeName.length; for (int i = this.memberTypes.length; --i >= 0;) { diff --git a/jdt-patch/e48/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ParameterizedTypeBinding.java b/jdt-patch/e48/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ParameterizedTypeBinding.java index d79d038deb..880cc4a844 100644 --- a/jdt-patch/e48/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ParameterizedTypeBinding.java +++ b/jdt-patch/e48/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ParameterizedTypeBinding.java @@ -1,3 +1,4 @@ +// GROOVY PATCHED /******************************************************************************* * Copyright (c) 2005, 2018 IBM Corporation and others. * All rights reserved. This program and the accompanying materials @@ -773,7 +774,7 @@ public MethodBinding getExactMethod(char[] selector, TypeBinding[] argumentTypes return null; } - /** + /** * @see org.eclipse.jdt.internal.compiler.lookup.ReferenceBinding#getField(char[], boolean) */ @Override @@ -781,12 +782,25 @@ public FieldBinding getField(char[] fieldName, boolean needResolve) { fields(); // ensure fields have been initialized... must create all at once unlike methods return ReferenceBinding.binarySearch(fieldName, this.fields); } - - /** + + /** * @see org.eclipse.jdt.internal.compiler.lookup.ReferenceBinding#getMemberType(char[]) */ @Override public ReferenceBinding getMemberType(char[] typeName) { + // GROOVY add + if (this.memberTypes == null && this.type.hasMemberTypes() && + this.type instanceof BinaryTypeBinding && this.type == this.type.prototype()) { //$IDENTITY-COMPARISON$ + boolean found = false; + for (ReferenceBinding memberType : ((BinaryTypeBinding) this.type).memberTypes) { + if (CharOperation.fragmentEquals(typeName, memberType.sourceName, CharOperation.lastIndexOf('$', memberType.sourceName) + 1, true)) { + found = true; + break; + } + } + if (!found) return null; + } + // GROOVY end memberTypes(); // ensure memberTypes have been initialized... must create all at once unlike methods int typeLength = typeName.length; for (int i = this.memberTypes.length; --i >= 0;) { diff --git a/jdt-patch/e49/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ParameterizedTypeBinding.java b/jdt-patch/e49/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ParameterizedTypeBinding.java index c4b6a82b5f..37d6dc4c85 100644 --- a/jdt-patch/e49/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ParameterizedTypeBinding.java +++ b/jdt-patch/e49/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ParameterizedTypeBinding.java @@ -1,3 +1,4 @@ +// GROOVY PATCHED /******************************************************************************* * Copyright (c) 2005, 2018 IBM Corporation and others. * @@ -42,7 +43,7 @@ * Bug 435805 - [1.8][compiler][null] Java 8 compiler does not recognize declaration style null annotations * Bug 456508 - Unexpected RHS PolyTypeBinding for: * Bug 390064 - [compiler][resource] Resource leak warning missing when extending parameterized class - * Jesper S Møller - Contributions for bug 381345 : [1.8] Take care of the Java 8 major version + * Jesper S M�ller - Contributions for bug 381345 : [1.8] Take care of the Java 8 major version * Bug 527554 - [18.3] Compiler support for JEP 286 Local-Variable Type *******************************************************************************/ package org.eclipse.jdt.internal.compiler.lookup; @@ -776,7 +777,7 @@ public MethodBinding getExactMethod(char[] selector, TypeBinding[] argumentTypes return null; } - /** + /** * @see org.eclipse.jdt.internal.compiler.lookup.ReferenceBinding#getField(char[], boolean) */ @Override @@ -784,12 +785,25 @@ public FieldBinding getField(char[] fieldName, boolean needResolve) { fields(); // ensure fields have been initialized... must create all at once unlike methods return ReferenceBinding.binarySearch(fieldName, this.fields); } - - /** + + /** * @see org.eclipse.jdt.internal.compiler.lookup.ReferenceBinding#getMemberType(char[]) */ @Override public ReferenceBinding getMemberType(char[] typeName) { + // GROOVY add + if (this.memberTypes == null && this.type.hasMemberTypes() && + this.type instanceof BinaryTypeBinding && this.type == this.type.prototype()) { //$IDENTITY-COMPARISON$ + boolean found = false; + for (ReferenceBinding memberType : ((BinaryTypeBinding) this.type).memberTypes) { + if (CharOperation.fragmentEquals(typeName, memberType.sourceName, CharOperation.lastIndexOf('$', memberType.sourceName) + 1, true)) { + found = true; + break; + } + } + if (!found) return null; + } + // GROOVY end memberTypes(); // ensure memberTypes have been initialized... must create all at once unlike methods int typeLength = typeName.length; for (int i = this.memberTypes.length; --i >= 0;) {