Skip to content
This repository was archived by the owner on Oct 16, 2020. It is now read-only.

Represent URIs as WHATWG URLs #203

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open
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: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
"vscode-jsonrpc": "^3.1.0",
"vscode-languageserver": "^3.1.0",
"vscode-languageserver-types": "^3.0.3",
"whatwg-url": "^4.7.1",
"yarn": "^0.21.3"
},
"devDependencies": {
Expand All @@ -61,7 +62,7 @@
"@types/lodash": "4.14.63",
"@types/mocha": "2.2.41",
"@types/mz": "0.0.31",
"@types/node": "7.0.14",
"@types/node": "6.0.70",
"@types/object-hash": "0.5.28",
"@types/rimraf": "0.0.28",
"@types/sinon": "2.1.3",
Expand Down
65 changes: 37 additions & 28 deletions src/fs.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import * as fs from 'mz/fs';
import * as path from 'path';
import { LanguageClient } from './lang-handler';
import glob = require('glob');
import iterate from 'iterare';
import { Span } from 'opentracing';
import Semaphore from 'semaphore-async-await';
import { URL } from 'whatwg-url';
import { InMemoryFileSystem } from './memfs';
import { normalizeDir, path2uri, toUnixPath, uri2path } from './util';
import { path2uri, uri2path } from './util';

export interface FileSystem {
/**
Expand All @@ -15,15 +15,15 @@ export interface FileSystem {
* @param base A URI under which to search, resolved relative to the rootUri
* @return A promise that is fulfilled with an array of URIs
*/
getWorkspaceFiles(base?: string, childOf?: Span): Promise<Iterable<string>>;
getWorkspaceFiles(base?: URL, childOf?: Span): Promise<Iterable<URL>>;

/**
* Returns the content of a text document
*
* @param uri The URI of the text document, resolved relative to the rootUri
* @return A promise that is fulfilled with the text document content
*/
getTextDocumentContent(uri: string, childOf?: Span): Promise<string>;
getTextDocumentContent(uri: URL, childOf?: Span): Promise<string>;
}

export class RemoteFileSystem implements FileSystem {
Expand All @@ -34,45 +34,54 @@ export class RemoteFileSystem implements FileSystem {
* The files request is sent from the server to the client to request a list of all files in the workspace or inside the directory of the base parameter, if given.
* A language server can use the result to index files by filtering and doing a content request for each text document of interest.
*/
async getWorkspaceFiles(base?: string, childOf = new Span()): Promise<Iterable<string>> {
return iterate(await this.client.workspaceXfiles({ base }, childOf))
.map(textDocument => textDocument.uri);
async getWorkspaceFiles(base?: URL, childOf = new Span()): Promise<Iterable<URL>> {
return iterate(await this.client.workspaceXfiles({ base: base && base.href }, childOf))
.map(textDocument => new URL(textDocument.uri));
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we somehow made it consistent - either new URI across the code or path2uri?
there can be only one

Copy link
Contributor Author

Choose a reason for hiding this comment

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

path2uri for converting file paths, new URL for converting a string-URL to a URL object

Copy link
Contributor

Choose a reason for hiding this comment

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

ok

}

/**
* The content request is sent from the server to the client to request the current content of any text document. This allows language servers to operate without accessing the file system directly.
*/
async getTextDocumentContent(uri: string, childOf = new Span()): Promise<string> {
const textDocument = await this.client.textDocumentXcontent({ textDocument: { uri } }, childOf);
async getTextDocumentContent(uri: URL, childOf = new Span()): Promise<string> {
const textDocument = await this.client.textDocumentXcontent({ textDocument: { uri: uri.href } }, childOf);
return textDocument.text;
}
}

/**
* FileSystem implementation that reads from the local disk
*/
export class LocalFileSystem implements FileSystem {

/**
* @param rootPath The root directory path that relative URIs should be resolved to
* @param rootUri The workspace root URI that is used when no base is given
*/
constructor(private rootPath: string) {}
constructor(protected rootUri: URL) {}

/**
* Converts the URI to an absolute path
* Returns the file path where a given URI should be located on disk
*/
protected resolveUriToPath(uri: string): string {
return toUnixPath(path.resolve(this.rootPath, uri2path(uri)));
protected resolveUriToPath(uri: URL): string {
return uri2path(uri);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd remove resolveUriToPath method then and replace it with direct uri2path invocation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tried it (if you look at commit log) but it is useful for overriding it. Otherwise the globbing etc. needs to be completely reimplemented.

}

async getWorkspaceFiles(base?: string): Promise<Iterable<string>> {
// Even if no base provided, still need to call resolveUriToPath which may be overridden
const root = this.resolveUriToPath(base || path2uri('', this.rootPath));
const baseUri = path2uri('', normalizeDir(root)) + '/';
async getWorkspaceFiles(base: URL = this.rootUri): Promise<Iterable<URL>> {
const files = await new Promise<string[]>((resolve, reject) => {
glob('*', { cwd: root, nodir: true, matchBase: true }, (err, matches) => err ? reject(err) : resolve(matches));
glob('*', {
// Search the base directory
cwd: this.resolveUriToPath(base),
// Don't return directories
nodir: true,
// Search directories recursively
matchBase: true,
// Return absolute file paths
absolute: true
} as any, (err, matches) => err ? reject(err) : resolve(matches));
});
return iterate(files).map(file => baseUri + file.split('/').map(encodeURIComponent).join('/'));
return iterate(files).map(filePath => path2uri(base, filePath));
}

async getTextDocumentContent(uri: string): Promise<string> {
async getTextDocumentContent(uri: URL): Promise<string> {
return fs.readFile(this.resolveUriToPath(uri), 'utf8');
}
}
Expand Down Expand Up @@ -107,19 +116,19 @@ export class FileSystemUpdater {
* @param uri URI of the file to fetch
* @param childOf A parent span for tracing
*/
async fetch(uri: string, childOf = new Span()): Promise<void> {
async fetch(uri: URL, childOf = new Span()): Promise<void> {
// Limit concurrent fetches
const promise = this.concurrencyLimit.execute(async () => {
try {
const content = await this.remoteFs.getTextDocumentContent(uri);
this.inMemoryFs.add(uri, content);
this.inMemoryFs.getContent(uri);
} catch (err) {
this.fetches.delete(uri);
this.fetches.delete(uri.href);
throw err;
}
});
this.fetches.set(uri, promise);
this.fetches.set(uri.href, promise);
return promise;
}

Expand All @@ -130,8 +139,8 @@ export class FileSystemUpdater {
* @param uri URI of the file to ensure
* @param span An OpenTracing span for tracing
*/
ensure(uri: string, span = new Span()): Promise<void> {
return this.fetches.get(uri) || this.fetch(uri, span);
ensure(uri: URL, span = new Span()): Promise<void> {
return this.fetches.get(uri.href) || this.fetch(uri, span);
}

/**
Expand Down Expand Up @@ -176,8 +185,8 @@ export class FileSystemUpdater {
*
* @param uri URI of the file that changed
*/
invalidate(uri: string): void {
this.fetches.delete(uri);
invalidate(uri: URL): void {
this.fetches.delete(uri.href);
}

/**
Expand Down
59 changes: 32 additions & 27 deletions src/memfs.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import * as fs_ from 'fs';
import iterate from 'iterare';
import * as path_ from 'path';
import * as ts from 'typescript';
import { URL } from 'whatwg-url';
import { Logger, NoopLogger } from './logging';
import * as match from './match-files';
import * as util from './util';
Expand Down Expand Up @@ -44,7 +46,10 @@ export class InMemoryFileSystem implements ts.ParseConfigHost, ts.ModuleResoluti
*/
rootNode: FileSystemNode;

constructor(path: string, private logger: Logger = new NoopLogger()) {
/**
* @param rootUri The workspace root URI
*/
constructor(private rootUri: URL, path: string, private logger: Logger = new NoopLogger()) {
this.path = path;
this.overlay = new Map<string, string>();
this.rootNode = { file: false, children: new Map<string, FileSystemNode>() };
Expand All @@ -53,8 +58,8 @@ export class InMemoryFileSystem implements ts.ParseConfigHost, ts.ModuleResoluti
/**
* Returns an IterableIterator for all URIs known to exist in the workspace (content loaded or not)
*/
uris(): IterableIterator<string> {
return this.files.keys();
uris(): IterableIterator<URL> {
return iterate(this.files.keys()).map(uri => new URL(uri));
Copy link
Contributor

Choose a reason for hiding this comment

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

Java does it better map(URL::new) 😀

}

/**
Expand All @@ -63,13 +68,13 @@ export class InMemoryFileSystem implements ts.ParseConfigHost, ts.ModuleResoluti
* @param uri The URI of the file
* @param content The optional content
*/
add(uri: string, content?: string): void {
add(uri: URL, content?: string): void {
// Make sure not to override existing content with undefined
if (content !== undefined || !this.files.has(uri)) {
this.files.set(uri, content);
if (content !== undefined || !this.files.has(uri.href)) {
this.files.set(uri.href, content);
}
// Add to directory tree
const filePath = util.uri2path(uri);
const filePath = util.toUnixPath(util.uri2path(uri));
const components = filePath.split('/').filter(c => c);
let node = this.rootNode;
for (const [i, component] of components.entries()) {
Expand All @@ -93,8 +98,8 @@ export class InMemoryFileSystem implements ts.ParseConfigHost, ts.ModuleResoluti
*
* @param uri URI to a file
*/
has(uri: string): boolean {
return this.files.has(uri) || this.fileExists(util.uri2path(uri));
has(uri: URL): boolean {
return this.files.has(uri.href) || this.fileExists(util.toUnixPath(util.uri2path(uri)));
}

/**
Expand All @@ -104,10 +109,10 @@ export class InMemoryFileSystem implements ts.ParseConfigHost, ts.ModuleResoluti
*
* TODO take overlay into account
*/
getContent(uri: string): string {
let content = this.files.get(uri);
getContent(uri: URL): string {
let content = this.files.get(uri.href);
if (content === undefined) {
content = typeScriptLibraries.get(util.uri2path(uri));
content = typeScriptLibraries.get(util.toUnixPath(util.uri2path(uri)));
}
if (content === undefined) {
throw new Error(`Content of ${uri} is not available in memory`);
Expand All @@ -118,28 +123,28 @@ export class InMemoryFileSystem implements ts.ParseConfigHost, ts.ModuleResoluti
/**
* Tells if a file denoted by the given name exists in the workspace (does not have to be loaded)
*
* @param path File path or URI (both absolute or relative file paths are accepted)
* @param filePath Path to file, absolute or relative to `rootUri`
*/
fileExists(path: string): boolean {
return this.readFileIfExists(path) !== undefined || this.files.has(path) || this.files.has(util.path2uri(this.path, path));
fileExists(filePath: string): boolean {
return this.readFileIfExists(filePath) !== undefined || this.files.has(filePath) || this.files.has(util.path2uri(this.rootUri, filePath).href);
}

/**
* @param path file path (both absolute or relative file paths are accepted)
* @return file's content in the following order (overlay then cache).
* If there is no such file, returns empty string to match expected signature
*/
readFile(path: string): string {
return this.readFileIfExists(path) || '';
readFile(filePath: string): string {
return this.readFileIfExists(filePath) || '';
}

/**
* @param path file path (both absolute or relative file paths are accepted)
* @param filePath Path to the file, absolute or relative to `rootUri`
* @return file's content in the following order (overlay then cache).
* If there is no such file, returns undefined
*/
private readFileIfExists(path: string): string | undefined {
const uri = util.path2uri(this.path, path);
readFileIfExists(filePath: string): string | undefined {
const uri = util.path2uri(this.rootUri, filePath).href;
let content = this.overlay.get(uri);
if (content !== undefined) {
return content;
Expand All @@ -153,23 +158,23 @@ export class InMemoryFileSystem implements ts.ParseConfigHost, ts.ModuleResoluti
return content;
}

return typeScriptLibraries.get(path);
return typeScriptLibraries.get(filePath);
}

/**
* Invalidates temporary content denoted by the given URI
* @param uri file's URI
*/
didClose(uri: string) {
this.overlay.delete(uri);
didClose(uri: URL) {
this.overlay.delete(uri.href);
}

/**
* Adds temporary content denoted by the given URI
* @param uri file's URI
*/
didSave(uri: string) {
const content = this.overlay.get(uri);
didSave(uri: URL) {
const content = this.overlay.get(uri.href);
if (content !== undefined) {
this.add(uri, content);
}
Expand All @@ -179,8 +184,8 @@ export class InMemoryFileSystem implements ts.ParseConfigHost, ts.ModuleResoluti
* Updates temporary content denoted by the given URI
* @param uri file's URI
*/
didChange(uri: string, text: string) {
this.overlay.set(uri, text);
didChange(uri: URL, text: string) {
this.overlay.set(uri.href, text);
}

/**
Expand Down
Loading