diff --git a/framework/src/main/java/org/checkerframework/common/wholeprograminference/WholeProgramInferenceJavaParserStorage.java b/framework/src/main/java/org/checkerframework/common/wholeprograminference/WholeProgramInferenceJavaParserStorage.java index 6a3ba01473dc..d9d104416f10 100644 --- a/framework/src/main/java/org/checkerframework/common/wholeprograminference/WholeProgramInferenceJavaParserStorage.java +++ b/framework/src/main/java/org/checkerframework/common/wholeprograminference/WholeProgramInferenceJavaParserStorage.java @@ -103,7 +103,6 @@ import org.checkerframework.javacutil.ElementUtils; import org.checkerframework.javacutil.TreeUtils; import org.checkerframework.javacutil.TypeSystemError; -import org.checkerframework.javacutil.TypesUtils; import org.checkerframework.javacutil.UserError; import org.plumelib.util.ArraySet; import org.plumelib.util.CollectionsP; @@ -1182,8 +1181,9 @@ boolean annotationIsRelevant(AnnotationExpr anno) { if (parentNode instanceof Type type) { // JavaParser's `TypeParameter` is a `Type`, so an annotation on a type parameter - // declaration, as in `<@Anno T>`, takes this branch. `typeToTypeMirror` returns null for a - // type parameter declaration, so such an annotation is retained. + // declaration, as in `<@Anno T>`, takes this branch. + // `JavaParserUtil.typeToTypeMirrorErasingTypeVariables` returns null for a type parameter + // declaration, so such an annotation is retained. return typeIsRelevant(gatf, type); } if (parentNode instanceof ArrayCreationLevel level) { @@ -1255,6 +1255,10 @@ private boolean typeIsRelevant(GenericAnnotatedTypeFactory gatf, Typ * type is {@code componentType} wrapped in {@code arrayLevels} array levels. This implementation * is conservative and only returns false if such a qualifier is definitely not relevant. * + *

A use of a type variable is resolved to the type variable's upper bound, which is sound + * because {@link GenericAnnotatedTypeFactory#isRelevant} treats a type variable as relevant + * exactly when its upper bound is. + * * @param gatf the type factory associated with this * @param componentType a JavaParser type * @param arrayLevels the number of array levels to wrap {@code componentType} in; may be 0 @@ -1262,12 +1266,14 @@ private boolean typeIsRelevant(GenericAnnotatedTypeFactory gatf, Typ */ private boolean typeIsRelevant( GenericAnnotatedTypeFactory gatf, Type componentType, int arrayLevels) { - TypeMirror tm = typeToTypeMirror(componentType); + Types types = atypeFactory.getProcessingEnv().getTypeUtils(); + TypeMirror tm = + JavaParserUtil.typeToTypeMirrorErasingTypeVariables( + elements, types, componentType, typeElementCache); if (tm == null) { // The type could not be determined. Be conservative. return true; } - Types types = atypeFactory.getProcessingEnv().getTypeUtils(); for (int i = 0; i < arrayLevels; i++) { tm = types.getArrayType(tm); } @@ -1301,76 +1307,6 @@ private boolean arrayCreationLevelIsRelevant( return true; } - /** - * Returns the TypeMirror for the given JavaParser type, or null if it cannot be determined. - * - *

This differs from {@link JavaParserUtil#typeToTypeMirror} in that a use of a type variable, - * for which no TypeMirror can be constructed from its JavaParser declaration, yields the type - * variable's effective upper bound as computed by {@link #typeVariableUpperBound}. That is sound - * for deciding relevance, because {@code GenericAnnotatedTypeFactory.isRelevant} erases the type - * and treats a type variable as relevant exactly when its upper bound is. - * - * @param type a JavaParser type - * @return the TypeMirror for {@code type}, or null if it cannot be determined - */ - private @Nullable TypeMirror typeToTypeMirror(Type type) { - Types types = atypeFactory.getProcessingEnv().getTypeUtils(); - // An array type's element type is what might name a type variable, and a non-array type is - // its own element type. - Type elementType = type.getElementType(); - TypeMirror result; - if (elementType instanceof ClassOrInterfaceType classType) { - // Resolving the name once yields both what it might name: a type or a type variable. - // Looking each up separately would walk the enclosing scopes twice. - JavaParserUtil.ResolvedTypeName resolved = - JavaParserUtil.resolveTypeName(elements, classType, typeElementCache); - TypeElement typeElt = resolved.typeElement(); - TypeParameter typeParameter = resolved.typeParameter(); - if (typeElt != null) { - result = typeElt.asType(); - } else if (typeParameter != null) { - // No TypeMirror can be built from a JavaParser type parameter, so use its upper bound. - result = typeVariableUpperBound(typeParameter); - } else { - return null; - } - } else { - result = JavaParserUtil.typeToTypeMirror(elements, types, elementType, typeElementCache); - } - if (result == null) { - return null; - } - for (int i = type.getArrayLevel(); i > 0; i--) { - result = types.getArrayType(result); - } - return result; - } - - /** - * Returns the TypeMirror for the given type variable's upper bound, or null if the upper bound - * cannot be determined. - * - *

When the upper bound is an intersection type, which {@code Types} cannot create, this - * returns the intersection type's erasure -- that is, its leftmost bound. - * - * @param typeParameter the declaration of a type variable - * @return the TypeMirror for the upper bound of {@code typeParameter}, erased if that bound is an - * intersection type, or null - */ - private @Nullable TypeMirror typeVariableUpperBound(TypeParameter typeParameter) { - NodeList bounds = typeParameter.getTypeBound(); - if (bounds.isEmpty()) { - // The implicit upper bound is `Object`. - return TypesUtils.getObjectTypeMirror(atypeFactory.getProcessingEnv()); - } - // If there are multiple bounds, the upper bound is an intersection type, which `Types` cannot - // create. Use its leftmost bound, which is its erasure; that is sufficient because the client, - // `GenericAnnotatedTypeFactory.isRelevant`, erases the type before testing relevance. - // The bound may itself be a type variable, as in ``; the - // recursion terminates because Java forbids a cycle among type variable bounds. - return typeToTypeMirror(bounds.get(0)); - } - /** * Returns the relative directory that corresponds to a package name. For example, if {@code * separatorChar} is {@code '/'}, then the package name {@code "org.example"} yields {@code diff --git a/framework/src/main/java/org/checkerframework/framework/util/JavaParserUtil.java b/framework/src/main/java/org/checkerframework/framework/util/JavaParserUtil.java index 0d2bba0a71eb..ca1a91bf4e68 100644 --- a/framework/src/main/java/org/checkerframework/framework/util/JavaParserUtil.java +++ b/framework/src/main/java/org/checkerframework/framework/util/JavaParserUtil.java @@ -1131,6 +1131,89 @@ public static TypeKind typeKindForPrimitive(PrimitiveType primitiveType) { return null; } + /** + * Returns the TypeMirror for the given JavaParser type, except that a use of a type variable + * yields the TypeMirror for the type variable's upper bound. For example, given the declaration + * {@code class C}, this method returns {@code java.lang.Number} for the type + * {@code T} and {@code java.lang.Number[]} for the type {@code T[]}. + * + *

The upper bound stands in for the type variable itself, which has no TypeMirror here, + * because a type variable has no {@code TypeElement} that {@code Elements} can look up. A caller + * that must distinguish a type variable from its bound should call {@link + * #resolveTypeNameAsTypeParameter(Elements, ClassOrInterfaceType, Map)}. + * + *

When a type variable's upper bound is an intersection type, which {@code Types} cannot + * create, this yields the intersection type's erasure -- that is, its leftmost bound. + * + *

Returns null if the TypeMirror cannot be determined, for the same types for which {@link + * #typeToTypeMirror(Elements, Types, Type, Map)} returns null. + * + * @param elements used for looking up names + * @param types used for creating types + * @param type a JavaParser type + * @param cache maps a name to the type it names, or to null if it names no type; this method both + * reads and writes it. See {@link #resolveTypeName(Elements, ClassOrInterfaceType, Map)} for + * restrictions on it. + * @return the TypeMirror for {@code type}, with a type variable replaced by its upper bound, or + * null if it cannot be determined + */ + public static @Nullable TypeMirror typeToTypeMirrorErasingTypeVariables( + Elements elements, Types types, Type type, Map cache) { + if (type instanceof ArrayType arrayType) { + TypeMirror componentType = + typeToTypeMirrorErasingTypeVariables( + elements, types, arrayType.getComponentType(), cache); + return componentType == null ? null : types.getArrayType(componentType); + } + if (type instanceof ClassOrInterfaceType classType) { + ResolvedTypeName resolved = resolveTypeName(elements, classType, cache); + TypeElement typeElt = resolved.typeElement(); + if (typeElt != null) { + return typeElt.asType(); + } + TypeParameter typeParameter = resolved.typeParameter(); + if (typeParameter == null) { + return null; + } + return typeVariableUpperBound(elements, types, typeParameter, cache); + } + return typeToTypeMirror(elements, types, type, cache); + } + + /** + * Returns the TypeMirror for the upper bound of the given type variable declaration, or null if + * the upper bound cannot be determined. + * + *

When the upper bound is an intersection type, which {@code Types} cannot create, this + * returns the intersection type's erasure -- that is, its leftmost bound. + * + * @param elements used for looking up names + * @param types used for creating types + * @param typeParameter the declaration of a type variable + * @param cache maps a name to the type it names, or to null if it names no type; this method both + * reads and writes it + * @return the TypeMirror for the upper bound of {@code typeParameter}, erased if that bound is an + * intersection type, or null + */ + private static @Nullable TypeMirror typeVariableUpperBound( + Elements elements, + Types types, + TypeParameter typeParameter, + Map cache) { + List bounds = typeParameter.getTypeBound(); + if (bounds.isEmpty()) { + // The implicit upper bound is `Object`. + TypeElement objectElt = elements.getTypeElement("java.lang.Object"); + return objectElt == null ? null : objectElt.asType(); + } + // If there are multiple bounds, the upper bound is an intersection type, which `Types` cannot + // create. Use its leftmost bound, which is its erasure; that is sufficient for the clients of + // this method, which erase the type anyway. + // The bound may itself be a type variable, as in ``; the + // recursion terminates because Java forbids a cycle among type variable bounds. + return typeToTypeMirrorErasingTypeVariables(elements, types, bounds.get(0), cache); + } + // // Perform side effects // diff --git a/framework/src/test/java/org/checkerframework/framework/util/JavaParserUtilTest.java b/framework/src/test/java/org/checkerframework/framework/util/JavaParserUtilTest.java index 06fd36946d69..e19f9169c2f7 100644 --- a/framework/src/test/java/org/checkerframework/framework/util/JavaParserUtilTest.java +++ b/framework/src/test/java/org/checkerframework/framework/util/JavaParserUtilTest.java @@ -78,6 +78,88 @@ public void testResolveInheritedMemberType() { "Visible"); } + /** + * Tests that {@link JavaParserUtil#resolveTypeName} resolves the simple name of a member type + * that a local class or an anonymous class inherits only when that class really inherits it. Such + * a class has no name that {@link Elements} can look up, so its own package -- that of the + * compilation unit that declares it -- determines whether it inherits a package-private member + * type of its supertype. + */ + @Test + public void testResolveMemberTypeInheritedByUnnameableClass() { + // A public member type is inherited, even across packages. + assertResolvesTo(SUPERPKG + ".Base.Visible", SUPERPKG, localClass("Visible"), "Visible"); + assertResolvesTo(SUPERPKG + ".Base.Visible", SUBPKG, localClass("Visible"), "Visible"); + assertResolvesTo(SUPERPKG + ".Base.Visible", SUPERPKG, anonymousClass("Visible"), "Visible"); + assertResolvesTo(SUPERPKG + ".Base.Visible", SUBPKG, anonymousClass("Visible"), "Visible"); + + // A package-private member type is inherited only within the package that declares it. + assertResolvesTo(SUPERPKG + ".Base.Hidden", SUPERPKG, localClass("Hidden"), "Hidden"); + assertResolvesTo(null, SUBPKG, localClass("Hidden"), "Hidden"); + assertResolvesTo(SUPERPKG + ".Base.Hidden", SUPERPKG, anonymousClass("Hidden"), "Hidden"); + assertResolvesTo(null, SUBPKG, anonymousClass("Hidden"), "Hidden"); + + // A private member type is never inherited. + assertResolvesTo(null, SUPERPKG, localClass("Secret"), "Secret"); + assertResolvesTo(null, SUPERPKG, anonymousClass("Secret"), "Secret"); + + // When the class does not inherit the package-private member type, the name resolves to the + // type of the same simple name that is declared in the class's own package. + assertResolvesTo(SUPERPKG + ".Base.Shadowed", SUPERPKG, localClass("Shadowed"), "Shadowed"); + assertResolvesTo(SUBPKG + ".Shadowed", SUBPKG, localClass("Shadowed"), "Shadowed"); + assertResolvesTo(SUPERPKG + ".Base.Shadowed", SUPERPKG, anonymousClass("Shadowed"), "Shadowed"); + assertResolvesTo(SUBPKG + ".Shadowed", SUBPKG, anonymousClass("Shadowed"), "Shadowed"); + } + + /** + * Tests that {@link JavaParserUtil#resolveTypeName} resolves a nested type name whose first + * component names a member type of an enclosing class and whose later components name inherited + * member types. Such a name is not canonical, so it cannot be looked up directly. + */ + @Test + public void testResolveNestedNameOfDeclaredMemberType() { + String memberOwner = + "class MemberOwner { private static class Member extends Base {}" + + " Member.Visible f; Member.Secret g; }"; + + // The name of the member type itself is canonical. + assertResolvesTo(SUPERPKG + ".MemberOwner.Member", SUPERPKG, memberOwner, "Member"); + + // `Member` inherits `Visible` rather than declaring it, so `Member.Visible` is not canonical + // and the member types of `Member`'s supertypes must be searched. + assertResolvesTo(SUPERPKG + ".Base.Visible", SUPERPKG, memberOwner, "Member.Visible"); + + // A member type of an enclosing class shadows every other type whose name starts with the + // member type's simple name, so a suffix that names no member type of `Member` names nothing. + assertResolvesTo(null, SUPERPKG, memberOwner, "Member.Secret"); + } + + /** + * Returns the declaration of a class named {@code Outer}, which is not a subtype of {@code Base}, + * that declares a local class that extends {@code Base} and that uses the given type name. + * + * @param typeName the type name that the local class uses + * @return a type declaration that contains a local class that uses {@code typeName} + */ + private static String localClass(String typeName) { + return "class Outer { void m() { class Local extends " + + SUPERPKG + + ".Base { " + + typeName + + " f; } } }"; + } + + /** + * Returns the declaration of a class named {@code Outer}, which is not a subtype of {@code Base}, + * that declares an anonymous subclass of {@code Base} that uses the given type name. + * + * @param typeName the type name that the anonymous class uses + * @return a type declaration that contains an anonymous class that uses {@code typeName} + */ + private static String anonymousClass(String typeName) { + return "class Outer { Object o = new " + SUPERPKG + ".Base() { " + typeName + " f; }; }"; + } + /** * Tests that {@link JavaParserUtil#resolveTypeNameAsTypeParameter} resolves a name to a type * variable exactly when no type declaration shadows the type variable. diff --git a/framework/src/test/java/org/checkerframework/framework/util/javaparserutil/subpkg/Outer.java b/framework/src/test/java/org/checkerframework/framework/util/javaparserutil/subpkg/Outer.java new file mode 100644 index 000000000000..34cc177986f1 --- /dev/null +++ b/framework/src/test/java/org/checkerframework/framework/util/javaparserutil/subpkg/Outer.java @@ -0,0 +1,14 @@ +package org.checkerframework.framework.util.javaparserutil.subpkg; + +import org.checkerframework.framework.util.javaparserutil.superpkg.Base; + +/** + * Test data for {@code JavaParserUtilTest}: a class that is not a subtype of {@link Base}, so that + * a test of a local or an anonymous class within it cannot accidentally resolve a name through the + * enclosing class. + */ +public class Outer { + + /** Creates a new Outer. */ + public Outer() {} +} diff --git a/framework/src/test/java/org/checkerframework/framework/util/javaparserutil/subpkg/Shadowed.java b/framework/src/test/java/org/checkerframework/framework/util/javaparserutil/subpkg/Shadowed.java new file mode 100644 index 000000000000..e71d4c5452dc --- /dev/null +++ b/framework/src/test/java/org/checkerframework/framework/util/javaparserutil/subpkg/Shadowed.java @@ -0,0 +1,14 @@ +package org.checkerframework.framework.util.javaparserutil.subpkg; + +import org.checkerframework.framework.util.javaparserutil.superpkg.Base; + +/** + * Test data for {@code JavaParserUtilTest}: a top-level type whose simple name is also the simple + * name of a package-private member type of {@link Base}. A class in this package does not inherit + * {@code Base.Shadowed}, so the simple name {@code Shadowed} refers to this type. + */ +public class Shadowed { + + /** Creates a new Shadowed. */ + public Shadowed() {} +} diff --git a/framework/src/test/java/org/checkerframework/framework/util/javaparserutil/superpkg/Base.java b/framework/src/test/java/org/checkerframework/framework/util/javaparserutil/superpkg/Base.java index 9f55b8dcae6d..45106ea9c1fe 100644 --- a/framework/src/test/java/org/checkerframework/framework/util/javaparserutil/superpkg/Base.java +++ b/framework/src/test/java/org/checkerframework/framework/util/javaparserutil/superpkg/Base.java @@ -18,6 +18,13 @@ public static class Visible { public static class OnlyInBase {} } + /** + * A package-private member type whose simple name is also the simple name of a top-level type in + * the {@code subpkg} package. A class in that package does not inherit this type, so the simple + * name {@code Shadowed} refers there to the top-level type instead. + */ + static class Shadowed {} + /** A private member type, which no subtype inherits. */ private static class Secret {} diff --git a/framework/src/test/java/org/checkerframework/framework/util/javaparserutil/superpkg/MemberOwner.java b/framework/src/test/java/org/checkerframework/framework/util/javaparserutil/superpkg/MemberOwner.java new file mode 100644 index 000000000000..36413cb7c7fd --- /dev/null +++ b/framework/src/test/java/org/checkerframework/framework/util/javaparserutil/superpkg/MemberOwner.java @@ -0,0 +1,17 @@ +package org.checkerframework.framework.util.javaparserutil.superpkg; + +/** + * Test data for {@code JavaParserUtilTest}: a class that declares a member type that inherits the + * member types of {@link Base}. A name such as {@code Member.Visible} is therefore not canonical. + */ +public class MemberOwner { + + /** Creates a new MemberOwner. */ + public MemberOwner() {} + + /** A private member type, which is in scope in this class's body but in no other class. */ + private static class Member extends Base {} + + /** Uses {@code Member.Visible}, whose name is not canonical. */ + Member.Visible f; +} diff --git a/framework/src/test/java/org/checkerframework/framework/util/javaparserutil/superpkg/Outer.java b/framework/src/test/java/org/checkerframework/framework/util/javaparserutil/superpkg/Outer.java new file mode 100644 index 000000000000..0c7cfce30730 --- /dev/null +++ b/framework/src/test/java/org/checkerframework/framework/util/javaparserutil/superpkg/Outer.java @@ -0,0 +1,12 @@ +package org.checkerframework.framework.util.javaparserutil.superpkg; + +/** + * Test data for {@code JavaParserUtilTest}: a class that is not a subtype of {@link Base}, so that + * a test of a local or an anonymous class within it cannot accidentally resolve a name through the + * enclosing class. + */ +public class Outer { + + /** Creates a new Outer. */ + public Outer() {} +}