Skip to content

[parse] Recover better from malformed subscript decls for code-completion #13348

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 1 commit into from
Dec 13, 2017
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
10 changes: 8 additions & 2 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4554,9 +4554,15 @@ ObjCSubscriptKind SubscriptDecl::getObjCSubscriptKind(
}

SourceRange SubscriptDecl::getSourceRange() const {
if (getBracesRange().isValid())
if (getBracesRange().isValid()) {
return { getSubscriptLoc(), getBracesRange().End };
return { getSubscriptLoc(), ElementTy.getSourceRange().End };
} else if (ElementTy.getSourceRange().End.isValid()) {
return { getSubscriptLoc(), ElementTy.getSourceRange().End };
} else if (ArrowLoc.isValid()) {
return { getSubscriptLoc(), ArrowLoc };
} else {
return getSubscriptLoc();
}
}

SourceRange SubscriptDecl::getSignatureSourceRange() const {
Expand Down
53 changes: 34 additions & 19 deletions lib/Parse/ParseDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5603,21 +5603,37 @@ Parser::parseDeclSubscript(ParseDeclOptions Flags,
ParserResult<ParameterList> Indices
= parseSingleParameterClause(ParameterContextKind::Subscript,
&argumentNames);
if (Indices.isNull() || Indices.hasCodeCompletion())
return ParserStatus(Indices);
Status |= Indices;

SignatureHasCodeCompletion |= Indices.hasCodeCompletion();
if (SignatureHasCodeCompletion && !CodeCompletion)
return makeParserCodeCompletionStatus();

// '->'
if (!Tok.is(tok::arrow)) {
SourceLoc ArrowLoc;
if (!consumeIf(tok::arrow, ArrowLoc)) {
if (!Indices.isParseError())
diagnose(Tok, diag::expected_arrow_subscript);
return makeParserError();
Status.setIsParseError();
}

if (!ArrowLoc.isValid() && (Indices.isNull() || Indices.get()->size() == 0)) {
// This doesn't look much like a subscript, so let regular recovery take
// care of it.
return Status;
}
SourceLoc ArrowLoc = consumeToken();

// type
ParserResult<TypeRepr> ElementTy = parseType(diag::expected_type_subscript);
if (ElementTy.isNull() || ElementTy.hasCodeCompletion())
return ParserStatus(ElementTy);
Status |= ElementTy;
SignatureHasCodeCompletion |= ElementTy.hasCodeCompletion();
if (SignatureHasCodeCompletion && !CodeCompletion) {
return makeParserCodeCompletionStatus();
}
if (ElementTy.isNull()) {
// Always set an element type.
ElementTy = makeParserResult(ElementTy, new (Context) ErrorTypeRepr());
}

diagnoseWhereClauseInGenericParamList(GenericParams);

Expand Down Expand Up @@ -5646,8 +5662,9 @@ Parser::parseDeclSubscript(ParseDeclOptions Flags,
Subscript->setGenericParams(GenericParams);

// Pass the function signature to code completion.
if (SignatureHasCodeCompletion)
if (SignatureHasCodeCompletion && CodeCompletion) {
CodeCompletion->setParsedDecl(Subscript);
}

Decls.push_back(Subscript);

Expand All @@ -5657,12 +5674,15 @@ Parser::parseDeclSubscript(ParseDeclOptions Flags,
if (Tok.isNot(tok::l_brace)) {
// Subscript declarations must always have at least a getter, so they need
// to be followed by a {.
if (Flags.contains(PD_InProtocol))
diagnose(Tok, diag::expected_lbrace_subscript_protocol)
.fixItInsertAfter(ElementTy.get()->getEndLoc(), " { get set }");
else
diagnose(Tok, diag::expected_lbrace_subscript);
Status.setIsParseError();
if (!Status.isError()) {
if (Flags.contains(PD_InProtocol)) {
diagnose(Tok, diag::expected_lbrace_subscript_protocol)
.fixItInsertAfter(ElementTy.get()->getEndLoc(), " { get set }");
} else {
diagnose(Tok, diag::expected_lbrace_subscript);
}
Status.setIsParseError();
}
} else {
if (parseGetSet(Flags, GenericParams,
Indices.get(), ElementTy.get(),
Expand All @@ -5681,11 +5701,6 @@ Parser::parseDeclSubscript(ParseDeclOptions Flags,
Flags, /*static*/ SourceLoc(), Attributes,
ElementTy.get(), Indices.get(), Decls);

if (Invalid) {
Subscript->setInterfaceType(ErrorType::get(Context));
Subscript->setInvalid();
}

// No need to setLocalDiscriminator because subscripts cannot
// validly appear outside of type decls.
return makeParserResult(Status, Subscript);
Expand Down
2 changes: 0 additions & 2 deletions lib/Sema/TypeCheckDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4250,8 +4250,6 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
SD->setIsBeingValidated();

auto dc = SD->getDeclContext();
assert(dc->isTypeContext() &&
"Decl parsing must prevent subscripts outside of types!");

if (auto gp = SD->getGenericParams()) {
// Write up generic parameters and check the generic parameter list.
Expand Down
89 changes: 89 additions & 0 deletions test/IDE/complete_type_subscript.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=PARAM_0 | %FileCheck %s -check-prefix=TOP_LEVEL_0
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=RETURN_0 | %FileCheck %s -check-prefix=TOP_LEVEL_0

struct S0 {
subscript(x: #^PARAM_0^#) -> Int { return 0 }
subscript(x: Int) -> #^RETURN_0^# { }
}
// TOP_LEVEL_0: Keyword/None: Any[#Any#];
// TOP_LEVEL_0: Decl[Struct]/CurrModule: S0[#S0#];
// TOP_LEVEL_0: Decl[Struct]/OtherModule[Swift]: Int[#Int#];

// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=PARAM_1 | %FileCheck %s -check-prefix=MYSTRUCT_0
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=RETURN_1 | %FileCheck %s -check-prefix=MYSTRUCT_0
struct S1 {
struct MyStruct {}
subscript(x: MyStruct#^PARAM_1^#) -> Int { return 0 }
subscript(x: MyStruct) -> MyStruct#^RETURN_1^# { }
}
// MYSTRUCT_0: Keyword/None: .Type[#S1.MyStruct.Type#];
// MYSTRUCT_0: Keyword/CurrNominal: .self[#S1.MyStruct#];


// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=PARAM_2 | %FileCheck %s -check-prefix=MYSTRUCT_1
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=RETURN_2 | %FileCheck %s -check-prefix=MYSTRUCT_1
struct S2 {
struct MyStruct {}
subscript(x: MyStruct.#^PARAM_2^#) -> Int { return 0 }
subscript(x: MyStruct) -> MyStruct.#^RETURN_2^# { }
}
// MYSTRUCT_1: Keyword/None: Type[#S2.MyStruct.Type#];
// MYSTRUCT_1: Keyword/CurrNominal: self[#S2.MyStruct#];

// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=GEN_PARAM_0 | %FileCheck %s -check-prefix=GEN_TOP_LEVEL_0
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=GEN_RETURN_0 | %FileCheck %s -check-prefix=GEN_TOP_LEVEL_0
struct G0<T> {
subscript(x: #^GEN_PARAM_0^#) -> Int { return 0 }
subscript(x: T) -> #^GEN_RETURN_0^# { return 0 }
}
// GEN_TOP_LEVEL_0: Keyword/None: Any[#Any#];
// GEN_TOP_LEVEL_0: Decl[GenericTypeParam]/Local: T[#T#];
// GEN_TOP_LEVEL_0: Decl[Struct]/CurrModule: S0[#S0#];
// GEN_TOP_LEVEL_0: Decl[Struct]/OtherModule[Swift]: Int[#Int#];

// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=GEN_PARAM_1 | %FileCheck %s -check-prefix=GEN_PARAM_1
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=GEN_RETURN_1 | %FileCheck %s -check-prefix=GEN_PARAM_1
struct G1<T> {
subscript(x: T#^GEN_PARAM_1^#) -> Int { return 0 }
subscript(x: T) -> T#^GEN_RETURN_1^# { return 0 }
}
// GEN_PARAM_1: Keyword/None: .Type[#T.Type#];
// GEN_PARAM_1: Keyword/CurrNominal: .self[#T#];

// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=GEN_PARAM_2 | %FileCheck %s -check-prefix=GEN_PARAM_2
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=GEN_RETURN_2 | %FileCheck %s -check-prefix=GEN_PARAM_2
struct G2<T> {
subscript(x: T.#^GEN_PARAM_2^#) -> Int { return 0 }
subscript(x: T) -> T.#^GEN_RETURN_2^# { return 0 }
}
// GEN_PARAM_2: Keyword/None: Type[#T.Type#];
// GEN_PARAM_2: Keyword/CurrNominal: self[#T#];

// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=GEN_PARAM_3 | %FileCheck %s -check-prefix=GEN_TOP_LEVEL_1
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=GEN_RETURN_3 | %FileCheck %s -check-prefix=GEN_TOP_LEVEL_1
struct G3 {
subscript<T>(x: #^GEN_PARAM_3^#) -> Int { return 0 }
subscript<T>(x: T) -> #^GEN_RETURN_3^# { return 0 }
}
// GEN_TOP_LEVEL_1: Keyword/None: Any[#Any#];
// GEN_TOP_LEVEL_1: Decl[GenericTypeParam]/Local: T[#T#];
// GEN_TOP_LEVEL_1: Decl[Struct]/CurrModule: S0[#S0#];
// GEN_TOP_LEVEL_1: Decl[Struct]/OtherModule[Swift]: Int[#Int#];

// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=GEN_PARAM_4 | %FileCheck %s -check-prefix=GEN_PARAM_4
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=GEN_RETURN_4 | %FileCheck %s -check-prefix=GEN_PARAM_4
struct G4 {
subscript<T>(x: T#^GEN_PARAM_4^#) -> Int { return 0 }
subscript<T>(x: T) -> T#^GEN_RETURN_4^# { return 0 }
}
// GEN_PARAM_4: Keyword/None: .Type[#T.Type#];
// GEN_PARAM_4: Keyword/CurrNominal: .self[#T#];

// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=GEN_PARAM_5 | %FileCheck %s -check-prefix=GEN_PARAM_5
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=GEN_RETURN_5 | %FileCheck %s -check-prefix=GEN_PARAM_5
struct G5 {
subscript<T>(x: T.#^GEN_PARAM_5^#) -> Int { return 0 }
subscript<T>(x: T) -> T.#^GEN_RETURN_5^# { return 0 }
}
// GEN_PARAM_5: Keyword/None: Type[#T.Type#];
// GEN_PARAM_5: Keyword/CurrNominal: self[#T#];
14 changes: 9 additions & 5 deletions test/Parse/subscripting.swift
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// RUN: %target-typecheck-verify-swift

struct X { }
struct X { } // expected-note {{did you mean}}

// Simple examples
struct X1 {
Expand Down Expand Up @@ -101,10 +101,10 @@ struct A1 {
subscript (i : Int) // expected-error{{expected '->' for subscript element type}}
Int {
get {
return stored
return stored // expected-error{{use of unresolved identifier}}
}
set {
stored = value
stored = newValue// expected-error{{use of unresolved identifier}}
}
}
}
Expand All @@ -116,13 +116,14 @@ struct A2 {
return stored
}
set {
stored = value
stored = newValue // expected-error{{use of unresolved identifier}}
}
}
}

struct A3 {
subscript(i : Int) // expected-error {{expected '->' for subscript element type}}
// expected-error@-1 {{expected subscripting element type}}
{
get {
return i
Expand All @@ -132,6 +133,7 @@ struct A3 {

struct A4 {
subscript(i : Int) { // expected-error {{expected '->' for subscript element type}}
// expected-error@-1 {{expected subscripting element type}}
get {
return i
}
Expand All @@ -144,8 +146,10 @@ struct A5 {

struct A6 {
subscript(i: Int)(j: Int) -> Int { // expected-error {{expected '->' for subscript element type}}
// expected-error@-1 {{function types cannot have argument labels}}
// expected-note@-2 {{did you mean}}
get {
return i + j
return i + j // expected-error {{use of unresolved identifier}}
}
}
}
Expand Down