Skip to content

fix: restore handling of zipfile: schema for neovim + yarn berry #862

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

Merged
merged 6 commits into from
Feb 2, 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
3 changes: 3 additions & 0 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,11 @@ jobs:
with:
node-version: ${{ matrix.node-version }}
cache: yarn
- run: corepack enable
- name: yarn install
run: yarn
- name: prepare yarnpnp
run: cd test-data/yarn-pnp && yarn
- name: build
run: yarn build
- name: Unittest
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/release.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ jobs:
with:
node-version: 18
registry-url: https://registry.npmjs.org
- run: corepack enable
- name: Lint and Test
if: ${{ steps.release.outputs.release_created }}
run: yarn && yarn build && yarn lint
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/size-limit.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ jobs:
with:
node-version: ${{ matrix.node-version }}
cache: yarn
- run: corepack enable
- name: yarn install
run: yarn
- name: build
Expand Down
85 changes: 85 additions & 0 deletions src/configuration/fileSchemes.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import { URI } from 'vscode-uri';
import * as lsp from 'vscode-languageserver';
import { beforeAll, beforeEach, afterAll, describe, it, expect } from 'vitest';
import { uri, createServer, position, TestLspServer, openDocumentAndWaitForDiagnostics, readContents, filePath, isWindows } from '../test-utils.js';
import { ZipfileURI } from '../utils/uri.js';

const ZIPFILE_URI = 'zipfile:///dir/foo.zip::path/file.ts';

describe('uri handling', () => {
it('parses zipfile:// uri', () => {
const parsed = URI.parse(ZIPFILE_URI);
expect(parsed.scheme).toBe('zipfile');
expect(parsed.authority).toBe('');
expect(parsed.path).toBe('/dir/foo.zip::path/file.ts');
expect(parsed.fsPath).toBe(isWindows ? '\\dir\\foo.zip::path\\file.ts' : '/dir/foo.zip::path/file.ts');
expect(parsed.query).toBe('');
expect(parsed.fragment).toBe('');
});

it('stringifies zipfile uri without encoding', () => {
const parsed = URI.parse(ZIPFILE_URI);
expect(parsed.toString(true)).toBe('zipfile:/dir/foo.zip::path/file.ts');
});

it('stringifies zipfile uri with encoding', () => {
const parsed = URI.parse(ZIPFILE_URI);
expect(parsed.toString()).toBe('zipfile:/dir/foo.zip%3A%3Apath/file.ts');
});
});

describe('zipfileuri handling', () => {
it('parses zipfile:// uri', () => {
const parsed = ZipfileURI.parse(ZIPFILE_URI);
expect(parsed.scheme).toBe('zipfile');
expect(parsed.authority).toBe('');
expect(parsed.path).toBe('/dir/foo.zip::path/file.ts');
expect(parsed.fsPath).toBe(isWindows ? '\\dir\\foo.zip::path\\file.ts' : '/dir/foo.zip::path/file.ts');
expect(parsed.query).toBe('');
expect(parsed.fragment).toBe('');
});

it('stringifies zipfile uri with and without encoding', () => {
const parsed = ZipfileURI.parse(ZIPFILE_URI);
expect(parsed.toString(true)).toBe('zipfile:///dir/foo.zip::path/file.ts');
expect(parsed.toString()).toBe('zipfile:///dir/foo.zip::path/file.ts');
});
});

describe('neovim zipfile scheme handling with yarn pnp', () => {
let server: TestLspServer;

beforeAll(async () => {
server = await createServer({
rootUri: uri('yarn-pnp'),
initializationOptionsOverrides: {
hostInfo: 'neovim',
},
publishDiagnostics() {},
});
});

beforeEach(() => {
server.closeAllForTesting();
});

afterAll(() => {
server.closeAllForTesting();
server.shutdown();
});

it('returns zipfile: uri for definition inside node_modules', async () => {
const doc = {
uri: uri('yarn-pnp', 'testfile.ts'),
languageId: 'typescript',
version: 1,
text: readContents(filePath('yarn-pnp', 'testfile.ts')),
};
await openDocumentAndWaitForDiagnostics(server, doc);
const pos = position(doc, 'AxiosHeaderValue');
const results = await server.definition({ textDocument: doc, position: pos });
const defintion = Array.isArray(results) ? results[0] as lsp.Location : null;
expect(defintion).toBeDefined();
expect(defintion!.uri).toMatch(/zipfile:\/\/.+.zip::node_modules\/axios\/.+/);
});
});
15 changes: 15 additions & 0 deletions src/configuration/fileSchemes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ export const untitled = 'untitled';
export const git = 'git';
export const github = 'github';
export const azurerepos = 'azurerepos';
// Equivalent of "untitled" in Sublime Text.
export const buffer = 'buffer';
// For yarn berry support in neovim.
export const zipfile = 'zipfile';

/** Live share scheme */
export const vsls = 'vsls';
Expand All @@ -23,6 +27,17 @@ export const memFs = 'memfs';
export const vscodeVfs = 'vscode-vfs';
export const officeScript = 'office-script';

