From e2728f626b0932fe295db8c9324d2d30eb0ed8bc Mon Sep 17 00:00:00 2001 From: Rintaro Ishizaki Date: Thu, 18 Apr 2024 16:19:46 -0700 Subject: [PATCH] [CodeCompletion] Fix completion for 'catch' pattern bound values Previously code completion for 'catch' pattern bound values didn't work correctly because code completion type checker fails to type check the value decl in the pattern. That was because the body of the 'do' statement is not type checked, so the thrown error is not determined, then falled backed to the default 'Never', which doesn't matches any patterns. To resolve this, always type check the body when typechecking 'catch' patterns. Also, pretends 'do {}' throws 'any Error' even without any throwing expressions in the body. rdar://126699879 (cherry picked from commit 0e122544caa41617ae3f424c9e316f64c788f88e) --- lib/Sema/TypeCheckDecl.cpp | 8 +- lib/Sema/TypeCheckStmt.cpp | 8 ++ test/IDE/complete_exception.swift | 136 ++++++------------ test/Sema/redeclaration-checking.swift | 1 - .../Parse/forward-slash-regex.swift | 2 +- test/stmt/typed_throws.swift | 4 +- 6 files changed, 63 insertions(+), 96 deletions(-) diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp index d4a7befa57e12..ebe617713ca57 100644 --- a/lib/Sema/TypeCheckDecl.cpp +++ b/lib/Sema/TypeCheckDecl.cpp @@ -2746,9 +2746,15 @@ NamingPatternRequest::evaluate(Evaluator &evaluator, VarDecl *VD) const { // We have some other parent stmt. Type check it completely. if (auto CS = dyn_cast(parentStmt)) parentStmt = CS->getParentStmt(); + + bool LeaveBodyUnchecked = true; + // type-checking 'catch' patterns depends on the type checked body. + if (isa(parentStmt)) + LeaveBodyUnchecked = false; + ASTNode node(parentStmt); TypeChecker::typeCheckASTNode(node, VD->getDeclContext(), - /*LeaveBodyUnchecked=*/true); + LeaveBodyUnchecked); } namingPattern = VD->getCanonicalVarDecl()->NamingPattern; } diff --git a/lib/Sema/TypeCheckStmt.cpp b/lib/Sema/TypeCheckStmt.cpp index 01d0370e4415d..5105e39f81a17 100644 --- a/lib/Sema/TypeCheckStmt.cpp +++ b/lib/Sema/TypeCheckStmt.cpp @@ -1669,6 +1669,14 @@ class StmtChecker : public StmtVisitor { bool limitExhaustivityChecks = true; Type caughtErrorType = TypeChecker::catchErrorType(DC, S); + + // If there was no throwing expression in the body, let's pretend it can + // throw 'any Error' just for type checking the pattern. That avoids + // superfluous diagnostics. Note that we still diagnose unreachable 'catch' + // separately in TypeCheckEffects. + if (caughtErrorType->isNever()) + caughtErrorType = Ctx.getErrorExistentialType(); + auto catches = S->getCatches(); checkSiblingCaseStmts(catches.begin(), catches.end(), CaseParentKind::DoCatch, limitExhaustivityChecks, diff --git a/test/IDE/complete_exception.swift b/test/IDE/complete_exception.swift index bd77b41ef2106..9d520caebf1f5 100644 --- a/test/IDE/complete_exception.swift +++ b/test/IDE/complete_exception.swift @@ -1,54 +1,5 @@ -// RUN: %target-swift-ide-test(mock-sdk: %clang-importer-sdk) -code-completion -source-filename %s -code-completion-token=CATCH1 | %FileCheck %s -check-prefix=CATCH1 -// RUN: %target-swift-ide-test(mock-sdk: %clang-importer-sdk) -code-completion -source-filename %s -code-completion-token=THROW1 > %t.throw1 -// RUN: %FileCheck %s -check-prefix=THROW1 < %t.throw1 -// RUN: %FileCheck %s -check-prefix=THROW1-LOCAL < %t.throw1 -// RUN: %target-swift-ide-test(mock-sdk: %clang-importer-sdk) -code-completion -source-filename %s -code-completion-token=CATCH2 | %FileCheck %s -check-prefix=CATCH2 -// RUN: %target-swift-ide-test(mock-sdk: %clang-importer-sdk) -code-completion -source-filename %s -code-completion-token=THROW2 | %FileCheck %s -check-prefix=THROW2 -// RUN: %target-swift-ide-test(mock-sdk: %clang-importer-sdk) -code-completion -source-filename %s -code-completion-token=CATCH3 | %FileCheck %s -check-prefix=CATCH3 -// RUN: %target-swift-ide-test(mock-sdk: %clang-importer-sdk) -code-completion -source-filename %s -code-completion-token=THROW3 | %FileCheck %s -check-prefix=THROW3 -// RUN: %target-swift-ide-test(mock-sdk: %clang-importer-sdk) -code-completion -source-filename %s -code-completion-token=TOP_LEVEL_CATCH1 | %FileCheck %s -check-prefix=CATCH1 -// RUN: %target-swift-ide-test(mock-sdk: %clang-importer-sdk) -code-completion -source-filename %s -code-completion-token=TOP_LEVEL_THROW1 | %FileCheck %s -check-prefix=THROW1 - -// RUN: %target-swift-ide-test(mock-sdk: %clang-importer-sdk) -code-completion -source-filename %s -code-completion-token=TOP_LEVEL_CATCH2 | %FileCheck %s -check-prefix=CATCH2 -// RUN: %target-swift-ide-test(mock-sdk: %clang-importer-sdk) -code-completion -source-filename %s -code-completion-token=TOP_LEVEL_THROW2 | %FileCheck %s -check-prefix=THROW2 -// RUN: %target-swift-ide-test(mock-sdk: %clang-importer-sdk) -code-completion -source-filename %s -code-completion-token=TOP_LEVEL_THROW3 | %FileCheck %s -check-prefix=THROW3 - -// RUN: %target-swift-ide-test(mock-sdk: %clang-importer-sdk) -code-completion -source-filename %s -code-completion-token=INSIDE_CATCH1 > %t.inside_catch1 -// RUN: %FileCheck %s -check-prefix=STMT < %t.inside_catch1 -// RUN: %FileCheck %s -check-prefix=IMPLICIT_ERROR < %t.inside_catch1 - -// RUN: %target-swift-ide-test(mock-sdk: %clang-importer-sdk) -code-completion -source-filename %s -code-completion-token=INSIDE_CATCH2 > %t.inside_catch2 -// RUN: %FileCheck %s -check-prefix=STMT < %t.inside_catch2 -// RUN: %FileCheck %s -check-prefix=EXPLICIT_ERROR_E < %t.inside_catch2 - -// RUN: %target-swift-ide-test(mock-sdk: %clang-importer-sdk) -code-completion -source-filename %s -code-completion-token=INSIDE_CATCH3 > %t.inside_catch3 -// RUN: %FileCheck %s -check-prefix=STMT < %t.inside_catch3 -// RUN: %FileCheck %s -check-prefix=EXPLICIT_NSERROR_E < %t.inside_catch3 - -// RUN: %target-swift-ide-test(mock-sdk: %clang-importer-sdk) -code-completion -source-filename %s -code-completion-token=INSIDE_CATCH4 > %t.inside_catch4 -// RUN: %FileCheck %s -check-prefix=STMT < %t.inside_catch4 -// RUN: %FileCheck %s -check-prefix=EXPLICIT_ERROR_PAYLOAD_I < %t.inside_catch4 - -// RUN: %target-swift-ide-test(mock-sdk: %clang-importer-sdk) -code-completion -source-filename %s -code-completion-token=INSIDE_CATCH5 > %t.inside_catch5 -// RUN: %FileCheck %s -check-prefix=STMT < %t.inside_catch5 -// RUN: %FileCheck %s -check-prefix=EXPLICIT_ERROR_E < %t.inside_catch5 -// RUN: %FileCheck %s -check-prefix=NO_ERROR_AND_A < %t.inside_catch5 - -// RUN: %target-swift-ide-test(mock-sdk: %clang-importer-sdk) -code-completion -source-filename %s -code-completion-token=INSIDE_CATCH6 > %t.inside_catch6 -// RUN: %FileCheck %s -check-prefix=STMT < %t.inside_catch6 -// RUN: %FileCheck %s -check-prefix=NO_E < %t.inside_catch6 -// RUN: %FileCheck %s -check-prefix=NO_ERROR_AND_A < %t.inside_catch6 - -// RUN: %target-swift-ide-test(mock-sdk: %clang-importer-sdk) -code-completion -source-filename %s -code-completion-token=INSIDE_CATCH_ERR_DOT1 | %FileCheck %s -check-prefix=ERROR_DOT -// RUN: %target-swift-ide-test(mock-sdk: %clang-importer-sdk) -code-completion -source-filename %s -code-completion-token=INSIDE_CATCH_ERR_DOT2 | %FileCheck %s -check-prefix=ERROR_DOT -// RUN: %target-swift-ide-test(mock-sdk: %clang-importer-sdk) -code-completion -source-filename %s -code-completion-token=INSIDE_CATCH_ERR_DOT3 | %FileCheck %s -check-prefix=NSERROR_DOT -// RUNFIXME: %target-swift-ide-test(mock-sdk: %clang-importer-sdk) -code-completion -source-filename %s -code-completion-token=INSIDE_CATCH_ERR_DOT4 | %FileCheck %s -check-prefix=INT_DOT - -// RUN: %target-swift-ide-test(mock-sdk: %clang-importer-sdk) -code-completion -source-filename %s -code-completion-token=TOP_LEVEL_INSIDE_CATCH1 > %t.top_level_inside_catch1 -// RUN: %FileCheck %s -check-prefix=STMT < %t.top_level_inside_catch1 -// RUN: %FileCheck %s -check-prefix=IMPLICIT_ERROR < %t.top_level_inside_catch1 - -// RUN: %target-swift-ide-test(mock-sdk: %clang-importer-sdk) -code-completion -source-filename %s -code-completion-token=TOP_LEVEL_INSIDE_CATCH_ERR_DOT1 | %FileCheck %s -check-prefix=ERROR_DOT +// RUN: %empty-directory(%t/batch-code-completion) +// RUN: %target-swift-ide-test(mock-sdk: %clang-importer-sdk) -batch-code-completion -source-filename %s -filecheck %raw-FileCheck -completion-output-dir %t/batch-code-completion // REQUIRES: objc_interop @@ -71,10 +22,10 @@ func getNSError() -> NSError { return NSError(domain: "", code: 1, userInfo: [:] func test001() { do {} catch #^CATCH1^# -// CATCH1-DAG: Decl[Enum]/CurrModule: Error4[#Error4#]; name=Error4{{$}} -// CATCH1-DAG: Decl[Class]/CurrModule: Error3[#Error3#]; name=Error3{{$}} -// CATCH1-DAG: Decl[Class]/CurrModule: Error2[#Error2#]; name=Error2{{$}} -// CATCH1-DAG: Decl[Class]/CurrModule: Error1[#Error1#]; name=Error1{{$}} +// CATCH1-DAG: Decl[Enum]/CurrModule/TypeRelation[Convertible]: Error4[#Error4#]; name=Error4{{$}} +// CATCH1-DAG: Decl[Class]/CurrModule/TypeRelation[Convertible]: Error3[#Error3#]; name=Error3{{$}} +// CATCH1-DAG: Decl[Class]/CurrModule/TypeRelation[Convertible]: Error2[#Error2#]; name=Error2{{$}} +// CATCH1-DAG: Decl[Class]/CurrModule/TypeRelation[Convertible]: Error1[#Error1#]; name=Error1{{$}} // CATCH1-DAG: Keyword[let]/None: let{{; name=.+$}} // CATCH1-DAG: Decl[Class]/CurrModule: NoneError1[#NoneError1#]; name=NoneError1{{$}} // CATCH1-DAG: Decl[Class]/OtherModule[Foundation]/IsSystem: NSError[#NSError#]{{; name=.+$}} @@ -84,7 +35,7 @@ func test002() { let text = "NonError" let e1 = Error1() let e2 = Error2() - throw #^THROW1^# + throw #^THROW1?check=THROW1,THROW1-LOCAL^# // THROW1-DAG: Decl[Enum]/CurrModule/TypeRelation[Convertible]: Error4[#Error4#]; name=Error4{{$}} // THROW1-DAG: Decl[Class]/CurrModule/TypeRelation[Convertible]: Error3[#Error3#]; name=Error3{{$}} @@ -93,11 +44,8 @@ func test002() { // THROW1-DAG: Decl[Protocol]/CurrModule/Flair[RareType]/TypeRelation[Convertible]: ErrorPro1[#ErrorPro1#]; name=ErrorPro1{{$}} // THROW1-DAG: Decl[FreeFunction]/CurrModule/TypeRelation[Convertible]: getError1()[#Error1#]{{; name=.+$}} // THROW1-DAG: Decl[FreeFunction]/CurrModule/TypeRelation[Convertible]: getNSError()[#NSError#]{{; name=.+$}} +// THROW1-DAG: Decl[Class]/CurrModule: NoneError1[#NoneError1#]; name=NoneError1{{$}} -// If we could prove that there is no way to get to an Error value by -// starting from these, we could remove them. But that may be infeasible in -// the presence of overloaded operators. -// THROW1-DAG: Decl[Class]/CurrModule: NoneError1[#NoneError1#]; name=NoneError1{{$}} // THROW1-LOCAL-DAG: Decl[LocalVar]/Local: text[#String#]; name=text{{$}} // THROW1-LOCAL-DAG: Decl[LocalVar]/Local/TypeRelation[Convertible]: e1[#Error1#]; name=e1{{$}} // THROW1-LOCAL-DAG: Decl[LocalVar]/Local/TypeRelation[Convertible]: e2[#Error2#]; name=e2{{$}} @@ -105,33 +53,33 @@ func test002() { func test003() { do {} catch Error4.#^CATCH2^# -// CATCH2: Decl[EnumElement]/CurrNominal: E1[#Error4#]{{; name=.+$}} -// CATCH2: Decl[EnumElement]/CurrNominal: E2({#Int32#})[#Error4#]{{; name=.+$}} +// CATCH2-DAG: Decl[EnumElement]/CurrNominal: E1[#Error4#]{{; name=.+$}} +// CATCH2-DAG: Decl[EnumElement]/CurrNominal: E2({#Int32#})[#Error4#]{{; name=.+$}} } func test004() { throw Error4.#^THROW2^# -// THROW2: Decl[EnumElement]/CurrNominal/TypeRelation[Convertible]: E1[#Error4#]{{; name=.+$}} -// THROW2: Decl[EnumElement]/CurrNominal/TypeRelation[Convertible]: E2({#Int32#})[#Error4#]{{; name=.+$}} +// THROW2-DAG: Decl[EnumElement]/CurrNominal/TypeRelation[Convertible]: E1[#Error4#]{{; name=.+$}} +// THROW2-DAG: Decl[EnumElement]/CurrNominal/TypeRelation[Convertible]: E2({#Int32#})[#Error4#]{{; name=.+$}} } func test005() { do {} catch Error4.E2#^CATCH3^# -// CATCH3: Pattern/CurrModule/Flair[ArgLabels]: ({#Int32#})[#Error4#]{{; name=.+$}} +// CATCH3-DAG: Pattern/CurrModule/Flair[ArgLabels]: ({#Int32#})[#Error4#]{{; name=.+$}} } func testInvalid() { try throw Error4.#^THROW3^# -// THROW3: Decl[EnumElement]/CurrNominal/TypeRelation[Convertible]: E1[#Error4#]{{; name=.+$}} -// THROW3: Decl[EnumElement]/CurrNominal/TypeRelation[Convertible]: E2({#Int32#})[#Error4#]{{; name=.+$}} +// THROW3-DAG: Decl[EnumElement]/CurrNominal/TypeRelation[Convertible]: E1[#Error4#]{{; name=.+$}} +// THROW3-DAG: Decl[EnumElement]/CurrNominal/TypeRelation[Convertible]: E2({#Int32#})[#Error4#]{{; name=.+$}} } //===--- Top-level throw/catch -do {} catch #^TOP_LEVEL_CATCH1^# {} -throw #^TOP_LEVEL_THROW1^# -do {} catch Error4.#^TOP_LEVEL_CATCH2^# {} -throw Error4.#^TOP_LEVEL_THROW2^# -try throw Error4.#^TOP_LEVEL_THROW3^# +do {} catch #^TOP_LEVEL_CATCH1?check=CATCH1^# {} +throw #^TOP_LEVEL_THROW1?check=THROW1^# +do {} catch Error4.#^TOP_LEVEL_CATCH2?check=CATCH2^# {} +throw Error4.#^TOP_LEVEL_THROW2?check=THROW2^# +try throw Error4.#^TOP_LEVEL_THROW3?check=THROW3^# //===--- Inside catch body @@ -145,38 +93,37 @@ try throw Error4.#^TOP_LEVEL_THROW3^# func test006() { do { } catch { - #^INSIDE_CATCH1^# + #^INSIDE_CATCH1?check=STMT,IMPLICIT_ERROR^# } -// IMPLICIT_ERROR: Decl[LocalVar]/Local: error[#any Error#]; name=error +// IMPLICIT_ERROR-DAG: Decl[LocalVar]/Local: error[#any Error#]; name=error } func test007() { do { } catch let e { - #^INSIDE_CATCH2^# + #^INSIDE_CATCH2?check=STMT,EXPLICIT_ERROR_E^# } -// EXPLICIT_ERROR_E: Decl[LocalVar]/Local: e[#any Error#]; name=e +// EXPLICIT_ERROR_E-DAG: Decl[LocalVar]/Local: e[#any Error#]; name=e } func test008() { do { } catch let e as NSError { - #^INSIDE_CATCH3^# + #^INSIDE_CATCH3?check=STMT,EXPLICIT_NSERROR_E^# } -// EXPLICIT_NSERROR_E: Decl[LocalVar]/Local: e[#NSError#]; name=e +// EXPLICIT_NSERROR_E-DAG: Decl[LocalVar]/Local: e[#NSError#]; name=e } func test009() { do { } catch Error4.E2(let i) { - #^INSIDE_CATCH4^# + #^INSIDE_CATCH4?check=STMT,EXPLICIT_ERROR_PAYLOAD_I^# } - // FIXME: we're getting parentheses around the type when it's unnamed... -// EXPLICIT_ERROR_PAYLOAD_I: Decl[LocalVar]/Local: i[#<>#]; name=i +// EXPLICIT_ERROR_PAYLOAD_I-DAG: Decl[LocalVar]/Local: i[#(Int32)#]; name=i } func test010() { do { } catch let awesomeError { } catch let e { - #^INSIDE_CATCH5^# + #^INSIDE_CATCH5?check=STMT,EXPLICIT_ERROR_E;check=NO_ERROR_AND_A^# } catch {} // NO_ERROR_AND_A-NOT: awesomeError // NO_ERROR_AND_A-NOT: Decl[LocalVar]/Local: error @@ -186,26 +133,26 @@ func test011() { } catch let awesomeError { } catch let excellentError { } catch {} - #^INSIDE_CATCH6^# + #^INSIDE_CATCH6?check=STMT;check=NO_ERROR_AND_A,NO_E^# // NO_E-NOT: excellentError } func test012() { do { } catch { - error.#^INSIDE_CATCH_ERR_DOT1^# + error.#^INSIDE_CATCH_ERR_DOT1?check=ERROR_DOT^# } } -// ERROR_DOT: Keyword[self]/CurrNominal: self[#any Error#]; name=self +// ERROR_DOT-DAG: Keyword[self]/CurrNominal: self[#any Error#]; name=self func test013() { do { } catch let e { - e.#^INSIDE_CATCH_ERR_DOT2^# + e.#^INSIDE_CATCH_ERR_DOT2?check=ERROR_DOT^# } } func test014() { do { } catch let e as NSError { - e.#^INSIDE_CATCH_ERR_DOT3^# + e.#^INSIDE_CATCH_ERR_DOT3?check=NSERROR_DOT^# } // NSERROR_DOT-DAG: Decl[InstanceVar]/CurrNominal/IsSystem: domain[#String#]; name=domain // NSERROR_DOT-DAG: Decl[InstanceVar]/CurrNominal/IsSystem: code[#Int#]; name=code @@ -218,7 +165,7 @@ func test014() { func test015() { do { } catch Error4.E2(let i) where i == 2 { - i.#^INSIDE_CATCH_ERR_DOT4^# + i.#^INSIDE_CATCH_ERR_DOT4?check=INT_DOT^# } } // Check that we can complete on the bound value; Not exhaustive.. @@ -228,9 +175,18 @@ func test015() { //===--- Inside catch body top-level do { } catch { - #^TOP_LEVEL_INSIDE_CATCH1^# + #^TOP_LEVEL_INSIDE_CATCH1?check=STMT,IMPLICIT_ERROR^# } do { } catch { - error.#^TOP_LEVEL_INSIDE_CATCH_ERR_DOT1^# + error.#^TOP_LEVEL_INSIDE_CATCH_ERR_DOT1?check=ERROR_DOT^# +} + +func canThrowError4() throws(Error4) {} +func test016() { + do { + try canThrowError4() + } catch .E2(let i) { + i.#^INSIDE_CATCH_TYPEDERR_DOT?check=INT_DOT^# + } } diff --git a/test/Sema/redeclaration-checking.swift b/test/Sema/redeclaration-checking.swift index 67c0be0eeaf4b..f256c01eece7c 100644 --- a/test/Sema/redeclaration-checking.swift +++ b/test/Sema/redeclaration-checking.swift @@ -87,7 +87,6 @@ func stmtTest() { // expected-note@-1 {{'x' previously declared here}} // expected-error@-2 {{invalid redeclaration of 'x'}} // expected-warning@-3 {{unreachable}} - // expected-error@-4{{pattern of type 'MyError' cannot match 'Never'}} } func fullNameTest() { diff --git a/test/StringProcessing/Parse/forward-slash-regex.swift b/test/StringProcessing/Parse/forward-slash-regex.swift index 18463688db5bb..f4990d4fcb4d4 100644 --- a/test/StringProcessing/Parse/forward-slash-regex.swift +++ b/test/StringProcessing/Parse/forward-slash-regex.swift @@ -262,7 +262,7 @@ default: } do {} catch /x/ {} -// expected-error@-1 {{expression pattern of type 'Regex' cannot match values of type 'Never'}} +// expected-error@-1 {{expression pattern of type 'Regex' cannot match values of type 'any Error'}} // expected-warning@-2 {{'catch' block is unreachable because no errors are thrown in 'do' block}} switch /x/ { diff --git a/test/stmt/typed_throws.swift b/test/stmt/typed_throws.swift index 19f17d9332131..0d0878c9144a9 100644 --- a/test/stmt/typed_throws.swift +++ b/test/stmt/typed_throws.swift @@ -134,9 +134,8 @@ func testTryIncompatibleTyped(cond: Bool) throws(HomeworkError) { } } catch let error as Never { // expected-warning@-1{{'catch' block is unreachable because no errors are thrown in 'do' block}} - // expected-warning@-2{{'as' test is always true}} throw .forgot - } + } // expected-error {{thrown expression type 'any Error' cannot be converted to error type 'HomeworkError'}} } func doSomethingWithoutThrowing() { } @@ -145,7 +144,6 @@ func testDoCatchWithoutThrowing() { do { try doSomethingWithoutThrowing() // expected-warning{{no calls to throwing functions occur within 'try' expression}} } catch HomeworkError.forgot { // expected-warning{{'catch' block is unreachable because no errors are thrown in 'do' block}} - // expected-error@-1{{pattern of type 'HomeworkError' cannot match 'Never'}} } catch { } }