Skip to content
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

Implement Client Rename Provider Settings #4950

Closed
wants to merge 20 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
b02df6f
registering new rename provider feature
Razmo99 Mar 26, 2023
d26601e
hints for dispose
Razmo99 Oct 13, 2023
d533c2b
implemented prepar rename symbol
Razmo99 Oct 13, 2023
056cb4b
added more detection and handeling for preparerenamesymbol
Razmo99 Oct 13, 2023
1c1a38e
removed substring to avoid confusion
Razmo99 Oct 13, 2023
e56a794
updating rename sumbol to new languageclient format
Razmo99 Mar 24, 2024
6ee4eb5
removed command register and not implemented method
Razmo99 Jun 4, 2024
6a27fd8
implemented renamesymbol as a feature and structured under languageCl…
Razmo99 Jun 4, 2024
6f28123
added logging. Now using forof. throw instead of promise.reject. remo…
Razmo99 Jun 4, 2024
9245c93
adjusting unhandled error preifx
Razmo99 Jun 6, 2024
a48c88e
added new option to specifiy if an Alias should be generated when a f…
Razmo99 Jun 7, 2024
0b497f6
renaming ShouldGenerateAlias to create CreateAlias
Razmo99 Jun 10, 2024
4a2196c
Move RenameSymbolSettings to RenameSymbol.ts
JustinGrote Sep 12, 2024
8b3b660
Revert Settings, I was wrong and created a circular dependency, oops
JustinGrote Sep 12, 2024
df907ec
Add Rename Disclaimer. Fully detailed disclaimer to come next
JustinGrote Sep 12, 2024
81e2e99
Add First-Run rename disclaimer
JustinGrote Sep 12, 2024
8e456e1
Refresh config when fetching after update
JustinGrote Sep 12, 2024
76ba1dd
Refine format to use Unicode
JustinGrote Sep 12, 2024
fb60faf
Add note about unsaved files
JustinGrote Sep 12, 2024
627ec9b
Adjusted VScode renameSymbol requests arguments to support new LSP fr…
Razmo99 Sep 27, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions media/RenameDisclaimer.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
⚠️⚠️⚠️ PowerShell Extension Rename Disclaimer ⚠️⚠️⚠️

PowerShell is not a statically typed language. As such, the renaming of functions, parameters, and other symbols can only be done on a best effort basis. While this is sufficient for the majority of use cases, it cannot be relied upon to find all instances of a symbol and rename them across an entire code base such as in C# or TypeScript.

There are several edge case scenarios which may exist where rename is difficult or impossible, or unable to be determined due to the dynamic scoping nature of PowerShell.

🤚🤚 Unsupported Scenarios

❌ Renaming can only be done within a single file. Renaming symbols across multiple files is not supported.
❌ Unsaved/Virtual files are currently not supported, you must save a file to disk before you can rename values.

👍👍 Implemented and Tested Rename Scenaiors

See the supported rename scenarios we are currently testing at:
https://github.com/PowerShell/PowerShellEditorServices/blob/main/test/PowerShellEditorServices.Test.Shared/Refactoring

📄📄 Filing a Rename Issue

If there is a rename scenario you feel can be reasonably supported in PowerShell, please file a bug report in the PowerShellEditorServices repository with the "Expected" and "Actual" being the before and after rename. We will evaluate it and accept or reject it and give reasons why. Items that fall under the Unsupported Scenarios above will be summarily rejected, however that does not mean that they may not be supported in the future if we come up with a reasonably safe way to implement a scenario.
12 changes: 11 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -1011,7 +1011,17 @@
"verbose"
],
"default": "off",
"markdownDescription": "Traces the communication between VS Code and the PowerShell Editor Services language server. **This setting is only meant for extension developers!**"
"description": "Traces the communication between VS Code and the PowerShell Editor Services language server. **This setting is only meant for extension developers!**"
},
"powershell.renameSymbol.createAlias": {
"type": "boolean",
"default": true,
"description": "If set, an [Alias] attribute will be added when renaming a function parameter so that the previous parameter name still works. This helps avoid breaking changes."
},
"powershell.renameSymbol.acceptRenameDisclaimer": {
"type": "boolean",
"default": false,
"description": "Accepts the disclaimer about risks and limitations to the PowerShell rename functionality"
}
}
},
Expand Down
2 changes: 2 additions & 0 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { SessionManager } from "./session";
import { LogLevel, getSettings } from "./settings";
import { PowerShellLanguageId } from "./utils";
import { LanguageClientConsumer } from "./languageClientConsumer";
import { RenameSymbolFeature } from "./features/RenameSymbol";