export function getSemanticSupportedSchemes(): string[] {
return [
file,
untitled,
buffer,
// walkThroughSnippet,
// vscodeNotebookCell,
zipfile,
];
}

/**
* File scheme for which JS/TS language feature should be disabled
*/
Expand Down
8 changes: 4 additions & 4 deletions src/diagnostic-queue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ export class DiagnosticEventQueue {
if (this.ignoredDiagnosticCodes.size) {
diagnostics = diagnostics.filter(diagnostic => !this.isDiagnosticIgnored(diagnostic));
}
const uri = this.client.toResource(file).toString();
const uri = this.client.toResourceUri(file);
const diagnosticsForFile = this.diagnostics.get(uri) || new FileDiagnostics(uri, this.publishDiagnostics, this.client, this.features);
diagnosticsForFile.update(kind, diagnostics);
this.diagnostics.set(uri, diagnosticsForFile);
Expand All @@ -98,12 +98,12 @@ export class DiagnosticEventQueue {
}

public getDiagnosticsForFile(file: string): lsp.Diagnostic[] {
const uri = this.client.toResource(file).toString();
const uri = this.client.toResourceUri(file);
return this.diagnostics.get(uri)?.getDiagnostics() || [];
}

public onDidCloseFile(file: string): void {
const uri = this.client.toResource(file).toString();
const uri = this.client.toResourceUri(file);
const diagnosticsForFile = this.diagnostics.get(uri);
diagnosticsForFile?.onDidClose();
this.diagnostics.delete(uri);
Expand All @@ -113,7 +113,7 @@ export class DiagnosticEventQueue {
* A testing function to clear existing file diagnostics, request fresh ones and wait for all to arrive.
*/
public async waitForDiagnosticsForTesting(file: string): Promise<void> {
const uri = this.client.toResource(file).toString();
const uri = this.client.toResourceUri(file);
let diagnosticsForFile = this.diagnostics.get(uri);
if (diagnosticsForFile) {
diagnosticsForFile.onDidClose();
Expand Down
2 changes: 1 addition & 1 deletion src/features/call-hierarchy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export function fromProtocolCallHierarchyItem(item: ts.server.protocol.CallHiera
kind: fromProtocolScriptElementKind(item.kind),
name,
detail,
uri: client.toResource(item.file).toString(),
uri: client.toResourceUri(item.file),
range: Range.fromTextSpan(item.span),
selectionRange: Range.fromTextSpan(item.selectionSpan),
};
Expand Down
2 changes: 1 addition & 1 deletion src/features/code-lens/implementationsCodeLens.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export default class TypeScriptImplementationsCodeLensProvider extends TypeScrip
const locations = response.body
.map(reference =>
// Only take first line on implementation: https://github.com/microsoft/vscode/issues/23924
Location.create(this.client.toResource(reference.file).toString(),
Location.create(this.client.toResourceUri(reference.file),
reference.start.line === reference.end.line
? typeConverters.Range.fromTextSpan(reference)
: Range.create(
Expand Down
2 changes: 1 addition & 1 deletion src/features/code-lens/referencesCodeLens.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export class TypeScriptReferencesCodeLensProvider extends TypeScriptBaseCodeLens
const locations = response.body.refs
.filter(reference => !reference.isDefinition)
.map(reference =>
typeConverters.Location.fromTextSpan(this.client.toResource(reference.file).toString(), reference));
typeConverters.Location.fromTextSpan(this.client.toResourceUri(reference.file), reference));

codeLens.command = {
title: this.getCodeLensLabel(locations),
Expand Down
2 changes: 1 addition & 1 deletion src/features/source-definition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export class SourceDefinitionCommand {
return;
}

const document = client.toOpenDocument(client.toResource(file).toString());
const document = client.toOpenDocument(client.toResourceUri(file));

if (!document) {
lspClient.showErrorMessage('Go to Source Definition failed. File not opened in the editor.');
Expand Down
15 changes: 8 additions & 7 deletions src/lsp-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ export class LspServer {
disableAutomaticTypingAcquisition,
maxTsServerMemory,
npmLocation,
hostInfo,
locale,
plugins: plugins || [],
onEvent: this.onTsEvent.bind(this),
Expand Down Expand Up @@ -550,7 +551,7 @@ export class LspServer {

async completionResolve(item: lsp.CompletionItem, token?: lsp.CancellationToken): Promise<lsp.CompletionItem> {
item.data = item.data?.cacheId !== undefined ? this.completionDataCache.get(item.data.cacheId) : item.data;
const uri = this.tsClient.toResource(item.data.file).toString();
const uri = this.tsClient.toResourceUri(item.data.file);
const document = item.data?.file ? this.tsClient.toOpenDocument(uri) : undefined;
if (!document) {
return item;
Expand Down Expand Up @@ -636,7 +637,7 @@ export class LspServer {
const changes: lsp.WorkspaceEdit['changes'] = {};
result.locs
.forEach((spanGroup) => {
const uri = this.tsClient.toResource(spanGroup.file).toString();
const uri = this.tsClient.toResourceUri(spanGroup.file);
const textEdits = changes[uri] || (changes[uri] = []);

spanGroup.locs.forEach((textSpan) => {
Expand Down Expand Up @@ -868,7 +869,7 @@ export class LspServer {
if (renameLocation) {
await this.options.lspClient.rename({
textDocument: {
uri: this.tsClient.toResource(args.file).toString(),
uri: this.tsClient.toResourceUri(args.file),
},
position: Position.fromLocation(renameLocation),
});
Expand All @@ -878,7 +879,7 @@ export class LspServer {
this.tsClient.configurePlugin(pluginName, configuration);
} else if (params.command === Commands.ORGANIZE_IMPORTS && params.arguments) {
const file = params.arguments[0] as string;
const uri = this.tsClient.toResource(file).toString();
const uri = this.tsClient.toResourceUri(file);
const document = this.tsClient.toOpenDocument(uri);
if (!document) {
return;
Expand Down Expand Up @@ -944,7 +945,7 @@ export class LspServer {
}
const changes: { [uri: string]: lsp.TextEdit[]; } = {};
for (const edit of edits) {
changes[this.tsClient.toResource(edit.fileName).toString()] = edit.textChanges.map(toTextEdit);
changes[this.tsClient.toResourceUri(edit.fileName)] = edit.textChanges.map(toTextEdit);
}
const { applied } = await this.options.lspClient.applyWorkspaceEdit({
edit: { changes },
Expand All @@ -957,7 +958,7 @@ export class LspServer {
for (const rename of params.files) {
const codeEdits = await this.getEditsForFileRename(rename.oldUri, rename.newUri, token);
for (const codeEdit of codeEdits) {
const uri = this.tsClient.toResource(codeEdit.fileName).toString();
const uri = this.tsClient.toResourceUri(codeEdit.fileName);
const textEdits = changes[uri] || (changes[uri] = []);
textEdits.push(...codeEdit.textChanges.map(toTextEdit));
}
Expand Down Expand Up @@ -1061,7 +1062,7 @@ export class LspServer {
return response.body.map(item => {
return <lsp.SymbolInformation>{
location: {
uri: this.tsClient.toResource(item.file).toString(),
uri: this.tsClient.toResourceUri(item.file),
range: {
start: Position.fromLocation(item.start),
end: Position.fromLocation(item.end),
Expand Down
10 changes: 5 additions & 5 deletions src/protocol-translation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ import type { ts } from './ts-protocol.js';
import { Position, Range } from './utils/typeConverters.js';

export function toLocation(fileSpan: ts.server.protocol.FileSpan, client: TsClient): lsp.Location {
const uri = client.toResource(fileSpan.file);
const uri = client.toResourceUri(fileSpan.file);
return {
uri: uri.toString(),
uri,
range: {
start: Position.fromLocation(fileSpan.start),
end: Position.fromLocation(fileSpan.end),
Expand Down Expand Up @@ -125,11 +125,11 @@ export function toTextEdit(edit: ts.server.protocol.CodeEdit): lsp.TextEdit {
}

export function toTextDocumentEdit(change: ts.server.protocol.FileCodeEdits, client: TsClient): lsp.TextDocumentEdit {
const uri = client.toResource(change.fileName);
const document = client.toOpenDocument(uri.toString());
const uri = client.toResourceUri(change.fileName);
const document = client.toOpenDocument(uri);
return {
textDocument: {
uri: uri.toString(),
uri,
version: document?.version ?? null,
},
edits: change.textChanges.map(c => toTextEdit(c)),
Expand Down
5 changes: 4 additions & 1 deletion src/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ const DEFAULT_TEST_CLIENT_INITIALIZATION_OPTIONS: TypeScriptInitializationOption

const DEFAULT_WORKSPACE_SETTINGS: WorkspaceConfiguration = {};

export const isWindows = process.platform === 'win32';

export async function openDocumentAndWaitForDiagnostics(server: TestLspServer, textDocument: lsp.TextDocumentItem): Promise<void> {
server.didOpenTextDocument({ textDocument });
await server.waitForDiagnosticsForFile(textDocument.uri);
Expand Down Expand Up @@ -204,6 +206,7 @@ interface TestLspServerOptions {
rootUri: string | null;
publishDiagnostics: (args: lsp.PublishDiagnosticsParams) => void;
clientCapabilitiesOverride?: lsp.ClientCapabilities;
initializationOptionsOverrides?: TypeScriptInitializationOptions;
}

export async function createServer(options: TestLspServerOptions): Promise<TestLspServer> {
Expand All @@ -223,7 +226,7 @@ export async function createServer(options: TestLspServerOptions): Promise<TestL
rootUri: options.rootUri,
processId: 42,
capabilities: deepmerge(DEFAULT_TEST_CLIENT_CAPABILITIES, options.clientCapabilitiesOverride || {}),
initializationOptions: DEFAULT_TEST_CLIENT_INITIALIZATION_OPTIONS,
initializationOptions: deepmerge(DEFAULT_TEST_CLIENT_INITIALIZATION_OPTIONS, options.initializationOptionsOverrides || {}),
workspaceFolders: null,
});
server.updateWorkspaceSettings({});
Expand Down
Loading