Skip to content

Commit 45e9534

Browse files
ahoppenahoppen
authored
Merge pull request #73066 from ahoppen/inherit-doc-comment-from-protocol-default-impl
Inherit doc comments from default implementations of inherited protocols
2 parents 6686bd9 + eec3d75 commit 45e9534

File tree

3 files changed

+196
-22
lines changed

3 files changed

+196
-22
lines changed

‎lib/AST/DocComment.cpp

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -427,24 +427,49 @@ class CommentProviderFinder final {
427427
return std::nullopt;
428428
}
429429

430+
/// Check if there is an inherited protocol that has a default implementation
431+
/// of `VD` with a doc comment.
430432
std::optional<ResultWithDecl> findDefaultProvidedDecl(const ValueDecl *VD) {
431-
// Only applies to protocol extension member.
432-
auto *protocol = VD->getDeclContext()->getExtendedProtocolDecl();
433-
if (!protocol)
433+
NominalTypeDecl *nominalType =
434+
dyn_cast_or_null<NominalTypeDecl>(VD->getDeclContext()->getAsDecl());
435+
if (!nominalType) {
436+
nominalType = VD->getDeclContext()->getExtendedProtocolDecl();
437+
}
438+
if (!nominalType)
434439
return std::nullopt;
435440

436441
SmallVector<ValueDecl *, 2> members;
437-
protocol->lookupQualified(const_cast<ProtocolDecl *>(protocol),
438-
DeclNameRef(VD->getName()),
439-
VD->getLoc(), NLOptions::NL_ProtocolMembers,
440-
members);
442+
nominalType->lookupQualified(nominalType, DeclNameRef(VD->getName()),
443+
VD->getLoc(), NLOptions::NL_ProtocolMembers,
444+
members);
441445

442446
std::optional<ResultWithDecl> result;
443-
for (auto *member : members) {
444-
if (!isa<ProtocolDecl>(member->getDeclContext()) ||
445-
!member->isProtocolRequirement())
446-
continue;
447+
Type vdComparisonTy = VD->getInterfaceType();
448+
if (!vdComparisonTy) {
449+
return std::nullopt;
450+
}
451+
if (auto fnTy = vdComparisonTy->getAs<AnyFunctionType>()) {
452+
// Strip off the 'Self' parameter.
453+
vdComparisonTy = fnTy->getResult();
454+
}
447455

456+
for (auto *member : members) {
457+
if (isa<AbstractFunctionDecl>(member) || isa<AbstractStorageDecl>(member)) {
458+
if (VD->isStatic() != member->isStatic()) {
459+
continue;
460+
}
461+
Type memberComparisonTy = member->getInterfaceType();
462+
if (!memberComparisonTy) {
463+
continue;
464+
}
465+
if (auto fnTy = memberComparisonTy->getAs<AnyFunctionType>()) {
466+
// Strip off the 'Self' parameter.
467+
memberComparisonTy = fnTy->getResult();
468+
}
469+
if (!vdComparisonTy->matches(memberComparisonTy, TypeMatchFlags::AllowOverride)) {
470+
continue;
471+
}
472+
}
448473
auto newResult = visit(member);
449474
if (!newResult)
450475
continue;
@@ -487,10 +512,10 @@ class CommentProviderFinder final {
487512
if (auto result = findOverriddenDecl(VD))
488513
return result;
489514

490-
if (auto result = findDefaultProvidedDecl(VD))
515+
if (auto result = findRequirementDecl(VD))
491516
return result;
492517

493-
if (auto result = findRequirementDecl(VD))
518+
if (auto result = findDefaultProvidedDecl(VD))
494519
return result;
495520

496521
return std::nullopt;

‎test/SourceKit/CursorInfo/cursor_inherit_doc_comment.swift

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
protocol BaseProtocol {}
2+
3+
extension BaseProtocol {
4+
/// Say hello
5+
func hello() { }
6+
}
7+
8+
protocol InheritedProtocol: BaseProtocol {}
9+
10+
extension InheritedProtocol {
11+
func hello() { }
12+
}
13+
14+
func testInheritBetweenExtensions(x: MyStruct) {
15+
struct MyStruct: InheritedProtocol {}
16+
17+
// RUN: %sourcekitd-test -req=cursor -req-opts=retrieve_symbol_graph=1 -pos=%(line + 1):14 %s -- %s | %FileCheck %s --check-prefix BETWEEN_EXTENSIONS
18+
MyStruct().hello()
19+
// BETWEEN_EXTENSIONS: DOC COMMENT
20+
// BETWEEN_EXTENSIONS-NEXT: Say hello
21+
// BETWEEN_EXTENSIONS-NEXT: DOC COMMENT XML
22+
// BETWEEN_EXTENSIONS: SYMBOL GRAPH BEGIN
23+
// BETWEEN_EXTENSIONS: Say hello
24+
// BETWEEN_EXTENSIONS: SYMBOL GRAPH END
25+
}
26+
27+
func testInheritFromExtensionToStruct(x: MyStruct) {
28+
struct MyStruct: BaseProtocol {
29+
func hello() {}
30+
}
31+
32+
// RUN: %sourcekitd-test -req=cursor -req-opts=retrieve_symbol_graph=1 -pos=%(line + 1):14 %s -- %s | %FileCheck %s --check-prefix EXTENSION_TO_STRUCT
33+
MyStruct().hello()
34+
// EXTENSION_TO_STRUCT: DOC COMMENT
35+
// EXTENSION_TO_STRUCT-NEXT: Say hello
36+
// EXTENSION_TO_STRUCT-NEXT: DOC COMMENT XML
37+
// EXTENSION_TO_STRUCT: SYMBOL GRAPH BEGIN
38+
// EXTENSION_TO_STRUCT: Say hello
39+
// EXTENSION_TO_STRUCT: SYMBOL GRAPH END
40+
}
41+
42+
protocol ProtocolWithDefaultedRequirement {
43+
/// Doc for the requirement
44+
func hello()
45+
}
46+
47+
extension ProtocolWithDefaultedRequirement {
48+
/// Doc for the default implementation
49+
func hello() {}
50+
}
51+
52+
func testProtocolWithDefaultedRequirement(foo: any ProtocolWithDefaultedRequirement) {
53+
// RUN: %sourcekitd-test -req=cursor -req-opts=retrieve_symbol_graph=1 -pos=%(line + 1):7 %s -- %s | %FileCheck %s --check-prefix DEFAULTED_REQUIREMENT
54+
foo.hello()
55+
// DEFAULTED_REQUIREMENT: DOC COMMENT
56+
// DEFAULTED_REQUIREMENT-NEXT: Doc for the requirement
57+
// DEFAULTED_REQUIREMENT-NEXT: DOC COMMENT XML
58+
// DEFAULTED_REQUIREMENT: SYMBOL GRAPH BEGIN
59+
// DEFAULTED_REQUIREMENT: Doc for the requirement
60+
// DEFAULTED_REQUIREMENT: SYMBOL GRAPH END
61+
}
62+
63+
64+
func testDontInheritFromFunctionWithDifferentType() {
65+
struct MyStruct: InheritedProtocol {
66+
func hello() -> String { return "Hello" }
67+
}
68+
// RUN: %sourcekitd-test -req=cursor -pos=%(line + 1):30 %s -- %s | %FileCheck %s --check-prefix FROM_FUNCTION_WITH_DIFFERENT_TYPE
69+
let x: String = MyStruct().hello()
70+
// FROM_FUNCTION_WITH_DIFFERENT_TYPE: DOC COMMENT
71+
// FROM_FUNCTION_WITH_DIFFERENT_TYPE-NEXT: DOC COMMENT XML
72+
}
73+
74+
protocol ProtocolWithThrowingFunction {
75+
/// Say hello
76+
func hello() throws
77+
}
78+
79+
func testInheritEvenIfThrowingNonThrowingMismatches() {
80+
struct MyStruct: ProtocolWithThrowingFunction {
81+
func hello() { }
82+
}
83+
// RUN: %sourcekitd-test -req=cursor -pos=%(line + 1):14 %s -- %s | %FileCheck %s --check-prefix SATISFY_THROWING_WITH_NON_THROWING
84+
MyStruct().hello()
85+
// SATISFY_THROWING_WITH_NON_THROWING: DOC COMMENT
86+
// SATISFY_THROWING_WITH_NON_THROWING-NEXT: Say hello
87+
// SATISFY_THROWING_WITH_NON_THROWING-NEXT: DOC COMMENT XML
88+
}
89+
90+
protocol ProtocolWithAccessor {
91+
/// The greeting
92+
var greeting: String { get }
93+
}
94+
95+
func testSatisfyAccessorRequirement() {
96+
struct MyStruct: ProtocolWithAccessor {
97+
var greeting: String = "hello"
98+
}
99+
// RUN: %sourcekitd-test -req=cursor -pos=%(line + 1):14 %s -- %s | %FileCheck %s --check-prefix ACCESSOR_REQUIREMENT
100+
MyStruct().greeting
101+
// ACCESSOR_REQUIREMENT: DOC COMMENT
102+
// ACCESSOR_REQUIREMENT-NEXT: The greeting
103+
// ACCESSOR_REQUIREMENT-NEXT: DOC COMMENT XML
104+
}
105+
106+
protocol ProtocolWithMethodReturningOptional {}
107+
extension ProtocolWithMethodReturningOptional {
108+
/// hello
109+
func hello() -> String? { nil }
110+
}
111+
112+
func testInheritFromLessSpecificOverridden() {
113+
struct MyStruct: ProtocolWithMethodReturningOptional {
114+
func hello() -> String { "hello" }
115+
}
116+
117+
// RUN: %sourcekitd-test -req=cursor -pos=%(line + 1):14 %s -- %s | %FileCheck %s --check-prefix INHERIT_FROM_LESS_SPECIFIC_OVERRIDDEN
118+
MyStruct().hello()
119+
// INHERIT_FROM_LESS_SPECIFIC_OVERRIDDEN: DOC COMMENT
120+
// INHERIT_FROM_LESS_SPECIFIC_OVERRIDDEN-NEXT: hello
121+
// INHERIT_FROM_LESS_SPECIFIC_OVERRIDDEN-NEXT: DOC COMMENT XML
122+
}
123+
124+
125+
protocol ProtocolWithStaticMethod {}
126+
extension ProtocolWithStaticMethod {
127+
/// hello
128+
static func hello() {}
129+
}
130+
131+
func testDontInheritFromStaticToNonStatic() {
132+
struct MyStruct: ProtocolWithStaticMethod {
133+
func hello() {}
134+
}
135+
136+
// RUN: %sourcekitd-test -req=cursor -pos=%(line + 1):14 %s -- %s | %FileCheck %s --check-prefix DONT_INHERIT_FROM_STATIC_TO_NON_STATIC
137+
MyStruct().hello()
138+
// DONT_INHERIT_FROM_STATIC_TO_NON_STATIC: DOC COMMENT
139+
// DONT_INHERIT_FROM_STATIC_TO_NON_STATIC-NEXT: DOC COMMENT XML
140+
}
141+
142+
protocol ProtocolWithStaticMethodReturningSelf {
143+
/// hello
144+
static func hello(_ greeting: String) -> Self {}
145+
}
146+
147+
func testInheritStaticFuncToEnumCase() {
148+
enum MyEnum: ProtocolWithStaticMethodReturningSelf {
149+
case hello(String)
150+
}
151+
152+
// RUN: %sourcekitd-test -req=cursor -pos=%(line + 1):10 %s -- %s | %FileCheck %s --check-prefix INHERIT_FROM_STATIC_FUNC_TO_ENUM_CASE
153+
MyEnum.hello
154+
// INHERIT_FROM_STATIC_FUNC_TO_ENUM_CASE: DOC COMMENT
155+
// INHERIT_FROM_STATIC_FUNC_TO_ENUM_CASE-NEXT: hello
156+
// INHERIT_FROM_STATIC_FUNC_TO_ENUM_CASE-NEXT: DOC COMMENT XML
157+
}

‎test/SymbolGraph/Relationships/Synthesized/RemoteInheritedDocs.swift

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
// RUN: %FileCheck %s --input-file %t/RemoteInheritedDocs.symbols.json --check-prefix BONUS
99
// RUN: %FileCheck %s --input-file %t/RemoteInheritedDocs.symbols.json --check-prefix INHERIT
1010
// RUN: %FileCheck %s --input-file %t/RemoteInheritedDocs.symbols.json --check-prefix LOCAL
11-
// RUN: %FileCheck %s --input-file %t/RemoteInheritedDocs.symbols.json --check-prefix OVERRIDE
1211
// RUN: %FileCheck %s --input-file %t/RemoteInheritedDocs@RemoteP.symbols.json --check-prefix EXTENSION
1312

1413
// RUN: %target-swift-symbolgraph-extract -module-name RemoteInheritedDocs -I %t -pretty-print -output-dir %t -skip-inherited-docs
@@ -17,12 +16,11 @@
1716
// RUN: %FileCheck %s --input-file %t/RemoteInheritedDocs.symbols.json --check-prefix BONUS
1817
// RUN: %FileCheck %s --input-file %t/RemoteInheritedDocs.symbols.json --check-prefix SKIP
1918
// RUN: %FileCheck %s --input-file %t/RemoteInheritedDocs.symbols.json --check-prefix LOCAL
20-
// RUN: %FileCheck %s --input-file %t/RemoteInheritedDocs.symbols.json --check-prefix OVERRIDE
2119

2220
// SOME: "source": "s:19RemoteInheritedDocs1SV8someFuncyyF"
2321
// SOME-NEXT: "target": "s:19RemoteInheritedDocs1SV"
2422
// SOME-NEXT: "sourceOrigin"
25-
// SOME-NEXT: "identifier": "s:7RemoteP1PP8someFuncyyF"
23+
// SOME-NEXT: "identifier": "s:7RemoteP1PP0A13InheritedDocsE8someFuncyyF"
2624
// SOME-NEXT: "displayName": "P.someFunc()"
2725

2826
// OTHER: "source": "s:19RemoteInheritedDocs1SV9otherFuncyyF"
@@ -42,9 +40,6 @@
4240

4341
// LOCAL: Local docs override!
4442

45-
// OVERRIDE-NOT: Extra default docs!
46-
// OVERRIDE-NOT: Extension override!
47-
4843
// EXTENSION-NOT: Some Protocol
4944

5045
import RemoteP
@@ -53,9 +48,6 @@ import RemoteP
5348
// but `otherFunc` does. Regardless, each one needs a `sourceOrigin` field connecting it to
5449
// upstream.
5550

56-
// `RemoteP.P` also has an extension with a default implementation for `extraFunc` that does have
57-
// docs, but overriding it here should prevent those from appearing
58-
5951
// When emitting extension block symbols, local extension blocks should never inherit documentation
6052
// from the original type declaration.
6153

0 commit comments

Comments
 (0)