Skip to content

[CodeCompletion] Improve accuracy of unresolved member completion #19584

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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
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
72 changes: 59 additions & 13 deletions lib/IDE/CodeCompletion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2617,7 +2617,7 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {

void addConstructorCallsForType(Type type, Identifier name,
DeclVisibilityKind Reason) {
if (!Ctx.LangOpts.CodeCompleteInitsInPostfixExpr)
if (!Ctx.LangOpts.CodeCompleteInitsInPostfixExpr && !IsUnresolvedMember)
return;

assert(CurrDeclContext);
Expand Down Expand Up @@ -2677,6 +2677,8 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {

void addNominalTypeRef(const NominalTypeDecl *NTD,
DeclVisibilityKind Reason) {
if (IsUnresolvedMember)
return;
CommandWordsPairs Pairs;
CodeCompletionResultBuilder Builder(
Sink,
Expand All @@ -2690,6 +2692,8 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
}

void addTypeAliasRef(const TypeAliasDecl *TAD, DeclVisibilityKind Reason) {
if (IsUnresolvedMember)
return;
CommandWordsPairs Pairs;
CodeCompletionResultBuilder Builder(
Sink,
Expand Down Expand Up @@ -2908,7 +2912,7 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
// initializer completions if we do not have a left parenthesis and either
// the initializer is required, the base type's instance type is not a class,
// or this is a 'self' or 'super' reference.
if (IsStaticMetatype || Ty->is<ArchetypeType>())
if (IsStaticMetatype || IsUnresolvedMember || Ty->is<ArchetypeType>())
addConstructorCall(CD, Reason, None, Result, /*isOnType*/true);
else if ((IsSelfRefExpr || IsSuperRefExpr || !Ty->is<ClassType>() ||
CD->isRequired()) && !HaveLParen)
Expand Down Expand Up @@ -3759,17 +3763,59 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
// We can only say .foo where foo is a static member of the contextual
// type and has the same type (or if the member is a function, then the
// same result type) as the contextual type.
FilteredDeclConsumer consumer(*this, [=](ValueDecl *VD, DeclVisibilityKind reason) {
FilteredDeclConsumer consumer(*this, [=](ValueDecl *VD,
DeclVisibilityKind reason) {
if (!VD->hasInterfaceType()) {
TypeResolver->resolveDeclSignature(VD);
if (!VD->hasInterfaceType())
return false;
}

auto declTy = VD->getInterfaceType();
while (auto FT = declTy->getAs<AnyFunctionType>())
// Enum element decls can always be referenced by implicit member
// expression.
if (isa<EnumElementDecl>(VD))
return true;

// Only non-failable constructors are implicitly referenceable.
if (auto CD = dyn_cast<ConstructorDecl>(VD)) {
switch (CD->getFailability()) {
case OTK_None:
case OTK_ImplicitlyUnwrappedOptional:
return true;
case OTK_Optional:
return false;
}
}

// Otherwise, check the result type matches the contextual type.
auto declTy = getTypeOfMember(VD, T);
if (declTy->hasError())
return false;

DeclContext *DC = const_cast<DeclContext *>(CurrDeclContext);

// Member types can also be implicitly referenceable as long as it's
// convertible to the contextual type.
if (auto CD = dyn_cast<TypeDecl>(VD)) {
declTy = declTy->getMetatypeInstanceType();
return swift::isConvertibleTo(declTy, T, *DC);
}

// Only static member can be referenced.
if (!VD->isStatic())
return false;

if (isa<FuncDecl>(VD)) {
// Strip '(Self.Type) ->' and parameters.
declTy = declTy->castTo<AnyFunctionType>()->getResult();
declTy = declTy->castTo<AnyFunctionType>()->getResult();
} else if (auto FT = declTy->getAs<AnyFunctionType>()) {
// The compiler accepts 'static var factory: () -> T' for implicit
// member expression.
// FIXME: This emits just 'factory'. We should emit 'factory()' instead.
declTy = FT->getResult();
return declTy->isEqual(T);
}
return swift::isConvertibleTo(declTy, T, *DC);
});

auto baseType = MetatypeType::get(T);
Expand All @@ -3784,14 +3830,14 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
void getUnresolvedMemberCompletions(ArrayRef<Type> Types) {
NeedLeadingDot = !HaveDot;
for (auto T : Types) {
if (T) {
// FIXME: we should also include .some/.none from optional itself but
// getUnresolvedMemberCompletions doesn't ever return them since the
// interface type in the FilteredDeclConsumer will not match the bound
// generic type expected.
T = T->lookThroughAllOptionalTypes();
getUnresolvedMemberCompletions(T);
if (!T)
continue;
if (auto objT = T->getOptionalObjectType()) {
// If this is optional type, perform completion for the object type.
// i.e. 'let _: Enum??? = .enumMember' is legal.
getUnresolvedMemberCompletions(objT->lookThroughAllOptionalTypes());
}
getUnresolvedMemberCompletions(T);
}
}

Expand Down
101 changes: 98 additions & 3 deletions test/IDE/complete_unresolved_members.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@

// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_8 | %FileCheck %s -check-prefix=UNRESOLVED_3
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_9 | %FileCheck %s -check-prefix=UNRESOLVED_3
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_OPT_1 | %FileCheck %s -check-prefix=UNRESOLVED_3
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_OPT_2 | %FileCheck %s -check-prefix=UNRESOLVED_3
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_OPT_1 | %FileCheck %s -check-prefix=UNRESOLVED_3_OPT
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_OPT_2 | %FileCheck %s -check-prefix=UNRESOLVED_3_OPT
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_OPT_3 | %FileCheck %s -check-prefix=UNRESOLVED_3_OPTOPTOPT

// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_12 | %FileCheck %s -check-prefix=UNRESOLVED_3
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_13 | %FileCheck %s -check-prefix=UNRESOLVED_3
Expand Down Expand Up @@ -54,6 +55,16 @@

// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=STRING_INTERPOLATION_1 | %FileCheck %s -check-prefix=STRING_INTERPOLATION_1
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=STRING_INTERPOLATION_INVALID

// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=SUBTYPE_1 | %FileCheck %s -check-prefix=SUBTYPE_1
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=SUBTYPE_2 | %FileCheck %s -check-prefix=SUBTYPE_2

// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=GENERIC_1 | %FileCheck %s -check-prefix=GENERIC_1 -check-prefix=GENERIC_1_INT
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=GENERIC_2 | %FileCheck %s -check-prefix=GENERIC_1 -check-prefix=GENERIC_1_INT
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=GENERIC_3 | %FileCheck %s -check-prefix=GENERIC_1 -check-prefix=GENERIC_1_U

// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=STATIC_CLOSURE_1 | %FileCheck %s -check-prefix=STATIC_CLOSURE_1

enum SomeEnum1 {
case South
case North
Expand Down Expand Up @@ -188,12 +199,33 @@ class C4 {
func f5() {
optionalEnumTaker1(.#^UNRESOLVED_OPT_2^#)
}
func f6() {
var _: SomeEnum1??? = .#^UNRESOLVED_OPT_3^#
}
}
// UNRESOLVED_3: Begin completions
// UNRESOLVED_3-DAG: Decl[EnumElement]/ExprSpecific: North[#SomeEnum1#]; name=North
// UNRESOLVED_3-DAG: Decl[EnumElement]/ExprSpecific: South[#SomeEnum1#]; name=South
// UNRESOLVED_3-NOT: SomeOptions1
// UNRESOLVED_3-NOT: SomeOptions2
// UNRESOLVED_3-NOT: none
// UNRESOLVED_3-NOT: some(

// UNRESOLVED_3_OPT: Begin completions
// UNRESOLVED_3_OPT-DAG: Decl[EnumElement]/ExprSpecific: North[#SomeEnum1#];
// UNRESOLVED_3_OPT-DAG: Decl[EnumElement]/ExprSpecific: South[#SomeEnum1#];
// UNRESOLVED_3_OPT-DAG: Decl[EnumElement]/ExprSpecific: none[#Optional<Wrapped>#]; name=none
// UNRESOLVED_3_OPT-DAG: Decl[EnumElement]/ExprSpecific: some({#Wrapped#})[#(Wrapped) -> Optional<Wrapped>#];
// UNRESOLVED_3_OPT-DAG: Decl[Constructor]/CurrNominal: init({#(some): SomeEnum1#})[#Optional<SomeEnum1>#];
// UNRESOLVED_3_OPT-DAG: Decl[Constructor]/CurrNominal: init({#nilLiteral: ()#})[#Optional<SomeEnum1>#];

// UNRESOLVED_3_OPTOPTOPT: Begin completions
// UNRESOLVED_3_OPTOPTOPT-DAG: Decl[EnumElement]/ExprSpecific: North[#SomeEnum1#];
// UNRESOLVED_3_OPTOPTOPT-DAG: Decl[EnumElement]/ExprSpecific: South[#SomeEnum1#];
// UNRESOLVED_3_OPTOPTOPT-DAG: Decl[EnumElement]/ExprSpecific: none[#Optional<Wrapped>#]; name=none
// UNRESOLVED_3_OPTOPTOPT-DAG: Decl[EnumElement]/ExprSpecific: some({#Wrapped#})[#(Wrapped) -> Optional<Wrapped>#];
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we know why the generic Wrapped isn't resolved here?

Copy link
Member Author

@rintaro rintaro Sep 29, 2018

Choose a reason for hiding this comment

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

Yeah, that is what I'm wondering too. I'll look into it in followups.

// UNRESOLVED_3_OPTOPTOPT-DAG: Decl[Constructor]/CurrNominal: init({#(some): SomeEnum1??#})[#Optional<SomeEnum1??>#];
// UNRESOLVED_3_OPTOPTOPT-DAG: Decl[Constructor]/CurrNominal: init({#nilLiteral: ()#})[#Optional<SomeEnum1??>#];

class C5 {
func f1() {
Expand Down Expand Up @@ -287,7 +319,7 @@ func testAvail1(_ x: EnumAvail1) {
func testAvail2(_ x: OptionsAvail1) {
testAvail2(.#^OPTIONS_AVAIL_1^#)
}
// OPTIONS_AVAIL_1: Begin completions, 3 items
// OPTIONS_AVAIL_1: Begin completions
// ENUM_AVAIL_1-NOT: AAA
// OPTIONS_AVAIL_1-DAG: Decl[StaticVar]/CurrNominal/TypeRelation[Identical]: aaa[#OptionsAvail1#];
// OPTIONS_AVAIL_1-DAG: Decl[StaticVar]/CurrNominal/NotRecommended/TypeRelation[Identical]: BBB[#OptionsAvail1#];
Expand Down Expand Up @@ -387,3 +419,66 @@ func testInStringInterpolation() {
// STRING_INTERPOLATION_1-DAG: Decl[EnumElement]/ExprSpecific: foo[#MyEnum#];
// STRING_INTERPOLATION_1-DAG: Decl[EnumElement]/ExprSpecific: bar[#MyEnum#];
// STRING_INTERPOLATION_1: End completions

class BaseClass {
class SubClass : BaseClass { init() {} }
static var subInstance: SubClass = SubClass()
}
protocol MyProtocol {
typealias Concrete1 = BaseClass
typealias Concrete2 = AnotherTy
}
extension BaseClass : MyProtocol {}
struct AnotherTy: MyProtocol {}
func testSubType() {
var _: BaseClass = .#^SUBTYPE_1^#
}
// SUBTYPE_1: Begin completions, 4 items
// SUBTYPE_1-DAG: Decl[Constructor]/CurrNominal/TypeRelation[Identical]: init()[#BaseClass#];
// SUBTYPE_1-DAG: Decl[Constructor]/CurrNominal/TypeRelation[Convertible]: SubClass()[#BaseClass.SubClass#];
// SUBTYPE_1-DAG: Decl[StaticVar]/CurrNominal/TypeRelation[Convertible]: subInstance[#BaseClass.SubClass#];
// SUBTYPE_1-DAG: Decl[Constructor]/Super/TypeRelation[Identical]: Concrete1()[#BaseClass#];
// SUBTYPE_1: End completions

func testMemberTypealias() {
var _: MyProtocol = .#^SUBTYPE_2^#
}
// SUBTYPE_2: Begin completions, 2 items
// SUBTYPE_2-DAG: Decl[Constructor]/CurrNominal/TypeRelation[Convertible]: Concrete1()[#BaseClass#];
// SUBTYPE_2-DAG: Decl[Constructor]/CurrNominal/TypeRelation[Convertible]: Concrete2()[#AnotherTy#];
// SUBTYPE_2: End completions

enum Generic<T> {
case contains(content: T)
case empty
static func create(_: T) -> Generic<T> { fatalError() }
}
func takeGenericInt(_: Generic<Int>) { }
func takeGenericU<U>(_: Generic<U>) { }
func testGeneric() {
do {
let _: Generic<Int> = .#^GENERIC_1^#
}
takeGenericInt(.#^GENERIC_2^#)
takeGenericU(.#^GENERIC_3^#)
}
// GENERIC_1: Begin completions
// GENERIC_1: Decl[EnumElement]/ExprSpecific: contains({#content: T#})[#(T) -> Generic<T>#];
// GENERIC_1: Decl[EnumElement]/ExprSpecific: empty[#Generic<T>#];
// GENERIC_1_INT: Decl[StaticMethod]/CurrNominal: create({#Int#})[#Generic<Int>#];
// GENERIC_1_U: Decl[StaticMethod]/CurrNominal: create({#U#})[#Generic<U>#];
// GENERIC_1: End completions

struct HasCreator {
static var create: () -> HasCreator = { fatalError() }
static var create_curried: () -> () -> HasCreator = { fatalError() }
}
func testHasStaticClosure() {
let _: HasCreator = .#^STATIC_CLOSURE_1^#
}
// STATIC_CLOSURE_1: Begin completions, 2 items
// STATIC_CLOSURE_1-DAG: Decl[Constructor]/CurrNominal/TypeRelation[Identical]: init()[#HasCreator#];
// FIXME: Suggest 'create()[#HasCreateor#]', not 'create'.
// STATIC_CLOSURE_1-DAG: Decl[StaticVar]/CurrNominal: create[#() -> HasCreator#];
// STATIC_CLOSURE_1-NOT: create_curried
// STATIC_CLOSURE_1: End completions
27 changes: 27 additions & 0 deletions test/expr/delayed-ident/nested_type.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// RUN: %target-typecheck-verify-swift

// Nested subclass, typealias Self.
class Base {
class Derived : Base {
init(x: Int) {}
}
typealias Ident = Base
}

let _: Base = .Derived(x: 12)
let _: Base = .Ident()

// Typealias in protocol.
protocol P {
typealias Impl1 = ConcreteP
}
extension P {
typealias Impl2 = ConcreteP
}
struct ConcreteP : P {
}

let _: P = .Impl1()
let _: P = .Impl2()
let _: ConcreteP = .Impl1()
let _: ConcreteP = .Impl2()
13 changes: 13 additions & 0 deletions test/expr/delayed-ident/static_var.swift
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,16 @@ struct Foo {
func & (x: Foo, y: Foo) -> Foo { }

var fooValue: Foo = .Bar & .Wibble

// Static closure variables.
struct HasClosure {
static var factoryNormal: (Int) -> HasClosure = { _ in .init() }
static var factoryReturnOpt: (Int) -> HasClosure? = { _ in .init() }
static var factoryIUO: ((Int) -> HasClosure)! = { _ in .init() }
static var factoryOpt: ((Int) -> HasClosure)? = { _ in .init() }
}
var _: HasClosure = .factoryNormal(0)
var _: HasClosure = .factoryReturnOpt(1)!
var _: HasClosure = .factoryIUO(2)
var _: HasClosure = .factoryOpt(3) // expected-error {{static property 'factoryOpt' is not a function}}
var _: HasClosure = .factoryOpt!(4) // expected-error {{type of expression is ambiguous without more context}}