Skip to content

Commit fa84e11

Browse files
allevatoallevato
authored
Merge pull request swiftlang#212 from frankus/add-throws-comment-support
Add support for Throws doc comments
2 parents 33f0b37 + 610e824 commit fa84e11

File tree

4 files changed

+141
-21
lines changed

4 files changed

+141
-21
lines changed

‎Sources/SwiftFormatRules/DeclSyntaxProtocol+Comments.swift

Lines changed: 41 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,9 @@ extension DeclSyntaxProtocol {
8383
let comments = docComment.components(separatedBy: .newlines)
8484
var params = [ParseComment.Parameter]()
8585
var commentParagraphs = [String]()
86-
var hasFoundParameterList = false
87-
var hasFoundReturn = false
86+
var currentSection: DocCommentSection = .commentParagraphs
8887
var returnsDescription: String?
88+
var throwsDescription: String?
8989
// Takes the first sentence of the comment, and counts the number of lines it uses.
9090
let oneSenteceSummary = docComment.components(separatedBy: ".").first
9191
let numOfOneSentenceLines = oneSenteceSummary!.components(separatedBy: .newlines).count
@@ -96,42 +96,64 @@ extension DeclSyntaxProtocol {
9696
let trimmedLine = line.trimmingCharacters(in: .whitespaces)
9797

9898
if trimmedLine.starts(with: "- Parameters") {
99-
hasFoundParameterList = true
99+
currentSection = .parameters
100100
} else if trimmedLine.starts(with: "- Parameter") {
101-
// If it's only a parameter it's information is inline eith the parameter
101+
// If it's only a parameter it's information is inline with the parameter
102102
// tag, just after the ':'.
103103
guard let index = trimmedLine.firstIndex(of: ":") else { continue }
104104
let name = trimmedLine.dropFirst("- Parameter".count)[..<index]
105105
.trimmingCharacters(in: .init(charactersIn: " -:"))
106106
let summary = trimmedLine[index...].trimmingCharacters(in: .init(charactersIn: " -:"))
107107
params.append(ParseComment.Parameter(name: name, summary: summary))
108+
} else if trimmedLine.starts(with: "- Throws:") {
109+
currentSection = .throwsDescription
110+
throwsDescription = String(trimmedLine.dropFirst("- Throws:".count))
108111
} else if trimmedLine.starts(with: "- Returns:") {
109-
hasFoundParameterList = false
110-
hasFoundReturn = true
112+
currentSection = .returnsDescription
111113
returnsDescription = String(trimmedLine.dropFirst("- Returns:".count))
112-
} else if hasFoundParameterList {
113-
// After the paramters tag is found the following lines should be the parameters
114-
// description.
115-
guard let index = trimmedLine.firstIndex(of: ":") else { continue }
116-
let name = trimmedLine[..<index].trimmingCharacters(in: .init(charactersIn: " -:"))
117-
let summary = trimmedLine[index...].trimmingCharacters(in: .init(charactersIn: " -:"))
118-
params.append(ParseComment.Parameter(name: name, summary: summary))
119-
} else if hasFoundReturn {
120-
// Appends the return description that is not inline with the return tag.
121-
returnsDescription!.append(trimmedLine)
122-
} else if trimmedLine != "" {
123-
commentParagraphs.append(" " + trimmedLine)
114+
} else {
115+
switch currentSection {
116+
case .parameters:
117+
// After the paramters tag is found the following lines should be the parameters
118+
// description.
119+
guard let index = trimmedLine.firstIndex(of: ":") else { continue }
120+
let name = trimmedLine[..<index].trimmingCharacters(in: .init(charactersIn: " -:"))
121+
let summary = trimmedLine[index...].trimmingCharacters(in: .init(charactersIn: " -:"))
122+
params.append(ParseComment.Parameter(name: name, summary: summary))
123+
124+
case .returnsDescription:
125+
// Appends the return description that is not inline with the return tag.
126+
returnsDescription!.append(trimmedLine)
127+
128+
case .throwsDescription:
129+
// Appends the throws description that is not inline with the throws tag.
130+
throwsDescription!.append(trimmedLine)
131+
132+
case .commentParagraphs:
133+
if trimmedLine != "" {
134+
commentParagraphs.append(" " + trimmedLine)
135+
}
136+
}
124137
}
125138
}
139+
126140
return ParseComment(
127141
oneSentenceSummary: oneSenteceSummary,
128142
commentParagraphs: commentParagraphs,
129143
parameters: params,
144+
throwsDescription: throwsDescription,
130145
returnsDescription: returnsDescription
131146
)
132147
}
133148
}
134149