// The most reliable way to get the name and version of the current extension.
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-var-requires
Expand Down Expand Up @@ -151,6 +152,7 @@ export async function activate(context: vscode.ExtensionContext): Promise<IPower
new RemoteFilesFeature(),
new DebugSessionFeature(context, sessionManager, logger),
new HelpCompletionFeature(),
new RenameSymbolFeature(documentSelector,logger)
];

sessionManager.setLanguageClientConsumers(languageClientConsumers);
Expand Down
190 changes: 190 additions & 0 deletions src/features/RenameSymbol.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import vscode = require("vscode");
import { RequestType, TextDocumentIdentifier } from "vscode-languageclient";
import { LanguageClientConsumer } from "../languageClientConsumer";
import { RenameProvider, WorkspaceEdit, TextDocument, CancellationToken, Position,Uri,Range, DocumentSelector } from "vscode";
import { LanguageClient } from "vscode-languageclient/node";
import { ILogger } from "../logging";
// eslint-disable-next-line @typescript-eslint/no-empty-interface

interface IRenameSymbolRequestArguments {
TextDocument:TextDocumentIdentifier
Position:Position
NewName:string
}
interface IPrepareRenameSymbolRequestArguments {
TextDocument:TextDocumentIdentifier
Position:Position
NewName:string
}

// eslint-disable-next-line @typescript-eslint/no-empty-interface
interface TextChange {

newText: string;
startLine: number;
startColumn: number;
endLine: number;
endColumn: number;
}
interface ModifiedFileResponse{
fileName: string;
changes : TextChange[]
}

interface IRenameSymbolRequestResponse {
changes : ModifiedFileResponse[]
}

interface IPrepareRenameSymbolRequestResponse {
message : string
}


const RenameSymbolRequestType = new RequestType<IRenameSymbolRequestArguments, IRenameSymbolRequestResponse, void>("textDocument/rename");
const PrepareRenameSymbolRequestType = new RequestType<IPrepareRenameSymbolRequestArguments, IPrepareRenameSymbolRequestResponse, void>("textDocument/prepareRename");

