-
Notifications
You must be signed in to change notification settings - Fork 307
Add refactoring actions for adding a test targets and products to a package manifest #1193
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
d11230c
531d577
0830a95
5428b9c
f6d286e
e540f16
502c9a8
3674e7f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add test cases for these refactorings? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in 4c1e119 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,243 @@ | ||
//===----------------------------------------------------------------------===// | ||
// | ||
// 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 LanguageServerProtocol | ||
import PackageModel | ||
import PackageModelSyntax | ||
import SwiftRefactor | ||
import SwiftSyntax | ||
|
||
/// Syntactic code action provider to provide refactoring actions that | ||
/// edit a package manifest. | ||
struct PackageManifestEdits: SyntaxCodeActionProvider { | ||
static func codeActions(in scope: SyntaxCodeActionScope) -> [CodeAction] { | ||
guard let token = scope.firstToken, | ||
let call = token.findEnclosingCall() | ||
else { | ||
return [] | ||
} | ||
|
||
return addTestTargetActions(call: call, in: scope) + addProductActions(call: call, in: scope) | ||
} | ||
|
||
/// Produce code actions to add test target(s) if we are currently on | ||
/// a target for which we know how to create a test. | ||
static func addTestTargetActions( | ||
call: FunctionCallExprSyntax, | ||
in scope: SyntaxCodeActionScope | ||
) -> [CodeAction] { | ||
guard let calledMember = call.findMemberAccessCallee(), | ||
targetsThatAllowTests.contains(calledMember), | ||
let targetName = call.findStringArgument(label: "name") | ||
else { | ||
return [] | ||
} | ||
|
||
do { | ||
// Describe the target we are going to create. | ||
let target = try TargetDescription( | ||
name: "\(targetName)Tests", | ||
dependencies: [.byName(name: targetName, condition: nil)], | ||
type: .test | ||
) | ||
|
||
let edits = try AddTarget.addTarget(target, to: scope.file) | ||
return [ | ||
CodeAction( | ||
title: "Add test target", | ||
kind: .refactor, | ||
edit: edits.asWorkspaceEdit(snapshot: scope.snapshot) | ||
) | ||
] | ||
Comment on lines
+53
to
+60
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it make sense to do some rough check whether a test target with that name already exists? I think even if we just do a textual search for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we're going to do that check, it should happen down in the SwiftPM code. Personally, I'm not sure... someone might want to use the action and then go rename the text. Are we helping them by taking the action away? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That’s a good point. Let’s leave it for now 👍🏽 |
||
} catch { | ||
return [] | ||
} | ||
} | ||
|
||
/// A list of target kinds that allow the creation of tests. | ||
static let targetsThatAllowTests: Set<String> = [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know this isn't particularly performance-sensitive, but this should probably be an Array. A Set is overkill. |
||
"executableTarget", | ||
"macro", | ||
"target", | ||
] | ||
|
||
/// Produce code actions to add a product if we are currently on | ||
/// a target for which we can create a product. | ||
static func addProductActions( | ||
call: FunctionCallExprSyntax, | ||
in scope: SyntaxCodeActionScope | ||
) -> [CodeAction] { | ||
guard let calledMember = call.findMemberAccessCallee(), | ||
targetsThatAllowProducts.contains(calledMember), | ||
let targetName = call.findStringArgument(label: "name") | ||
else { | ||
return [] | ||
} | ||
|
||
do { | ||
let type: ProductType = | ||
calledMember == "executableTarget" | ||
? .executable | ||
: .library(.automatic) | ||
|
||
// Describe the target we are going to create. | ||
let product = try ProductDescription( | ||
name: targetName, | ||
type: type, | ||
targets: [targetName] | ||
) | ||
|
||
let edits = try AddProduct.addProduct(product, to: scope.file) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar here, do you think there’s a way to check if we already have a product for this target? I think textual search wouldn’t work here. |
||
return [ | ||
CodeAction( | ||
title: "Add product to export this target", | ||
kind: .refactor, | ||
edit: edits.asWorkspaceEdit(snapshot: scope.snapshot) | ||
) | ||
] | ||
} catch { | ||
return [] | ||
} | ||
} | ||
|
||
/// A list of target kinds that allow the creation of tests. | ||
static let targetsThatAllowProducts: Set<String> = [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. An Array would probably be better. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, you're right. I'll clean it up in a subsequent PR, because I'm drowning in unmerged PRs right now |
||
"executableTarget", | ||
"target", | ||
] | ||
} | ||
|
||
fileprivate extension PackageEditResult { | ||
/// Translate package manifest edits into a workspace edit. This can | ||
/// involve both modifications to the manifest file as well as the creation | ||
/// of new files. | ||
ahoppen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/// `snapshot` is the latest snapshot of the `Package.swift` file. | ||
func asWorkspaceEdit(snapshot: DocumentSnapshot) -> WorkspaceEdit { | ||
// The edits to perform on the manifest itself. | ||
let manifestTextEdits = manifestEdits.map { edit in | ||
TextEdit( | ||
range: snapshot.range(of: edit.range), | ||
newText: edit.replacement | ||
) | ||
} | ||
|
||
// If we couldn't figure out the manifest directory, or there are no | ||
// files to add, the only changes are the manifest edits. We're done | ||
// here. | ||
let manifestDirectoryURL = snapshot.uri.fileURL? | ||
.deletingLastPathComponent() | ||
guard let manifestDirectoryURL, !auxiliaryFiles.isEmpty else { | ||
return WorkspaceEdit( | ||
changes: [snapshot.uri: manifestTextEdits] | ||
) | ||
} | ||
|
||
// Use the more full-featured documentChanges, which takes precedence | ||
// over the individual changes to documents. | ||
var documentChanges: [WorkspaceEditDocumentChange] = [] | ||
|
||
// Put the manifest changes into the array. | ||
documentChanges.append( | ||
.textDocumentEdit( | ||
TextDocumentEdit( | ||
textDocument: .init(snapshot.uri, version: snapshot.version), | ||
edits: manifestTextEdits.map { .textEdit($0) } | ||
) | ||
) | ||
) | ||
|
||
// Create an populate all of the auxiliary files. | ||
for (relativePath, contents) in auxiliaryFiles { | ||
guard | ||
let url = URL( | ||
string: relativePath.pathString, | ||
relativeTo: manifestDirectoryURL | ||
) | ||
else { | ||
continue | ||
} | ||
|
||
let documentURI = DocumentURI(url) | ||
let createFile = CreateFile( | ||
uri: documentURI | ||
) | ||
|
||
let zeroPosition = Position(line: 0, utf16index: 0) | ||
let edit = TextEdit( | ||
range: zeroPosition..<zeroPosition, | ||
newText: contents.description | ||
) | ||
|
||
documentChanges.append(.createFile(createFile)) | ||
documentChanges.append( | ||
.textDocumentEdit( | ||
TextDocumentEdit( | ||
textDocument: .init(documentURI, version: snapshot.version), | ||
edits: [.textEdit(edit)] | ||
) | ||
) | ||
) | ||
} | ||
|
||
return WorkspaceEdit( | ||
changes: [snapshot.uri: manifestTextEdits], | ||
documentChanges: documentChanges | ||
) | ||
} | ||
} | ||
|
||
fileprivate extension SyntaxProtocol { | ||
// Find an enclosing call syntax expression. | ||
func findEnclosingCall() -> FunctionCallExprSyntax? { | ||
var current = Syntax(self) | ||
while true { | ||
if let call = current.as(FunctionCallExprSyntax.self) { | ||
return call | ||
} | ||
|
||
if let parent = current.parent { | ||
current = parent | ||
continue | ||
} | ||
|
||
return nil | ||
} | ||
} | ||
} | ||
|
||
fileprivate extension FunctionCallExprSyntax { | ||
/// Find an argument with the given label that has a string literal as | ||
/// its argument. | ||
func findStringArgument(label: String) -> String? { | ||
for arg in arguments { | ||
if arg.label?.text == label { | ||
return arg.expression.as(StringLiteralExprSyntax.self)? | ||
.representedLiteralValue | ||
} | ||
} | ||
|
||
return nil | ||
} | ||
|
||
/// Find the callee when it is a member access expression referencing | ||
/// a declaration when a specific name. | ||
func findMemberAccessCallee() -> String? { | ||
guard | ||
let memberAccess = self.calledExpression | ||
.as(MemberAccessExprSyntax.self) | ||
else { | ||
return nil | ||
} | ||
|
||
return memberAccess.declName.baseName.text | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.