150+
private enum DocCommentSection {
151+
case commentParagraphs
152+
case parameters
153+
case throwsDescription
154+
case returnsDescription
155+
}
156+
135157
struct ParseComment {
136158
struct Parameter {
137159
var name: String
@@ -141,5 +163,6 @@ struct ParseComment {
141163
var oneSentenceSummary: String?
142164
var commentParagraphs: [String]?
143165
var parameters: [Parameter]?
166+
var throwsDescription: String?
144167
var returnsDescription: String?
145168
}

‎Sources/SwiftFormatRules/ValidateDocumentationComments.swift

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,20 @@ public final class ValidateDocumentationComments: SyntaxLintRule {
2424

2525
public override func visit(_ node: InitializerDeclSyntax) -> SyntaxVisitorContinueKind {
2626
return checkFunctionLikeDocumentation(
27-
DeclSyntax(node), name: "init", parameters: node.parameters.parameterList)
27+
DeclSyntax(node), name: "init", parameters: node.parameters.parameterList, throwsOrRethrowsKeyword: node.throwsOrRethrowsKeyword)
2828
}
2929

3030
public override func visit(_ node: FunctionDeclSyntax) -> SyntaxVisitorContinueKind {
3131
return checkFunctionLikeDocumentation(
32-
DeclSyntax(node), name: node.identifier.text, parameters: node.signature.input.parameterList,
32+
DeclSyntax(node), name: node.identifier.text, parameters: node.signature.input.parameterList, throwsOrRethrowsKeyword: node.signature.throwsOrRethrowsKeyword,
3333
returnClause: node.signature.output)
3434
}
3535

3636
private func checkFunctionLikeDocumentation(
3737
_ node: DeclSyntax,
3838
name: String,
3939
parameters: FunctionParameterListSyntax,
40+
throwsOrRethrowsKeyword: TokenSyntax?,
4041
returnClause: ReturnClauseSyntax? = nil
4142
) -> SyntaxVisitorContinueKind {
4243
guard let declComment = node.docComment else { return .skipChildren }
@@ -57,6 +58,7 @@ public final class ValidateDocumentationComments: SyntaxLintRule {
5758
$0.trimmingCharacters(in: .whitespaces).starts(with: "- Parameters")
5859
}
5960

61+
validateThrows(throwsOrRethrowsKeyword, name: name, throwsDesc: commentInfo.throwsDescription)
6062
validateReturn(returnClause, name: name, returnDesc: commentInfo.returnsDescription)
6163
let funcParameters = funcParametersIdentifiers(in: parameters)
6264

@@ -95,6 +97,25 @@ public final class ValidateDocumentationComments: SyntaxLintRule {
9597
diagnose(.documentReturnValue(funcName: name), on: returnClause)
9698
}
9799
}
100+
101+
/// Ensures the function has throws documentation if it may actually throw
102+
/// an error.
103+
private func validateThrows(
104+
_ throwsOrRethrowsKeyword: TokenSyntax?,
105+
name: String,
106+
throwsDesc: String?
107+
) {
108+
// If a function is marked as `rethrows`, it doesn't have any errors of its
109+
// own that should be documented. So only require documentation for
110+
// functions marked `throws`.
111+
let needsThrowsDesc = throwsOrRethrowsKeyword?.tokenKind == .throwsKeyword
112+
113+
if !needsThrowsDesc && throwsDesc != nil {
114+
diagnose(.removeThrowsComment(funcName: name), on: throwsOrRethrowsKeyword)
115+
} else if needsThrowsDesc && throwsDesc == nil {
116+
diagnose(.documentErrorsThrown(funcName: name), on: throwsOrRethrowsKeyword)
117+
}
118+
}
98119
}
99120

100121
/// Iterates through every parameter of paramList and returns a list of the
@@ -153,4 +174,18 @@ extension Diagnostic.Message {
153174
"replace the singular inline form of 'Parameter' tag with a plural 'Parameters' tag "
154175
+ "and group each parameter as a nested list"
155176
)
177+
178+
public static func removeThrowsComment(funcName: String) -> Diagnostic.Message {
179+
return Diagnostic.Message(
180+
.warning,
181+
"remove the 'Throws' tag for non-throwing function \(funcName)"
182+
)
183+
}
184+
185+
public static func documentErrorsThrown(funcName: String) -> Diagnostic.Message {
186+
return Diagnostic.Message(
187+
.warning,
188+
"add a 'Throws' tag describing the errors thrown by \(funcName)"
189+
)
190+
}
156191
}

‎Tests/SwiftFormatRulesTests/ValidateDocumentationCommentsTests.swift

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,38 @@ final class ValidateDocumentationCommentsTests: LintOrFormatRuleTestCase {
6161
XCTAssertDiagnosed(.parametersDontMatch(funcName: "foo"))
6262
}
6363