export class RenameSymbolFeature extends LanguageClientConsumer implements RenameProvider {
private languageRenameProvider:vscode.Disposable;
// Used to singleton the disclaimer prompt in case multiple renames are triggered
private disclaimerPromise?: Promise<boolean>;

constructor(documentSelector:DocumentSelector,private logger: ILogger){
super();

this.languageRenameProvider = vscode.languages.registerRenameProvider(documentSelector,this);
}

// eslint-disable-next-line @typescript-eslint/no-empty-function
public override onLanguageClientSet(_languageClient: LanguageClient): void {}

public async provideRenameEdits(document: TextDocument, position: Position, newName: string, _token: CancellationToken): Promise<WorkspaceEdit | undefined | null> {

const disclaimerAccepted = await this.acknowledgeDisclaimer();
if (!disclaimerAccepted) {return undefined;}

const req:IRenameSymbolRequestArguments = {
TextDocument : {uri:document.uri.toString()},
Position : position,
NewName : newName
};

try {
const client = await LanguageClientConsumer.getLanguageClient();
const response = await client.sendRequest(RenameSymbolRequestType, req);

if (!response.changes.length) {
return undefined;
}

const edit = new WorkspaceEdit();
for (const file of response.changes) {
const uri = Uri.file(file.fileName);
for (const change of file.changes) {
edit.replace(
uri,
new Range(change.startLine, change.startColumn, change.endLine, change.endColumn),
change.newText
);
}
}
return edit;
} catch (error) {
return undefined;
}
}

public async prepareRename(
document: vscode.TextDocument,
position: vscode.Position,
_token: vscode.CancellationToken
): Promise<vscode.Range | {range: vscode.Range; placeholder: string;} | undefined | null> {

const disclaimerAccepted = await this.acknowledgeDisclaimer();
if (!disclaimerAccepted) {return undefined;}

const req:IRenameSymbolRequestArguments = {
TextDocument : {uri:document.uri.toString()},
Position : position,
NewName : ""
};

try {
const client = await LanguageClientConsumer.getLanguageClient();
const response = await client.sendRequest(PrepareRenameSymbolRequestType, req);

if (!response.message) {
return null;
}
const wordRange = document.getWordRangeAtPosition(position);
if (!wordRange) {
throw new Error("Not a valid location for renaming.");

}
const wordText = document.getText(wordRange);
if (response.message) {
throw new Error(response.message);
}

return {
range: wordRange,
placeholder: wordText
};
}catch (error) {
const msg = `RenameSymbol: ${error}`;
this.logger.writeError(msg);
throw new Error(msg);
}
}


/** Prompts the user to acknowledge the risks inherent with the rename provider and does not proceed until it is accepted */
async acknowledgeDisclaimer(): Promise<boolean> {
if (!this.disclaimerPromise) {
this.disclaimerPromise = this.acknowledgeDisclaimerImpl();
}
return this.disclaimerPromise;
}

/** This is a separate function so that it only runs once as a singleton and the promise only resolves once */
async acknowledgeDisclaimerImpl(): Promise<boolean>
{
const config = vscode.workspace.getConfiguration();
const acceptRenameDisclaimer = config.get<boolean>("powershell.renameSymbol.acceptRenameDisclaimer", false);

if (!acceptRenameDisclaimer) {
const extensionPath = vscode.extensions.getExtension("ms-vscode.PowerShell")?.extensionPath;
const disclaimerPath = vscode.Uri.file(`${extensionPath}/media/RenameDisclaimer.txt`);

const result = await vscode.window.showWarningMessage(
//TODO: Provide a link to a markdown document that appears in the editor window, preferably one hosted with the extension itself.
`The PowerShell Rename functionality has limitations and risks, please [review the disclaimer](${disclaimerPath}).`,
"I Accept",
"I Accept [Workspace]",
"No"
);

switch (result) {
case "I Accept":
await config.update("powershell.renameSymbol.acceptRenameDisclaimer", true, vscode.ConfigurationTarget.Global);
break;
case "I Accept [Workspace]":
await config.update("powershell.renameSymbol.acceptRenameDisclaimer", true, vscode.ConfigurationTarget.Workspace);
break;
default:
void vscode.window.showInformationMessage("Rename operation cancelled and rename has been disabled until the extension is restarted.");
break;
}
}

// Refresh the config to ensure it was set
return vscode.workspace.getConfiguration().get<boolean>("powershell.renameSymbol.acceptRenameDisclaimer", false);
}

public dispose(): void {
this.languageRenameProvider.dispose();
}

}
9 changes: 8 additions & 1 deletion src/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import path = require("path");
// Perhaps we just get rid of this entirely?

// eslint-disable-next-line @typescript-eslint/no-extraneous-class
class PartialSettings { }
export class PartialSettings { }

export class Settings extends PartialSettings {
powerShellAdditionalExePaths: PowerShellAdditionalExePathSettings = {};
Expand All @@ -39,6 +39,7 @@ export class Settings extends PartialSettings {
cwd = ""; // NOTE: use validateCwdSetting() instead of this directly!
enableReferencesCodeLens = true;
analyzeOpenDocumentsOnly = false;
renameSymbol = new RenameSymbolSettings();
// TODO: Add (deprecated) useX86Host (for testing)
}

Expand Down Expand Up @@ -155,6 +156,12 @@ class ButtonSettings extends PartialSettings {
showPanelMovementButtons = false;
}

class RenameSymbolSettings extends PartialSettings {
createAlias = true;
acceptRenameDisclaimer = false;
}


// This is a recursive function which unpacks a WorkspaceConfiguration into our settings.
function getSetting<TSetting>(key: string | undefined, value: TSetting, configuration: vscode.WorkspaceConfiguration): TSetting {
// Base case where we're looking at a primitive type (or our special record).
Expand Down