Skip to content

[CodeCompletion] Suggest the property name in its didSet clause #28319

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 1 commit into from
Dec 4, 2019
Merged

[CodeCompletion] Suggest the property name in its didSet clause #28319

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 8 additions & 0 deletions lib/IDE/CodeCompletion.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2168,6 +2168,14 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
addValueBaseName(Builder, Name);
setClangDeclKeywords(VD, Pairs, Builder);

// "not recommended" in its own getter.
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this impact keypaths? I think that would still be "okay". Technically you can also call the setter safely from the getter if it doesn't mutually recurse, but I don't care much about that case.

Copy link
Member Author

@rintaro rintaro Dec 4, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

It did. I updated the PR to ensure it doesn't mark it "NotRecommended" in case of keypaths or explicit self.
I want to mark it because the compiler emit warnings for them:

class MyClass {
  var value: Int {
    get {
      value = 12 // warning: attempting to access 'value' within its own getter
                 // note: access 'self' explicitly to silence this warning
      return 1
    }
    set {
    }
  }
}

Copy link
Contributor

Choose a reason for hiding this comment

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

SGTM, thanks!

if (Kind == LookupKind::ValueInDeclContext) {
if (auto accessor = dyn_cast<AccessorDecl>(CurrDeclContext)) {
if (accessor->getStorage() == VD && accessor->isGetter())
Builder.setNotRecommended(CodeCompletionResult::NoReason);
}
}

if (!VD->hasInterfaceType())
return;

