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

Add quickfix and refactoring to install @types packages #19130

Merged
12 commits merged into from
Oct 17, 2017
33 changes: 18 additions & 15 deletions scripts/buildProtocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,22 +51,25 @@ class DeclarationsWalker {
return this.processType((<any>type).typeArguments[0]);
}
else {
for (const decl of s.getDeclarations()) {
const sourceFile = decl.getSourceFile();
if (sourceFile === this.protocolFile || path.basename(sourceFile.fileName) === "lib.d.ts") {
return;
}
if (decl.kind === ts.SyntaxKind.EnumDeclaration && !isStringEnum(decl as ts.EnumDeclaration)) {
this.removedTypes.push(type);
return;
}
else {
// splice declaration in final d.ts file
let text = decl.getFullText();
this.text += `${text}\n`;
// recursively pull all dependencies into result dts file
const declarations = s.getDeclarations();
if (declarations) {
for (const decl of declarations) {
const sourceFile = decl.getSourceFile();
if (sourceFile === this.protocolFile || path.basename(sourceFile.fileName) === "lib.d.ts") {
return;
}
if (decl.kind === ts.SyntaxKind.EnumDeclaration && !isStringEnum(decl as ts.EnumDeclaration)) {
this.removedTypes.push(type);
return;
}
else {
// splice declaration in final d.ts file
let text = decl.getFullText();
this.text += `${text}\n`;
// recursively pull all dependencies into result dts file

this.visitTypeNodes(decl);
this.visitTypeNodes(decl);
}
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/compiler/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1829,7 +1829,9 @@ namespace ts {
return i < 0 ? path : path.substring(i + 1);
}

export function combinePaths(path1: string, path2: string) {
export function combinePaths(path1: Path, path2: string): Path;
Copy link
Member

Choose a reason for hiding this comment

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

This is not correct.. Path handle case sensitivity.. since path2 doesnt have Path as type you would never be sure if combined paths would be Path or just string (that needs to handle case sensitivity)

export function combinePaths(path1: string, path2: string): string;
export function combinePaths(path1: string, path2: string): string {
if (!(path1 && path1.length)) return path2;
if (!(path2 && path2.length)) return path1;
if (getRootLength(path2) !== 0) return path2;
Expand Down
73 changes: 71 additions & 2 deletions src/compiler/moduleNameResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,11 @@ namespace ts {
}
}

export function getEffectiveTypeRoots(options: CompilerOptions, host: { directoryExists?: (directoryName: string) => boolean, getCurrentDirectory?: () => string }): string[] | undefined {
export interface GetEffectiveTypeRootsHost {
directoryExists?: (directoryName: string) => boolean;
getCurrentDirectory?: () => string;
}
export function getEffectiveTypeRoots(options: CompilerOptions, host: GetEffectiveTypeRootsHost): string[] | undefined {
if (options.typeRoots) {
return options.typeRoots;
}
Expand Down Expand Up @@ -985,7 +989,8 @@ namespace ts {
return withPackageId(packageId, pathAndExtension);
}

function getPackageName(moduleName: string): { packageName: string, rest: string } {
/* @internal */
export function getPackageName(moduleName: string): { packageName: string, rest: string } {
let idx = moduleName.indexOf(directorySeparator);
if (moduleName[0] === "@") {
idx = moduleName.indexOf(directorySeparator, idx + 1);
Expand Down Expand Up @@ -1153,4 +1158,68 @@ namespace ts {
function toSearchResult<T>(value: T | undefined): SearchResult<T> {
return value !== undefined ? { value } : undefined;
}

/* @internal */
export const enum PackageNameValidationResult {
Copy link
Member

Choose a reason for hiding this comment

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

These types shouldnt be in compiler .. They should be in Language service since they arent needed by compiler.

Ok,
ScopedPackagesNotSupported,
EmptyName,
NameTooLong,
NameStartsWithDot,
NameStartsWithUnderscore,
NameContainsNonURISafeCharacters
}

const MaxPackageNameLength = 214;

/**
* Validates package name using rules defined at https://docs.npmjs.com/files/package.json
*/
/* @internal */
export function validatePackageName(packageName: string): PackageNameValidationResult {
Copy link
Member

Choose a reason for hiding this comment

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

These functions too..

if (!packageName) {
return PackageNameValidationResult.EmptyName;
}
if (packageName.length > MaxPackageNameLength) {
return PackageNameValidationResult.NameTooLong;
}
if (packageName.charCodeAt(0) === CharacterCodes.dot) {
return PackageNameValidationResult.NameStartsWithDot;
}
if (packageName.charCodeAt(0) === CharacterCodes._) {
return PackageNameValidationResult.NameStartsWithUnderscore;
}
// check if name is scope package like: starts with @ and has one '/' in the middle
// scoped packages are not currently supported
// TODO: when support will be added we'll need to split and check both scope and package name
if (/^@[^/]+\/[^/]+$/.test(packageName)) {
return PackageNameValidationResult.ScopedPackagesNotSupported;
}
if (encodeURIComponent(packageName) !== packageName) {
return PackageNameValidationResult.NameContainsNonURISafeCharacters;
}
return PackageNameValidationResult.Ok;
}

/* @internal */
export function renderPackageNameValidationFailure(result: PackageNameValidationResult, typing: string): string {
switch (result) {
case PackageNameValidationResult.EmptyName:
return `Package name '${typing}' cannot be empty`;
case PackageNameValidationResult.NameTooLong:
return `Package name '${typing}' should be less than ${MaxPackageNameLength} characters`;
case PackageNameValidationResult.NameStartsWithDot:
return `Package name '${typing}' cannot start with '.'`;
case PackageNameValidationResult.NameStartsWithUnderscore:
return `Package name '${typing}' cannot start with '_'`;
case PackageNameValidationResult.ScopedPackagesNotSupported:
return `Package '${typing}' is scoped and currently is not supported`;
case PackageNameValidationResult.NameContainsNonURISafeCharacters:
return `Package name '${typing}' contains non URI safe characters`;
case PackageNameValidationResult.Ok:
throw Debug.fail(); // Shouldn't have called this.
default:
Debug.assertNever(result);
}
}
}
55 changes: 48 additions & 7 deletions src/harness/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -953,6 +953,10 @@ namespace FourSlash {
return this.getChecker().getSymbolsInScope(node, ts.SymbolFlags.Value | ts.SymbolFlags.Type | ts.SymbolFlags.Namespace);
}

public setTypesRegistry(map: ts.MapLike<void>): void {
this.languageServiceAdapterHost.typesRegistry = ts.createMapFromTemplate(map);
}

public verifyTypeOfSymbolAtLocation(range: Range, symbol: ts.Symbol, expected: string): void {
const node = this.goToAndGetNode(range);
const checker = this.getChecker();
Expand Down Expand Up @@ -2750,16 +2754,26 @@ Actual: ${stringify(fullActual)}`);
}
}

public verifyCodeFixAvailable(negative: boolean) {
const codeFix = this.getCodeFixActions(this.activeFile.fileName);
public verifyCodeFixAvailable(negative: boolean, info: FourSlashInterface.VerifyCodeFixAvailableOptions[] | undefined) {
const codeFixes = this.getCodeFixActions(this.activeFile.fileName);

if (negative && codeFix.length) {
this.raiseError(`verifyCodeFixAvailable failed - expected no fixes but found one.`);
if (negative) {
if (codeFixes.length) {
this.raiseError(`verifyCodeFixAvailable failed - expected no fixes but found one.`);
}
return;
}

if (!(negative || codeFix.length)) {
if (!codeFixes.length) {
this.raiseError(`verifyCodeFixAvailable failed - expected code fixes but none found.`);
}
if (info) {
assert.equal(info.length, codeFixes.length);
ts.zipWith(codeFixes, info, (fix, info) => {
assert.equal(fix.description, info.description);
this.assertObjectsEqual(fix.commands, info.commands);
});
}
}

public verifyApplicableRefactorAvailableAtMarker(negative: boolean, markerName: string) {
Expand Down Expand Up @@ -2803,6 +2817,14 @@ Actual: ${stringify(fullActual)}`);
}
}

public verifyRefactor({ name, actionName, refactors }: FourSlashInterface.VerifyRefactorOptions) {
const selection = this.getSelection();

const actualRefactors = (this.languageService.getApplicableRefactors(this.activeFile.fileName, selection) || ts.emptyArray)
.filter(r => r.name === name && r.actions.some(a => a.name === actionName));
this.assertObjectsEqual(actualRefactors, refactors);
}

public verifyApplicableRefactorAvailableForRange(negative: boolean) {
const ranges = this.getRanges();
if (!(ranges && ranges.length === 1)) {
Expand Down Expand Up @@ -3577,6 +3599,10 @@ namespace FourSlashInterface {
public symbolsInScope(range: FourSlash.Range): ts.Symbol[] {
return this.state.symbolsInScope(range);
}

public setTypesRegistry(map: ts.MapLike<void>): void {
this.state.setTypesRegistry(map);
}
}

export class GoTo {
Expand Down Expand Up @@ -3752,8 +3778,8 @@ namespace FourSlashInterface {
this.state.verifyCodeFix(options);
}

public codeFixAvailable() {
this.state.verifyCodeFixAvailable(this.negative);
public codeFixAvailable(options?: VerifyCodeFixAvailableOptions[]) {
this.state.verifyCodeFixAvailable(this.negative, options);
}

public applicableRefactorAvailableAtMarker(markerName: string) {
Expand All @@ -3764,6 +3790,10 @@ namespace FourSlashInterface {
this.state.verifyApplicableRefactorAvailableForRange(this.negative);
}

public refactor(options: VerifyRefactorOptions) {
this.state.verifyRefactor(options);
}

public refactorAvailable(name: string, actionName?: string) {
this.state.verifyRefactorAvailable(this.negative, name, actionName);
}
Expand Down Expand Up @@ -4404,4 +4434,15 @@ namespace FourSlashInterface {
errorCode?: number;
index?: number;
}

export interface VerifyCodeFixAvailableOptions {
description: string;
commands?: ts.CodeActionCommand[];
}

export interface VerifyRefactorOptions {
name: string;
actionName: string;
refactors: ts.ApplicableRefactorInfo[];
}
}
10 changes: 10 additions & 0 deletions src/harness/harnessLanguageService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ namespace Harness.LanguageService {
}

export class LanguageServiceAdapterHost {
public typesRegistry: ts.Map<void> | undefined;
protected virtualFileSystem: Utils.VirtualFileSystem = new Utils.VirtualFileSystem(virtualFileSystemRoot, /*useCaseSensitiveFilenames*/false);

constructor(protected cancellationToken = DefaultHostCancellationToken.Instance,
Expand Down Expand Up @@ -182,6 +183,14 @@ namespace Harness.LanguageService {

/// Native adapter
class NativeLanguageServiceHost extends LanguageServiceAdapterHost implements ts.LanguageServiceHost {
tryGetTypesRegistry(): ts.Map<void> | undefined {
if (this.typesRegistry === undefined) {
ts.Debug.fail("fourslash test should set types registry.");
}
return this.typesRegistry;
}
installPackage = ts.notImplemented;

getCompilationSettings() { return this.settings; }
getCancellationToken() { return this.cancellationToken; }
getDirectories(path: string): string[] {
Expand Down Expand Up @@ -493,6 +502,7 @@ namespace Harness.LanguageService {
getCodeFixesAtPosition(): ts.CodeAction[] {
throw new Error("Not supported on the shim.");
}
applyCodeActionCommand = ts.notImplemented;
getCodeFixDiagnostics(): ts.Diagnostic[] {
throw new Error("Not supported on the shim.");
}
Expand Down
2 changes: 1 addition & 1 deletion src/harness/unittests/compileOnSave.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace ts.projectSystem {

describe("CompileOnSave affected list", () => {
function sendAffectedFileRequestAndCheckResult(session: server.Session, request: server.protocol.Request, expectedFileList: { projectFileName: string, files: FileOrFolder[] }[]) {
const response: server.protocol.CompileOnSaveAffectedFileListSingleProject[] = session.executeCommand(request).response;
const response = session.executeCommand(request).response as server.protocol.CompileOnSaveAffectedFileListSingleProject[];
const actualResult = response.sort((list1, list2) => compareStrings(list1.projectFileName, list2.projectFileName));
expectedFileList = expectedFileList.sort((list1, list2) => compareStrings(list1.projectFileName, list2.projectFileName));

Expand Down
10 changes: 10 additions & 0 deletions src/harness/unittests/extractTestHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,14 @@ namespace ts {
return rulesProvider;
}

const notImplementedHost: LanguageServiceHost = {
getCompilationSettings: notImplemented,
getScriptFileNames: notImplemented,
getScriptVersion: notImplemented,
getScriptSnapshot: notImplemented,
getDefaultLibFileName: notImplemented,
};

export function testExtractSymbol(caption: string, text: string, baselineFolder: string, description: DiagnosticMessage) {
const t = extractTest(text);
const selectionRange = t.ranges.get("selection");
Expand Down Expand Up @@ -125,6 +133,7 @@ namespace ts {
file: sourceFile,
startPosition: selectionRange.start,
endPosition: selectionRange.end,
host: notImplementedHost,
rulesProvider: getRuleProvider()
};
const rangeToExtract = refactor.extractSymbol.getRangeToExtract(sourceFile, createTextSpanFromBounds(selectionRange.start, selectionRange.end));
Expand Down Expand Up @@ -188,6 +197,7 @@ namespace ts {
file: sourceFile,
startPosition: selectionRange.start,
endPosition: selectionRange.end,
host: notImplementedHost,
rulesProvider: getRuleProvider()
};
const rangeToExtract = refactor.extractSymbol.getRangeToExtract(sourceFile, createTextSpanFromBounds(selectionRange.start, selectionRange.end));
Expand Down
2 changes: 2 additions & 0 deletions src/harness/unittests/languageService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ export function Component(x: Config): any;`
// to write an alias to a module's default export was referrenced across files and had no default export
it("should be able to create a language service which can respond to deinition requests without throwing", () => {
const languageService = ts.createLanguageService({
tryGetTypesRegistry: notImplemented,
installPackage: notImplemented,
getCompilationSettings() {
return {};
},
Expand Down
10 changes: 5 additions & 5 deletions src/harness/unittests/projectErrors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,23 +57,23 @@ namespace ts.projectSystem {
});

checkNumberOfProjects(projectService, { externalProjects: 1 });
const diags = session.executeCommand(compilerOptionsRequest).response;
const diags = session.executeCommand(compilerOptionsRequest).response as server.protocol.DiagnosticWithLinePosition[];
// only file1 exists - expect error
checkDiagnosticsWithLinePos(diags, ["File '/a/b/applib.ts' not found."]);
}
host.reloadFS([file2, libFile]);
{
// only file2 exists - expect error
checkNumberOfProjects(projectService, { externalProjects: 1 });
const diags = session.executeCommand(compilerOptionsRequest).response;
const diags = session.executeCommand(compilerOptionsRequest).response as server.protocol.DiagnosticWithLinePosition[];
checkDiagnosticsWithLinePos(diags, ["File '/a/b/app.ts' not found."]);
}

host.reloadFS([file1, file2, libFile]);
{
// both files exist - expect no errors
checkNumberOfProjects(projectService, { externalProjects: 1 });
const diags = session.executeCommand(compilerOptionsRequest).response;
const diags = session.executeCommand(compilerOptionsRequest).response as server.protocol.DiagnosticWithLinePosition[];
checkDiagnosticsWithLinePos(diags, []);
}
});
Expand Down Expand Up @@ -103,13 +103,13 @@ namespace ts.projectSystem {
seq: 2,
arguments: { projectFileName: project.getProjectName() }
};
let diags = session.executeCommand(compilerOptionsRequest).response;
let diags = session.executeCommand(compilerOptionsRequest).response as server.protocol.DiagnosticWithLinePosition[];
checkDiagnosticsWithLinePos(diags, ["File '/a/b/applib.ts' not found."]);

host.reloadFS([file1, file2, config, libFile]);

checkNumberOfProjects(projectService, { configuredProjects: 1 });
diags = session.executeCommand(compilerOptionsRequest).response;
diags = session.executeCommand(compilerOptionsRequest).response as server.protocol.DiagnosticWithLinePosition[];
checkDiagnosticsWithLinePos(diags, []);
});

Expand Down
Loading