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

Fix handling files with non-Ascii characters by sending a TextDocumentIdentifier for dynamic file requests/responses #7442

Merged
merged 3 commits into from
Aug 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 6 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@
- Debug from .csproj and .sln [#5876](https://github.com/dotnet/vscode-csharp/issues/5876)

# Latest
* Bump Roslyn to 4.12.0-2.24408.4 (PR: [#7429](https://github.com/dotnet/vscode-csharp/pull/7429))
* Fix handling Razor files with non-ascii characters (PR: [#7442](https://github.com/dotnet/vscode-csharp/pull/7442))
* Bump Roslyn to 4.12.0-2.24413.5 (PR: [#7442](https://github.com/dotnet/vscode-csharp/pull/7442))
* Fix URI comparisons for different casing (PR: [#74746](https://github.com/dotnet/roslyn/pull/74746))
* Remove implicit unsafe cast in foreach(PR: [#74747](https://github.com/dotnet/roslyn/pull/74747))
* Send a TextDocumentidentifier for razor dynamic file requests/responses (PR: [#74727](https://github.com/dotnet/roslyn/pull/74727))
* Fix issues with VSCode LSP EA causing handlers to fail to load (PR: [#74700](https://github.com/dotnet/roslyn/pull/74700))
* Reduce allocations in SyntaxEquivalence.AreEquivalent by using a more appropriate pooling mechanism for the stack it uses to walk trees. (PR: [#74610](https://github.com/dotnet/roslyn/pull/74610))
* Reduce allocations in SyntaxNodeExtensions.GetMembers to instead execute a given lambda over the collection. (PR: [#74628](https://github.com/dotnet/roslyn/pull/74628))
* Modify ISyntaxFacts methods to allocate less (PR: [#74596](https://github.com/dotnet/roslyn/pull/74596))
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
}
},
"defaults": {
"roslyn": "4.12.0-2.24408.4",
"roslyn": "4.12.0-2.24413.5",
"omniSharp": "1.39.11",
"razor": "9.0.0-preview.24366.2",
"razorOmnisharp": "7.0.0-preview.23363.1",
Expand Down
11 changes: 9 additions & 2 deletions src/lsptoolshost/roslynLanguageServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ import { getComponentPaths } from './builtInComponents';
import { OnAutoInsertFeature } from './onAutoInsertFeature';
import { registerLanguageStatusItems } from './languageStatusBar';
import { ProjectContextService } from './services/projectContextService';
import { ProvideDynamicFileResponse } from '../razor/src/dynamicFile/provideDynamicFileResponse';
import { ProvideDynamicFileParams } from '../razor/src/dynamicFile/provideDynamicFileParams';

let _channel: vscode.OutputChannel;
let _traceChannel: vscode.OutputChannel;
Expand Down Expand Up @@ -730,11 +732,16 @@ export class RoslynLanguageServer {
};
}

private ProvideDyanmicFileInfoType: RequestType<ProvideDynamicFileParams, ProvideDynamicFileResponse, any> =
new RequestType(RoslynLanguageServer.provideRazorDynamicFileInfoMethodName);

private registerDynamicFileInfo() {
// When the Roslyn language server sends a request for Razor dynamic file info, we forward that request along to Razor via
// a command.
this._languageClient.onRequest(RoslynLanguageServer.provideRazorDynamicFileInfoMethodName, async (request) =>
vscode.commands.executeCommand(DynamicFileInfoHandler.provideDynamicFileInfoCommand, request)
this._languageClient.onRequest<ProvideDynamicFileParams, ProvideDynamicFileResponse, any>(
this.ProvideDyanmicFileInfoType,
async (request) =>
vscode.commands.executeCommand(DynamicFileInfoHandler.provideDynamicFileInfoCommand, request)
);
this._languageClient.onNotification(
RoslynLanguageServer.removeRazorDynamicFileInfoMethodName,
Expand Down
4 changes: 2 additions & 2 deletions src/lsptoolshost/services/projectContextService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,10 @@ export class ProjectContextService {
private async getVirtualCSharpUri(uri: vscode.Uri): Promise<vscode.Uri | undefined> {
const response = await vscode.commands.executeCommand<ProvideDynamicFileResponse>(
DynamicFileInfoHandler.provideDynamicFileInfoCommand,
new ProvideDynamicFileParams([uri.fsPath])
new ProvideDynamicFileParams({ uri: UriConverter.serialize(uri) })
);

const responseUri = response.generatedFiles[0];
const responseUri = response.csharpDocument?.uri;
if (!responseUri) {
return undefined;
}
Expand Down
2 changes: 1 addition & 1 deletion src/lsptoolshost/uriConverter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,6 @@ export class UriConverter {
}

public static deserialize(value: string): vscode.Uri {
return vscode.Uri.parse(value);
return vscode.Uri.parse(value, true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does true do here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's the strict parameter. It causes it to throw if the Uri doesn't have a schema

}
}
45 changes: 22 additions & 23 deletions src/razor/src/dynamicFile/dynamicFileInfoHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,23 +36,21 @@ export class DynamicFileInfoHandler {
}

// Given Razor document URIs, returns associated generated doc URIs
private async provideDynamicFileInfo(request: ProvideDynamicFileParams): Promise<ProvideDynamicFileResponse> {
const uris = request.razorFiles;
const virtualUris = new Array<DocumentUri | null>();
private async provideDynamicFileInfo(
request: ProvideDynamicFileParams
): Promise<ProvideDynamicFileResponse | null> {
let virtualUri: DocumentUri | null = null;
try {
for (const razorDocumentPath of uris) {
const vscodeUri = vscode.Uri.file(razorDocumentPath);
const razorDocument = await this.documentManager.getDocument(vscodeUri);
if (razorDocument === undefined) {
virtualUris.push(null);
this.logger.logWarning(
`Could not find Razor document ${razorDocumentPath}; adding null as a placeholder in URI array.`
);
} else {
// Retrieve generated doc URIs for each Razor URI we are given
const virtualCsharpUri = UriConverter.serialize(razorDocument.csharpDocument.uri);
virtualUris.push(virtualCsharpUri);
}
const vscodeUri = vscode.Uri.parse(request.razorDocument.uri, true);
const razorDocument = await this.documentManager.getDocument(vscodeUri);
if (razorDocument === undefined) {
this.logger.logWarning(
`Could not find Razor document ${vscodeUri.fsPath}; adding null as a placeholder in URI array.`
);
} else {
// Retrieve generated doc URIs for each Razor URI we are given
const virtualCsharpUri = UriConverter.serialize(razorDocument.csharpDocument.uri);
virtualUri = virtualCsharpUri;
}

this.documentManager.roslynActivated = true;
Expand All @@ -64,17 +62,18 @@ export class DynamicFileInfoHandler {
this.logger.logWarning(`${DynamicFileInfoHandler.provideDynamicFileInfoCommand} failed with ${error}`);
}

return new ProvideDynamicFileResponse(virtualUris);
if (virtualUri) {
return new ProvideDynamicFileResponse({ uri: virtualUri });
}

return null;
}

private async removeDynamicFileInfo(request: RemoveDynamicFileParams) {
try {
const uris = request.razorFiles;
for (const razorDocumentPath of uris) {
const vscodeUri = vscode.Uri.file(razorDocumentPath);
if (this.documentManager.isRazorDocumentOpenInCSharpWorkspace(vscodeUri)) {
this.documentManager.didCloseRazorCSharpDocument(vscodeUri);
}
const vscodeUri = UriConverter.deserialize(request.csharpDocument.uri);
if (this.documentManager.isRazorDocumentOpenInCSharpWorkspace(vscodeUri)) {
this.documentManager.didCloseRazorCSharpDocument(vscodeUri);
}
} catch (error) {
this.logger.logWarning(`${DynamicFileInfoHandler.removeDynamicFileInfoCommand} failed with ${error}`);
Expand Down
5 changes: 3 additions & 2 deletions src/razor/src/dynamicFile/provideDynamicFileParams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { DocumentUri } from 'vscode-languageclient/node';
import { TextDocumentIdentifier } from 'vscode-languageserver-protocol';

// matches https://github.com/dotnet/roslyn/blob/9e91ca6590450e66e0041ee3135bbf044ac0687a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/RazorDynamicFileInfoProvider.cs#L22
export class ProvideDynamicFileParams {
constructor(public readonly razorFiles: DocumentUri[]) {}
constructor(public readonly razorDocument: TextDocumentIdentifier) {}
}
5 changes: 3 additions & 2 deletions src/razor/src/dynamicFile/provideDynamicFileResponse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { DocumentUri } from 'vscode-languageclient/node';
import { TextDocumentIdentifier } from 'vscode-languageclient/node';

// matches https://github.com/dotnet/roslyn/blob/9e91ca6590450e66e0041ee3135bbf044ac0687a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/RazorDynamicFileInfoProvider.cs#L28
export class ProvideDynamicFileResponse {
constructor(public readonly generatedFiles: (DocumentUri | null)[]) {}
constructor(public readonly csharpDocument: TextDocumentIdentifier | null) {}
}
5 changes: 3 additions & 2 deletions src/razor/src/dynamicFile/removeDynamicFileParams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { DocumentUri } from 'vscode-languageclient/node';
import { TextDocumentIdentifier } from 'vscode-languageclient/node';

// matches https://github.com/dotnet/roslyn/blob/9e91ca6590450e66e0041ee3135bbf044ac0687a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/RazorDynamicFileInfoProvider.cs#L28
export class RemoveDynamicFileParams {
constructor(public readonly razorFiles: DocumentUri[]) {}
constructor(public readonly csharpDocument: TextDocumentIdentifier) {}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so ironically I named this wrong in the original PR 😅. I'll get that fixed later

}
Loading