Skip to content

Commit

Permalink
AIC: bypass checkTypeArgumentRedundancy instead of setting IsDiamond
Browse files Browse the repository at this point in the history 
for #1358 #1419
  • Loading branch information
@eric-milles
eric-milles committed Nov 20, 2022
1 parent d53cebd commit f3616f0
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 55 deletions.
54 changes: 21 additions & 33 deletions ...groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/basic/GenericsTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2170,17 +2170,13 @@ public void testAbstractCovariance5() {
};
//@formatter:on

if (isAtLeastJava(JDK9)) {
runWarningFreeTest(sources);
} else {
runNegativeTest(sources,
"----------\n" +
"1. WARNING in Impl.groovy (at line 3)\n" +
"\tCharSequence process(CharSequence chars) { chars }\n" +
"\t^^^^^^^^^^^^\n" +
"Type safety: The return type CharSequence for process(CharSequence) from the type new Face<CharSequence>(){} needs unchecked conversion to conform to IO from the type Face<T>\n" +
"----------\n");
}
runNegativeTest(sources,
"----------\n" +
"1. WARNING in Impl.groovy (at line 3)\n" +
"\tCharSequence process(CharSequence chars) { chars }\n" +
"\t^^^^^^^^^^^^\n" +
"Type safety: The return type CharSequence for process(CharSequence) from the type new Face<CharSequence>(){} needs unchecked conversion to conform to IO from the type Face<T>\n" +
"----------\n");
}

@Test // https://issues.apache.org/jira/projects/GROOVY/issues/GROOVY-9059
Expand All @@ -2200,17 +2196,13 @@ public void testAbstractCovariance6() {
};
//@formatter:on

if (isAtLeastJava(JDK9)) {
runWarningFreeTest(sources);
} else {
runNegativeTest(sources,
"----------\n" +
"1. WARNING in Impl.groovy (at line 3)\n" +
"\tString process(String string) { string }\n" +
"\t^^^^^^\n" +
"Type safety: The return type String for process(String) from the type new Face<String>(){} needs unchecked conversion to conform to IO from the type Face<T>\n" +
"----------\n");
}
runNegativeTest(sources,
"----------\n" +
"1. WARNING in Impl.groovy (at line 3)\n" +
"\tString process(String string) { string }\n" +
"\t^^^^^^\n" +
"Type safety: The return type String for process(String) from the type new Face<String>(){} needs unchecked conversion to conform to IO from the type Face<T>\n" +
"----------\n");
}

@Test // https://issues.apache.org/jira/projects/GROOVY/issues/GROOVY-9059
Expand All @@ -2230,17 +2222,13 @@ public void testAbstractCovariance7() {
};
//@formatter:on

if (isAtLeastJava(JDK9)) {
runWarningFreeTest(sources);
} else {
runNegativeTest(sources,
"----------\n" +
"1. WARNING in Impl.groovy (at line 3)\n" +
"\tString process(String string) { string }\n" +
"\t^^^^^^\n" +
"Type safety: The return type String for process(String) from the type new Face<String>(){} needs unchecked conversion to conform to O from the type Face<T>\n" +
"----------\n");
}
runNegativeTest(sources,
"----------\n" +
"1. WARNING in Impl.groovy (at line 3)\n" +
"\tString process(String string) { string }\n" +
"\t^^^^^^\n" +
"Type safety: The return type String for process(String) from the type new Face<String>(){} needs unchecked conversion to conform to O from the type Face<T>\n" +
"----------\n");
}

