Skip to content

Commit

Permalink
Refactored fourslash framework to use mount paths ability added for t…
Browse files Browse the repository at this point in the history
…ypeshed as well (microsoft#95)

* Squashed 'server/pyright/' changes from 9521693..b1e0ff8

b1e0ff8 made reportMissingTypeStub diagnostics warning by default and some refactoring around code actions. (microsoft#507)

git-subtree-dir: server/pyright
git-subtree-split: b1e0ff8

* pure style changes

* add configuration to run each tests individually.

* refactor mount path code so that typeshed path also uses same mechanism

* added option to ignore warning on fourslash test file and removed files no longer needed

* introduced consts to save all well knowns consts so that we don't spread string literals all over the places

* added test to confirm typeshed directory to mount can be controlled by fourslash test file itself.
  • Loading branch information
heejaechang authored Feb 13, 2020
1 parent 0b55276 commit 26cf0a6
Show file tree
Hide file tree
Showing 13 changed files with 165 additions and 101 deletions.
4 changes: 2 additions & 2 deletions server/.vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,13 @@
"windows": {
"program": "${workspaceFolder}/node_modules/jest/bin/jest",
}
}
},
{
"type": "node",
"name": "fourslash current file",
"request": "launch",
"args": [
"fourslashrunner.test.ts",
"fourSlashRunner.test.ts",
"-t ${fileBasenameNoExtension}",
"--config",
"jest.config.js"
Expand Down
5 changes: 3 additions & 2 deletions server/src/analyzer/pythonPathUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import * as child_process from 'child_process';
import { ConfigOptions } from '../common/configOptions';
import * as consts from '../common/consts';
import {
combinePaths, ensureTrailingDirectorySeparator, getDirectoryPath,
getFileSystemEntries, isDirectory, normalizePath
Expand All @@ -22,7 +23,7 @@ export function getTypeShedFallbackPath(moduleDirectory?: string) {
moduleDirectory = normalizePath(moduleDirectory);
return combinePaths(getDirectoryPath(
ensureTrailingDirectorySeparator(moduleDirectory)),
'typeshed-fallback');
consts.TYPESHED_FALLBACK);
}

return undefined;
Expand Down Expand Up @@ -74,7 +75,7 @@ export function findPythonSearchPaths(fs: VirtualFileSystem, configOptions: Conf

// We didn't find a site-packages directory directly in the lib
// directory. Scan for a "python*" directory instead.
const entries = getFileSystemEntries(this._fs, libPath);
const entries = getFileSystemEntries(fs, libPath);
for (let i = 0; i < entries.directories.length; i++) {
const dirName = entries.directories[i];
if (dirName.startsWith('python')) {
Expand Down
9 changes: 9 additions & 0 deletions server/src/common/consts.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/*
* consts.ts
* Copyright (c) Microsoft Corporation.
* Licensed under the MIT license.
*
* Defines well known consts and names
*/

export const TYPESHED_FALLBACK = 'typeshed-fallback';
99 changes: 55 additions & 44 deletions server/src/common/pathUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ import { URI } from 'vscode-uri';
import { some } from './collectionUtils';
import { compareValues, Comparison, GetCanonicalFileName, identity } from './core';
import * as debug from './debug';
import { compareStringsCaseInsensitive, compareStringsCaseSensitive, equateStringsCaseInsensitive,
equateStringsCaseSensitive, getStringComparer } from './stringUtils';
import {
compareStringsCaseInsensitive, compareStringsCaseSensitive, equateStringsCaseInsensitive,
equateStringsCaseSensitive, getStringComparer
} from './stringUtils';
import { VirtualFileSystem } from './vfs';

export interface FileSpec {
Expand Down Expand Up @@ -81,7 +83,7 @@ export function getPathComponents(pathString: string) {
}

export function reducePathComponents(components: readonly string[]) {
if (!some(components)) return [];
if (!some(components)) { return []; }

// Reduce the path components by eliminating
// any '.' or '..'.
Expand Down Expand Up @@ -109,7 +111,7 @@ export function reducePathComponents(components: readonly string[]) {
}

export function combinePathComponents(components: string[]): string {
if (components.length === 0) return "";
if (components.length === 0) { return ''; }

const root = components[0] && ensureTrailingDirectorySeparator(components[0]);
return normalizeSlashes(root + components.slice(1).join(path.sep));
Expand Down Expand Up @@ -155,8 +157,7 @@ export function getFileSize(fs: VirtualFileSystem, path: string) {
if (stat.isFile()) {
return stat.size;
}
}
catch { /*ignore*/ }
} catch { /*ignore*/ }
return 0;
}

Expand Down Expand Up @@ -218,11 +219,10 @@ export function comparePaths(a: string, b: string, currentDirectory?: string | b
a = normalizePath(a);
b = normalizePath(b);

if (typeof currentDirectory === "string") {
if (typeof currentDirectory === 'string') {
a = combinePaths(currentDirectory, a);
b = combinePaths(currentDirectory, b);
}
else if (typeof currentDirectory === "boolean") {
} else if (typeof currentDirectory === 'boolean') {
ignoreCase = currentDirectory;
}
return comparePathsWorker(a, b, getStringComparer(ignoreCase));
Expand All @@ -234,16 +234,15 @@ export function comparePaths(a: string, b: string, currentDirectory?: string | b
export function containsPath(parent: string, child: string, ignoreCase?: boolean): boolean;
export function containsPath(parent: string, child: string, currentDirectory: string, ignoreCase?: boolean): boolean;
export function containsPath(parent: string, child: string, currentDirectory?: string | boolean, ignoreCase?: boolean) {
if (typeof currentDirectory === "string") {
if (typeof currentDirectory === 'string') {
parent = combinePaths(currentDirectory, parent);
child = combinePaths(currentDirectory, child);
}
else if (typeof currentDirectory === "boolean") {
} else if (typeof currentDirectory === 'boolean') {
ignoreCase = currentDirectory;
}

if (parent === undefined || child === undefined) return false;
if (parent === child) return true;
if (parent === undefined || child === undefined) { return false; }
if (parent === child) { return true; }

const parentComponents = getPathComponents(parent);
const childComponents = getPathComponents(child);
Expand Down Expand Up @@ -283,8 +282,10 @@ export function changeAnyExtension(path: string, ext: string): string;
*/
export function changeAnyExtension(path: string, ext: string, extensions: string | readonly string[], ignoreCase: boolean): string;
export function changeAnyExtension(path: string, ext: string, extensions?: string | readonly string[], ignoreCase?: boolean): string {
const pathext = extensions !== undefined && ignoreCase !== undefined ? getAnyExtensionFromPath(path, extensions, ignoreCase) : getAnyExtensionFromPath(path);
return pathext ? path.slice(0, path.length - pathext.length) + (ext.startsWith(".") ? ext : "." + ext) : path;
const pathext = extensions !== undefined && ignoreCase !== undefined ?
getAnyExtensionFromPath(path, extensions, ignoreCase) : getAnyExtensionFromPath(path);

return pathext ? path.slice(0, path.length - pathext.length) + (ext.startsWith('.') ? ext : '.' + ext) : path;
}

/**
Expand Down Expand Up @@ -312,14 +313,15 @@ export function getAnyExtensionFromPath(path: string, extensions?: string | read
// Retrieves any string from the final "." onwards from a base file name.
// Unlike extensionFromPath, which throws an exception on unrecognized extensions.
if (extensions) {
return getAnyExtensionFromPathWorker(stripTrailingDirectorySeparator(path), extensions, ignoreCase ? equateStringsCaseInsensitive : equateStringsCaseSensitive);
return getAnyExtensionFromPathWorker(stripTrailingDirectorySeparator(path), extensions,
ignoreCase ? equateStringsCaseInsensitive : equateStringsCaseSensitive);
}
const baseFileName = getBaseFileName(path);
const extensionIndex = baseFileName.lastIndexOf(".");
const extensionIndex = baseFileName.lastIndexOf('.');
if (extensionIndex >= 0) {
return baseFileName.substring(extensionIndex);
}
return "";
return '';
}

/**
Expand Down Expand Up @@ -357,13 +359,15 @@ export function getBaseFileName(pathString: string, extensions?: string | readon

// if the path provided is itself the root, then it has not file name.
const rootLength = getRootLength(pathString);
if (rootLength === pathString.length) return "";
if (rootLength === pathString.length) { return ''; }

// return the trailing portion of the path starting after the last (non-terminal) directory
// separator but not including any trailing directory separator.
pathString = stripTrailingDirectorySeparator(pathString);
const name = pathString.slice(Math.max(getRootLength(pathString), pathString.lastIndexOf(path.sep) + 1));
const extension = extensions !== undefined && ignoreCase !== undefined ? getAnyExtensionFromPath(name, extensions, ignoreCase) : undefined;
const extension = extensions !== undefined && ignoreCase !== undefined ?
getAnyExtensionFromPath(name, extensions, ignoreCase) : undefined;

return extension ? name.slice(0, name.length - extension.length) : name;
}

Expand All @@ -375,11 +379,15 @@ export function getRelativePathFromDirectory(from: string, to: string, ignoreCas
* Gets a relative path that can be used to traverse between `from` and `to`.
*/
export function getRelativePathFromDirectory(fromDirectory: string, to: string, getCanonicalFileName: GetCanonicalFileName): string;
export function getRelativePathFromDirectory(fromDirectory: string, to: string, getCanonicalFileNameOrIgnoreCase: GetCanonicalFileName | boolean) {
debug.assert((getRootLength(fromDirectory) > 0) === (getRootLength(to) > 0), "Paths must either both be absolute or both be relative");
const getCanonicalFileName = typeof getCanonicalFileNameOrIgnoreCase === "function" ? getCanonicalFileNameOrIgnoreCase : identity;
const ignoreCase = typeof getCanonicalFileNameOrIgnoreCase === "boolean" ? getCanonicalFileNameOrIgnoreCase : false;
const pathComponents = getPathComponentsRelativeTo(fromDirectory, to, ignoreCase ? equateStringsCaseInsensitive : equateStringsCaseSensitive, getCanonicalFileName);
export function getRelativePathFromDirectory(fromDirectory: string, to: string,
getCanonicalFileNameOrIgnoreCase: GetCanonicalFileName | boolean) {

debug.assert((getRootLength(fromDirectory) > 0) === (getRootLength(to) > 0), 'Paths must either both be absolute or both be relative');
const getCanonicalFileName = typeof getCanonicalFileNameOrIgnoreCase === 'function' ? getCanonicalFileNameOrIgnoreCase : identity;
const ignoreCase = typeof getCanonicalFileNameOrIgnoreCase === 'boolean' ? getCanonicalFileNameOrIgnoreCase : false;
const pathComponents = getPathComponentsRelativeTo(fromDirectory, to, ignoreCase ?
equateStringsCaseInsensitive : equateStringsCaseSensitive, getCanonicalFileName);

return combinePathComponents(pathComponents);
}

Expand Down Expand Up @@ -505,7 +513,7 @@ export function getWildcardRegexPattern(rootPath: string, fileSpec: string): str

const escapedSeparator = getRegexEscapedSeparator();
const doubleAsteriskRegexFragment = `(${ escapedSeparator }[^${ escapedSeparator }.][^${ escapedSeparator }]*)*?`;
const reservedCharacterPattern = new RegExp(`[^\\w\\s${ escapedSeparator }]`, "g");
const reservedCharacterPattern = new RegExp(`[^\\w\\s${ escapedSeparator }]`, 'g');

// Strip the directory separator from the root component.
if (pathComponents.length > 0) {
Expand Down Expand Up @@ -616,9 +624,9 @@ export function isDiskPathRoot(path: string) {

//// Path Comparisons
function comparePathsWorker(a: string, b: string, componentComparer: (a: string, b: string) => Comparison) {
if (a === b) return Comparison.EqualTo;
if (a === undefined) return Comparison.LessThan;
if (b === undefined) return Comparison.GreaterThan;
if (a === b) { return Comparison.EqualTo; }
if (a === undefined) { return Comparison.LessThan; }
if (b === undefined) { return Comparison.GreaterThan; }

// NOTE: Performance optimization - shortcut if the root segments differ as there would be no
// need to perform path reduction.
Expand All @@ -631,7 +639,7 @@ function comparePathsWorker(a: string, b: string, componentComparer: (a: string,

// check path for these segments: '', '.'. '..'
const escapedSeparator = getRegexEscapedSeparator();
const relativePathSegmentRegExp = new RegExp(`(^|${escapedSeparator}).{0,2}($|${escapedSeparator})`);
const relativePathSegmentRegExp = new RegExp(`(^|${ escapedSeparator }).{0,2}($|${ escapedSeparator })`);

// NOTE: Performance optimization - shortcut if there are no relative path segments in
// the non-root portion of the path
Expand All @@ -656,19 +664,21 @@ function comparePathsWorker(a: string, b: string, componentComparer: (a: string,
return compareValues(aComponents.length, bComponents.length);
}

function getAnyExtensionFromPathWorker(path: string, extensions: string | readonly string[], stringEqualityComparer: (a: string, b: string) => boolean) {
if (typeof extensions === "string") {
return tryGetExtensionFromPath(path, extensions, stringEqualityComparer) || "";
function getAnyExtensionFromPathWorker(path: string, extensions: string | readonly string[],
stringEqualityComparer: (a: string, b: string) => boolean) {

if (typeof extensions === 'string') {
return tryGetExtensionFromPath(path, extensions, stringEqualityComparer) || '';
}
for (const extension of extensions) {
const result = tryGetExtensionFromPath(path, extension, stringEqualityComparer);
if (result) return result;
if (result) { return result; }
}
return "";
return '';
}

function tryGetExtensionFromPath(path: string, extension: string, stringEqualityComparer: (a: string, b: string) => boolean) {
if (!extension.startsWith(".")) extension = "." + extension;
if (!extension.startsWith('.')) { extension = '.' + extension; }
if (path.length >= extension.length && path.charCodeAt(path.length - extension.length) === Char.Period) {
const pathExtension = path.slice(path.length - extension.length);
if (stringEqualityComparer(pathExtension, extension)) {
Expand All @@ -679,7 +689,9 @@ function tryGetExtensionFromPath(path: string, extension: string, stringEquality
return undefined;
}

function getPathComponentsRelativeTo(from: string, to: string, stringEqualityComparer: (a: string, b: string) => boolean, getCanonicalFileName: GetCanonicalFileName) {
function getPathComponentsRelativeTo(from: string, to: string, stringEqualityComparer: (a: string, b: string) => boolean,
getCanonicalFileName: GetCanonicalFileName) {

const fromComponents = getPathComponents(from);
const toComponents = getPathComponents(to);

Expand All @@ -688,7 +700,7 @@ function getPathComponentsRelativeTo(from: string, to: string, stringEqualityCom
const fromComponent = getCanonicalFileName(fromComponents[start]);
const toComponent = getCanonicalFileName(toComponents[start]);
const comparer = start === 0 ? equateStringsCaseInsensitive : stringEqualityComparer;
if (!comparer(fromComponent, toComponent)) break;
if (!comparer(fromComponent, toComponent)) { break; }
}

if (start === 0) {
Expand All @@ -698,14 +710,14 @@ function getPathComponentsRelativeTo(from: string, to: string, stringEqualityCom
const components = toComponents.slice(start);
const relative: string[] = [];
for (; start < fromComponents.length; start++) {
relative.push("..");
relative.push('..');
}
return ["", ...relative, ...components];
return ['', ...relative, ...components];
}

const enum FileSystemEntryKind {
File,
Directory,
Directory
}

function fileSystemEntryExists(fs: VirtualFileSystem, path: string, entryKind: FileSystemEntryKind): boolean {
Expand All @@ -716,8 +728,7 @@ function fileSystemEntryExists(fs: VirtualFileSystem, path: string, entryKind: F
case FileSystemEntryKind.Directory: return stat.isDirectory();
default: return false;
}
}
catch (e) {
} catch (e) {
return false;
}
}
Expand Down
6 changes: 3 additions & 3 deletions server/src/languageServerBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ import {
ParameterInformation, RemoteConsole, SignatureInformation, SymbolInformation, TextDocuments, TextEdit, WorkspaceEdit
} from 'vscode-languageserver';

import { ImportResolver } from './analyzer/importResolver';
import { AnalyzerService } from './analyzer/service';
import { CommandLineOptions } from './common/commandLineOptions';
import { ConfigOptions } from './common/configOptions';
import { Diagnostic as AnalyzerDiagnostic, DiagnosticCategory } from './common/diagnostic';
import './common/extensions';
import { combinePaths, convertPathToUri, convertUriToPath, normalizePath } from './common/pathUtils';
import { ConfigOptions } from './common/configOptions';
import { ImportResolver } from './analyzer/importResolver';
import { Position } from './common/textRange';
import { createFromRealFileSystem, VirtualFileSystem } from './common/vfs';
import { CompletionItemData } from './languageService/completionProvider';
Expand Down Expand Up @@ -97,7 +97,7 @@ export abstract class LanguageServerBase {
protected createImportResolver(fs: VirtualFileSystem, options: ConfigOptions): ImportResolver {
return new ImportResolver(fs, options);
}

// Creates a service instance that's used for analyzing a
// program within a workspace.
createAnalyzerService(name: string): AnalyzerService {
Expand Down
3 changes: 2 additions & 1 deletion server/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import * as path from 'path';
import { isArray } from 'util';
import { CodeAction, CodeActionParams, Command, ExecuteCommandParams } from 'vscode-languageserver';
import { CommandController } from './commands/commandController';
import * as consts from './common/consts';
import * as debug from './common/debug';
import { convertUriToPath, getDirectoryPath, normalizeSlashes } from './common/pathUtils';
import { LanguageServerBase, ServerSettings, WorkspaceServiceInstance } from './languageServerBase';
Expand All @@ -27,7 +28,7 @@ class Server extends LanguageServerBase {
// 1. to find "typeshed-fallback" folder.
// 2. to set "cwd" to run python to find search path.
const rootDirectory = getDirectoryPath(__dirname);
debug.assert(fs.existsSync(path.join(rootDirectory, 'typeshed-fallback')), `Unable to locate typeshed fallback folder at '${ rootDirectory }'`);
debug.assert(fs.existsSync(path.join(rootDirectory, consts.TYPESHED_FALLBACK)), `Unable to locate typeshed fallback folder at '${ rootDirectory }'`);
super('Pyright', rootDirectory);

this._controller = new CommandController(this);
Expand Down
12 changes: 4 additions & 8 deletions server/src/tests/filesystem.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,7 @@ test('createFromFileSystem1', () => {
const content = '# test';

// file system will map physical file system to virtual one
const resolver = factory.createResolver(host.HOST);
const fs = factory.createFromFileSystem(host.HOST, resolver, false,
const fs = factory.createFromFileSystem(host.HOST, false,
{ documents: [new factory.TextDocument(filepath, content)], cwd: factory.srcFolder });

// check existing typeshed folder on virtual path inherited from base snapshot from physical file system
Expand All @@ -157,16 +156,14 @@ test('createFromFileSystem1', () => {
});

test('createFromFileSystem2', () => {
const resolver = factory.createResolver(host.HOST);
const fs = factory.createFromFileSystem(host.HOST, resolver, /* ignoreCase */ true, { cwd: factory.srcFolder });
const fs = factory.createFromFileSystem(host.HOST, /* ignoreCase */ true, { cwd: factory.srcFolder });
const entries = fs.readdirSync(factory.typeshedFolder.toUpperCase());
assert(entries.length > 0);
});

test('createFromFileSystemWithCustomTypeshedPath', () => {
const invalidpath = normalizeSlashes(combinePaths(host.HOST.getWorkspaceRoot(), '../docs'));
const resolver = factory.createResolver(host.HOST);
const fs = factory.createFromFileSystem(host.HOST, resolver, /* ignoreCase */ false, {
const fs = factory.createFromFileSystem(host.HOST, /* ignoreCase */ false, {
cwd: factory.srcFolder, meta: { [factory.typeshedFolder]: invalidpath }
});

Expand All @@ -175,8 +172,7 @@ test('createFromFileSystemWithCustomTypeshedPath', () => {
});

test('createFromFileSystemWithMetadata', () => {
const resolver = factory.createResolver(host.HOST);
const fs = factory.createFromFileSystem(host.HOST, resolver, /* ignoreCase */ false, {
const fs = factory.createFromFileSystem(host.HOST, /* ignoreCase */ false, {
cwd: factory.srcFolder, meta: { 'unused': 'unused' }
});

Expand Down
Loading

0 comments on commit 26cf0a6

Please sign in to comment.