diff --git a/Sources/SourceKitLSP/CMakeLists.txt b/Sources/SourceKitLSP/CMakeLists.txt index 12558497a..3b9202799 100644 --- a/Sources/SourceKitLSP/CMakeLists.txt +++ b/Sources/SourceKitLSP/CMakeLists.txt @@ -22,6 +22,7 @@ target_sources(SourceKitLSP PRIVATE Clang/ClangLanguageService.swift) target_sources(SourceKitLSP PRIVATE Swift/AdjustPositionToStartOfIdentifier.swift + Swift/CodeActions/AddDocumentation.swift Swift/CodeActions/ConvertIntegerLiteral.swift Swift/CodeActions/PackageManifestEdits.swift Swift/CodeActions/SyntaxCodeActionProvider.swift diff --git a/Sources/SourceKitLSP/Swift/CodeActions/AddDocumentation.swift b/Sources/SourceKitLSP/Swift/CodeActions/AddDocumentation.swift new file mode 100644 index 000000000..3118913fd --- /dev/null +++ b/Sources/SourceKitLSP/Swift/CodeActions/AddDocumentation.swift @@ -0,0 +1,156 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +import SwiftParser +import SwiftRefactor +import SwiftSyntax + +/// Insert a documentation template associated with a function or macro. +/// +/// ## Before +/// +/// ```swift +/// static func refactor(syntax: DeclSyntax, in context: Void) -> DeclSyntax? {} +/// ``` +/// +/// ## After +/// +/// ```swift +/// /// +/// /// - Parameters: +/// /// - syntax: +/// /// - context: +/// /// - Returns: +/// static func refactor(syntax: DeclSyntax, in context: Void) -> DeclSyntax? {} +/// ``` +@_spi(Testing) +public struct AddDocumentation: EditRefactoringProvider { + @_spi(Testing) + public static func textRefactor(syntax: DeclSyntax, in context: Void) -> [SourceEdit] { + let hasDocumentation = syntax.leadingTrivia.contains(where: { trivia in + switch trivia { + case .blockComment(_), .docBlockComment(_), .lineComment(_), .docLineComment(_): + return true + default: + return false + } + }) + + guard !hasDocumentation else { + return [] + } + + let indentation = [.newlines(1)] + syntax.leadingTrivia.lastLineIndentation() + var content: [TriviaPiece] = [] + content.append(contentsOf: indentation) + content.append(.docLineComment("/// A description")) + + if let parameters = syntax.parameters?.parameters { + if let onlyParam = parameters.only { + let paramToken = onlyParam.secondName?.text ?? onlyParam.firstName.text + content.append(contentsOf: indentation) + content.append(.docLineComment("/// - Parameter \(paramToken):")) + } else { + content.append(contentsOf: indentation) + content.append(.docLineComment("/// - Parameters:")) + content.append( + contentsOf: parameters.flatMap({ param in + indentation + [ + .docLineComment("/// - \(param.secondName?.text ?? param.firstName.text):") + ] + }) + ) + content.append(contentsOf: indentation) + content.append(.docLineComment("///")) + } + } + + if syntax.throwsKeyword != nil { + content.append(contentsOf: indentation) + content.append(.docLineComment("/// - Throws:")) + } + + if syntax.returnType != nil { + content.append(contentsOf: indentation) + content.append(.docLineComment("/// - Returns:")) + } + + let insertPos = syntax.position + return [ + SourceEdit( + range: insertPos.. Trivia { + guard let lastNewline = pieces.lastIndex(where: { $0.isNewline }) else { + return self + } + + return Trivia(pieces: pieces[(lastNewline + 1)...]) + } +} diff --git a/Sources/SourceKitLSP/Swift/CodeActions/SyntaxCodeActions.swift b/Sources/SourceKitLSP/Swift/CodeActions/SyntaxCodeActions.swift index b4b2ce85c..15c86cf7c 100644 --- a/Sources/SourceKitLSP/Swift/CodeActions/SyntaxCodeActions.swift +++ b/Sources/SourceKitLSP/Swift/CodeActions/SyntaxCodeActions.swift @@ -15,6 +15,7 @@ import SwiftRefactor /// List of all of the syntactic code action providers, which can be used /// to produce code actions using only the swift-syntax tree of a file. let allSyntaxCodeActions: [SyntaxCodeActionProvider.Type] = [ + AddDocumentation.self, AddSeparatorsToIntegerLiteral.self, ConvertIntegerLiteral.self, FormatRawStringLiteral.self, diff --git a/Sources/SourceKitLSP/Swift/CodeActions/SyntaxRefactoringCodeActionProvider.swift b/Sources/SourceKitLSP/Swift/CodeActions/SyntaxRefactoringCodeActionProvider.swift index 2b725b421..1fb72e0ed 100644 --- a/Sources/SourceKitLSP/Swift/CodeActions/SyntaxRefactoringCodeActionProvider.swift +++ b/Sources/SourceKitLSP/Swift/CodeActions/SyntaxRefactoringCodeActionProvider.swift @@ -16,7 +16,7 @@ import SwiftSyntax /// Protocol that adapts a SyntaxRefactoringProvider (that comes from /// swift-syntax) into a SyntaxCodeActionProvider. -protocol SyntaxRefactoringCodeActionProvider: SyntaxCodeActionProvider, SyntaxRefactoringProvider { +protocol SyntaxRefactoringCodeActionProvider: SyntaxCodeActionProvider, EditRefactoringProvider { static var title: String { get } } @@ -31,20 +31,23 @@ extension SyntaxRefactoringCodeActionProvider where Self.Context == Void { return [] } - guard let refactored = Self.refactor(syntax: node) else { + let sourceEdits = Self.textRefactor(syntax: node) + if sourceEdits.isEmpty { return [] } - let edit = TextEdit( - range: scope.snapshot.range(of: node), - newText: refactored.description - ) + let textEdits = sourceEdits.map { edit in + TextEdit( + range: scope.snapshot.range(of: edit.range), + newText: edit.replacement + ) + } return [ CodeAction( title: Self.title, kind: .refactorInline, - edit: WorkspaceEdit(changes: [scope.snapshot.uri: [edit]]) + edit: WorkspaceEdit(changes: [scope.snapshot.uri: textEdits]) ) ] } diff --git a/Tests/SourceKitLSPTests/CodeActionTests.swift b/Tests/SourceKitLSPTests/CodeActionTests.swift index 657c5c78a..77763c4f3 100644 --- a/Tests/SourceKitLSPTests/CodeActionTests.swift +++ b/Tests/SourceKitLSPTests/CodeActionTests.swift @@ -348,7 +348,23 @@ final class CodeActionTests: XCTestCase { command: expectedCommand ) - XCTAssertEqual(result, .codeActions([expectedCodeAction])) + guard case .codeActions(var resultActions) = result else { + XCTFail("Result doesn't have code actions: \(String(describing: result))") + return + } + + // Filter out "Add documentation"; we test it elsewhere + if let addDocIndex = resultActions.firstIndex(where: { + $0.title == "Add documentation" + } + ) { + resultActions.remove(at: addDocIndex) + } else { + XCTFail("Missing 'Add documentation'.") + return + } + + XCTAssertEqual(resultActions, [expectedCodeAction]) } func testCodeActionsRemovePlaceholders() async throws { @@ -441,6 +457,36 @@ final class CodeActionTests: XCTestCase { try await fulfillmentOfOrThrow([editReceived]) } + func testAddDocumentationCodeActionResult() async throws { + let testClient = try await TestSourceKitLSPClient(capabilities: clientCapabilitiesWithCodeActionSupport()) + let uri = DocumentURI.for(.swift) + let positions = testClient.openDocument( + """ + 2️⃣func refacto1️⃣r(syntax: DeclSyntax, in context: Void) -> DeclSyntax? { }3️⃣ + """, + uri: uri + ) + + let testPosition = positions["1️⃣"] + let request = CodeActionRequest( + range: Range(testPosition), + context: .init(), + textDocument: TextDocumentIdentifier(uri) + ) + let result = try await testClient.send(request) + + guard case .codeActions(let codeActions) = result else { + XCTFail("Expected code actions") + return + } + + // Make sure we get an add-documentation action. + let addDocAction = codeActions.first { action in + return action.title == "Add documentation" + } + XCTAssertNotNil(addDocAction) + } + func testCodeActionForFixItsProducedBySwiftSyntax() async throws { let project = try await MultiFileTestProject(files: [ "test.swift": "protocol 1️⃣Multi 2️⃣ident 3️⃣{}", diff --git a/Tests/SourceKitLSPTests/PullDiagnosticsTests.swift b/Tests/SourceKitLSPTests/PullDiagnosticsTests.swift index c6b412792..b64013a1e 100644 --- a/Tests/SourceKitLSPTests/PullDiagnosticsTests.swift +++ b/Tests/SourceKitLSPTests/PullDiagnosticsTests.swift @@ -93,17 +93,18 @@ final class PullDiagnosticsTests: XCTestCase { return } - XCTAssertEqual(actions.count, 1) - let action = try XCTUnwrap(actions.first) - // Allow the action message to be the one before or after - // https://github.com/apple/swift/pull/67909, ensuring this test passes with - // a sourcekitd that contains the change from that PR as well as older - // toolchains that don't contain the change yet. + XCTAssertEqual(actions.count, 2) XCTAssert( - [ - "Add stubs for conformance", - "Do you want to add protocol stubs?", - ].contains(action.title) + actions.contains { action in + // Allow the action message to be the one before or after + // https://github.com/apple/swift/pull/67909, ensuring this test passes with + // a sourcekitd that contains the change from that PR as well as older + // toolchains that don't contain the change yet. + [ + "Add stubs for conformance", + "Do you want to add protocol stubs?", + ].contains(action.title) + } ) } diff --git a/Tests/SourceKitLSPTests/SyntaxRefactorTests.swift b/Tests/SourceKitLSPTests/SyntaxRefactorTests.swift new file mode 100644 index 000000000..f1207284b --- /dev/null +++ b/Tests/SourceKitLSPTests/SyntaxRefactorTests.swift @@ -0,0 +1,265 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2019 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +@_spi(Testing) import SourceKitLSP +import SwiftParser +import SwiftRefactor +import SwiftSyntax +import XCTest + +final class SyntaxRefactorTests: XCTestCase { + func testAddDocumentationRefactor() throws { + try assertRefactor( + """ + func refactor(syntax: DeclSyntax, in context: Void) -> DeclSyntax? { } + """, + context: (), + provider: AddDocumentation.self, + expected: [ + SourceEdit( + range: AbsolutePosition(utf8Offset: 0)..( + malformedInput input: String, + context: R.Context, + provider: R.Type, + expected: [SourceEdit], + file: StaticString = #filePath, + line: UInt = #line +) throws where R.Input == Syntax { + var parser = Parser(input) + let syntax = ExprSyntax.parse(from: &parser) + try assertRefactor( + Syntax(syntax), + context: context, + provider: provider, + expected: expected, + file: file, + line: line + ) +} + +// Borrowed from the swift-syntax library's SwiftRefactor tests. + +func assertRefactor( + _ input: R.Input, + context: R.Context, + provider: R.Type, + expected: [SourceEdit], + file: StaticString = #filePath, + line: UInt = #line +) throws { + let edits = R.textRefactor(syntax: input, in: context) + guard !edits.isEmpty else { + if !expected.isEmpty { + XCTFail( + """ + Refactoring produced empty result, expected: + \(expected) + """, + file: file, + line: line + ) + } + return + } + + if edits.count != expected.count { + XCTFail( + """ + Refactoring produced incorrect number of edits, expected \(expected.count) not \(edits.count). + + Actual: + \(edits.map({ $0.debugDescription }).joined(separator: "\n")) + + Expected: + \(expected.map({ $0.debugDescription }).joined(separator: "\n")) + + """, + file: file, + line: line + ) + return + } + + for (actualEdit, expectedEdit) in zip(edits, expected) { + XCTAssertEqual( + actualEdit, + expectedEdit, + "Incorrect edit, expected \(expectedEdit.debugDescription) but actual was \(actualEdit.debugDescription)", + file: file, + line: line + ) + assertStringsEqualWithDiff( + actualEdit.replacement, + expectedEdit.replacement, + file: file, + line: line + ) + } +} + +/// Asserts that the two strings are equal, providing Unix `diff`-style output if they are not. +/// +/// - Parameters: +/// - actual: The actual string. +/// - expected: The expected string. +/// - message: An optional description of the failure. +/// - additionalInfo: Additional information about the failed test case that will be printed after the diff +/// - file: The file in which failure occurred. Defaults to the file name of the test case in +/// which this function was called. +/// - line: The line number on which failure occurred. Defaults to the line number on which this +/// function was called. +public func assertStringsEqualWithDiff( + _ actual: String, + _ expected: String, + _ message: String = "", + additionalInfo: @autoclosure () -> String? = nil, + file: StaticString = #filePath, + line: UInt = #line +) { + if actual == expected { + return + } + failStringsEqualWithDiff( + actual, + expected, + message, + additionalInfo: additionalInfo(), + file: file, + line: line + ) +} + +/// Asserts that the two data are equal, providing Unix `diff`-style output if they are not. +/// +/// - Parameters: +/// - actual: The actual string. +/// - expected: The expected string. +/// - message: An optional description of the failure. +/// - additionalInfo: Additional information about the failed test case that will be printed after the diff +/// - file: The file in which failure occurred. Defaults to the file name of the test case in +/// which this function was called. +/// - line: The line number on which failure occurred. Defaults to the line number on which this +/// function was called. +public func assertDataEqualWithDiff( + _ actual: Data, + _ expected: Data, + _ message: String = "", + additionalInfo: @autoclosure () -> String? = nil, + file: StaticString = #filePath, + line: UInt = #line +) { + if actual == expected { + return + } + + // NOTE: Converting to `Stirng` here looses invalid UTF8 sequence difference, + // but at least we can see something is different. + failStringsEqualWithDiff( + String(decoding: actual, as: UTF8.self), + String(decoding: expected, as: UTF8.self), + message, + additionalInfo: additionalInfo(), + file: file, + line: line + ) +} + +/// `XCTFail` with `diff`-style output. +public func failStringsEqualWithDiff( + _ actual: String, + _ expected: String, + _ message: String = "", + additionalInfo: @autoclosure () -> String? = nil, + file: StaticString = #filePath, + line: UInt = #line +) { + let stringComparison: String + + // Use `CollectionDifference` on supported platforms to get `diff`-like line-based output. On + // older platforms, fall back to simple string comparison. + if #available(macOS 10.15, *) { + let actualLines = actual.components(separatedBy: .newlines) + let expectedLines = expected.components(separatedBy: .newlines) + + let difference = actualLines.difference(from: expectedLines) + + var result = "" + + var insertions = [Int: String]() + var removals = [Int: String]() + + for change in difference { + switch change { + case .insert(let offset, let element, _): + insertions[offset] = element + case .remove(let offset, let element, _): + removals[offset] = element + } + } + + var expectedLine = 0 + var actualLine = 0 + + while expectedLine < expectedLines.count || actualLine < actualLines.count { + if let removal = removals[expectedLine] { + result += "–\(removal)\n" + expectedLine += 1 + } else if let insertion = insertions[actualLine] { + result += "+\(insertion)\n" + actualLine += 1 + } else { + result += " \(expectedLines[expectedLine])\n" + expectedLine += 1 + actualLine += 1 + } + } + + stringComparison = result + } else { + // Fall back to simple message on platforms that don't support CollectionDifference. + stringComparison = """ + Expected: + \(expected) + + Actual: + \(actual) + """ + } + + var fullMessage = """ + \(message.isEmpty ? "Actual output does not match the expected" : message) + \(stringComparison) + """ + if let additional = additionalInfo() { + fullMessage = """ + \(fullMessage) + \(additional) + """ + } + XCTFail(fullMessage, file: file, line: line) +}