Skip to content

Commit

Permalink
Fix for #714: defer initialization/resolution of inner types
Browse files Browse the repository at this point in the history
  • Loading branch information
@eric-milles
eric-milles committed Sep 14, 2018
1 parent a53198c commit 5f5755d
Show file tree
Hide file tree
Showing 10 changed files with 108 additions and 80 deletions.
15 changes: 1 addition & 14 deletions base/org.codehaus.groovy24/src/org/codehaus/groovy/ast/ClassNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1662,12 +1662,6 @@ public Iterator<InnerClassNode> getInnerClasses() {
}

// GRECLIPSE add
public void forgetInnerClass(InnerClassNode icn) {
if (innerClasses != null) {
innerClasses.remove(icn); // GRECLIPSE-1167
}
}

public String getClassInternalName() {
return (isRedirectNode() ? redirect().getClassInternalName() : null);
}
Expand All @@ -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

Expand Down
15 changes: 1 addition & 14 deletions base/org.codehaus.groovy25/src/org/codehaus/groovy/ast/ClassNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1604,12 +1604,6 @@ public Iterator<InnerClassNode> getInnerClasses() {
}

// GRECLIPSE add
public void forgetInnerClass(InnerClassNode icn) {
if (innerClasses != null) {
innerClasses.remove(icn); // GRECLIPSE-1167
}
}

public String getClassInternalName() {
return (isRedirectNode() ? redirect().getClassInternalName() : null);
}
Expand All @@ -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

Expand Down
15 changes: 1 addition & 14 deletions base/org.codehaus.groovy26/src/org/codehaus/groovy/ast/ClassNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1639,12 +1639,6 @@ public Iterator<InnerClassNode> getInnerClasses() {
}

// GRECLIPSE add
public void forgetInnerClass(InnerClassNode icn) {
if (innerClasses != null) {
innerClasses.remove(icn); // GRECLIPSE-1167
}
}

public String getClassInternalName() {
return (isRedirectNode() ? redirect().getClassInternalName() : null);
}
Expand All @@ -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

Expand Down
49 changes: 37 additions & 12 deletions ...lipse.jdt.groovy.core/src/org/codehaus/jdt/groovy/internal/compiler/ast/JDTClassNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -476,7 +469,7 @@ public List<AnnotationNode> 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;
}
Expand Down Expand Up @@ -585,6 +578,38 @@ public List<PropertyNode> getProperties() {
return Collections.unmodifiableList(super.getProperties());
}

@Override
public Iterator<InnerClassNode> 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()) {
Expand Down Expand Up @@ -643,6 +668,6 @@ public boolean isResolved() {

@Override
public boolean mightHaveInners() {
return (jdtBinding.memberTypes().length > 0);
return jdtBinding.hasMemberTypes();
}
}
11 changes: 6 additions & 5 deletions ...rg.eclipse.jdt.groovy.core/src/org/codehaus/jdt/groovy/internal/compiler/ast/JDTNode.java
View file
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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<AnnotationNode> getAnnotations(ClassNode type);

List<AnnotationNode> getAnnotations();

List<AnnotationNode> getAnnotations(ClassNode type);
JDTResolver getResolver();

Binding getJdtBinding();

Expand Down
6 changes: 3 additions & 3 deletions ...odehaus/groovy/eclipse/refactoring/core/extract/ConvertGroovyLocalToFieldRefactoring.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -401,9 +401,9 @@ public void visitVariableExpression(VariableExpression variableExpression) {
};

referencesVisitor.visitClass(getContainingClassNode());
Iterator<InnerClassNode> innerClasses = getContainingClassNode().getInnerClasses();
while (innerClasses != null && innerClasses.hasNext()) {
ClassNode innerClass = innerClasses.next();

for (Iterator<InnerClassNode> it = getContainingClassNode().getInnerClasses(); it.hasNext();) {
InnerClassNode innerClass = it.next();
referencesVisitor.visitClass(innerClass);
}

Expand Down
13 changes: 6 additions & 7 deletions ....eclipse.refactoring/src/org/codehaus/groovy/eclipse/refactoring/core/utils/ASTTools.java
View file
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -284,12 +284,11 @@ public static ClassNode getContainingClassNode(ModuleNode moduleNode, int offset
}
} else {
// look for inner classes
Iterator<InnerClassNode> 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<InnerClassNode> it = containingClassNode.getInnerClasses(); it.hasNext();) {
InnerClassNode innerClass = it.next();
if (innerClass.getStart() <= offset && innerClass.getEnd() >= offset) {
containingClassNode = innerClass;
it = innerClass.getInnerClasses();
}
}
}
Expand Down
22 changes: 18 additions & 4 deletions ....jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ParameterizedTypeBinding.java
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// GROOVY PATCHED
/*******************************************************************************
* Copyright (c) 2005, 2018 IBM Corporation and others.
* All rights reserved. This program and the accompanying materials
Expand Down Expand Up @@ -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: <code-snippet>
* 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;
Expand Down Expand Up @@ -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;) {
Expand Down
20 changes: 17 additions & 3 deletions ....jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ParameterizedTypeBinding.java
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// GROOVY PATCHED
/*******************************************************************************
* Copyright (c) 2005, 2018 IBM Corporation and others.
* All rights reserved. This program and the accompanying materials
Expand Down Expand Up @@ -773,20 +774,33 @@ public MethodBinding getExactMethod(char[] selector, TypeBinding[] argumentTypes
return null;
}

/**
/**
* @see org.eclipse.jdt.internal.compiler.lookup.ReferenceBinding#getField(char[], boolean)
*/
@Override
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;) {
Expand Down
22 changes: 18 additions & 4 deletions ....jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/ParameterizedTypeBinding.java
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// GROOVY PATCHED
/*******************************************************************************
* Copyright (c) 2005, 2018 IBM Corporation and others.
*
Expand Down Expand Up @@ -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: <code-snippet>
* 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;
Expand Down Expand Up @@ -776,20 +777,33 @@ public MethodBinding getExactMethod(char[] selector, TypeBinding[] argumentTypes
return null;
}

/**
/**
* @see org.eclipse.jdt.internal.compiler.lookup.ReferenceBinding#getField(char[], boolean)
*/
@Override
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;) {
Expand Down

0 comments on commit 5f5755d

Please sign in to comment.