64+
func testThrowsDocumentation() {
65+
let input =
66+
"""
67+
/// One sentence summary.
68+
///
69+
/// - Parameters:
70+
/// - p1: Parameter 1.
71+
/// - p2: Parameter 2.
72+
/// - p3: Parameter 3.
73+
/// - Throws: an error.
74+
func doesNotThrow(p1: Int, p2: Int, p3: Int) {}
75+
76+
/// One sentence summary.
77+
///
78+
/// - Parameters:
79+
/// - p1: Parameter 1.
80+
/// - p2: Parameter 2.
81+
/// - p3: Parameter 3.
82+
func doesThrow(p1: Int, p2: Int, p3: Int) throws {}
83+
84+
/// One sentence summary.
85+
///
86+
/// - Parameter p1: Parameter 1.
87+
/// - Throws: doesn't really throw, just rethrows
88+
func doesRethrow(p1: (() throws -> ())) rethrows {}
89+
"""
90+
performLint(ValidateDocumentationComments.self, input: input)
91+
XCTAssertDiagnosed(.removeThrowsComment(funcName: "doesNotThrow"))
92+
XCTAssertDiagnosed(.documentErrorsThrown(funcName: "doesThrow"))
93+
XCTAssertDiagnosed(.removeThrowsComment(funcName: "doesRethrow"))
94+
}
95+
6496
func testReturnDocumentation() {
6597
let input =
6698
"""
@@ -104,9 +136,17 @@ final class ValidateDocumentationCommentsTests: LintOrFormatRuleTestCase {
104136
/// - Parameters:
105137
/// - command: The command to execute in the shell environment.
106138
/// - stdin: The string to use as standard input.
139+
/// - Throws: An error, possibly.
107140
/// - Returns: A string containing the contents of the invoked process's
108141
/// standard output.
109-
func pluralParam(command: String, stdin: String) -> String {
142+
func pluralParam(command: String, stdin: String) throws -> String {
143+
// ...
144+
}
145+
146+
/// One sentence summary.
147+
///
148+
/// - Parameter p1: Parameter 1.
149+
func rethrower(p1: (() throws -> ())) rethrows {
110150
// ...
111151
}
112152
@@ -126,6 +166,11 @@ final class ValidateDocumentationCommentsTests: LintOrFormatRuleTestCase {
126166
XCTAssertNotDiagnosed(.documentReturnValue(funcName: "pluralParam"))
127167
XCTAssertNotDiagnosed(.removeReturnComment(funcName: "pluralParam"))
128168
XCTAssertNotDiagnosed(.parametersDontMatch(funcName: "pluralParam"))
169+
XCTAssertNotDiagnosed(.documentErrorsThrown(funcName: "pluralParam"))
170+
XCTAssertNotDiagnosed(.removeThrowsComment(funcName: "pluralParam"))
171+
172+
XCTAssertNotDiagnosed(.documentErrorsThrown(funcName: "pluralParam"))
173+
XCTAssertNotDiagnosed(.removeThrowsComment(funcName: "pluralParam"))
129174

130175
XCTAssertNotDiagnosed(.documentReturnValue(funcName: "ommitedFunc"))
131176
XCTAssertNotDiagnosed(.removeReturnComment(funcName: "ommitedFunc"))
@@ -207,6 +252,16 @@ final class ValidateDocumentationCommentsTests: LintOrFormatRuleTestCase {
207252
init(label command: String, label2 stdin: String) {
208253
// ...
209254
}
255+
256+
/// Brief summary.
257+
///
258+
/// - Parameters:
259+
/// - command: The command to execute in the shell environment.
260+
/// - stdin: The string to use as standard input.
261+
/// - Throws: An error.
262+
init(label command: String, label2 stdin: String) throws {
263+
// ...
264+
}
210265
}
211266
"""
212267
performLint(ValidateDocumentationComments.self, input: input)
@@ -223,5 +278,11 @@ final class ValidateDocumentationCommentsTests: LintOrFormatRuleTestCase {
223278
XCTAssertNotDiagnosed(.documentReturnValue(funcName: "init"))
224279
XCTAssertNotDiagnosed(.removeReturnComment(funcName: "init"))
225280
XCTAssertNotDiagnosed(.parametersDontMatch(funcName: "init"))
281+
282+
XCTAssertNotDiagnosed(.documentReturnValue(funcName: "init"))
283+
XCTAssertNotDiagnosed(.removeReturnComment(funcName: "init"))
284+
XCTAssertNotDiagnosed(.parametersDontMatch(funcName: "init"))
285+
XCTAssertNotDiagnosed(.documentErrorsThrown(funcName: "init"))
286+
XCTAssertNotDiagnosed(.removeThrowsComment(funcName: "init"))
226287
}
227288
}

‎Tests/SwiftFormatRulesTests/XCTestManifests.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,7 @@ extension ValidateDocumentationCommentsTests {
398398
("testInitializer", testInitializer),
399399
("testParameterDocumentation", testParameterDocumentation),
400400
("testParametersName", testParametersName),
401+
("testThrowsDocumentation", testThrowsDocumentation),
401402
("testReturnDocumentation", testReturnDocumentation),
402403
("testSeparateLabelAndIdentifier", testSeparateLabelAndIdentifier),
403404
("testValidDocumentation", testValidDocumentation),

0 commit comments

Comments
 (0)