Skip to content

[Completion] Remove uses of unbounded generic types #79585

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 27, 2025
Merged

[Completion] Remove uses of unbounded generic types #79585

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0fc825b
[Completion] Remove uses of unbounded generic types
hamishknight Feb 25, 2025
f73054a
[Completion] Don't suggest nested type in where clause
hamishknight Feb 25, 2025
be0f6f9
[Completion] NFC: Factor out `getMemberBaseType()`
hamishknight Feb 25, 2025
8975bf6
[Completion] Substitute base type for nested types
hamishknight Feb 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions include/swift/IDE/CompletionLookup.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,11 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
/// \p selfTy must be a \c Self type of the context.
static bool canBeUsedAsRequirementFirstType(Type selfTy, TypeAliasDecl *TAD);

/// Retrieve the type to use as the base for a member completion.
Type getMemberBaseType() const {
return BaseType ? BaseType : ExprType;
}

public:
struct RequestedResultsTy {
const ModuleDecl *TheModule;
Expand Down
3 changes: 1 addition & 2 deletions lib/IDE/CodeCompletionResultType.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,8 +395,7 @@ static TypeRelation calculateTypeRelation(Type Ty, Type ExpectedTy,

// Equality/Conversion of GenericTypeParameterType won't account for
// requirements – ignore them
if (!Ty->hasTypeParameter() && !ExpectedTy->hasTypeParameter() &&
!Ty->hasUnboundGenericType() && !ExpectedTy->hasUnboundGenericType()) {
if (!Ty->hasTypeParameter() && !ExpectedTy->hasTypeParameter()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also get rid of type parameters at some point too, in favor of the caller mapping the type into the correct environment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup I'm leaving that for a future PR

if (Ty->isEqual(ExpectedTy))
return TypeRelation::Convertible;
bool isAny = false;
Expand Down
57 changes: 34 additions & 23 deletions lib/IDE/CompletionLookup.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,11 @@ static Type defaultTypeLiteralKind(CodeCompletionLiteralKind kind,
case CodeCompletionLiteralKind::ArrayLiteral:
if (!Ctx.getArrayDecl())
return Type();
return Ctx.getArrayDecl()->getDeclaredType();
return Ctx.getArrayDecl()->getDeclaredInterfaceType();
case CodeCompletionLiteralKind::DictionaryLiteral:
if (!Ctx.getDictionaryDecl())
return Type();
return Ctx.getDictionaryDecl()->getDeclaredType();
return Ctx.getDictionaryDecl()->getDeclaredInterfaceType();
case CodeCompletionLiteralKind::NilLiteral:
case CodeCompletionLiteralKind::ColorLiteral:
case CodeCompletionLiteralKind::ImageLiteral:
Expand Down Expand Up @@ -603,7 +603,7 @@ Type CompletionLookup::getTypeOfMember(const ValueDecl *VD,
DynamicLookupInfo dynamicLookupInfo) {
switch (dynamicLookupInfo.getKind()) {
case DynamicLookupInfo::None:
return getTypeOfMember(VD, this->ExprType);
return getTypeOfMember(VD, getMemberBaseType());
case DynamicLookupInfo::AnyObject:
return getTypeOfMember(VD, Type());
case DynamicLookupInfo::KeyPathDynamicMember: {
Expand Down Expand Up @@ -676,7 +676,14 @@ Type CompletionLookup::getTypeOfMember(const ValueDecl *VD,
}

Type CompletionLookup::getTypeOfMember(const ValueDecl *VD, Type ExprType) {
Type T = VD->getInterfaceType();
Type T;
if (auto *TD = dyn_cast<TypeDecl>(VD)) {
// For a type decl we're interested in the declared interface type, i.e
// we don't want a metatype.
T = TD->getDeclaredInterfaceType();
} else {
T = VD->getInterfaceType();
}
assert(!T.isNull());

if (ExprType) {
Expand Down Expand Up @@ -763,9 +770,7 @@ Type CompletionLookup::getTypeOfMember(const ValueDecl *VD, Type ExprType) {
}

Type CompletionLookup::getAssociatedTypeType(const AssociatedTypeDecl *ATD) {
Type BaseTy = BaseType;
if (!BaseTy)
BaseTy = ExprType;
Type BaseTy = getMemberBaseType();
if (!BaseTy && CurrDeclContext)
BaseTy =
CurrDeclContext->getInnermostTypeContext()->getDeclaredTypeInContext();
Expand Down Expand Up @@ -1709,13 +1714,16 @@ void CompletionLookup::addNominalTypeRef(const NominalTypeDecl *NTD,
addLeadingDot(Builder);
Builder.addBaseName(NTD->getName().str());

// Substitute the base type for a nested type if needed.
auto nominalTy = getTypeOfMember(NTD, dynamicLookupInfo);

// "Fake" annotation for custom attribute types.
SmallVector<char, 0> stash;
StringRef customAttributeAnnotation = getTypeAnnotationString(NTD, stash);
if (!customAttributeAnnotation.empty()) {
Builder.addTypeAnnotation(customAttributeAnnotation);
} else {
addTypeAnnotation(Builder, NTD->getDeclaredType());
addTypeAnnotation(Builder, nominalTy);
}

// Override the type relation for NominalTypes. Use the better relation
Expand All @@ -1725,8 +1733,7 @@ void CompletionLookup::addNominalTypeRef(const NominalTypeDecl *NTD,
// func receiveMetatype(_: Int.Type) {}
//
// We want to suggest 'Int' as 'Identical' for both arguments.
Builder.setResultTypes(
{NTD->getInterfaceType(), NTD->getDeclaredInterfaceType()});
Builder.setResultTypes({MetatypeType::get(nominalTy), nominalTy});
Builder.setTypeContext(expectedTypeContext, CurrDeclContext);
}

Expand All @@ -1739,17 +1746,18 @@ void CompletionLookup::addTypeAliasRef(const TypeAliasDecl *TAD,
Builder.setAssociatedDecl(TAD);
addLeadingDot(Builder);
Builder.addBaseName(TAD->getName().str());
if (auto underlyingType = TAD->getUnderlyingType()) {
if (underlyingType->hasError()) {
addTypeAnnotation(Builder,
TAD->isGeneric()
? TAD->getUnboundGenericType()
: TAD->getDeclaredInterfaceType());

} else {
addTypeAnnotation(Builder, underlyingType);
}
// Substitute the base type for a nested typealias if needed.
auto ty = getTypeOfMember(TAD, dynamicLookupInfo);

// If the underlying type has an error, prefer to print the full typealias,
// otherwise get the underlying type.
if (auto *TA = dyn_cast<TypeAliasType>(ty.getPointer())) {
auto underlyingTy = TA->getSinglyDesugaredType();
if (!underlyingTy->hasError())
ty = underlyingTy;
}
addTypeAnnotation(Builder, ty);
}

void CompletionLookup::addGenericTypeParamRef(
Expand Down Expand Up @@ -3010,11 +3018,14 @@ void CompletionLookup::getGenericRequirementCompletions(
/*includeDerivedRequirements*/ false,
/*includeProtocolExtensionMembers*/ true);
// We not only allow referencing nested types/typealiases directly, but also
// qualified by the current type. Thus also suggest current self type so the
// qualified by the current type, as long as it's a top-level type (nested
// types need to be qualified). Thus also suggest current self type so the
// user can do a memberwise lookup on it.
if (auto SelfType = typeContext->getSelfNominalTypeDecl()) {
addNominalTypeRef(SelfType, DeclVisibilityKind::LocalDecl,
DynamicLookupInfo());
if (auto *NTD = typeContext->getSelfNominalTypeDecl()) {
if (!NTD->getDeclContext()->isTypeContext()) {
addNominalTypeRef(NTD, DeclVisibilityKind::LocalDecl,
DynamicLookupInfo());
}
}

// Self is also valid in all cases in which it can be used in function
Expand Down
4 changes: 3 additions & 1 deletion lib/IDE/TypeCheckCompletionCallback.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,10 @@ Type swift::ide::getTypeForCompletion(const constraints::Solution &S,
// Use the contextual type, unless it is still unresolved, in which case fall
// back to getting the type from the expression.
if (auto ContextualType = S.getContextualType(Node)) {
if (!ContextualType->hasUnresolvedType())
if (!ContextualType->hasUnresolvedType() &&
!ContextualType->hasUnboundGenericType()) {
return ContextualType;
}
}

if (!S.hasType(Node)) {
Expand Down
4 changes: 2 additions & 2 deletions test/IDE/complete_annotation.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ func testGlobal() {
// GLOBAL_EXPR-DAG: Literal[Boolean]/None: <name>false</name>; typename=<typeid.sys>Bool</typeid.sys>;
// GLOBAL_EXPR-DAG: Literal[Nil]/None: <name>nil</name>; typename=;
// GLOBAL_EXPR-DAG: Literal[String]/None: &quot;<callarg><callarg.param>abc</callarg.param></callarg>&quot;; typename=<typeid.sys>String</typeid.sys>;
// GLOBAL_EXPR-DAG: Literal[Array]/None: [<callarg><callarg.param>values</callarg.param></callarg>]; typename=<typeid.sys>Array</typeid.sys>;
// GLOBAL_EXPR-DAG: Literal[Dictionary]/None: [<callarg><callarg.param>key</callarg.param></callarg>: <callarg><callarg.param>value</callarg.param></callarg>]; typename=<typeid.sys>Dictionary</typeid.sys>;
// GLOBAL_EXPR-DAG: Literal[Array]/None: [<callarg><callarg.param>values</callarg.param></callarg>]; typename=<typeid.sys>Array</typeid.sys>&lt;<typeid.sys>Element</typeid.sys>&gt;;
// GLOBAL_EXPR-DAG: Literal[Dictionary]/None: [<callarg><callarg.param>key</callarg.param></callarg>: <callarg><callarg.param>value</callarg.param></callarg>]; typename=<typeid.sys>Dictionary</typeid.sys>&lt;<typeid.sys>Key</typeid.sys>, <typeid.sys>Value</typeid.sys>&gt;;
// GLOBAL_EXPR-DAG: Literal[_Color]/None: <name>#colorLiteral</name>(<callarg><callarg.label>red</callarg.label>: <callarg.type><typeid.sys>Float</typeid.sys></callarg.type></callarg>, <callarg><callarg.label>green</callarg.label>: <callarg.type><typeid.sys>Float</typeid.sys></callarg.type></callarg>, <callarg><callarg.label>blue</callarg.label>: <callarg.type><typeid.sys>Float</typeid.sys></callarg.type></callarg>, <callarg><callarg.label>alpha</callarg.label>: <callarg.type><typeid.sys>Float</typeid.sys></callarg.type></callarg>); typename=;
// GLOBAL_EXPR-DAG: Literal[_Image]/None: <name>#imageLiteral</name>(<callarg><callarg.label>resourceName</callarg.label>: <callarg.type><typeid.sys>String</typeid.sys></callarg.type></callarg>); typename=;
// GLOBAL_EXPR-DAG: Literal[Tuple]/None: (<callarg><callarg.param>values</callarg.param></callarg>); typename=;
Expand Down
8 changes: 4 additions & 4 deletions test/IDE/complete_at_top_level_library.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ protocol MyProtocol {}
// LIBRARY-DAG: Literal[Boolean]/None/Flair[ExprAtFileScope]: false[#Bool#]; name=false
// LIBRARY-DAG: Literal[Nil]/None/Flair[ExprAtFileScope]: nil; name=nil
// LIBRARY-DAG: Literal[String]/None/Flair[ExprAtFileScope]: "{#(abc)#}"[#String#]; name=""
// LIBRARY-DAG: Literal[Array]/None/Flair[ExprAtFileScope]: [{#(values)#}][#Array#]; name=[]
// LIBRARY-DAG: Literal[Dictionary]/None/Flair[ExprAtFileScope]: [{#(key)#}: {#(value)#}][#Dictionary#]; name=[: ]
// LIBRARY-DAG: Literal[Array]/None/Flair[ExprAtFileScope]: [{#(values)#}][#Array<Element>#]; name=[]
// LIBRARY-DAG: Literal[Dictionary]/None/Flair[ExprAtFileScope]: [{#(key)#}: {#(value)#}][#Dictionary<Key, Value>#]; name=[: ]
// LIBRARY-DAG: Literal[Tuple]/None/Flair[ExprAtFileScope]: ({#(values)#}); name=()
// LIBRARY-DAG: Decl[Struct]/CurrModule/Flair[ExprAtFileScope]: MyStruct[#MyStruct#]; name=MyStruct
// LIBRARY-DAG: Decl[Protocol]/CurrModule/Flair[RareType,ExprAtFileScope]: MyProtocol[#MyProtocol#]; name=MyProtocol
Expand Down Expand Up @@ -136,8 +136,8 @@ protocol MyProtocol {}
// SCRIPT-DAG: Literal[Boolean]/None: false[#Bool#]; name=false
// SCRIPT-DAG: Literal[Nil]/None: nil; name=nil
// SCRIPT-DAG: Literal[String]/None: "{#(abc)#}"[#String#]; name=""
// SCRIPT-DAG: Literal[Array]/None: [{#(values)#}][#Array#]; name=[]
// SCRIPT-DAG: Literal[Dictionary]/None: [{#(key)#}: {#(value)#}][#Dictionary#]; name=[: ]
// SCRIPT-DAG: Literal[Array]/None: [{#(values)#}][#Array<Element>#]; name=[]
// SCRIPT-DAG: Literal[Dictionary]/None: [{#(key)#}: {#(value)#}][#Dictionary<Key, Value>#]; name=[: ]
// SCRIPT-DAG: Literal[Tuple]/None: ({#(values)#}); name=()
// SCRIPT-DAG: Decl[Struct]/CurrModule: MyStruct[#MyStruct#]; name=MyStruct
// SCRIPT-DAG: Decl[Protocol]/CurrModule/Flair[RareType]: MyProtocol[#MyProtocol#]; name=MyProtocol
Expand Down
2 changes: 1 addition & 1 deletion test/IDE/complete_constrained.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func foo(s: MyStruct<Int>) {
// META_MYSTRUCT_INT_DOT: Begin completions, 11 items
// META_MYSTRUCT_INT_DOT-DAG: Keyword[self]/CurrNominal: self[#MyStruct<Int>.Type#]; name=self
// META_MYSTRUCT_INT_DOT-DAG: Keyword/CurrNominal: Type[#MyStruct<Int>.Type#]; name=Type
// META_MYSTRUCT_INT_DOT-DAG: Decl[TypeAlias]/CurrNominal: Assoc[#T#]; name=Assoc
// META_MYSTRUCT_INT_DOT-DAG: Decl[TypeAlias]/CurrNominal: Assoc[#Int#]; name=Assoc
// META_MYSTRUCT_INT_DOT-DAG: Decl[Constructor]/CurrNominal: init({#int: U#})[#MyStruct<Int>#]; name=init(int:)
// META_MYSTRUCT_INT_DOT-DAG: Decl[Constructor]/CurrNominal: init({#withConstrainedGenericParam: SomeProto#})[#MyStruct<Int>#]; name=init(withConstrainedGenericParam:)
// META_MYSTRUCT_INT_DOT-DAG: Decl[InstanceMethod]/CurrNominal: methodWithConstrainedGenericParam({#(self): MyStruct<Int>#})[#(x: SomeProto) -> Int#]; name=methodWithConstrainedGenericParam(:)
Expand Down
6 changes: 3 additions & 3 deletions test/IDE/complete_constructor.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,8 +311,8 @@ class DependentTypeInClosure<Data: DataType> {
func testDependentTypeInClosure() {
let _: DependentTypeInClosure = .#^DEPENDENT_IN_CLOSURE_3^#
// DEPENDENT_IN_CLOSURE_3: Begin completions, 2 items
// DEPENDENT_IN_CLOSURE_3-DAG: Decl[Constructor]/CurrNominal: init({#(arg): DataType#}, {#fn: (DataType.Content) -> Void##(DataType.Content) -> Void#})[#DependentTypeInClosure<DataType>#];
// DEPENDENT_IN_CLOSURE_3-DAG: Decl[Constructor]/CurrNominal: init({#arg: DataType#}, {#fn: () -> DataType.Content##() -> DataType.Content#})[#DependentTypeInClosure<DataType>#];
// DEPENDENT_IN_CLOSURE_3-DAG: Decl[Constructor]/CurrNominal/TypeRelation[Convertible]: init({#(arg): DataType#}, {#fn: (Data.Content) -> Void##(Data.Content) -> Void#})[#DependentTypeInClosure<DataType>#];
// DEPENDENT_IN_CLOSURE_3-DAG: Decl[Constructor]/CurrNominal/TypeRelation[Convertible]: init({#arg: DataType#}, {#fn: () -> Data.Content##() -> Data.Content#})[#DependentTypeInClosure<DataType>#];

let _ = DependentTypeInClosure(#^DEPENDENT_IN_CLOSURE_1^#)
// DEPENDENT_IN_CLOSURE_1-DAG: Decl[Constructor]/CurrNominal/Flair[ArgLabels]: ['(']{#(arg): DataType#}, {#fn: (_) -> Void##(_) -> Void#}[')'][#DependentTypeInClosure<DataType>#];
Expand Down Expand Up @@ -348,6 +348,6 @@ func testIgnoreGenericArgsAfterCompletionToken() {
func deserializeRecord() throws -> HostRecord<IPv4> {
var position = 42
return try #^IGNORE_GENERIC_ARGS_AFTER_COMPLETION_TOKEN^#HostRecord<IPv4>(position: &position)
// IGNORE_GENERIC_ARGS_AFTER_COMPLETION_TOKEN-DAG: Decl[Struct]/Local: HostRecord[#HostRecord#];
// IGNORE_GENERIC_ARGS_AFTER_COMPLETION_TOKEN-DAG: Decl[Struct]/Local: HostRecord[#HostRecord<IPType>#];
}
}
4 changes: 2 additions & 2 deletions test/IDE/complete_embedded_linux.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ func test() {
// GLOBAL: Literal[Boolean]/None: false[#Bool#];
// GLOBAL: Literal[Nil]/None: nil;
// GLOBAL: Literal[String]/None: "{#(abc)#}"[#String#];
// GLOBAL: Literal[Array]/None: [{#(values)#}][#Array#];
// GLOBAL: Literal[Dictionary]/None: [{#(key)#}: {#(value)#}][#Dictionary#];
// GLOBAL: Literal[Array]/None: [{#(values)#}][#Array<Element>#];
// GLOBAL: Literal[Dictionary]/None: [{#(key)#}: {#(value)#}][#Dictionary<Key, Value>#];
}
4 changes: 2 additions & 2 deletions test/IDE/complete_embedded_macos.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ func test() {
// GLOBAL: Literal[Boolean]/None: false[#Bool#];
// GLOBAL: Literal[Nil]/None: nil;
// GLOBAL: Literal[String]/None: "{#(abc)#}"[#String#];
// GLOBAL: Literal[Array]/None: [{#(values)#}][#Array#];
// GLOBAL: Literal[Dictionary]/None: [{#(key)#}: {#(value)#}][#Dictionary#];
// GLOBAL: Literal[Array]/None: [{#(values)#}][#Array<Element>#];
// GLOBAL: Literal[Dictionary]/None: [{#(key)#}: {#(value)#}][#Dictionary<Key, Value>#];
}
2 changes: 1 addition & 1 deletion test/IDE/complete_from_stdlib.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

#^PLAIN_TOP_LEVEL_1?check=PLAIN_TOP_LEVEL;check=NO_STDLIB_PRIVATE^#

// PLAIN_TOP_LEVEL-DAG: Decl[Struct]/OtherModule[Swift]/IsSystem: Array[#Array#]{{; name=.+$}}
// PLAIN_TOP_LEVEL-DAG: Decl[Struct]/OtherModule[Swift]/IsSystem: Array[#Array<Element>#]{{; name=.+$}}

func privateNominalMembers(_ a: String) {
a.#^PRIVATE_NOMINAL_MEMBERS_1?check=PRIVATE_NOMINAL_MEMBERS_1;check=NO_STDLIB_PRIVATE^#
Expand Down
6 changes: 3 additions & 3 deletions test/IDE/complete_from_swift_module.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,15 +134,15 @@ struct Foo: Swift.Array.#^STDLIB_TYPE_QUALIFIED_NESTED^# {}

struct Bar: Swift.#^STDLIB_TYPE_QUALIFIED^# {}
// STDLIB_TYPE_QUALIFIED-NOT: Decl[Module]
// STDLIB_TYPE_QUALIFIED: Decl[Struct]/OtherModule[Swift]/IsSystem: AnyCollection[#AnyCollection#]; name=AnyCollection
// STDLIB_TYPE_QUALIFIED: Decl[Struct]/OtherModule[Swift]/IsSystem: AnyCollection[#AnyCollection<Element>#]; name=AnyCollection
// STDLIB_TYPE_QUALIFIED-NOT: Decl[Module]

func foo() -> foo_swift_module.#^MODULE_TYPE_QUALIFIED^# {}
// MODULE_TYPE_QUALIFIED: Decl[Protocol]/OtherModule[foo_swift_module]: BarProtocol[#BarProtocol#]; name=BarProtocol
// MODULE_TYPE_QUALIFIED: Decl[Enum]/OtherModule[foo_swift_module]: MyQuickLookObject[#MyQuickLookObject#]; name=MyQuickLookObject
// MODULE_TYPE_QUALIFIED: Decl[Struct]/OtherModule[foo_swift_module]: BarGenericSwiftStruct1[#BarGenericSwiftStruct1#]; name=BarGenericSwiftStruct1
// MODULE_TYPE_QUALIFIED: Decl[Struct]/OtherModule[foo_swift_module]: BarGenericSwiftStruct1[#BarGenericSwiftStruct1<T>#]; name=BarGenericSwiftStruct1
// MODULE_TYPE_QUALIFIED: Decl[Struct]/OtherModule[foo_swift_module]: FooSwiftStruct[#FooSwiftStruct#]; name=FooSwiftStruct
// MODULE_TYPE_QUALIFIED: Decl[Struct]/OtherModule[foo_swift_module]: BarGenericSwiftStruct2[#BarGenericSwiftStruct2#]; name=BarGenericSwiftStruct2
// MODULE_TYPE_QUALIFIED: Decl[Struct]/OtherModule[foo_swift_module]: BarGenericSwiftStruct2[#BarGenericSwiftStruct2<T, U>#]; name=BarGenericSwiftStruct2

// rdar://92048610
func testAmbiguousResultBuilder() {
Expand Down
Loading