[CodeCompletion] Missing init completions for dotExpr

[AnthonyLatsis]

Fixes non-visible inherited convenience initializers
Fixes non-visible initializers from protocol extensions
Fixes visible initializers on dynamic metatypes for postfix and parenPostfix expressions

Resolves SR-2206, SR-7256, SR-7789, the last being a bug found while fixing the other 2.

Note

The following logic causes global completion results (all of the literals, stdlib decls etc.) to show when there aren't any function completions found for ParenPostfix. This includes cases when nothing is found, some of which are directly related to SR-7789 – before, they showed initializers, now, being fixed, they show nothing (empty completion results) and thus fall under the case when global lookup is done and shown.
https://github.com/apple/swift/blob/ba9cc174e457704c1d9e959a0bac3cb28b9fd9d8/lib/IDE/CodeCompletion.cpp#L5404-L5407

Example:

class A { init(foo: Int) {}}
func test() {
  var B = A.self
  B(#^HERE^# // will show global completion results 
}

IMO this is incorrect and there shouldn't be any completions at all if the sink is empty to begin with, but nevertheless, is this is supposed to be correct?

UDP

SR-7789 is fixed, however. I think this should be moved to a separate bug report if needed. I recently found out it's present in some cases which aren't related to SR-7789. In particular, parenPostfix initializer completions for some generic types.

[AnthonyLatsis]

/cc @benlangmuir

[benlangmuir]

Thanks for working on this!

class A { init(foo: Int) {}}
func test() {
  var B = A.self
  B(#^HERE^# // will show global completion results 
}

IMO this is incorrect and there shouldn't be any completions at all if the sink is empty to begin with, but nevertheless, is this is supposed to be correct?

If something fails - e.g. cannot typecheck B - then we should assume conservatively that expressions are valid. Also while B(1) is rejected, B\n(1) is not, so it's not a bad idea to be lenient here in general.

[benlangmuir]

We shouldn't be using ExprType directly here, since we don't know its relation to this call to addConstructorCall - see addConstructorCallsFromType for example where we don't know what ExprType will be. It looks like the intention was that this code either uses BaseType (if it is specified) or ExprType - e.g. inside of getTypeOfMember. That's pretty hard to understand, and in practice it looks like BaseType won't always be a metatype even though it sets IsOnMetatype=true.

How do you feel about the following?

• Make BaseType required when IsOnMetatype=true and pass in ExprType when we call this method.
• Either accept both metatypes and non-metatypes for BaseType and handle that explicitly (e.g. if it's a metatype, get the instance type otherwise use the type directly), or require callers to always provide one or the other and assert that (e.g. always pass a metatype or always pass a non-metatype).

[AnthonyLatsis]

To be honest I don't like the name isOnMetatype at all. We don't call initializers on metatypes to begin with, right? I renamed it to isOnType, made some clean-up and changed the code not to have a need to use ExprType here. I decided not to touch anything else yet, since getTypeOfMember, which is the only place where baseType is used, is incorrectly used as well (see SR-7802). After fixing that, we might not need baseType anymore.

[benlangmuir]

This condition needs a comment explaining what's going on. For example, why are we checking IsSelfRefExpr or is<ArchetypeType>?

[AnthonyLatsis]

I agree. In fact I got it wrong and I might have avoided mistakes with a comment. I will add failing tests to make sure the correct expression doesn't break without consequences.

[AnthonyLatsis]

Roughly, I've fixed all of the following cases concerning SR-7789 and cleaned up the code.

class Foo {
  init() {}
  init(a : Int) {}
  init(foo: String) {
    self.#^HERE^# // shows initializers
  }
  class func foo() {
    self.#^HERE^# // doesn't show initializers
  }
}
class Derived: Foo {
  class func foo() {
    super.#^HERE^# // doesn't show initializers
  }
  convenience init() {
    super.#^HERE^# // shows initializers
  }
}

func foo() {
  var MT= Foo.self
  MT.#^HERE^# // no initializers (OK)
  MT(#^HERE^# // shows initializers 
  MT#^HERE^#  // shows initializers
}
protocol P {
  init(bar: Int)
}
func test<S: P>(_ arg: S) {
  S.#^HERE^# // doesn't show initializers
}

[AnthonyLatsis]

But there's a problem: I can't make the

  class func foo() {
    super.#^HERE^# // shows super init completions
  }

test to pass in complete_after_super.swift, so I haven't included it. At the same time, super#^HERE^# passes fine.
Of course, all of them work as expected when I build and run swift-ide-test. I suspect the reason is the language version used to run the tests in this file, but I do not know how to find it out. I've tried using different versions with -swift-version on swift-ide-test, but there is no difference.
At this point I though you might have an idea of what's going on.

[benlangmuir]

Of course, all of them work as expected when I build and run swift-ide-test. I suspect the reason is the language version used to run the tests in this file, but I do not know how to find it out. I've tried using different versions with -swift-version on swift-ide-test, but there is no difference.
At this point I though you might have an idea of what's going on.

When the test fails it should print out the exact command line that it runs for every RUN line, so you should be able to use that to reproduce.

[AnthonyLatsis]

Great...It didn't work because the two checks were together and the first one apparently prevents the following one from typechecking.
This breaks CLASS_FUNC_SUPER_DOT

class func test3() {
  super#^CLASS_FUNC_SUPER_NODOT^#
  super.#^CLASS_FUNC_SUPER_DOT^#
}

This breaks CLASS_FUNC_SUPER_NODOT

class func test3() {
  super.#^CLASS_FUNC_SUPER_DOT^#
  super#^CLASS_FUNC_SUPER_NODOT^#
}

[AnthonyLatsis]

Just in case, ready to proceed :)

[benlangmuir]

This brace is now unmatched after your changes (it was originally matching the one in test3() {).

[benlangmuir]

@swift-ci please smoke test

[AnthonyLatsis]

Ready to ship?

[benlangmuir]

Yup, thanks for the reminder!