From eec3d752cbc2ef27198b74a5ebe2c8a957835ef8 Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Thu, 18 Apr 2024 22:19:27 -0700 Subject: [PATCH] Inherit doc comments from default implementations of inherited protocols For example when we have ```swift protocol BaseProtocol {} extension BaseProtocol { /// Say hello func hello() { } } struct MyStruct: BaseProtocol { func hello() {} } ``` Then `MyStruct.hello` should inherit the doc comment from the implementation of `BaseProtocol.hello`. Currently no doc comment is associated with `MyStruct.hello`. rdar://126240546 --- lib/AST/DocComment.cpp | 51 ++++-- .../cursor_inherit_doc_comment.swift | 157 ++++++++++++++++++ .../Synthesized/RemoteInheritedDocs.swift | 10 +- 3 files changed, 196 insertions(+), 22 deletions(-) create mode 100644 test/SourceKit/CursorInfo/cursor_inherit_doc_comment.swift diff --git a/lib/AST/DocComment.cpp b/lib/AST/DocComment.cpp index 0dc55d67b438c..deb2e81f38c8b 100644 --- a/lib/AST/DocComment.cpp +++ b/lib/AST/DocComment.cpp @@ -427,24 +427,49 @@ class CommentProviderFinder final { return std::nullopt; } + /// Check if there is an inherited protocol that has a default implementation + /// of `VD` with a doc comment. std::optional findDefaultProvidedDecl(const ValueDecl *VD) { - // Only applies to protocol extension member. - auto *protocol = VD->getDeclContext()->getExtendedProtocolDecl(); - if (!protocol) + NominalTypeDecl *nominalType = + dyn_cast_or_null(VD->getDeclContext()->getAsDecl()); + if (!nominalType) { + nominalType = VD->getDeclContext()->getExtendedProtocolDecl(); + } + if (!nominalType) return std::nullopt; SmallVector members; - protocol->lookupQualified(const_cast(protocol), - DeclNameRef(VD->getName()), - VD->getLoc(), NLOptions::NL_ProtocolMembers, - members); + nominalType->lookupQualified(nominalType, DeclNameRef(VD->getName()), + VD->getLoc(), NLOptions::NL_ProtocolMembers, + members); std::optional result; - for (auto *member : members) { - if (!isa(member->getDeclContext()) || - !member->isProtocolRequirement()) - continue; + Type vdComparisonTy = VD->getInterfaceType(); + if (!vdComparisonTy) { + return std::nullopt; + } + if (auto fnTy = vdComparisonTy->getAs()) { + // Strip off the 'Self' parameter. + vdComparisonTy = fnTy->getResult(); + } + for (auto *member : members) { + if (isa(member) || isa(member)) { + if (VD->isStatic() != member->isStatic()) { + continue; + } + Type memberComparisonTy = member->getInterfaceType(); + if (!memberComparisonTy) { + continue; + } + if (auto fnTy = memberComparisonTy->getAs()) { + // Strip off the 'Self' parameter. + memberComparisonTy = fnTy->getResult(); + } + if (!vdComparisonTy->matches(memberComparisonTy, TypeMatchFlags::AllowOverride)) { + continue; + } + } auto newResult = visit(member); if (!newResult) continue; @@ -487,10 +512,10 @@ class CommentProviderFinder final { if (auto result = findOverriddenDecl(VD)) return result; - if (auto result = findDefaultProvidedDecl(VD)) + if (auto result = findRequirementDecl(VD)) return result; - if (auto result = findRequirementDecl(VD)) + if (auto result = findDefaultProvidedDecl(VD)) return result; return std::nullopt; diff --git a/test/SourceKit/CursorInfo/cursor_inherit_doc_comment.swift b/test/SourceKit/CursorInfo/cursor_inherit_doc_comment.swift new file mode 100644 index 0000000000000..ef5d094122726 --- /dev/null +++ b/test/SourceKit/CursorInfo/cursor_inherit_doc_comment.swift @@ -0,0 +1,157 @@ +protocol BaseProtocol {} + +extension BaseProtocol { + /// Say hello + func hello() { } +} + +protocol InheritedProtocol: BaseProtocol {} + +extension InheritedProtocol { + func hello() { } +} + +func testInheritBetweenExtensions(x: MyStruct) { + struct MyStruct: InheritedProtocol {} + + // RUN: %sourcekitd-test -req=cursor -req-opts=retrieve_symbol_graph=1 -pos=%(line + 1):14 %s -- %s | %FileCheck %s --check-prefix BETWEEN_EXTENSIONS + MyStruct().hello() + // BETWEEN_EXTENSIONS: DOC COMMENT + // BETWEEN_EXTENSIONS-NEXT: Say hello + // BETWEEN_EXTENSIONS-NEXT: DOC COMMENT XML + // BETWEEN_EXTENSIONS: SYMBOL GRAPH BEGIN + // BETWEEN_EXTENSIONS: Say hello + // BETWEEN_EXTENSIONS: SYMBOL GRAPH END +} + +func testInheritFromExtensionToStruct(x: MyStruct) { + struct MyStruct: BaseProtocol { + func hello() {} + } + + // RUN: %sourcekitd-test -req=cursor -req-opts=retrieve_symbol_graph=1 -pos=%(line + 1):14 %s -- %s | %FileCheck %s --check-prefix EXTENSION_TO_STRUCT + MyStruct().hello() + // EXTENSION_TO_STRUCT: DOC COMMENT + // EXTENSION_TO_STRUCT-NEXT: Say hello + // EXTENSION_TO_STRUCT-NEXT: DOC COMMENT XML + // EXTENSION_TO_STRUCT: SYMBOL GRAPH BEGIN + // EXTENSION_TO_STRUCT: Say hello + // EXTENSION_TO_STRUCT: SYMBOL GRAPH END +} + +protocol ProtocolWithDefaultedRequirement { + /// Doc for the requirement + func hello() +} + +extension ProtocolWithDefaultedRequirement { + /// Doc for the default implementation + func hello() {} +} + +func testProtocolWithDefaultedRequirement(foo: any ProtocolWithDefaultedRequirement) { + // RUN: %sourcekitd-test -req=cursor -req-opts=retrieve_symbol_graph=1 -pos=%(line + 1):7 %s -- %s | %FileCheck %s --check-prefix DEFAULTED_REQUIREMENT + foo.hello() + // DEFAULTED_REQUIREMENT: DOC COMMENT + // DEFAULTED_REQUIREMENT-NEXT: Doc for the requirement + // DEFAULTED_REQUIREMENT-NEXT: DOC COMMENT XML + // DEFAULTED_REQUIREMENT: SYMBOL GRAPH BEGIN + // DEFAULTED_REQUIREMENT: Doc for the requirement + // DEFAULTED_REQUIREMENT: SYMBOL GRAPH END +} + + +func testDontInheritFromFunctionWithDifferentType() { + struct MyStruct: InheritedProtocol { + func hello() -> String { return "Hello" } + } + // RUN: %sourcekitd-test -req=cursor -pos=%(line + 1):30 %s -- %s | %FileCheck %s --check-prefix FROM_FUNCTION_WITH_DIFFERENT_TYPE + let x: String = MyStruct().hello() + // FROM_FUNCTION_WITH_DIFFERENT_TYPE: DOC COMMENT + // FROM_FUNCTION_WITH_DIFFERENT_TYPE-NEXT: DOC COMMENT XML +} + +protocol ProtocolWithThrowingFunction { + /// Say hello + func hello() throws +} + +func testInheritEvenIfThrowingNonThrowingMismatches() { + struct MyStruct: ProtocolWithThrowingFunction { + func hello() { } + } + // RUN: %sourcekitd-test -req=cursor -pos=%(line + 1):14 %s -- %s | %FileCheck %s --check-prefix SATISFY_THROWING_WITH_NON_THROWING + MyStruct().hello() + // SATISFY_THROWING_WITH_NON_THROWING: DOC COMMENT + // SATISFY_THROWING_WITH_NON_THROWING-NEXT: Say hello + // SATISFY_THROWING_WITH_NON_THROWING-NEXT: DOC COMMENT XML +} + +protocol ProtocolWithAccessor { + /// The greeting + var greeting: String { get } +} + +func testSatisfyAccessorRequirement() { + struct MyStruct: ProtocolWithAccessor { + var greeting: String = "hello" + } + // RUN: %sourcekitd-test -req=cursor -pos=%(line + 1):14 %s -- %s | %FileCheck %s --check-prefix ACCESSOR_REQUIREMENT + MyStruct().greeting + // ACCESSOR_REQUIREMENT: DOC COMMENT + // ACCESSOR_REQUIREMENT-NEXT: The greeting + // ACCESSOR_REQUIREMENT-NEXT: DOC COMMENT XML +} + +protocol ProtocolWithMethodReturningOptional {} +extension ProtocolWithMethodReturningOptional { + /// hello + func hello() -> String? { nil } +} + +func testInheritFromLessSpecificOverridden() { + struct MyStruct: ProtocolWithMethodReturningOptional { + func hello() -> String { "hello" } + } + + // RUN: %sourcekitd-test -req=cursor -pos=%(line + 1):14 %s -- %s | %FileCheck %s --check-prefix INHERIT_FROM_LESS_SPECIFIC_OVERRIDDEN + MyStruct().hello() + // INHERIT_FROM_LESS_SPECIFIC_OVERRIDDEN: DOC COMMENT + // INHERIT_FROM_LESS_SPECIFIC_OVERRIDDEN-NEXT: hello + // INHERIT_FROM_LESS_SPECIFIC_OVERRIDDEN-NEXT: DOC COMMENT XML +} + + +protocol ProtocolWithStaticMethod {} +extension ProtocolWithStaticMethod { + /// hello + static func hello() {} +} + +func testDontInheritFromStaticToNonStatic() { + struct MyStruct: ProtocolWithStaticMethod { + func hello() {} + } + + // RUN: %sourcekitd-test -req=cursor -pos=%(line + 1):14 %s -- %s | %FileCheck %s --check-prefix DONT_INHERIT_FROM_STATIC_TO_NON_STATIC + MyStruct().hello() + // DONT_INHERIT_FROM_STATIC_TO_NON_STATIC: DOC COMMENT + // DONT_INHERIT_FROM_STATIC_TO_NON_STATIC-NEXT: DOC COMMENT XML +} + +protocol ProtocolWithStaticMethodReturningSelf { + /// hello + static func hello(_ greeting: String) -> Self {} +} + +func testInheritStaticFuncToEnumCase() { + enum MyEnum: ProtocolWithStaticMethodReturningSelf { + case hello(String) + } + + // RUN: %sourcekitd-test -req=cursor -pos=%(line + 1):10 %s -- %s | %FileCheck %s --check-prefix INHERIT_FROM_STATIC_FUNC_TO_ENUM_CASE + MyEnum.hello + // INHERIT_FROM_STATIC_FUNC_TO_ENUM_CASE: DOC COMMENT + // INHERIT_FROM_STATIC_FUNC_TO_ENUM_CASE-NEXT: hello + // INHERIT_FROM_STATIC_FUNC_TO_ENUM_CASE-NEXT: DOC COMMENT XML +} diff --git a/test/SymbolGraph/Relationships/Synthesized/RemoteInheritedDocs.swift b/test/SymbolGraph/Relationships/Synthesized/RemoteInheritedDocs.swift index 22f4c4f86978d..dc253594d7044 100644 --- a/test/SymbolGraph/Relationships/Synthesized/RemoteInheritedDocs.swift +++ b/test/SymbolGraph/Relationships/Synthesized/RemoteInheritedDocs.swift @@ -8,7 +8,6 @@ // RUN: %FileCheck %s --input-file %t/RemoteInheritedDocs.symbols.json --check-prefix BONUS // RUN: %FileCheck %s --input-file %t/RemoteInheritedDocs.symbols.json --check-prefix INHERIT // RUN: %FileCheck %s --input-file %t/RemoteInheritedDocs.symbols.json --check-prefix LOCAL -// RUN: %FileCheck %s --input-file %t/RemoteInheritedDocs.symbols.json --check-prefix OVERRIDE // RUN: %FileCheck %s --input-file %t/RemoteInheritedDocs@RemoteP.symbols.json --check-prefix EXTENSION // RUN: %target-swift-symbolgraph-extract -module-name RemoteInheritedDocs -I %t -pretty-print -output-dir %t -skip-inherited-docs @@ -17,12 +16,11 @@ // RUN: %FileCheck %s --input-file %t/RemoteInheritedDocs.symbols.json --check-prefix BONUS // RUN: %FileCheck %s --input-file %t/RemoteInheritedDocs.symbols.json --check-prefix SKIP // RUN: %FileCheck %s --input-file %t/RemoteInheritedDocs.symbols.json --check-prefix LOCAL -// RUN: %FileCheck %s --input-file %t/RemoteInheritedDocs.symbols.json --check-prefix OVERRIDE // SOME: "source": "s:19RemoteInheritedDocs1SV8someFuncyyF" // SOME-NEXT: "target": "s:19RemoteInheritedDocs1SV" // SOME-NEXT: "sourceOrigin" -// SOME-NEXT: "identifier": "s:7RemoteP1PP8someFuncyyF" +// SOME-NEXT: "identifier": "s:7RemoteP1PP0A13InheritedDocsE8someFuncyyF" // SOME-NEXT: "displayName": "P.someFunc()" // OTHER: "source": "s:19RemoteInheritedDocs1SV9otherFuncyyF" @@ -42,9 +40,6 @@ // LOCAL: Local docs override! -// OVERRIDE-NOT: Extra default docs! -// OVERRIDE-NOT: Extension override! - // EXTENSION-NOT: Some Protocol import RemoteP @@ -53,9 +48,6 @@ import RemoteP // but `otherFunc` does. Regardless, each one needs a `sourceOrigin` field connecting it to // upstream. -// `RemoteP.P` also has an extension with a default implementation for `extraFunc` that does have -// docs, but overriding it here should prevent those from appearing - // When emitting extension block symbols, local extension blocks should never inherit documentation // from the original type declaration.