Expand Down
8 changes: 4 additions & 4 deletions lib/Sema/LookupVisibleDecls.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1208,11 +1208,11 @@ void swift::lookupVisibleDecls(VisibleDeclConsumer &Consumer,
bool isUsableValue(ValueDecl *VD, DeclVisibilityKind Reason) {

// Check "use within its own initial value" case.
if (auto *varD = dyn_cast<VarDecl>(VD))
if (auto *PBD = varD->getParentPatternBinding())
if (!PBD->isImplicit() &&
SM.rangeContainsTokenLoc(PBD->getSourceRange(), Loc))
if (auto *varD = dyn_cast<VarDecl>(VD)) {
if (auto *initExpr = varD->getParentInitializer())
if (SM.rangeContainsTokenLoc(initExpr->getSourceRange(), Loc))
return false;
}

switch (Reason) {
case DeclVisibilityKind::LocalVariable:
Expand Down
2 changes: 1 addition & 1 deletion test/IDE/complete_crashes.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ while true {
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=GENERIC_PARAM_AND_ASSOC_TYPE | %FileCheck %s -check-prefix=GENERIC_PARAM_AND_ASSOC_TYPE
struct CustomGenericCollection<Key> : ExpressibleByDictionaryLiteral {
// GENERIC_PARAM_AND_ASSOC_TYPE: Begin completions
// GENERIC_PARAM_AND_ASSOC_TYPE-DAG: Decl[InstanceVar]/CurrNominal/TypeRelation[Identical]: count[#Int#]; name=count
// GENERIC_PARAM_AND_ASSOC_TYPE-DAG: Decl[InstanceVar]/CurrNominal/NotRecommended/TypeRelation[Identical]: count[#Int#]; name=count
// GENERIC_PARAM_AND_ASSOC_TYPE-DAG: Decl[GenericTypeParam]/Local: Key[#Key#]; name=Key
// GENERIC_PARAM_AND_ASSOC_TYPE-DAG: Decl[TypeAlias]/CurrNominal: Value[#CustomGenericCollection<Key>.Value#]; name=Value
// GENERIC_PARAM_AND_ASSOC_TYPE: End completions
Expand Down
79 changes: 79 additions & 0 deletions test/IDE/complete_expr_postfix_begin.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,23 @@
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_INIT_6 | %FileCheck %s -check-prefix=OWN_INIT_6
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_INIT_7 | %FileCheck %s -check-prefix=OWN_INIT_7

// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_ACCESSOR_1 | %FileCheck %s -check-prefix=OWN_ACCESSOR_1
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_ACCESSOR_2 | %FileCheck %s -check-prefix=OWN_ACCESSOR_2
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_ACCESSOR_3 | %FileCheck %s -check-prefix=OWN_ACCESSOR_3
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_ACCESSOR_4 | %FileCheck %s -check-prefix=OWN_ACCESSOR_3
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_ACCESSOR_5 | %FileCheck %s -check-prefix=OWN_ACCESSOR_5
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_ACCESSOR_6 | %FileCheck %s -check-prefix=OWN_ACCESSOR_6
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_ACCESSOR_7 | %FileCheck %s -check-prefix=OWN_ACCESSOR_7
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_ACCESSOR_8 | %FileCheck %s -check-prefix=OWN_ACCESSOR_7
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_ACCESSOR_9 | %FileCheck %s -check-prefix=OWN_ACCESSOR_9
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_ACCESSOR_10 | %FileCheck %s -check-prefix=OWN_ACCESSOR_10
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_ACCESSOR_11 | %FileCheck %s -check-prefix=OWN_ACCESSOR_11
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_ACCESSOR_12 | %FileCheck %s -check-prefix=OWN_ACCESSOR_11
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_ACCESSOR_13 | %FileCheck %s -check-prefix=OWN_ACCESSOR_13
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_ACCESSOR_14 | %FileCheck %s -check-prefix=OWN_ACCESSOR_13
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_ACCESSOR_15 | %FileCheck %s -check-prefix=OWN_ACCESSOR_13
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_ACCESSOR_16 | %FileCheck %s -check-prefix=OWN_ACCESSOR_13

//
// Test code completion at the beginning of expr-postfix.
//
Expand Down Expand Up @@ -588,3 +605,65 @@ func ownInitTestingShadow(ownInit7: Int) {
// OWN_INIT_7: Begin completions
// OWN_INIT_7: Decl[LocalVar]/Local/TypeRelation[Identical]: ownInit7[#Int#];
}

var inAccessor1: Int {
get { #^OWN_ACCESSOR_1^# }
// OWN_ACCESSOR_1: Begin completions
// OWN_ACCESSOR_1: Decl[GlobalVar]/CurrModule/NotRecommended/TypeRelation[Identical]: inAccessor1[#Int#];
set { #^OWN_ACCESSOR_2^# }
// OWN_ACCESSOR_2: Begin completions
// OWN_ACCESSOR_2: Decl[GlobalVar]/CurrModule: inAccessor1[#Int#];
}
var inAccessor2: Int = 1 {
didSet { #^OWN_ACCESSOR_3^# }
// OWN_ACCESSOR_3: Begin completions
// OWN_ACCESSOR_3: Decl[GlobalVar]/CurrModule: inAccessor2[#Int#];
willSet { #^OWN_ACCESSOR_4^# }
}
class InAccessorTest {
var inAccessor3: Int {
get { #^OWN_ACCESSOR_5^# }
// OWN_ACCESSOR_5: Begin completions
// OWN_ACCESSOR_5: Decl[InstanceVar]/CurrNominal/NotRecommended/TypeRelation[Identical]: inAccessor3[#Int#];
set { #^OWN_ACCESSOR_6^# }
// OWN_ACCESSOR_6: Begin completions
// OWN_ACCESSOR_6: Decl[InstanceVar]/CurrNominal: inAccessor3[#Int#];
}
var inAccessor4: Int = 1 {
didSet { #^OWN_ACCESSOR_7^# }
// OWN_ACCESSOR_7: Begin completions
// OWN_ACCESSOR_7: Decl[InstanceVar]/CurrNominal: inAccessor4[#Int#];
willSet { #^OWN_ACCESSOR_8^# }
}
}
func inAccessorTest() {
var inAccessor5: Int {
get { #^OWN_ACCESSOR_9^# }
// OWN_ACCESSOR_9: Begin completions
// OWN_ACCESSOR_9: Decl[LocalVar]/Local/NotRecommended/TypeRelation[Identical]: inAccessor5[#Int#];
set { #^OWN_ACCESSOR_10^# }
// OWN_ACCESSOR_10: Begin completions
// OWN_ACCESSOR_10: Decl[LocalVar]/Local: inAccessor5[#Int#];
}
var inAccessor6: Int = 1 {
didSet { #^OWN_ACCESSOR_11^# }
// OWN_ACCESSOR_11: Begin completions
// OWN_ACCESSOR_11: Decl[LocalVar]/Local: inAccessor6[#Int#];
willSet { #^OWN_ACCESSOR_12^# }
}
}
class InAccessorTestQualified {
var inAccessorProp: Int {
get {
let _ = self.#^OWN_ACCESSOR_13^#
// OWN_ACCESSOR_13: Begin completions
// OWN_ACCESSOR_13-DAG: Decl[InstanceVar]/CurrNominal: inAccessorProp[#Int#];
// OWN_ACCESSOR_13: End completions
let _ = \InAccessorTestQualified.#^OWN_ACCESSOR_14^#
}
set {
let _ = self.#^OWN_ACCESSOR_15^#
let _ = \InAccessorTestQualified.#^OWN_ACCESSOR_16^#
}
}
}
4 changes: 2 additions & 2 deletions test/Parse/implicit_getter_incomplete.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ func test1() {

// Would trigger assertion when AST verifier checks source ranges ("child source range not contained within its parent")
func test2() { // expected-note {{match}}
var a : Int { // expected-note {{match}}
switch i { // expected-error {{unresolved identifier}} expected-error{{'switch' statement body must have at least one 'case'}}
var a : Int { // expected-note {{match}} expected-note {{'a' declared here}}
switch i { // expected-error {{use of unresolved identifier 'i'; did you mean 'a'}} expected-error{{'switch' statement body must have at least one 'case'}}
}
// expected-error@+1 2 {{expected '}'}}
8 changes: 4 additions & 4 deletions test/decl/var/static_var.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ struct InMemberFunc {
}
}

struct S { // expected-note 3{{extended type declared here}} expected-note{{did you mean 'S'?}}
struct S { // expected-note 3{{extended type declared here}}
static var v1: Int = 0
class var v2: Int = 0 // expected-error {{class properties are only allowed within classes; use 'static' to declare a static property}} {{3-8=static}}

Expand All @@ -116,7 +116,7 @@ extension S {
class let el2: Int = 0 // expected-error {{class properties are only allowed within classes; use 'static' to declare a static property}} {{3-8=static}}
}

enum E { // expected-note 3{{extended type declared here}} expected-note{{did you mean 'E'?}}
enum E { // expected-note 3{{extended type declared here}}
static var v1: Int = 0
class var v2: Int = 0 // expected-error {{class properties are only allowed within classes; use 'static' to declare a static property}} {{3-8=static}}

Expand All @@ -141,7 +141,7 @@ extension E {
class let el2: Int = 0 // expected-error {{class properties are only allowed within classes; use 'static' to declare a static property}} {{3-8=static}}
}

class C { // expected-note{{did you mean 'C'?}}
class C {
static var v1: Int = 0
class final var v3: Int = 0 // expected-error {{class stored properties not supported}}
class var v4: Int = 0 // expected-error {{class stored properties not supported}}
Expand Down Expand Up @@ -171,7 +171,7 @@ extension C {
static final let el4: Int = 0 // expected-error {{static declarations are already final}} {{10-16=}}
}

protocol P { // expected-note{{did you mean 'P'?}} expected-note{{extended type declared here}}
protocol P { // expected-note{{extended type declared here}}
// Both `static` and `class` property requirements are equivalent in protocols rdar://problem/17198298
static var v1: Int { get }
class var v2: Int { get } // expected-error {{class properties are only allowed within classes; use 'static' to declare a requirement fulfilled by either a static or class property}} {{3-8=static}}
Expand Down