@Test // https://issues.apache.org/jira/projects/GROOVY/issues/GROOVY-10675
Expand Down
43 changes: 21 additions & 22 deletions ...e/src/org/codehaus/jdt/groovy/internal/compiler/ast/GroovyCompilationUnitDeclaration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,11 @@
import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants;
import org.eclipse.jdt.internal.compiler.impl.CompilerOptions;
import org.eclipse.jdt.internal.compiler.impl.ReferenceContext;
import org.eclipse.jdt.internal.compiler.lookup.BlockScope;
import org.eclipse.jdt.internal.compiler.lookup.CompilationUnitScope;
import org.eclipse.jdt.internal.compiler.lookup.ExtraCompilerModifiers;
import org.eclipse.jdt.internal.compiler.lookup.LookupEnvironment;
import org.eclipse.jdt.internal.compiler.lookup.ParameterizedTypeBinding;
import org.eclipse.jdt.internal.compiler.lookup.SourceTypeBinding;
import org.eclipse.jdt.internal.compiler.lookup.TypeConstants;
import org.eclipse.jdt.internal.compiler.lookup.TypeIds;
Expand Down Expand Up @@ -1205,16 +1207,18 @@ private void createTypeDeclarations(ModuleNode moduleNode) {
typeDeclaration.name = CharOperation.NO_CHAR;
typeDeclaration.bits |= (ASTNode.IsAnonymousType | ASTNode.IsLocalType);
//typeDeclaration.bits |= (typeDeclaration.superclass.bits & ASTNode.HasTypeAnnotations);
QualifiedAllocationExpression allocation = new QualifiedAllocationExpression(typeDeclaration);
QualifiedAllocationExpression allocation = new QualifiedAllocationExpression(typeDeclaration) {
@Override
public void checkTypeArgumentRedundancy(ParameterizedTypeBinding atype, BlockScope scope) {
// https://github.com/groovy/groovy-eclipse/issues/1358
}
};
allocation.sourceStart = isEnum ? typeDeclaration.sourceStart : typeDeclaration.sourceStart - 4; // approx. offset of "new"
allocation.sourceEnd = typeDeclaration.bodyEnd;
if (!isEnum) {
allocation.type = typeDeclaration.superclass;
if (allocation.type.isParameterizedTypeReference() && unitDeclaration.compilerOptions.targetJDK >= ClassFileConstants.JDK9) {
allocation.type.bits |= ASTNode.IsDiamond; // https://github.com/groovy/groovy-eclipse/issues/1358
}
}
// TODO: allocation.typeArguments = something
// TODO (GROOVY-10501): allocation.typeArguments = something
} else {
typeDeclaration.name = innerClassNode.getNameWithoutPackage().substring(outerClassNode.getNameWithoutPackage().length() + 1).toCharArray();
}
Expand Down Expand Up @@ -1707,7 +1711,7 @@ private void configurePermittedSubtypes(TypeDeclaration typeDeclaration, ClassNo
if (e instanceof VariableExpression) {
char[] simpleName = e.getText().toCharArray(); // assume it's a type name
TypeReference t = verify(new SingleTypeReference(simpleName, toPos(e.getStart(), e.getEnd() - 1)));
t.bits |= ASTNode.IgnoreRawTypeCheck;
if (!checkGenerics) t.bits |= ASTNode.IgnoreRawTypeCheck;
return t;
}
if (e instanceof PropertyExpression) {
Expand All @@ -1717,7 +1721,7 @@ private void configurePermittedSubtypes(TypeDeclaration typeDeclaration, ClassNo
}
char[][] compoundName = CharOperation.splitOn('.', e.getText().toCharArray()); // assume it's a type name
TypeReference t = new QualifiedTypeReference(compoundName, positionsFor(compoundName, e.getStart(), e.getEnd()));
t.bits |= ASTNode.IgnoreRawTypeCheck;
if (!checkGenerics) t.bits |= ASTNode.IgnoreRawTypeCheck;
return t;
}
Util.log(IStatus.WARNING, "Unhandled reference type: " + e.getClass().getName());
Expand Down Expand Up @@ -2307,27 +2311,23 @@ private TypeReference createTypeReferenceForClassNode(ClassNode classNode, int s

if (compoundName.length == 1) {
if (typeArguments == null) {
TypeReference tr = verify(new SingleTypeReference(typeName, toPos(sourceStart, sourceEnd - 1)));
if (!checkGenerics) {
tr.bits |= ASTNode.IgnoreRawTypeCheck;
}
return tr;
TypeReference t = verify(new SingleTypeReference(typeName, toPos(sourceStart, sourceEnd - 1)));
if (!checkGenerics) t.bits |= ASTNode.IgnoreRawTypeCheck;
return t;
} else {
TypeReference[] typeRefs = typeArguments.toArray(new TypeReference[typeArguments.size()]);
return new ParameterizedSingleTypeReference(typeName, typeRefs, 0, toPos(sourceStart, sourceEnd - 1));
}
} else {
if (typeArguments == null) {
TypeReference tr = new QualifiedTypeReference(compoundName, positionsFor(compoundName, sourceStart, sourceEnd));
if (!checkGenerics) {
tr.bits |= ASTNode.IgnoreRawTypeCheck;
}
return tr;
TypeReference t = new QualifiedTypeReference(compoundName, positionsFor(compoundName, sourceStart, sourceEnd));
if (!checkGenerics) t.bits |= ASTNode.IgnoreRawTypeCheck;
return t;
} else {
// TODO: Support individual component parameterization: A<X>.B<Y>
TypeReference[][] typeRefs = new TypeReference[compoundName.length][];
typeRefs[compoundName.length - 1] = typeArguments.toArray(new TypeReference[typeArguments.size()]);
return new ParameterizedQualifiedTypeReference(compoundName, typeRefs, 0, positionsFor(compoundName, sourceStart, sourceEnd));
TypeReference[][] types = new TypeReference[compoundName.length][];
types[compoundName.length - 1] = typeArguments.toArray(new TypeReference[typeArguments.size()]);
return new ParameterizedQualifiedTypeReference(compoundName, types, 0, positionsFor(compoundName, sourceStart, sourceEnd));
}
}
}
Expand Down Expand Up @@ -2360,8 +2360,7 @@ private TypeReference createTypeReferenceForGenerics(GenericsType genericsType)
}
// FIXASC what does the check on this next really line mean?
} else if (!genericsType.getType().isGenericsPlaceHolder()) {
TypeReference typeReference = createTypeReferenceForClassNode(genericsType.getType());
return typeReference;
return createTypeReferenceForClassNode(genericsType.getType());
} else {
// this means it is a placeholder. As an example, if the reference is to 'List'
// then the genericsType info may include a placeholder for the type variable (as the user
Expand Down

0 comments on commit f3616f0

Please sign in to comment.