diff --git a/.vscode/tasks.json b/.vscode/tasks.json index f3c59a6171768..31928f73ce217 100644 --- a/.vscode/tasks.json +++ b/.vscode/tasks.json @@ -18,6 +18,13 @@ "problemMatcher": [ "$tsc" ] + }, + { + "taskName": "tests", + "showOutput": "silent", + "problemMatcher": [ + "$tsc" + ] } ] } \ No newline at end of file diff --git a/Jakefile.js b/Jakefile.js index 31243f77c4284..3f4439a8cb7fa 100644 --- a/Jakefile.js +++ b/Jakefile.js @@ -133,6 +133,7 @@ var harnessSources = harnessCoreSources.concat([ "projectErrors.ts", "matchFiles.ts", "initializeTSConfig.ts", + "extractMethods.ts", "printer.ts", "textChanges.ts", "telemetry.ts", diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 1c0f15e27aa5b..68546b7ff9f41 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -223,11 +223,10 @@ namespace ts { getSuggestionForNonexistentProperty: (node, type) => unescapeLeadingUnderscores(getSuggestionForNonexistentProperty(node, type)), getSuggestionForNonexistentSymbol: (location, name, meaning) => unescapeLeadingUnderscores(getSuggestionForNonexistentSymbol(location, escapeLeadingUnderscores(name), meaning)), getBaseConstraintOfType, - getJsxNamespace: () => unescapeLeadingUnderscores(getJsxNamespace()), - resolveNameAtLocation(location: Node, name: string, meaning: SymbolFlags): Symbol | undefined { - location = getParseTreeNode(location); - return resolveName(location, escapeLeadingUnderscores(name), meaning, /*nameNotFoundMessage*/ undefined, escapeLeadingUnderscores(name)); + resolveName(name, location, meaning) { + return resolveName(location, name as __String, meaning, /*nameNotFoundMessage*/ undefined, /*nameArg*/ undefined); }, + getJsxNamespace: () => unescapeLeadingUnderscores(getJsxNamespace()), }; const tupleTypes: GenericType[] = []; diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 3b4b0ddf66711..34742a57ed16b 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -3673,5 +3673,15 @@ "Convert function '{0}' to class": { "category": "Message", "code": 95002 + }, + + "Extract function": { + "category": "Message", + "code": 95003 + }, + + "Extract function into '{0}'": { + "category": "Message", + "code": 95004 } } diff --git a/src/compiler/transformers/es2015.ts b/src/compiler/transformers/es2015.ts index f97de018d4ad0..da827d7b63e9d 100644 --- a/src/compiler/transformers/es2015.ts +++ b/src/compiler/transformers/es2015.ts @@ -394,7 +394,7 @@ namespace ts { function shouldVisitNode(node: Node): boolean { return (node.transformFlags & TransformFlags.ContainsES2015) !== 0 || convertedLoopState !== undefined - || (hierarchyFacts & HierarchyFacts.ConstructorWithCapturedSuper && isStatement(node)) + || (hierarchyFacts & HierarchyFacts.ConstructorWithCapturedSuper && isStatementOrBlock(node)) || (isIterationStatement(node, /*lookInLabeledStatements*/ false) && shouldConvertIterationStatementBody(node)) || isTypeScriptClassWrapper(node); } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 09f1b5cfcc53c..fd7bb665e7281 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -2606,9 +2606,8 @@ namespace ts { * Does not include properties of primitive types. */ /* @internal */ getAllPossiblePropertiesOfType(type: Type): Symbol[]; - + /* @internal */ resolveName(name: string, location: Node, meaning: SymbolFlags): Symbol | undefined; /* @internal */ getJsxNamespace(): string; - /* @internal */ resolveNameAtLocation(location: Node, name: string, meaning: SymbolFlags): Symbol | undefined; } export enum NodeBuilderFlags { diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 76d242e8b08d8..619966725a7cb 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -693,7 +693,7 @@ namespace ts { // At this point, node is either a qualified name or an identifier Debug.assert(node.kind === SyntaxKind.Identifier || node.kind === SyntaxKind.QualifiedName || node.kind === SyntaxKind.PropertyAccessExpression, "'node' was expected to be a qualified name, identifier or property access in 'isPartOfTypeNode'."); - // falls through + // falls through case SyntaxKind.QualifiedName: case SyntaxKind.PropertyAccessExpression: case SyntaxKind.ThisKeyword: @@ -968,7 +968,7 @@ namespace ts { if (!includeArrowFunctions) { continue; } - // falls through + // falls through case SyntaxKind.FunctionDeclaration: case SyntaxKind.FunctionExpression: case SyntaxKind.ModuleDeclaration: @@ -1027,7 +1027,7 @@ namespace ts { if (!stopOnFunctions) { continue; } - // falls through + // falls through case SyntaxKind.PropertyDeclaration: case SyntaxKind.PropertySignature: case SyntaxKind.MethodDeclaration: @@ -1211,7 +1211,7 @@ namespace ts { if (node.parent.kind === SyntaxKind.TypeQuery || isJSXTagName(node)) { return true; } - // falls through + // falls through case SyntaxKind.NumericLiteral: case SyntaxKind.StringLiteral: case SyntaxKind.ThisKeyword: @@ -1499,8 +1499,8 @@ namespace ts { parent.parent.kind === SyntaxKind.VariableStatement; const variableStatementNode = isInitializerOfVariableDeclarationInStatement ? parent.parent.parent : - isVariableOfVariableDeclarationStatement ? parent.parent : - undefined; + isVariableOfVariableDeclarationStatement ? parent.parent : + undefined; if (variableStatementNode) { getJSDocCommentsAndTagsWorker(variableStatementNode); } @@ -1614,7 +1614,7 @@ namespace ts { if (isInJavaScriptFile(node)) { if (node.type && node.type.kind === SyntaxKind.JSDocVariadicType || forEach(getJSDocParameterTags(node), - t => t.typeExpression && t.typeExpression.type.kind === SyntaxKind.JSDocVariadicType)) { + t => t.typeExpression && t.typeExpression.type.kind === SyntaxKind.JSDocVariadicType)) { return true; } } @@ -1907,7 +1907,7 @@ namespace ts { if (node.asteriskToken) { flags |= FunctionFlags.Generator; } - // falls through + // falls through case SyntaxKind.ArrowFunction: if (hasModifier(node, ModifierFlags.Async)) { flags |= FunctionFlags.Async; @@ -5123,6 +5123,19 @@ namespace ts { return isUnaryExpressionKind(skipPartiallyEmittedExpressions(node).kind); } + /* @internal */ + export function isUnaryExpressionWithWrite(expr: Node): expr is PrefixUnaryExpression | PostfixUnaryExpression { + switch (expr.kind) { + case SyntaxKind.PostfixUnaryExpression: + return true; + case SyntaxKind.PrefixUnaryExpression: + return (expr).operator === SyntaxKind.PlusPlusToken || + (expr).operator === SyntaxKind.MinusMinusToken; + default: + return false; + } + } + function isExpressionKind(kind: SyntaxKind) { return kind === SyntaxKind.ConditionalExpression || kind === SyntaxKind.YieldExpression @@ -5337,7 +5350,25 @@ namespace ts { const kind = node.kind; return isStatementKindButNotDeclarationKind(kind) || isDeclarationStatementKind(kind) - || kind === SyntaxKind.Block; + || isBlockStatement(node); + } + + /* @internal */ + export function isStatementOrBlock(node: Node): node is Statement { + const kind = node.kind; + return isStatementKindButNotDeclarationKind(kind) + || isDeclarationStatementKind(kind) + || node.kind === SyntaxKind.Block; + } + + function isBlockStatement(node: Node): node is Block { + if (node.kind !== SyntaxKind.Block) return false; + if (node.parent !== undefined) { + if (node.parent.kind === SyntaxKind.TryStatement || node.parent.kind === SyntaxKind.CatchClause) { + return false; + } + } + return !isFunctionBlock(node); } // Module references diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index c6ec0f257b606..7f8ded3e2599b 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -187,6 +187,9 @@ namespace FourSlash { // The current caret position in the active file public currentCaretPosition = 0; + // The position of the end of the current selection, or -1 if nothing is selected + public selectionEnd = -1; + public lastKnownMarker = ""; // The file that's currently 'opened' @@ -433,11 +436,19 @@ namespace FourSlash { public goToPosition(pos: number) { this.currentCaretPosition = pos; + this.selectionEnd = -1; + } + + public select(startMarker: string, endMarker: string) { + const start = this.getMarkerByName(startMarker), end = this.getMarkerByName(endMarker); + this.goToPosition(start.position); + this.selectionEnd = end.position; } public moveCaretRight(count = 1) { this.currentCaretPosition += count; this.currentCaretPosition = Math.min(this.currentCaretPosition, this.getFileContent(this.activeFile.fileName).length); + this.selectionEnd = -1; } // Opens a file given its 0-based index or fileName @@ -980,9 +991,9 @@ namespace FourSlash { } for (const reference of expectedReferences) { - const {fileName, start, end} = reference; + const { fileName, start, end } = reference; if (reference.marker && reference.marker.data) { - const {isWriteAccess, isDefinition} = reference.marker.data; + const { isWriteAccess, isDefinition } = reference.marker.data; this.verifyReferencesWorker(actualReferences, fileName, start, end, isWriteAccess, isDefinition); } else { @@ -1193,7 +1204,7 @@ namespace FourSlash { displayParts: ts.SymbolDisplayPart[], documentation: ts.SymbolDisplayPart[], tags: ts.JSDocTagInfo[] - ) { + ) { const actualQuickInfo = this.languageService.getQuickInfoAtPosition(this.activeFile.fileName, this.currentCaretPosition); assert.equal(actualQuickInfo.kind, kind, this.messageAtLastKnownMarker("QuickInfo kind")); @@ -1789,19 +1800,16 @@ namespace FourSlash { // We get back a set of edits, but langSvc.editScript only accepts one at a time. Use this to keep track // of the incremental offset from each edit to the next. We assume these edit ranges don't overlap - edits = edits.sort((a, b) => a.span.start - b.span.start); - for (let i = 0; i < edits.length - 1; i++) { - const firstEditSpan = edits[i].span; - const firstEditEnd = firstEditSpan.start + firstEditSpan.length; - assert.isTrue(firstEditEnd <= edits[i + 1].span.start); - } + // Copy this so we don't ruin someone else's copy + edits = JSON.parse(JSON.stringify(edits)); // Get a snapshot of the content of the file so we can make sure any formatting edits didn't destroy non-whitespace characters const oldContent = this.getFileContent(fileName); let runningOffset = 0; - for (const edit of edits) { - const offsetStart = edit.span.start + runningOffset; + for (let i = 0; i < edits.length; i++) { + const edit = edits[i]; + const offsetStart = edit.span.start; const offsetEnd = offsetStart + edit.span.length; this.editScriptAndUpdateMarkers(fileName, offsetStart, offsetEnd, edit.newText); const editDelta = edit.newText.length - edit.span.length; @@ -1816,8 +1824,13 @@ namespace FourSlash { } } runningOffset += editDelta; - // TODO: Consider doing this at least some of the time for higher fidelity. Currently causes a failure (bug 707150) - // this.languageService.getScriptLexicalStructure(fileName); + + // Update positions of any future edits affected by this change + for (let j = i + 1; j < edits.length; j++) { + if (edits[j].span.start >= edits[i].span.start) { + edits[j].span.start += editDelta; + } + } } if (isFormattingEdit) { @@ -1901,7 +1914,7 @@ namespace FourSlash { this.goToPosition(len); } - public goToRangeStart({fileName, start}: Range) { + public goToRangeStart({ fileName, start }: Range) { this.openFile(fileName); this.goToPosition(start); } @@ -2075,7 +2088,7 @@ namespace FourSlash { return result; } - private rangeText({fileName, start, end}: Range): string { + private rangeText({ fileName, start, end }: Range): string { return this.getFileContent(fileName).slice(start, end); } @@ -2361,7 +2374,7 @@ namespace FourSlash { private applyCodeActions(actions: ts.CodeAction[], index?: number): void { if (index === undefined) { if (!(actions && actions.length === 1)) { - this.raiseError(`Should find exactly one codefix, but ${actions ? actions.length : "none"} found. ${actions ? actions.map(a => `${Harness.IO.newLine()} "${a.description}"`) : "" }`); + this.raiseError(`Should find exactly one codefix, but ${actions ? actions.length : "none"} found. ${actions ? actions.map(a => `${Harness.IO.newLine()} "${a.description}"`) : ""}`); } index = 0; } @@ -2736,6 +2749,30 @@ namespace FourSlash { } } + private getSelection() { + return ({ + pos: this.currentCaretPosition, + end: this.selectionEnd === -1 ? this.currentCaretPosition : this.selectionEnd + }); + } + + public verifyRefactorAvailable(negative: boolean, name?: string, subName?: string) { + const selection = this.getSelection(); + + let refactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, selection) || []; + if (name) { + refactors = refactors.filter(r => r.name === name && (subName === undefined || r.actions.some(a => a.name === subName))); + } + const isAvailable = refactors.length > 0; + + if (negative && isAvailable) { + this.raiseError(`verifyApplicableRefactorAvailableForRange failed - expected no refactor but found some.`); + } + else if (!negative && !isAvailable) { + this.raiseError(`verifyApplicableRefactorAvailableForRange failed - expected a refactor but found none.`); + } + } + public verifyApplicableRefactorAvailableForRange(negative: boolean) { const ranges = this.getRanges(); if (!(ranges && ranges.length === 1)) { @@ -2752,6 +2789,20 @@ namespace FourSlash { } } + public applyRefactor(refactorName: string, actionName: string) { + const range = this.getSelection(); + const refactors = this.languageService.getApplicableRefactors(this.activeFile.fileName, range); + const refactor = ts.find(refactors, r => r.name === refactorName); + if (!refactor) { + this.raiseError(`The expected refactor: ${refactorName} is not available at the marker location.`); + } + + const editInfo = this.languageService.getEditsForRefactor(this.activeFile.fileName, this.formatCodeSettings, range, refactorName, actionName); + for (const edit of editInfo.edits) { + this.applyEdits(edit.fileName, edit.textChanges, /*isFormattingEdit*/ false); + } + } + public verifyFileAfterApplyingRefactorAtMarker( markerName: string, expectedContent: string, @@ -3496,6 +3547,10 @@ namespace FourSlashInterface { public file(indexOrName: any, content?: string, scriptKindName?: string): void { this.state.openFile(indexOrName, content, scriptKindName); } + + public select(startMarker: string, endMarker: string) { + this.state.select(startMarker, endMarker); + } } export class VerifyNegatable { @@ -3617,6 +3672,10 @@ namespace FourSlashInterface { public applicableRefactorAvailableForRange() { this.state.verifyApplicableRefactorAvailableForRange(this.negative); } + + public refactorAvailable(name?: string, subName?: string) { + this.state.verifyRefactorAvailable(this.negative, name, subName); + } } export class Verify extends VerifyNegatable { @@ -4012,6 +4071,10 @@ namespace FourSlashInterface { public disableFormatting() { this.state.enableFormatting = false; } + + public applyRefactor(refactorName: string, actionName: string) { + this.state.applyRefactor(refactorName, actionName); + } } export class Debug { diff --git a/src/harness/unittests/extractMethods.ts b/src/harness/unittests/extractMethods.ts new file mode 100644 index 0000000000000..4b5d1911b4914 --- /dev/null +++ b/src/harness/unittests/extractMethods.ts @@ -0,0 +1,591 @@ +/// +/// + +namespace ts { + interface Range { + start: number; + end: number; + name: string; + } + + interface Test { + source: string; + ranges: Map; + } + + function extractTest(source: string): Test { + const activeRanges: Range[] = []; + let text = ""; + let lastPos = 0; + let pos = 0; + const ranges = createMap(); + + while (pos < source.length) { + if (source.charCodeAt(pos) === CharacterCodes.openBracket && + (source.charCodeAt(pos + 1) === CharacterCodes.hash || source.charCodeAt(pos + 1) === CharacterCodes.$)) { + const saved = pos; + pos += 2; + const s = pos; + consumeIdentifier(); + const e = pos; + if (source.charCodeAt(pos) === CharacterCodes.bar) { + pos++; + text += source.substring(lastPos, saved); + const name = s === e + ? source.charCodeAt(saved + 1) === CharacterCodes.hash ? "selection" : "extracted" + : source.substring(s, e); + activeRanges.push({ name, start: text.length, end: undefined }); + lastPos = pos; + continue; + } + else { + pos = saved; + } + } + else if (source.charCodeAt(pos) === CharacterCodes.bar && source.charCodeAt(pos + 1) === CharacterCodes.closeBracket) { + text += source.substring(lastPos, pos); + activeRanges[activeRanges.length - 1].end = text.length; + const range = activeRanges.pop(); + if (range.name in ranges) { + throw new Error(`Duplicate name of range ${range.name}`); + } + ranges.set(range.name, range); + pos += 2; + lastPos = pos; + continue; + } + pos++; + } + text += source.substring(lastPos, pos); + + function consumeIdentifier() { + while (isIdentifierPart(source.charCodeAt(pos), ScriptTarget.Latest)) { + pos++; + } + } + return { source: text, ranges }; + } + + const newLineCharacter = "\n"; + function getRuleProvider(action?: (opts: FormatCodeSettings) => void) { + const options = { + indentSize: 4, + tabSize: 4, + newLineCharacter, + convertTabsToSpaces: true, + indentStyle: ts.IndentStyle.Smart, + insertSpaceAfterConstructor: false, + insertSpaceAfterCommaDelimiter: true, + insertSpaceAfterSemicolonInForStatements: true, + insertSpaceBeforeAndAfterBinaryOperators: true, + insertSpaceAfterKeywordsInControlFlowStatements: true, + insertSpaceAfterFunctionKeywordForAnonymousFunctions: false, + insertSpaceAfterOpeningAndBeforeClosingNonemptyParenthesis: false, + insertSpaceAfterOpeningAndBeforeClosingNonemptyBrackets: false, + insertSpaceAfterOpeningAndBeforeClosingNonemptyBraces: true, + insertSpaceAfterOpeningAndBeforeClosingTemplateStringBraces: false, + insertSpaceAfterOpeningAndBeforeClosingJsxExpressionBraces: false, + insertSpaceBeforeFunctionParenthesis: false, + placeOpenBraceOnNewLineForFunctions: false, + placeOpenBraceOnNewLineForControlBlocks: false, + }; + if (action) { + action(options); + } + const rulesProvider = new formatting.RulesProvider(); + rulesProvider.ensureUpToDate(options); + return rulesProvider; + } + + function testExtractRangeFailed(caption: string, s: string, expectedErrors: string[]) { + return it(caption, () => { + const t = extractTest(s); + const file = createSourceFile("a.ts", t.source, ScriptTarget.Latest, /*setParentNodes*/ true); + const selectionRange = t.ranges.get("selection"); + if (!selectionRange) { + throw new Error(`Test ${s} does not specify selection range`); + } + const result = refactor.extractMethod.getRangeToExtract(file, createTextSpanFromBounds(selectionRange.start, selectionRange.end)); + assert(result.targetRange === undefined, "failure expected"); + const sortedErrors = result.errors.map(e => e.messageText).sort(); + assert.deepEqual(sortedErrors, expectedErrors.sort(), "unexpected errors"); + }); + } + + function testExtractRange(s: string): void { + const t = extractTest(s); + const f = createSourceFile("a.ts", t.source, ScriptTarget.Latest, /*setParentNodes*/ true); + const selectionRange = t.ranges.get("selection"); + if (!selectionRange) { + throw new Error(`Test ${s} does not specify selection range`); + } + const result = refactor.extractMethod.getRangeToExtract(f, createTextSpanFromBounds(selectionRange.start, selectionRange.end)); + const expectedRange = t.ranges.get("extracted"); + if (expectedRange) { + let start: number, end: number; + if (ts.isArray(result.targetRange.range)) { + start = result.targetRange.range[0].getStart(f); + end = ts.lastOrUndefined(result.targetRange.range).getEnd(); + } + else { + start = result.targetRange.range.getStart(f); + end = result.targetRange.range.getEnd(); + } + assert.equal(start, expectedRange.start, "incorrect start of range"); + assert.equal(end, expectedRange.end, "incorrect end of range"); + } + else { + assert.isTrue(!result.targetRange, `expected range to extract to be undefined`); + } + } + + describe("extractMethods", () => { + it("get extract range from selection", () => { + testExtractRange(` + [#| + [$|var x = 1; + var y = 2;|]|] + `); + testExtractRange(` + [#| + var x = 1; + var y = 2|]; + `); + testExtractRange(` + [#|var x = 1|]; + var y = 2; + `); + testExtractRange(` + if ([#|[#extracted|a && b && c && d|]|]) { + } + `); + testExtractRange(` + if [#|(a && b && c && d|]) { + } + `); + testExtractRange(` + if (a && b && c && d) { + [#| [$|var x = 1; + console.log(x);|] |] + } + `); + testExtractRange(` + [#| + if (a) { + return 100; + } |] + `); + testExtractRange(` + function foo() { + [#| [$|if (a) { + } + return 100|] |] + } + `); + testExtractRange(` + [#| + [$|l1: + if (x) { + break l1; + }|]|] + `); + testExtractRange(` + [#| + [$|l2: + { + if (x) { + } + break l2; + }|]|] + `); + testExtractRange(` + while (true) { + [#| if(x) { + } + break; |] + } + `); + testExtractRange(` + while (true) { + [#| if(x) { + } + continue; |] + } + `); + testExtractRange(` + l3: + { + [#| + if (x) { + } + break l3; |] + } + `); + testExtractRange(` + function f() { + while (true) { + [#| + if (x) { + return; + } |] + } + } + `); + testExtractRange(` + function f() { + while (true) { + [#| + [$|if (x) { + } + return;|] + |] + } + } + `); + testExtractRange(` + function f() { + return [#| [$|1 + 2|] |]+ 3; + } + } + `); + testExtractRange(` + function f() { + return [$|1 + [#|2 + 3|]|]; + } + } + `); + testExtractRange(` + function f() { + return [$|1 + 2 + [#|3 + 4|]|]; + } + } + `); + }); + + testExtractRangeFailed("extractRangeFailed1", + ` +namespace A { + function f() { + [#| + let x = 1 + if (x) { + return 10; + } + |] + } +} + `, + [ + "Cannot extract range containing conditional return statement." + ]); + + testExtractRangeFailed("extractRangeFailed2", + ` +namespace A { + function f() { + while (true) { + [#| + let x = 1 + if (x) { + break; + } + |] + } + } +} + `, + [ + "Cannot extract range containing conditional break or continue statements." + ]); + + testExtractRangeFailed("extractRangeFailed3", + ` +namespace A { + function f() { + while (true) { + [#| + let x = 1 + if (x) { + continue; + } + |] + } + } +} + `, + [ + "Cannot extract range containing conditional break or continue statements." + ]); + + testExtractRangeFailed("extractRangeFailed4", + ` +namespace A { + function f() { + l1: { + [#| + let x = 1 + if (x) { + break l1; + } + |] + } + } +} + `, + [ + "Cannot extract range containing labeled break or continue with target outside of the range." + ]); + + testExtractRangeFailed("extractRangeFailed5", + ` +namespace A { + function f() { + [#| + try { + f2() + return 10; + } + catch (e) { + } + |] + } + function f2() { + } +} + `, + [ + "Cannot extract range containing conditional return statement." + ]); + + testExtractRangeFailed("extractRangeFailed6", + ` +namespace A { + function f() { + [#| + try { + f2() + } + catch (e) { + return 10; + } + |] + } + function f2() { + } +} + `, + [ + "Cannot extract range containing conditional return statement." + ]); + + testExtractMethod("extractMethod1", + `namespace A { + let x = 1; + function foo() { + } + namespace B { + function a() { + let a = 1; + [#| + let y = 5; + let z = x; + a = y; + foo();|] + } + } +}`); + testExtractMethod("extractMethod2", + `namespace A { + let x = 1; + function foo() { + } + namespace B { + function a() { + [#| + let y = 5; + let z = x; + return foo();|] + } + } +}`); + testExtractMethod("extractMethod3", + `namespace A { + function foo() { + } + namespace B { + function* a(z: number) { + [#| + let y = 5; + yield z; + return foo();|] + } + } +}`); + testExtractMethod("extractMethod4", + `namespace A { + function foo() { + } + namespace B { + async function a(z: number, z1: any) { + [#| + let y = 5; + if (z) { + await z1; + } + return foo();|] + } + } +}`); + testExtractMethod("extractMethod5", + `namespace A { + let x = 1; + export function foo() { + } + namespace B { + function a() { + let a = 1; + [#| + let y = 5; + let z = x; + a = y; + foo();|] + } + } +}`); + testExtractMethod("extractMethod6", + `namespace A { + let x = 1; + export function foo() { + } + namespace B { + function a() { + let a = 1; + [#| + let y = 5; + let z = x; + a = y; + return foo();|] + } + } +}`); + testExtractMethod("extractMethod7", + `namespace A { + let x = 1; + export namespace C { + export function foo() { + } + } + namespace B { + function a() { + let a = 1; + [#| + let y = 5; + let z = x; + a = y; + return C.foo();|] + } + } +}`); + testExtractMethod("extractMethod8", + `namespace A { + let x = 1; + namespace B { + function a() { + let a1 = 1; + return 1 + [#|a1 + x|] + 100; + } + } +}`); + testExtractMethod("extractMethod9", + `namespace A { + export interface I { x: number }; + namespace B { + function a() { + [#|let a1: I = { x: 1 }; + return a1.x + 10;|] + } + } +}`); + testExtractMethod("extractMethod10", + `namespace A { + export interface I { x: number }; + class C { + a() { + let z = 1; + [#|let a1: I = { x: 1 }; + return a1.x + 10;|] + } + } +}`); + testExtractMethod("extractMethod11", + `namespace A { + let y = 1; + class C { + a() { + let z = 1; + [#|let a1 = { x: 1 }; + y = 10; + z = 42; + return a1.x + 10;|] + } + } +}`); + testExtractMethod("extractMethod12", + `namespace A { + let y = 1; + class C { + b() {} + a() { + let z = 1; + [#|let a1 = { x: 1 }; + y = 10; + z = 42; + this.b(); + return a1.x + 10;|] + } + } +}`); + }); + + + function testExtractMethod(caption: string, text: string) { + it(caption, () => { + Harness.Baseline.runBaseline(`extractMethod/${caption}.js`, () => { + const t = extractTest(text); + const selectionRange = t.ranges.get("selection"); + if (!selectionRange) { + throw new Error(`Test ${caption} does not specify selection range`); + } + const f = { + path: "/a.ts", + content: t.source + }; + const host = projectSystem.createServerHost([f]); + const projectService = projectSystem.createProjectService(host); + projectService.openClientFile(f.path); + const program = projectService.inferredProjects[0].getLanguageService().getProgram(); + const sourceFile = program.getSourceFile(f.path); + const context: RefactorContext = { + cancellationToken: { throwIfCancellationRequested() { }, isCancellationRequested() { return false; } }, + newLineCharacter, + program, + file: sourceFile, + startPosition: -1, + rulesProvider: getRuleProvider() + }; + const result = refactor.extractMethod.getRangeToExtract(sourceFile, createTextSpanFromBounds(selectionRange.start, selectionRange.end)); + assert.equal(result.errors, undefined, "expect no errors"); + const results = refactor.extractMethod.extractRange(result.targetRange, context); + const data: string[] = []; + data.push(`==ORIGINAL==`); + data.push(sourceFile.text); + for (const r of results) { + const changes = refactor.extractMethod.extractRange(result.targetRange, context, results.indexOf(r))[0].changes; + data.push(`==SCOPE::${r.scopeDescription}==`); + data.push(textChanges.applyChanges(sourceFile.text, changes[0].textChanges)); + } + return data.join(newLineCharacter); + }); + }); + } +} diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index d6fb8de9259b2..9fff51ee411d9 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -147,7 +147,7 @@ namespace ts.codefix { } else if (isJsxOpeningLikeElement(token.parent) && token.parent.tagName === token) { // The error wasn't for the symbolAtLocation, it was for the JSX tag itself, which needs access to e.g. `React`. - symbol = checker.getAliasedSymbol(checker.resolveNameAtLocation(token, checker.getJsxNamespace(), SymbolFlags.Value)); + symbol = checker.getAliasedSymbol(checker.resolveName(checker.getJsxNamespace(), token.parent.tagName, SymbolFlags.Value)); symbolName = symbol.name; } else { diff --git a/src/services/refactors/convertFunctionToEs6Class.ts b/src/services/refactors/convertFunctionToEs6Class.ts index 45adfb4b03900..bf0e4e22658b7 100644 --- a/src/services/refactors/convertFunctionToEs6Class.ts +++ b/src/services/refactors/convertFunctionToEs6Class.ts @@ -1,6 +1,6 @@ /* @internal */ -namespace ts.refactor { +namespace ts.refactor.convertFunctionToES6Class { const actionName = "convert"; const convertFunctionToES6Class: Refactor = { diff --git a/src/services/refactors/extractMethod.ts b/src/services/refactors/extractMethod.ts new file mode 100644 index 0000000000000..65e29138e9597 --- /dev/null +++ b/src/services/refactors/extractMethod.ts @@ -0,0 +1,1154 @@ +/// +/// + +/* @internal */ +namespace ts.refactor.extractMethod { + const extractMethod: Refactor = { + name: "Extract Method", + description: Diagnostics.Extract_function.message, + getAvailableActions, + getEditsForAction, + }; + + registerRefactor(extractMethod); + + /** Compute the associated code actions */ + function getAvailableActions(context: RefactorContext): ApplicableRefactorInfo[] | undefined { + const rangeToExtract = getRangeToExtract(context.file, { start: context.startPosition, length: context.endPosition - context.startPosition }); + + const targetRange: TargetRange = rangeToExtract.targetRange; + if (targetRange === undefined) { + return undefined; + } + + const extractions = extractRange(targetRange, context); + if (extractions === undefined) { + // No extractions possible + return undefined; + } + + const actions: RefactorActionInfo[] = []; + const usedNames: Map = createMap(); + + let i = 0; + for (const extr of extractions) { + // Skip these since we don't have a way to report errors yet + if (extr.errors && extr.errors.length) { + continue; + } + + // Don't issue refactorings with duplicated names + const description = formatStringFromArgs(Diagnostics.Extract_function_into_0.message, [extr.scopeDescription]); + if (!usedNames.get(description)) { + usedNames.set(description, true); + actions.push({ + description, + name: `scope_${i}` + }); + } + i++; + } + + if (actions.length === 0) { + return undefined; + } + + return [{ + name: extractMethod.name, + description: extractMethod.description, + inlineable: true, + actions + }]; + } + + function getEditsForAction(context: RefactorContext, actionName: string): RefactorEditInfo | undefined { + const length = context.endPosition === undefined ? 0 : context.endPosition - context.startPosition; + const rangeToExtract = getRangeToExtract(context.file, { start: context.startPosition, length }); + const targetRange: TargetRange = rangeToExtract.targetRange; + + const parsedIndexMatch = /scope_(\d+)/.exec(actionName); + if (!parsedIndexMatch) { + throw new Error("Expected to match the regexp"); + } + const index = +parsedIndexMatch[1]; + if (!isFinite(index)) { + throw new Error("Expected to parse a number from the scope index"); + } + + const extractions = extractRange(targetRange, context, index); + if (extractions === undefined) { + // Scope is no longer valid from when the user issued the refactor + // return undefined; + return undefined; + } + return ({ edits: extractions[0].changes }); + } + + // Move these into diagnostic messages if they become user-facing + namespace Messages { + function createMessage(message: string): DiagnosticMessage { + return { message, code: 0, category: DiagnosticCategory.Message, key: message }; + } + + export const CannotExtractFunction: DiagnosticMessage = createMessage("Cannot extract function."); + export const StatementOrExpressionExpected: DiagnosticMessage = createMessage("Statement or expression expected."); + export const CannotExtractRangeContainingConditionalBreakOrContinueStatements: DiagnosticMessage = createMessage("Cannot extract range containing conditional break or continue statements."); + export const CannotExtractRangeContainingConditionalReturnStatement: DiagnosticMessage = createMessage("Cannot extract range containing conditional return statement."); + export const CannotExtractRangeContainingLabeledBreakOrContinueStatementWithTargetOutsideOfTheRange: DiagnosticMessage = createMessage("Cannot extract range containing labeled break or continue with target outside of the range."); + export const CannotExtractRangeThatContainsWritesToReferencesLocatedOutsideOfTheTargetRangeInGenerators: DiagnosticMessage = createMessage("Cannot extract range containing writes to references located outside of the target range in generators."); + export const TypeWillNotBeVisibleInTheNewScope = createMessage("Type will not visible in the new scope."); + export const FunctionWillNotBeVisibleInTheNewScope = createMessage("Function will not visible in the new scope."); + export const InsufficientSelection = createMessage("Select more than a single identifier."); + export const CannotExtractExportedEntity = createMessage("Cannot extract exported declaration"); + export const CannotCombineWritesAndReturns = createMessage("Cannot combine writes and returns"); + export const CannotExtractReadonlyPropertyInitializerOutsideConstructor = createMessage("Cannot move initialization of read-only class property outside of the constructor"); + export const CannotExtractAmbientBlock = createMessage("Cannot extract code from ambient contexts"); + } + + export enum RangeFacts { + None = 0, + HasReturn = 1 << 0, + IsGenerator = 1 << 1, + IsAsyncFunction = 1 << 2, + UsesThis = 1 << 3, + /** + * The range is in a function which needs the 'static' modifier in a class + */ + InStaticRegion = 1 << 4 + } + + /** + * Represents an expression or a list of statements that should be extracted with some extra information + */ + export interface TargetRange { + readonly range: Expression | Statement[]; + readonly facts: RangeFacts; + /** + * A list of symbols that are declared in the selected range which are visible in the containing lexical scope + * Used to ensure we don't turn something used outside the range free (or worse, resolve to a different entity). + */ + readonly declarations: Symbol[]; + } + + /** + * Result of 'getRangeToExtract' operation: contains either a range or a list of errors + */ + export type RangeToExtract = { + readonly targetRange?: never; + readonly errors: ReadonlyArray; + } | { + readonly targetRange: TargetRange; + readonly errors?: never; + }; + + /* + * Scopes that can store newly extracted method + */ + export type Scope = FunctionLikeDeclaration | SourceFile | ModuleBlock | ClassLikeDeclaration; + + /** + * Result of 'extractRange' operation for a specific scope. + * Stores either a list of changes that should be applied to extract a range or a list of errors + */ + export interface ExtractResultForScope { + readonly scope: Scope; + readonly scopeDescription: string; + readonly changes?: FileTextChanges[]; + readonly errors?: Diagnostic[]; + } + + /** + * getRangeToExtract takes a span inside a text file and returns either an expression or an array + * of statements representing the minimum set of nodes needed to extract the entire span. This + * process may fail, in which case a set of errors is returned instead (these are currently + * not shown to the user, but can be used by us diagnostically) + */ + export function getRangeToExtract(sourceFile: SourceFile, span: TextSpan): RangeToExtract { + const length = span.length || 0; + // Walk up starting from the the start position until we find a non-SourceFile node that subsumes the selected span. + // This may fail (e.g. you select two statements in the root of a source file) + let start = getParentNodeInSpan(getTokenAtPosition(sourceFile, span.start, /*includeJsDocComment*/ false), sourceFile, span); + // Do the same for the ending position + let end = getParentNodeInSpan(findTokenOnLeftOfPosition(sourceFile, textSpanEnd(span)), sourceFile, span); + + const declarations: Symbol[] = []; + + // We'll modify these flags as we walk the tree to collect data + // about what things need to be done as part of the extraction. + let rangeFacts = RangeFacts.None; + + if (!start || !end) { + // cannot find either start or end node + return { errors: [createFileDiagnostic(sourceFile, span.start, length, Messages.CannotExtractFunction)] }; + } + + if (start.parent !== end.parent) { + // handle cases like 1 + [2 + 3] + 4 + // user selection is marked with []. + // in this case 2 + 3 does not belong to the same tree node + // instead the shape of the tree looks like this: + // + + // / \ + // + 4 + // / \ + // + 3 + // / \ + // 1 2 + // in this case there is no such one node that covers ends of selection and is located inside the selection + // to handle this we check if both start and end of the selection belong to some binary operation + // and start node is parented by the parent of the end node + // if this is the case - expand the selection to the entire parent of end node (in this case it will be [1 + 2 + 3] + 4) + const startParent = skipParentheses(start.parent); + const endParent = skipParentheses(end.parent); + if (isBinaryExpression(startParent) && isBinaryExpression(endParent) && isNodeDescendantOf(startParent, endParent)) { + start = end = endParent; + } + else { + // start and end nodes belong to different subtrees + return createErrorResult(sourceFile, span.start, length, Messages.CannotExtractFunction); + } + } + if (start !== end) { + // start and end should be statements and parent should be either block or a source file + if (!isBlockLike(start.parent)) { + return createErrorResult(sourceFile, span.start, length, Messages.CannotExtractFunction); + } + const statements: Statement[] = []; + for (const statement of (start.parent).statements) { + if (statement === start || statements.length) { + const errors = checkNode(statement); + if (errors) { + return { errors }; + } + statements.push(statement); + } + if (statement === end) { + break; + } + } + return { targetRange: { range: statements, facts: rangeFacts, declarations } }; + } + else { + // We have a single expression (start) + const errors = checkRootNode(start) || checkNode(start); + if (errors) { + return { errors }; + } + + // If our selection is the expression in an ExrpessionStatement, expand + // the selection to include the enclosing Statement (this stops us + // from trying to care about the return value of the extracted function + // and eliminates double semicolon insertion in certain scenarios) + const range = isStatement(start) + ? [start] + : start.parent && start.parent.kind === SyntaxKind.ExpressionStatement + ? [start.parent as Statement] + : start; + + return { targetRange: { range, facts: rangeFacts, declarations } }; + } + + function createErrorResult(sourceFile: SourceFile, start: number, length: number, message: DiagnosticMessage): RangeToExtract { + return { errors: [createFileDiagnostic(sourceFile, start, length, message)] }; + } + + function checkRootNode(node: Node): Diagnostic[] | undefined { + if (isIdentifier(node)) { + return [createDiagnosticForNode(node, Messages.InsufficientSelection)]; + } + return undefined; + } + + // Verifies whether we can actually extract this node or not. + function checkNode(nodeToCheck: Node): Diagnostic[] | undefined { + const enum PermittedJumps { + None = 0, + Break = 1 << 0, + Continue = 1 << 1, + Return = 1 << 2 + } + if (!isStatement(nodeToCheck) && !(isExpression(nodeToCheck) && isLegalExpressionExtraction(nodeToCheck))) { + return [createDiagnosticForNode(nodeToCheck, Messages.StatementOrExpressionExpected)]; + } + + if (isInAmbientContext(nodeToCheck)) { + return [createDiagnosticForNode(nodeToCheck, Messages.CannotExtractAmbientBlock)]; + } + + // If we're in a class, see if we're in a static region (static property initializer, static method, class constructor parameter default) or not + const stoppingPoint: Node = getContainingClass(nodeToCheck); + if (stoppingPoint) { + let current: Node = nodeToCheck; + while (current !== stoppingPoint) { + if (current.kind === SyntaxKind.PropertyDeclaration) { + if (hasModifier(current, ModifierFlags.Static)) { + rangeFacts |= RangeFacts.InStaticRegion; + } + break; + } + else if (current.kind === SyntaxKind.Parameter) { + const ctorOrMethod = getContainingFunction(current); + if (ctorOrMethod.kind === SyntaxKind.Constructor) { + rangeFacts |= RangeFacts.InStaticRegion; + } + break; + } + else if (current.kind === SyntaxKind.MethodDeclaration) { + if (hasModifier(current, ModifierFlags.Static)) { + rangeFacts |= RangeFacts.InStaticRegion; + } + } + current = current.parent; + } + } + + let errors: Diagnostic[]; + let permittedJumps = PermittedJumps.Return; + let seenLabels: Array<__String>; + + visit(nodeToCheck); + + return errors; + + function visit(node: Node) { + if (errors) { + // already found an error - can stop now + return true; + } + + if (isDeclaration(node)) { + const declaringNode = (node.kind === SyntaxKind.VariableDeclaration) ? node.parent.parent : node; + if (hasModifier(declaringNode, ModifierFlags.Export)) { + (errors || (errors = []).push(createDiagnosticForNode(node, Messages.CannotExtractExportedEntity))); + return true; + } + declarations.push(node.symbol); + } + + // Some things can't be extracted in certain situations + switch (node.kind) { + case SyntaxKind.ImportDeclaration: + (errors || (errors = [])).push(createDiagnosticForNode(node, Messages.CannotExtractFunction)); + return true; + case SyntaxKind.SuperKeyword: + // For a super *constructor call*, we have to be extracting the entire class, + // but a super *method call* simply implies a 'this' reference + if (node.parent.kind === SyntaxKind.CallExpression) { + // Super constructor call + const containingClass = getContainingClass(node); + if (containingClass.pos < span.start || containingClass.end >= (span.start + span.length)) { + (errors || (errors = [])).push(createDiagnosticForNode(node, Messages.CannotExtractFunction)); + return true; + } + } + else { + rangeFacts |= RangeFacts.UsesThis; + } + break; + } + + if (!node || isFunctionLike(node) || isClassLike(node)) { + switch (node.kind) { + case SyntaxKind.FunctionDeclaration: + case SyntaxKind.ClassDeclaration: + if (node.parent.kind === SyntaxKind.SourceFile && (node.parent as ts.SourceFile).externalModuleIndicator === undefined) { + // You cannot extract global declarations + (errors || (errors = [])).push(createDiagnosticForNode(node, Messages.FunctionWillNotBeVisibleInTheNewScope)); + } + break; + } + + // do not dive into functions or classes + return false; + } + const savedPermittedJumps = permittedJumps; + if (node.parent) { + switch (node.parent.kind) { + case SyntaxKind.IfStatement: + if ((node.parent).thenStatement === node || (node.parent).elseStatement === node) { + // forbid all jumps inside thenStatement or elseStatement + permittedJumps = PermittedJumps.None; + } + break; + case SyntaxKind.TryStatement: + if ((node.parent).tryBlock === node) { + // forbid all jumps inside try blocks + permittedJumps = PermittedJumps.None; + } + else if ((node.parent).finallyBlock === node) { + // allow unconditional returns from finally blocks + permittedJumps = PermittedJumps.Return; + } + break; + case SyntaxKind.CatchClause: + if ((node.parent).block === node) { + // forbid all jumps inside the block of catch clause + permittedJumps = PermittedJumps.None; + } + break; + case SyntaxKind.CaseClause: + if ((node).expression !== node) { + // allow unlabeled break inside case clauses + permittedJumps |= PermittedJumps.Break; + } + break; + default: + if (isIterationStatement(node.parent, /*lookInLabeledStatements*/ false)) { + if ((node.parent).statement === node) { + // allow unlabeled break/continue inside loops + permittedJumps |= PermittedJumps.Break | PermittedJumps.Continue; + } + } + break; + } + } + + switch (node.kind) { + case SyntaxKind.ThisType: + case SyntaxKind.ThisKeyword: + rangeFacts |= RangeFacts.UsesThis; + break; + case SyntaxKind.LabeledStatement: + { + const label = (node).label; + (seenLabels || (seenLabels = [])).push(label.escapedText); + forEachChild(node, visit); + seenLabels.pop(); + break; + } + case SyntaxKind.BreakStatement: + case SyntaxKind.ContinueStatement: + { + const label = (node).label; + if (label) { + if (!contains(seenLabels, label.escapedText)) { + // attempts to jump to label that is not in range to be extracted + (errors || (errors = [])).push(createDiagnosticForNode(node, Messages.CannotExtractRangeContainingLabeledBreakOrContinueStatementWithTargetOutsideOfTheRange)); + } + } + else { + if (!(permittedJumps & (SyntaxKind.BreakStatement ? PermittedJumps.Break : PermittedJumps.Continue))) { + // attempt to break or continue in a forbidden context + (errors || (errors = [])).push(createDiagnosticForNode(node, Messages.CannotExtractRangeContainingConditionalBreakOrContinueStatements)); + } + } + break; + } + case SyntaxKind.AwaitExpression: + rangeFacts |= RangeFacts.IsAsyncFunction; + break; + case SyntaxKind.YieldExpression: + rangeFacts |= RangeFacts.IsGenerator; + break; + case SyntaxKind.ReturnStatement: + if (permittedJumps & PermittedJumps.Return) { + rangeFacts |= RangeFacts.HasReturn; + } + else { + (errors || (errors = [])).push(createDiagnosticForNode(node, Messages.CannotExtractRangeContainingConditionalReturnStatement)); + } + break; + default: + forEachChild(node, visit); + break; + } + + permittedJumps = savedPermittedJumps; + } + } + } + + function isValidExtractionTarget(node: Node) { + // Note that we don't use isFunctionLike because we don't want to put the extracted closure *inside* a method + return (node.kind === SyntaxKind.FunctionDeclaration) || (node.kind === SyntaxKind.Constructor) || isSourceFile(node) || isModuleBlock(node) || isClassLike(node); + } + + /** + * Computes possible places we could extract the function into. For example, + * you may be able to extract into a class method *or* local closure *or* namespace function, + * depending on what's in the extracted body. + */ + export function collectEnclosingScopes(range: TargetRange): Scope[] | undefined { + let current: Node = isArray(range.range) ? firstOrUndefined(range.range) : range.range; + if (range.facts & RangeFacts.UsesThis) { + // if range uses this as keyword or as type inside the class then it can only be extracted to a method of the containing class + const containingClass = getContainingClass(current); + if (containingClass) { + return [containingClass]; + } + } + + const start = current; + + let scopes: Scope[] | undefined = undefined; + while (current) { + // We want to find the nearest parent where we can place an "equivalent" sibling to the node we're extracting out of. + // Walk up to the closest parent of a place where we can logically put a sibling: + // * Function declaration + // * Class declaration or expression + // * Module/namespace or source file + if (current !== start && isValidExtractionTarget(current)) { + (scopes = scopes || []).push(current as FunctionLikeDeclaration); + } + + // A function parameter's initializer is actually in the outer scope, not the function declaration + if (current && current.parent && current.parent.kind === SyntaxKind.Parameter) { + // Skip all the way to the outer scope of the function that declared this parameter + current = current.parent.parent.parent; + } + else { + current = current.parent; + } + + } + return scopes; + } + + /** + * Given a piece of text to extract ('targetRange'), computes a list of possible extractions. + * Each returned ExtractResultForScope corresponds to a possible target scope and is either a set of changes + * or an error explaining why we can't extract into that scope. + */ + export function extractRange(targetRange: TargetRange, context: RefactorContext, requestedChangesIndex: number = undefined): ReadonlyArray | undefined { + const { file: sourceFile } = context; + + if (targetRange === undefined) { + return undefined; + } + + const scopes = collectEnclosingScopes(targetRange); + if (scopes === undefined) { + return undefined; + } + + const enclosingTextRange = getEnclosingTextRange(targetRange, sourceFile); + const { target, usagesPerScope, errorsPerScope } = collectReadsAndWrites( + targetRange, + scopes, + enclosingTextRange, + sourceFile, + context.program.getTypeChecker()); + + context.cancellationToken.throwIfCancellationRequested(); + + if (requestedChangesIndex !== undefined) { + if (errorsPerScope[requestedChangesIndex].length) { + return undefined; + } + return [extractFunctionInScope(target, scopes[requestedChangesIndex], usagesPerScope[requestedChangesIndex], targetRange, context)]; + } + else { + return scopes.map((scope, i) => { + const errors = errorsPerScope[i]; + if (errors.length) { + return { + scope, + scopeDescription: getDescriptionForScope(scope), + errors + }; + } + return { scope, scopeDescription: getDescriptionForScope(scope) }; + }); + } + } + + function getDescriptionForScope(scope: Scope) { + if (isFunctionLike(scope)) { + switch (scope.kind) { + case SyntaxKind.Constructor: + return "constructor"; + case SyntaxKind.FunctionExpression: + return scope.name + ? `function expression ${scope.name.getText()}` + : "anonymous function expression"; + case SyntaxKind.FunctionDeclaration: + return `function ${scope.name.getText()}`; + case SyntaxKind.ArrowFunction: + return "arrow function"; + case SyntaxKind.MethodDeclaration: + return `method ${scope.name.getText()}`; + case SyntaxKind.GetAccessor: + return `get ${scope.name.getText()}`; + case SyntaxKind.SetAccessor: + return `set ${scope.name.getText()}`; + } + } + else if (isModuleBlock(scope)) { + return `namespace ${scope.parent.name.getText()}`; + } + else if (isClassLike(scope)) { + return scope.kind === SyntaxKind.ClassDeclaration + ? `class ${scope.name.text}` + : scope.name.text + ? `class expression ${scope.name.text}` + : "anonymous class expression"; + } + else if (isSourceFile(scope)) { + return `file '${scope.fileName}'`; + } + else { + return "unknown"; + } + } + + function getUniqueName(isNameOkay: (name: __String) => boolean) { + let functionNameText = "newFunction" as __String; + if (isNameOkay(functionNameText)) { + return functionNameText; + } + let i = 1; + while (!isNameOkay(functionNameText = `newFunction_${i}` as __String)) { + i++; + } + return functionNameText; + } + + export function extractFunctionInScope( + node: Statement | Expression | Block, + scope: Scope, + { usages: usagesInScope, substitutions }: ScopeUsages, + range: TargetRange, + context: RefactorContext): ExtractResultForScope { + + const checker = context.program.getTypeChecker(); + + // Make a unique name for the extracted function + let functionNameText: __String; + if (isClassLike(scope)) { + const type = range.facts & RangeFacts.InStaticRegion ? checker.getTypeOfSymbolAtLocation(scope.symbol, scope) : checker.getDeclaredTypeOfSymbol(scope.symbol); + const props = checker.getPropertiesOfType(type); + functionNameText = getUniqueName(n => props.every(p => p.name !== n)); + } + else { + const file = scope.getSourceFile(); + functionNameText = getUniqueName(n => !file.identifiers.has(n as string)); + } + const isJS = isInJavaScriptFile(scope); + + const functionName = createIdentifier(functionNameText as string); + const functionReference = createIdentifier(functionNameText as string); + + let returnType: TypeNode = undefined; + const parameters: ParameterDeclaration[] = []; + const callArguments: Identifier[] = []; + let writes: UsageEntry[]; + usagesInScope.forEach((usage, name) => { + let typeNode: TypeNode = undefined; + if (!isJS) { + let type = checker.getTypeOfSymbolAtLocation(usage.symbol, usage.node); + // Widen the type so we don't emit nonsense annotations like "function fn(x: 3) {" + type = checker.getBaseTypeOfLiteralType(type); + typeNode = checker.typeToTypeNode(type, node, NodeBuilderFlags.NoTruncation); + } + + const paramDecl = createParameter( + /*decorators*/ undefined, + /*modifiers*/ undefined, + /*dotDotDotToken*/ undefined, + /*name*/ name, + /*questionToken*/ undefined, + typeNode + ); + parameters.push(paramDecl); + if (usage.usage === Usage.Write) { + (writes || (writes = [])).push(usage); + } + callArguments.push(createIdentifier(name)); + }); + + // Provide explicit return types for contexutally-typed functions + // to avoid problems when there are literal types present + if (isExpression(node) && !isJS) { + const contextualType = checker.getContextualType(node); + returnType = checker.typeToTypeNode(contextualType); + } + + const { body, returnValueProperty } = transformFunctionBody(node); + let newFunction: MethodDeclaration | FunctionDeclaration; + + if (isClassLike(scope)) { + // always create private method in TypeScript files + const modifiers: Modifier[] = isJS ? [] : [createToken(SyntaxKind.PrivateKeyword)]; + if (range.facts & RangeFacts.IsAsyncFunction) { + modifiers.push(createToken(SyntaxKind.AsyncKeyword)); + } + if (range.facts & RangeFacts.InStaticRegion) { + modifiers.push(createToken(SyntaxKind.StaticKeyword)); + } + newFunction = createMethod( + /*decorators*/ undefined, + modifiers, + range.facts & RangeFacts.IsGenerator ? createToken(SyntaxKind.AsteriskToken) : undefined, + functionName, + /*questionToken*/ undefined, + /*typeParameters*/[], + parameters, + returnType, + body + ); + } + else { + newFunction = createFunctionDeclaration( + /*decorators*/ undefined, + range.facts & RangeFacts.IsAsyncFunction ? [createToken(SyntaxKind.AsyncKeyword)] : undefined, + range.facts & RangeFacts.IsGenerator ? createToken(SyntaxKind.AsteriskToken) : undefined, + functionName, + /*typeParameters*/[], + parameters, + returnType, + body + ); + } + + const changeTracker = textChanges.ChangeTracker.fromCodeFixContext(context); + // insert function at the end of the scope + changeTracker.insertNodeBefore(context.file, scope.getLastToken(), newFunction, { prefix: context.newLineCharacter, suffix: context.newLineCharacter }); + + const newNodes: Node[] = []; + // replace range with function call + let call: Expression = createCall( + isClassLike(scope) ? createPropertyAccess(range.facts & RangeFacts.InStaticRegion ? createIdentifier(scope.name.getText()) : createThis(), functionReference) : functionReference, + /*typeArguments*/ undefined, + callArguments); + if (range.facts & RangeFacts.IsGenerator) { + call = createYield(createToken(SyntaxKind.AsteriskToken), call); + } + if (range.facts & RangeFacts.IsAsyncFunction) { + call = createAwait(call); + } + + if (writes) { + if (returnValueProperty) { + // has both writes and return, need to create variable declaration to hold return value; + newNodes.push(createVariableStatement( + /*modifiers*/ undefined, + [createVariableDeclaration(returnValueProperty, createKeywordTypeNode(SyntaxKind.AnyKeyword))] + )); + } + + const assignments = getPropertyAssignmentsForWrites(writes); + if (returnValueProperty) { + assignments.unshift(createShorthandPropertyAssignment(returnValueProperty)); + } + + // propagate writes back + if (assignments.length === 1) { + if (returnValueProperty) { + newNodes.push(createReturn(createIdentifier(returnValueProperty))); + } + else { + newNodes.push(createStatement(createBinary(assignments[0].name, SyntaxKind.EqualsToken, call))); + } + } + else { + // emit e.g. + // { a, b, __return } = newFunction(a, b); + // return __return; + newNodes.push(createStatement(createBinary(createObjectLiteral(assignments), SyntaxKind.EqualsToken, call))); + if (returnValueProperty) { + newNodes.push(createReturn(createIdentifier(returnValueProperty))); + } + } + } + else { + if (range.facts & RangeFacts.HasReturn) { + newNodes.push(createReturn(call)); + } + else if (isArray(range.range)) { + newNodes.push(createStatement(call)); + } + else { + newNodes.push(call); + } + } + + if (isArray(range.range)) { + changeTracker.replaceNodesWithNodes(context.file, range.range, newNodes, { + nodeSeparator: context.newLineCharacter, + suffix: context.newLineCharacter // insert newline only when replacing statements + }); + } + else { + changeTracker.replaceNodeWithNodes(context.file, range.range, newNodes, { nodeSeparator: context.newLineCharacter }); + } + + return { + scope, + scopeDescription: getDescriptionForScope(scope), + changes: changeTracker.getChanges() + }; + + function getPropertyAssignmentsForWrites(writes: UsageEntry[]) { + return writes.map(w => createShorthandPropertyAssignment(w.symbol.name)); + } + + function generateReturnValueProperty() { + return "__return"; + } + + function transformFunctionBody(body: Node) { + if (isBlock(body) && !writes && substitutions.size === 0) { + // already block, no writes to propagate back, no substitutions - can use node as is + return { body: createBlock(body.statements, /*multLine*/ true), returnValueProperty: undefined }; + } + let returnValueProperty: string; + const statements = createNodeArray(isBlock(body) ? body.statements.slice(0) : [isStatement(body) ? body : createReturn(body)]); + // rewrite body if either there are writes that should be propagated back via return statements or there are substitutions + if (writes || substitutions.size) { + const rewrittenStatements = visitNodes(statements, visitor).slice(); + if (writes && !(range.facts & RangeFacts.HasReturn) && isStatement(body)) { + // add return at the end to propagate writes back in case if control flow falls out of the function body + // it is ok to know that range has at least one return since it we only allow unconditional returns + const assignments = getPropertyAssignmentsForWrites(writes); + if (assignments.length === 1) { + rewrittenStatements.push(createReturn(assignments[0].name)); + } + else { + rewrittenStatements.push(createReturn(createObjectLiteral(assignments))); + } + } + return { body: createBlock(rewrittenStatements, /*multiLine*/ true), returnValueProperty }; + } + else { + return { body: createBlock(statements, /*multiLine*/ true), returnValueProperty: undefined }; + } + + function visitor(node: Node): VisitResult { + if (node.kind === SyntaxKind.ReturnStatement && writes) { + const assignments: ObjectLiteralElementLike[] = getPropertyAssignmentsForWrites(writes); + if ((node).expression) { + if (!returnValueProperty) { + returnValueProperty = generateReturnValueProperty(); + } + assignments.unshift(createPropertyAssignment(returnValueProperty, visitNode((node).expression, visitor))); + } + if (assignments.length === 1) { + return createReturn(assignments[0].name as Expression); + } + else { + return createReturn(createObjectLiteral(assignments)); + } + } + else { + const substitution = substitutions.get(getNodeId(node).toString()); + return substitution || visitEachChild(node, visitor, nullTransformationContext); + } + } + } + } + + function isModuleBlock(n: Node): n is ModuleBlock { + return n.kind === SyntaxKind.ModuleBlock; + } + + function isReadonlyArray(v: any): v is ReadonlyArray { + return isArray(v); + } + + /** + * Produces a range that spans the entirety of nodes, given a selection + * that might start/end in the middle of nodes. + * + * For example, when the user makes a selection like this + * v---v + * var someThing = foo + bar; + * this returns ^-------^ + */ + function getEnclosingTextRange(targetRange: TargetRange, sourceFile: SourceFile): TextRange { + return isReadonlyArray(targetRange.range) + ? { pos: targetRange.range[0].getStart(sourceFile), end: targetRange.range[targetRange.range.length - 1].getEnd() } + : targetRange.range; + } + + const enum Usage { + // value should be passed to extracted method + Read = 1, + // value should be passed to extracted method and propagated back + Write = 2 + } + + interface UsageEntry { + readonly usage: Usage; + readonly symbol: Symbol; + readonly node: Node; + } + + interface ScopeUsages { + usages: Map; + substitutions: Map; + } + + function collectReadsAndWrites( + targetRange: TargetRange, + scopes: Scope[], + enclosingTextRange: TextRange, + sourceFile: SourceFile, + checker: TypeChecker) { + + const usagesPerScope: ScopeUsages[] = []; + const substitutionsPerScope: Map[] = []; + const errorsPerScope: Diagnostic[][] = []; + const visibleDeclarationsInExtractedRange: Symbol[] = []; + + // initialize results + for (const _ of scopes) { + usagesPerScope.push({ usages: createMap(), substitutions: createMap() }); + substitutionsPerScope.push(createMap()); + errorsPerScope.push([]); + } + const seenUsages = createMap(); + let valueUsage = Usage.Read; + const target = isReadonlyArray(targetRange.range) ? createBlock(targetRange.range) : targetRange.range; + const containingLexicalScopeOfExtraction = isBlockScope(scopes[0], scopes[0].parent) ? scopes[0] : getEnclosingBlockScopeContainer(scopes[0]); + + collectUsages(target); + + for (let i = 0; i < scopes.length; i++) { + let hasWrite = false; + let readonlyClassPropertyWrite: Declaration | undefined = undefined; + usagesPerScope[i].usages.forEach(value => { + if (value.usage === Usage.Write) { + hasWrite = true; + if (value.symbol.flags & SymbolFlags.ClassMember && + value.symbol.valueDeclaration && + hasModifier(value.symbol.valueDeclaration, ModifierFlags.Readonly)) { + readonlyClassPropertyWrite = value.symbol.valueDeclaration; + } + } + }); + + if (hasWrite && !isArray(targetRange.range) && isExpression(targetRange.range)) { + errorsPerScope[i].push(createDiagnosticForNode(targetRange.range, Messages.CannotCombineWritesAndReturns)); + } + else if (readonlyClassPropertyWrite && i > 0) { + errorsPerScope[i].push(createDiagnosticForNode(readonlyClassPropertyWrite, Messages.CannotCombineWritesAndReturns)); + } + } + + // If there are any declarations in the extracted block that are used in the same enclosing + // lexical scope, we can't move the extraction "up" as those declarations will become unreachable + if (visibleDeclarationsInExtractedRange.length) { + forEachChild(containingLexicalScopeOfExtraction, checkForUsedDeclarations); + } + + return { target, usagesPerScope, errorsPerScope }; + + function collectUsages(node: Node) { + if (isDeclaration(node) && node.symbol) { + visibleDeclarationsInExtractedRange.push(node.symbol); + } + + if (isAssignmentExpression(node)) { + const savedValueUsage = valueUsage; + // use 'write' as default usage for values + valueUsage = Usage.Write; + collectUsages(node.left); + valueUsage = savedValueUsage; + + collectUsages(node.right); + } + else if (isUnaryExpressionWithWrite(node)) { + const savedValueUsage = valueUsage; + valueUsage = Usage.Write; + collectUsages(node.operand); + valueUsage = savedValueUsage; + } + else if (isPropertyAccessExpression(node) || isElementAccessExpression(node)) { + const savedValueUsage = valueUsage; + // use 'write' as default usage for values + valueUsage = Usage.Read; + forEachChild(node, collectUsages); + valueUsage = savedValueUsage; + } + else if (isIdentifier(node)) { + if (!node.parent) { + return; + } + if (isQualifiedName(node.parent) && node !== node.parent.left) { + return; + } + if (isPropertyAccessExpression(node.parent) && node !== node.parent.expression) { + return; + } + recordUsage(node, valueUsage, /*isTypeNode*/ isPartOfTypeNode(node)); + } + else { + forEachChild(node, collectUsages); + } + } + + function recordUsage(n: Identifier, usage: Usage, isTypeNode: boolean) { + const symbolId = recordUsagebySymbol(n, usage, isTypeNode); + if (symbolId) { + for (let i = 0; i < scopes.length; i++) { + // push substitution from map to map to simplify rewriting + const substitition = substitutionsPerScope[i].get(symbolId); + if (substitition) { + usagesPerScope[i].substitutions.set(getNodeId(n).toString(), substitition); + } + } + } + } + + function recordUsagebySymbol(identifier: Identifier, usage: Usage, isTypeName: boolean) { + const symbol = checker.getSymbolAtLocation(identifier); + if (!symbol) { + // cannot find symbol - do nothing + return undefined; + } + const symbolId = getSymbolId(symbol).toString(); + const lastUsage = seenUsages.get(symbolId); + // there are two kinds of value usages + // - reads - if range contains a read from the value located outside of the range then value should be passed as a parameter + // - writes - if range contains a write to a value located outside the range the value should be passed as a parameter and + // returned as a return value + // 'write' case is a superset of 'read' so if we already have processed 'write' of some symbol there is not need to handle 'read' + // since all information is already recorded + if (lastUsage && lastUsage >= usage) { + return symbolId; + } + + seenUsages.set(symbolId, usage); + if (lastUsage) { + // if we get here this means that we are trying to handle 'write' and 'read' was already processed + // walk scopes and update existing records. + for (const perScope of usagesPerScope) { + const prevEntry = perScope.usages.get(identifier.text as string); + if (prevEntry) { + perScope.usages.set(identifier.text as string, { usage, symbol, node: identifier }); + } + } + return symbolId; + } + // find first declaration in this file + const declInFile = find(symbol.getDeclarations(), d => d.getSourceFile() === sourceFile); + if (!declInFile) { + return undefined; + } + if (rangeContainsRange(enclosingTextRange, declInFile)) { + // declaration is located in range to be extracted - do nothing + return undefined; + } + if (targetRange.facts & RangeFacts.IsGenerator && usage === Usage.Write) { + // this is write to a reference located outside of the target scope and range is extracted into generator + // currently this is unsupported scenario + for (const errors of errorsPerScope) { + errors.push(createDiagnosticForNode(identifier, Messages.CannotExtractRangeThatContainsWritesToReferencesLocatedOutsideOfTheTargetRangeInGenerators)); + } + } + for (let i = 0; i < scopes.length; i++) { + const scope = scopes[i]; + const resolvedSymbol = checker.resolveName(symbol.name, scope, symbol.flags); + if (resolvedSymbol === symbol) { + continue; + } + if (!substitutionsPerScope[i].has(symbolId)) { + const substitution = tryReplaceWithQualifiedNameOrPropertyAccess(symbol.exportSymbol || symbol, scope, isTypeName); + if (substitution) { + substitutionsPerScope[i].set(symbolId, substitution); + } + else if (isTypeName) { + errorsPerScope[i].push(createDiagnosticForNode(identifier, Messages.TypeWillNotBeVisibleInTheNewScope)); + } + else { + usagesPerScope[i].usages.set(identifier.text as string, { usage, symbol, node: identifier }); + } + } + } + return symbolId; + } + + function checkForUsedDeclarations(node: Node) { + // If this node is entirely within the original extraction range, we don't need to do anything. + if (node === targetRange.range || (isArray(targetRange.range) && targetRange.range.indexOf(node as Statement) >= 0)) { + return; + } + + // Otherwise check and recurse. + const sym = checker.getSymbolAtLocation(node); + if (sym && visibleDeclarationsInExtractedRange.some(d => d === sym)) { + for (const scope of errorsPerScope) { + scope.push(createDiagnosticForNode(node, Messages.CannotExtractExportedEntity)); + } + return true; + } + else { + forEachChild(node, checkForUsedDeclarations); + } + } + + function tryReplaceWithQualifiedNameOrPropertyAccess(symbol: Symbol, scopeDecl: Node, isTypeNode: boolean): PropertyAccessExpression | EntityName { + if (!symbol) { + return undefined; + } + if (symbol.getDeclarations().some(d => d.parent === scopeDecl)) { + return createIdentifier(symbol.name); + } + const prefix = tryReplaceWithQualifiedNameOrPropertyAccess(symbol.parent, scopeDecl, isTypeNode); + if (prefix === undefined) { + return undefined; + } + return isTypeNode ? createQualifiedName(prefix, createIdentifier(symbol.name)) : createPropertyAccess(prefix, symbol.name); + } + } + + function getParentNodeInSpan(node: Node, file: SourceFile, span: TextSpan): Node { + while (node) { + if (!node.parent) { + return undefined; + } + if (isSourceFile(node.parent) || !spanContainsNode(span, node.parent, file)) { + return node; + } + + node = node.parent; + } + } + + function spanContainsNode(span: TextSpan, node: Node, file: SourceFile): boolean { + return textSpanContainsPosition(span, node.getStart(file)) && + node.getEnd() <= textSpanEnd(span); + } + + /** + * Computes whether or not a node represents an expression in a position where it could + * be extracted. + * The isExpression() in utilities.ts returns some false positives we need to handle, + * such as `import x from 'y'` -- the 'y' is a StringLiteral but is *not* an expression + * in the sense of something that you could extract on + */ + function isLegalExpressionExtraction(node: Node): boolean { + switch (node.parent.kind) { + case SyntaxKind.EnumMember: + return false; + } + + switch (node.kind) { + case SyntaxKind.StringLiteral: + return node.parent.kind !== SyntaxKind.ImportDeclaration && + node.parent.kind !== SyntaxKind.ImportSpecifier; + + case SyntaxKind.SpreadElement: + case SyntaxKind.ObjectBindingPattern: + case SyntaxKind.BindingElement: + return false; + + case SyntaxKind.Identifier: + return node.parent.kind !== SyntaxKind.BindingElement && + node.parent.kind !== SyntaxKind.ImportSpecifier; + } + return true; + } + + function isBlockLike(node: Node): node is BlockLike { + switch (node.kind) { + case SyntaxKind.Block: + case SyntaxKind.SourceFile: + case SyntaxKind.ModuleBlock: + case SyntaxKind.CaseClause: + return true; + default: + return false; + } + } +} diff --git a/src/services/refactors/refactors.ts b/src/services/refactors/refactors.ts index 4c4ecb33416b1..3a33ccc83c2b8 100644 --- a/src/services/refactors/refactors.ts +++ b/src/services/refactors/refactors.ts @@ -1 +1,2 @@ /// +/// diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index d468263227e1e..5480c8f34a5fe 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -64,7 +64,7 @@ namespace ts.textChanges { */ export type ConfigurableStartEnd = ConfigurableStart & ConfigurableEnd; - export interface InsertNodeOptions { + interface InsertNodeOptions { /** * Text to be inserted before the new node */ @@ -83,16 +83,43 @@ namespace ts.textChanges { delta?: number; } - export type ChangeNodeOptions = ConfigurableStartEnd & InsertNodeOptions; + enum ChangeKind { + Remove, + ReplaceWithSingleNode, + ReplaceWithMultipleNodes + } + + type Change = ReplaceWithSingleNode | ReplaceWithMultipleNodes | RemoveNode; - interface Change { + interface BaseChange { readonly sourceFile: SourceFile; readonly range: TextRange; + } + + interface ChangeNodeOptions extends ConfigurableStartEnd, InsertNodeOptions { readonly useIndentationFromFile?: boolean; - readonly node?: Node; + } + interface ReplaceWithSingleNode extends BaseChange { + readonly kind: ChangeKind.ReplaceWithSingleNode; + readonly node: Node; readonly options?: ChangeNodeOptions; } + interface RemoveNode extends BaseChange { + readonly kind: ChangeKind.Remove; + readonly node?: never; + readonly options?: never; + } + + interface ChangeMultipleNodesOptions extends ChangeNodeOptions { + nodeSeparator: string; + } + interface ReplaceWithMultipleNodes extends BaseChange { + readonly kind: ChangeKind.ReplaceWithMultipleNodes; + readonly nodes: ReadonlyArray; + readonly options?: ChangeMultipleNodesOptions; + } + export function getSeparatorCharacter(separator: Token) { return tokenToString(separator.kind); } @@ -153,12 +180,16 @@ namespace ts.textChanges { return s; } + function getNewlineKind(context: { newLineCharacter: string }) { + return context.newLineCharacter === "\n" ? NewLineKind.LineFeed : NewLineKind.CarriageReturnLineFeed; + } + export class ChangeTracker { private changes: Change[] = []; private readonly newLineCharacter: string; - public static fromCodeFixContext(context: { newLineCharacter: string, rulesProvider: formatting.RulesProvider }) { - return new ChangeTracker(context.newLineCharacter === "\n" ? NewLineKind.LineFeed : NewLineKind.CarriageReturnLineFeed, context.rulesProvider); + public static fromCodeFixContext(context: { newLineCharacter: string, rulesProvider?: formatting.RulesProvider }) { + return new ChangeTracker(getNewlineKind(context), context.rulesProvider); } constructor( @@ -168,22 +199,22 @@ namespace ts.textChanges { this.newLineCharacter = getNewLineCharacter({ newLine }); } - public deleteNode(sourceFile: SourceFile, node: Node, options: ConfigurableStartEnd = {}) { - const startPosition = getAdjustedStartPosition(sourceFile, node, options, Position.FullStart); - const endPosition = getAdjustedEndPosition(sourceFile, node, options); - this.changes.push({ sourceFile, options, range: { pos: startPosition, end: endPosition } }); + public deleteRange(sourceFile: SourceFile, range: TextRange) { + this.changes.push({ kind: ChangeKind.Remove, sourceFile, range }); return this; } - public deleteRange(sourceFile: SourceFile, range: TextRange) { - this.changes.push({ sourceFile, range }); + public deleteNode(sourceFile: SourceFile, node: Node, options: ConfigurableStartEnd = {}) { + const startPosition = getAdjustedStartPosition(sourceFile, node, options, Position.FullStart); + const endPosition = getAdjustedEndPosition(sourceFile, node, options); + this.changes.push({ kind: ChangeKind.Remove, sourceFile, range: { pos: startPosition, end: endPosition } }); return this; } public deleteNodeRange(sourceFile: SourceFile, startNode: Node, endNode: Node, options: ConfigurableStartEnd = {}) { const startPosition = getAdjustedStartPosition(sourceFile, startNode, options, Position.FullStart); const endPosition = getAdjustedEndPosition(sourceFile, endNode, options); - this.changes.push({ sourceFile, options, range: { pos: startPosition, end: endPosition } }); + this.changes.push({ kind: ChangeKind.Remove, sourceFile, range: { pos: startPosition, end: endPosition } }); return this; } @@ -223,33 +254,74 @@ namespace ts.textChanges { } public replaceRange(sourceFile: SourceFile, range: TextRange, newNode: Node, options: InsertNodeOptions = {}) { - this.changes.push({ sourceFile, range, options, node: newNode }); + this.changes.push({ kind: ChangeKind.ReplaceWithSingleNode, sourceFile, range, options, node: newNode }); return this; } public replaceNode(sourceFile: SourceFile, oldNode: Node, newNode: Node, options: ChangeNodeOptions = {}) { const startPosition = getAdjustedStartPosition(sourceFile, oldNode, options, Position.Start); const endPosition = getAdjustedEndPosition(sourceFile, oldNode, options); - this.changes.push({ sourceFile, options, useIndentationFromFile: true, node: newNode, range: { pos: startPosition, end: endPosition } }); - return this; + return this.replaceWithSingle(sourceFile, startPosition, endPosition, newNode, options); } public replaceNodeRange(sourceFile: SourceFile, startNode: Node, endNode: Node, newNode: Node, options: ChangeNodeOptions = {}) { const startPosition = getAdjustedStartPosition(sourceFile, startNode, options, Position.Start); const endPosition = getAdjustedEndPosition(sourceFile, endNode, options); - this.changes.push({ sourceFile, options, useIndentationFromFile: true, node: newNode, range: { pos: startPosition, end: endPosition } }); + return this.replaceWithSingle(sourceFile, startPosition, endPosition, newNode, options); + } + + private replaceWithSingle(sourceFile: SourceFile, startPosition: number, endPosition: number, newNode: Node, options: ChangeNodeOptions): this { + this.changes.push({ + kind: ChangeKind.ReplaceWithSingleNode, + sourceFile, + options, + node: newNode, + range: { pos: startPosition, end: endPosition } + }); + return this; + } + + private replaceWithMultiple(sourceFile: SourceFile, startPosition: number, endPosition: number, newNodes: ReadonlyArray, options: ChangeMultipleNodesOptions): this { + this.changes.push({ + kind: ChangeKind.ReplaceWithMultipleNodes, + sourceFile, + options, + nodes: newNodes, + range: { pos: startPosition, end: endPosition } + }); return this; } + public replaceNodeWithNodes(sourceFile: SourceFile, oldNode: Node, newNodes: ReadonlyArray, options: ChangeMultipleNodesOptions) { + const startPosition = getAdjustedStartPosition(sourceFile, oldNode, options, Position.Start); + const endPosition = getAdjustedEndPosition(sourceFile, oldNode, options); + return this.replaceWithMultiple(sourceFile, startPosition, endPosition, newNodes, options); + } + + public replaceNodesWithNodes(sourceFile: SourceFile, oldNodes: ReadonlyArray, newNodes: ReadonlyArray, options: ChangeMultipleNodesOptions) { + const startPosition = getAdjustedStartPosition(sourceFile, oldNodes[0], options, Position.Start); + const endPosition = getAdjustedEndPosition(sourceFile, lastOrUndefined(oldNodes), options); + return this.replaceWithMultiple(sourceFile, startPosition, endPosition, newNodes, options); + } + + public replaceRangeWithNodes(sourceFile: SourceFile, range: TextRange, newNodes: ReadonlyArray, options: ChangeMultipleNodesOptions) { + return this.replaceWithMultiple(sourceFile, range.pos, range.end, newNodes, options); + } + + public replaceNodeRangeWithNodes(sourceFile: SourceFile, startNode: Node, endNode: Node, newNodes: ReadonlyArray, options: ChangeMultipleNodesOptions) { + const startPosition = getAdjustedStartPosition(sourceFile, startNode, options, Position.Start); + const endPosition = getAdjustedEndPosition(sourceFile, endNode, options); + return this.replaceWithMultiple(sourceFile, startPosition, endPosition, newNodes, options); + } + public insertNodeAt(sourceFile: SourceFile, pos: number, newNode: Node, options: InsertNodeOptions = {}) { - this.changes.push({ sourceFile, options, node: newNode, range: { pos, end: pos } }); + this.changes.push({ kind: ChangeKind.ReplaceWithSingleNode, sourceFile, options, node: newNode, range: { pos, end: pos } }); return this; } public insertNodeBefore(sourceFile: SourceFile, before: Node, newNode: Node, options: InsertNodeOptions & ConfigurableStart = {}) { const startPosition = getAdjustedStartPosition(sourceFile, before, options, Position.Start); - this.changes.push({ sourceFile, options, useIndentationFromFile: true, node: newNode, range: { pos: startPosition, end: startPosition } }); - return this; + return this.replaceWithSingle(sourceFile, startPosition, startPosition, newNode, options); } public insertNodeAfter(sourceFile: SourceFile, after: Node, newNode: Node, options: InsertNodeOptions & ConfigurableEnd = {}) { @@ -261,6 +333,7 @@ namespace ts.textChanges { // if not - insert semicolon to preserve the code from changing the meaning due to ASI if (sourceFile.text.charCodeAt(after.end - 1) !== CharacterCodes.semicolon) { this.changes.push({ + kind: ChangeKind.ReplaceWithSingleNode, sourceFile, options: {}, range: { pos: after.end, end: after.end }, @@ -269,8 +342,7 @@ namespace ts.textChanges { } } const endPosition = getAdjustedEndPosition(sourceFile, after, options); - this.changes.push({ sourceFile, options, useIndentationFromFile: true, node: newNode, range: { pos: endPosition, end: endPosition } }); - return this; + return this.replaceWithSingle(sourceFile, endPosition, endPosition, newNode, options); } /** @@ -339,10 +411,10 @@ namespace ts.textChanges { } this.changes.push({ + kind: ChangeKind.ReplaceWithSingleNode, sourceFile, range: { pos: startPos, end: containingList[index + 1].getStart(sourceFile) }, node: newNode, - useIndentationFromFile: true, options: { prefix, // write separator and leading trivia of the next element as suffix @@ -383,6 +455,7 @@ namespace ts.textChanges { if (multilineList) { // insert separator immediately following the 'after' node to preserve comments in trailing trivia this.changes.push({ + kind: ChangeKind.ReplaceWithSingleNode, sourceFile, range: { pos: end, end }, node: createToken(separator), @@ -396,6 +469,7 @@ namespace ts.textChanges { insertPos--; } this.changes.push({ + kind: ChangeKind.ReplaceWithSingleNode, sourceFile, range: { pos: insertPos, end: insertPos }, node: newNode, @@ -404,6 +478,7 @@ namespace ts.textChanges { } else { this.changes.push({ + kind: ChangeKind.ReplaceWithSingleNode, sourceFile, range: { pos: end, end }, node: newNode, @@ -446,38 +521,51 @@ namespace ts.textChanges { } private computeNewText(change: Change, sourceFile: SourceFile): string { - if (!change.node) { + if (change.kind === ChangeKind.Remove) { // deletion case return ""; } + const options = change.options || {}; - const nonFormattedText = getNonformattedText(change.node, sourceFile, this.newLine); + let text: string; + const pos = change.range.pos; + const posStartsLine = getLineStartPositionForPosition(pos, sourceFile) === pos; + if (change.kind === ChangeKind.ReplaceWithMultipleNodes) { + const parts = change.nodes.map(n => this.getFormattedTextOfNode(n, sourceFile, pos, options)); + text = parts.join(change.options.nodeSeparator); + } + else { + Debug.assert(change.kind === ChangeKind.ReplaceWithSingleNode, "change.kind === ReplaceWithSingleNode"); + text = this.getFormattedTextOfNode(change.node, sourceFile, pos, options); + } + // strip initial indentation (spaces or tabs) if text will be inserted in the middle of the line + text = (posStartsLine || options.indentation !== undefined) ? text : text.replace(/^\s+/, ""); + return (options.prefix || "") + text + (options.suffix || ""); + } + + private getFormattedTextOfNode(node: Node, sourceFile: SourceFile, pos: number, options: ChangeNodeOptions): string { + const nonformattedText = getNonformattedText(node, sourceFile, this.newLine); if (this.validator) { - this.validator(nonFormattedText); + this.validator(nonformattedText); } const formatOptions = this.rulesProvider.getFormatOptions(); - const pos = change.range.pos; const posStartsLine = getLineStartPositionForPosition(pos, sourceFile) === pos; const initialIndentation = - change.options.indentation !== undefined - ? change.options.indentation - : change.useIndentationFromFile - ? formatting.SmartIndenter.getIndentation(change.range.pos, sourceFile, formatOptions, posStartsLine || (change.options.prefix === this.newLineCharacter)) + options.indentation !== undefined + ? options.indentation + : (options.useIndentationFromFile !== false) + ? formatting.SmartIndenter.getIndentation(pos, sourceFile, formatOptions, posStartsLine || (options.prefix === this.newLineCharacter)) : 0; const delta = - change.options.delta !== undefined - ? change.options.delta - : formatting.SmartIndenter.shouldIndentChildNode(change.node) - ? formatOptions.indentSize + options.delta !== undefined + ? options.delta + : formatting.SmartIndenter.shouldIndentChildNode(node) + ? (formatOptions.indentSize || 0) : 0; - let text = applyFormatting(nonFormattedText, sourceFile, initialIndentation, delta, this.rulesProvider); - // strip initial indentation (spaces or tabs) if text will be inserted in the middle of the line - // however keep indentation if it is was forced - text = posStartsLine || change.options.indentation !== undefined ? text : text.replace(/^\s+/, ""); - return (options.prefix || "") + text + (options.suffix || ""); + return applyFormatting(nonformattedText, sourceFile, initialIndentation, delta, this.rulesProvider); } private static normalize(changes: Change[]): Change[] { @@ -654,4 +742,4 @@ namespace ts.textChanges { this.lastNonTriviaPosition = 0; } } -} \ No newline at end of file +} diff --git a/src/services/types.ts b/src/services/types.ts index 6ff7402f3ad0c..b034e2813c6e1 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -271,7 +271,6 @@ namespace ts { isValidBraceCompletionAtPosition(fileName: string, position: number, openingBrace: number): boolean; getCodeFixesAtPosition(fileName: string, start: number, end: number, errorCodes: number[], formatOptions: FormatCodeSettings): CodeAction[]; - getApplicableRefactors(fileName: string, positionOrRaneg: number | TextRange): ApplicableRefactorInfo[]; getEditsForRefactor(fileName: string, formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, refactorName: string, actionName: string): RefactorEditInfo | undefined; diff --git a/tests/baselines/reference/extractMethod/extractMethod1.js b/tests/baselines/reference/extractMethod/extractMethod1.js new file mode 100644 index 0000000000000..60c7e301616e8 --- /dev/null +++ b/tests/baselines/reference/extractMethod/extractMethod1.js @@ -0,0 +1,98 @@ +==ORIGINAL== +namespace A { + let x = 1; + function foo() { + } + namespace B { + function a() { + let a = 1; + + let y = 5; + let z = x; + a = y; + foo(); + } + } +} +==SCOPE::function a== +namespace A { + let x = 1; + function foo() { + } + namespace B { + function a() { + let a = 1; + + newFunction(); + + function newFunction() { + let y = 5; + let z = x; + a = y; + foo(); + } + } + } +} +==SCOPE::namespace B== +namespace A { + let x = 1; + function foo() { + } + namespace B { + function a() { + let a = 1; + + a = newFunction(a); + } + + function newFunction(a: number) { + let y = 5; + let z = x; + a = y; + foo(); + return a; + } + } +} +==SCOPE::namespace A== +namespace A { + let x = 1; + function foo() { + } + namespace B { + function a() { + let a = 1; + + a = newFunction(a); + } + } + + function newFunction(a: number) { + let y = 5; + let z = x; + a = y; + foo(); + return a; + } +} +==SCOPE::file '/a.ts'== +namespace A { + let x = 1; + function foo() { + } + namespace B { + function a() { + let a = 1; + + a = newFunction(x, a, foo); + } + } +} +function newFunction(x: number, a: number, foo: () => void) { + let y = 5; + let z = x; + a = y; + foo(); + return a; +} diff --git a/tests/baselines/reference/extractMethod/extractMethod10.js b/tests/baselines/reference/extractMethod/extractMethod10.js new file mode 100644 index 0000000000000..02923b7ab50c7 --- /dev/null +++ b/tests/baselines/reference/extractMethod/extractMethod10.js @@ -0,0 +1,55 @@ +==ORIGINAL== +namespace A { + export interface I { x: number }; + class C { + a() { + let z = 1; + let a1: I = { x: 1 }; + return a1.x + 10; + } + } +} +==SCOPE::class C== +namespace A { + export interface I { x: number }; + class C { + a() { + let z = 1; + return this.newFunction(); + } + + private newFunction() { + let a1: I = { x: 1 }; + return a1.x + 10; + } + } +} +==SCOPE::namespace A== +namespace A { + export interface I { x: number }; + class C { + a() { + let z = 1; + return newFunction(); + } + } + + function newFunction() { + let a1: I = { x: 1 }; + return a1.x + 10; + } +} +==SCOPE::file '/a.ts'== +namespace A { + export interface I { x: number }; + class C { + a() { + let z = 1; + return newFunction(); + } + } +} +function newFunction() { + let a1: A.I = { x: 1 }; + return a1.x + 10; +} diff --git a/tests/baselines/reference/extractMethod/extractMethod11.js b/tests/baselines/reference/extractMethod/extractMethod11.js new file mode 100644 index 0000000000000..77565e7b0a778 --- /dev/null +++ b/tests/baselines/reference/extractMethod/extractMethod11.js @@ -0,0 +1,69 @@ +==ORIGINAL== +namespace A { + let y = 1; + class C { + a() { + let z = 1; + let a1 = { x: 1 }; + y = 10; + z = 42; + return a1.x + 10; + } + } +} +==SCOPE::class C== +namespace A { + let y = 1; + class C { + a() { + let z = 1; + var __return: any; + ({ __return, z } = this.newFunction(z)); + return __return; + } + + private newFunction(z: number) { + let a1 = { x: 1 }; + y = 10; + z = 42; + return { __return: a1.x + 10, z }; + } + } +} +==SCOPE::namespace A== +namespace A { + let y = 1; + class C { + a() { + let z = 1; + var __return: any; + ({ __return, z } = newFunction(z)); + return __return; + } + } + + function newFunction(z: number) { + let a1 = { x: 1 }; + y = 10; + z = 42; + return { __return: a1.x + 10, z }; + } +} +==SCOPE::file '/a.ts'== +namespace A { + let y = 1; + class C { + a() { + let z = 1; + var __return: any; + ({ __return, y, z } = newFunction(y, z)); + return __return; + } + } +} +function newFunction(y: number, z: number) { + let a1 = { x: 1 }; + y = 10; + z = 42; + return { __return: a1.x + 10, y, z }; +} diff --git a/tests/baselines/reference/extractMethod/extractMethod12.js b/tests/baselines/reference/extractMethod/extractMethod12.js new file mode 100644 index 0000000000000..8ff6e130dad2a --- /dev/null +++ b/tests/baselines/reference/extractMethod/extractMethod12.js @@ -0,0 +1,36 @@ +==ORIGINAL== +namespace A { + let y = 1; + class C { + b() {} + a() { + let z = 1; + let a1 = { x: 1 }; + y = 10; + z = 42; + this.b(); + return a1.x + 10; + } + } +} +==SCOPE::class C== +namespace A { + let y = 1; + class C { + b() {} + a() { + let z = 1; + var __return: any; + ({ __return, z } = this.newFunction(z)); + return __return; + } + + private newFunction(z: number) { + let a1 = { x: 1 }; + y = 10; + z = 42; + this.b(); + return { __return: a1.x + 10, z }; + } + } +} \ No newline at end of file diff --git a/tests/baselines/reference/extractMethod/extractMethod2.js b/tests/baselines/reference/extractMethod/extractMethod2.js new file mode 100644 index 0000000000000..28c27993d7165 --- /dev/null +++ b/tests/baselines/reference/extractMethod/extractMethod2.js @@ -0,0 +1,85 @@ +==ORIGINAL== +namespace A { + let x = 1; + function foo() { + } + namespace B { + function a() { + + let y = 5; + let z = x; + return foo(); + } + } +} +==SCOPE::function a== +namespace A { + let x = 1; + function foo() { + } + namespace B { + function a() { + + return newFunction(); + + function newFunction() { + let y = 5; + let z = x; + return foo(); + } + } + } +} +==SCOPE::namespace B== +namespace A { + let x = 1; + function foo() { + } + namespace B { + function a() { + + return newFunction(); + } + + function newFunction() { + let y = 5; + let z = x; + return foo(); + } + } +} +==SCOPE::namespace A== +namespace A { + let x = 1; + function foo() { + } + namespace B { + function a() { + + return newFunction(); + } + } + + function newFunction() { + let y = 5; + let z = x; + return foo(); + } +} +==SCOPE::file '/a.ts'== +namespace A { + let x = 1; + function foo() { + } + namespace B { + function a() { + + return newFunction(x, foo); + } + } +} +function newFunction(x: number, foo: () => void) { + let y = 5; + let z = x; + return foo(); +} diff --git a/tests/baselines/reference/extractMethod/extractMethod3.js b/tests/baselines/reference/extractMethod/extractMethod3.js new file mode 100644 index 0000000000000..e5903ead18199 --- /dev/null +++ b/tests/baselines/reference/extractMethod/extractMethod3.js @@ -0,0 +1,80 @@ +==ORIGINAL== +namespace A { + function foo() { + } + namespace B { + function* a(z: number) { + + let y = 5; + yield z; + return foo(); + } + } +} +==SCOPE::function a== +namespace A { + function foo() { + } + namespace B { + function* a(z: number) { + + return yield* newFunction(); + + function* newFunction() { + let y = 5; + yield z; + return foo(); + } + } + } +} +==SCOPE::namespace B== +namespace A { + function foo() { + } + namespace B { + function* a(z: number) { + + return yield* newFunction(z); + } + + function* newFunction(z: number) { + let y = 5; + yield z; + return foo(); + } + } +} +==SCOPE::namespace A== +namespace A { + function foo() { + } + namespace B { + function* a(z: number) { + + return yield* newFunction(z); + } + } + + function* newFunction(z: number) { + let y = 5; + yield z; + return foo(); + } +} +==SCOPE::file '/a.ts'== +namespace A { + function foo() { + } + namespace B { + function* a(z: number) { + + return yield* newFunction(z, foo); + } + } +} +function* newFunction(z: number, foo: () => void) { + let y = 5; + yield z; + return foo(); +} diff --git a/tests/baselines/reference/extractMethod/extractMethod4.js b/tests/baselines/reference/extractMethod/extractMethod4.js new file mode 100644 index 0000000000000..6b9e2eed099d3 --- /dev/null +++ b/tests/baselines/reference/extractMethod/extractMethod4.js @@ -0,0 +1,90 @@ +==ORIGINAL== +namespace A { + function foo() { + } + namespace B { + async function a(z: number, z1: any) { + + let y = 5; + if (z) { + await z1; + } + return foo(); + } + } +} +==SCOPE::function a== +namespace A { + function foo() { + } + namespace B { + async function a(z: number, z1: any) { + + return await newFunction(); + + async function newFunction() { + let y = 5; + if(z) { + await z1; + } + return foo(); + } + } + } +} +==SCOPE::namespace B== +namespace A { + function foo() { + } + namespace B { + async function a(z: number, z1: any) { + + return await newFunction(z, z1); + } + + async function newFunction(z: number, z1: any) { + let y = 5; + if(z) { + await z1; + } + return foo(); + } + } +} +==SCOPE::namespace A== +namespace A { + function foo() { + } + namespace B { + async function a(z: number, z1: any) { + + return await newFunction(z, z1); + } + } + + async function newFunction(z: number, z1: any) { + let y = 5; + if(z) { + await z1; + } + return foo(); + } +} +==SCOPE::file '/a.ts'== +namespace A { + function foo() { + } + namespace B { + async function a(z: number, z1: any) { + + return await newFunction(z, z1, foo); + } + } +} +async function newFunction(z: number, z1: any, foo: () => void) { + let y = 5; + if(z) { + await z1; +} + return foo(); +} diff --git a/tests/baselines/reference/extractMethod/extractMethod5.js b/tests/baselines/reference/extractMethod/extractMethod5.js new file mode 100644 index 0000000000000..cf63cb2a93214 --- /dev/null +++ b/tests/baselines/reference/extractMethod/extractMethod5.js @@ -0,0 +1,98 @@ +==ORIGINAL== +namespace A { + let x = 1; + export function foo() { + } + namespace B { + function a() { + let a = 1; + + let y = 5; + let z = x; + a = y; + foo(); + } + } +} +==SCOPE::function a== +namespace A { + let x = 1; + export function foo() { + } + namespace B { + function a() { + let a = 1; + + newFunction(); + + function newFunction() { + let y = 5; + let z = x; + a = y; + foo(); + } + } + } +} +==SCOPE::namespace B== +namespace A { + let x = 1; + export function foo() { + } + namespace B { + function a() { + let a = 1; + + a = newFunction(a); + } + + function newFunction(a: number) { + let y = 5; + let z = x; + a = y; + foo(); + return a; + } + } +} +==SCOPE::namespace A== +namespace A { + let x = 1; + export function foo() { + } + namespace B { + function a() { + let a = 1; + + a = newFunction(a); + } + } + + function newFunction(a: number) { + let y = 5; + let z = x; + a = y; + foo(); + return a; + } +} +==SCOPE::file '/a.ts'== +namespace A { + let x = 1; + export function foo() { + } + namespace B { + function a() { + let a = 1; + + a = newFunction(x, a); + } + } +} +function newFunction(x: number, a: number) { + let y = 5; + let z = x; + a = y; + A.foo(); + return a; +} diff --git a/tests/baselines/reference/extractMethod/extractMethod6.js b/tests/baselines/reference/extractMethod/extractMethod6.js new file mode 100644 index 0000000000000..99f52e9febbe0 --- /dev/null +++ b/tests/baselines/reference/extractMethod/extractMethod6.js @@ -0,0 +1,101 @@ +==ORIGINAL== +namespace A { + let x = 1; + export function foo() { + } + namespace B { + function a() { + let a = 1; + + let y = 5; + let z = x; + a = y; + return foo(); + } + } +} +==SCOPE::function a== +namespace A { + let x = 1; + export function foo() { + } + namespace B { + function a() { + let a = 1; + + return newFunction(); + + function newFunction() { + let y = 5; + let z = x; + a = y; + return foo(); + } + } + } +} +==SCOPE::namespace B== +namespace A { + let x = 1; + export function foo() { + } + namespace B { + function a() { + let a = 1; + + var __return: any; + ({ __return, a } = newFunction(a)); + return __return; + } + + function newFunction(a: number) { + let y = 5; + let z = x; + a = y; + return { __return: foo(), a }; + } + } +} +==SCOPE::namespace A== +namespace A { + let x = 1; + export function foo() { + } + namespace B { + function a() { + let a = 1; + + var __return: any; + ({ __return, a } = newFunction(a)); + return __return; + } + } + + function newFunction(a: number) { + let y = 5; + let z = x; + a = y; + return { __return: foo(), a }; + } +} +==SCOPE::file '/a.ts'== +namespace A { + let x = 1; + export function foo() { + } + namespace B { + function a() { + let a = 1; + + var __return: any; + ({ __return, a } = newFunction(x, a)); + return __return; + } + } +} +function newFunction(x: number, a: number) { + let y = 5; + let z = x; + a = y; + return { __return: A.foo(), a }; +} diff --git a/tests/baselines/reference/extractMethod/extractMethod7.js b/tests/baselines/reference/extractMethod/extractMethod7.js new file mode 100644 index 0000000000000..09d5edaa2c05d --- /dev/null +++ b/tests/baselines/reference/extractMethod/extractMethod7.js @@ -0,0 +1,111 @@ +==ORIGINAL== +namespace A { + let x = 1; + export namespace C { + export function foo() { + } + } + namespace B { + function a() { + let a = 1; + + let y = 5; + let z = x; + a = y; + return C.foo(); + } + } +} +==SCOPE::function a== +namespace A { + let x = 1; + export namespace C { + export function foo() { + } + } + namespace B { + function a() { + let a = 1; + + return newFunction(); + + function newFunction() { + let y = 5; + let z = x; + a = y; + return C.foo(); + } + } + } +} +==SCOPE::namespace B== +namespace A { + let x = 1; + export namespace C { + export function foo() { + } + } + namespace B { + function a() { + let a = 1; + + var __return: any; + ({ __return, a } = newFunction(a)); + return __return; + } + + function newFunction(a: number) { + let y = 5; + let z = x; + a = y; + return { __return: C.foo(), a }; + } + } +} +==SCOPE::namespace A== +namespace A { + let x = 1; + export namespace C { + export function foo() { + } + } + namespace B { + function a() { + let a = 1; + + var __return: any; + ({ __return, a } = newFunction(a)); + return __return; + } + } + + function newFunction(a: number) { + let y = 5; + let z = x; + a = y; + return { __return: C.foo(), a }; + } +} +==SCOPE::file '/a.ts'== +namespace A { + let x = 1; + export namespace C { + export function foo() { + } + } + namespace B { + function a() { + let a = 1; + + var __return: any; + ({ __return, a } = newFunction(x, a)); + return __return; + } + } +} +function newFunction(x: number, a: number) { + let y = 5; + let z = x; + a = y; + return { __return: A.C.foo(), a }; +} diff --git a/tests/baselines/reference/extractMethod/extractMethod8.js b/tests/baselines/reference/extractMethod/extractMethod8.js new file mode 100644 index 0000000000000..fe9cf2a92f0fd --- /dev/null +++ b/tests/baselines/reference/extractMethod/extractMethod8.js @@ -0,0 +1,65 @@ +==ORIGINAL== +namespace A { + let x = 1; + namespace B { + function a() { + let a1 = 1; + return 1 + a1 + x + 100; + } + } +} +==SCOPE::function a== +namespace A { + let x = 1; + namespace B { + function a() { + let a1 = 1; + return newFunction() + 100; + + function newFunction() { + return 1 + a1 + x; + } + } + } +} +==SCOPE::namespace B== +namespace A { + let x = 1; + namespace B { + function a() { + let a1 = 1; + return newFunction(a1) + 100; + } + + function newFunction(a1: number) { + return 1 + a1 + x; + } + } +} +==SCOPE::namespace A== +namespace A { + let x = 1; + namespace B { + function a() { + let a1 = 1; + return newFunction(a1) + 100; + } + } + + function newFunction(a1: number) { + return 1 + a1 + x; + } +} +==SCOPE::file '/a.ts'== +namespace A { + let x = 1; + namespace B { + function a() { + let a1 = 1; + return newFunction(a1, x) + 100; + } + } +} +function newFunction(a1: number, x: number) { + return 1 + a1 + x; +} diff --git a/tests/baselines/reference/extractMethod/extractMethod9.js b/tests/baselines/reference/extractMethod/extractMethod9.js new file mode 100644 index 0000000000000..fcc5dcd23de07 --- /dev/null +++ b/tests/baselines/reference/extractMethod/extractMethod9.js @@ -0,0 +1,65 @@ +==ORIGINAL== +namespace A { + export interface I { x: number }; + namespace B { + function a() { + let a1: I = { x: 1 }; + return a1.x + 10; + } + } +} +==SCOPE::function a== +namespace A { + export interface I { x: number }; + namespace B { + function a() { + return newFunction(); + + function newFunction() { + let a1: I = { x: 1 }; + return a1.x + 10; + } + } + } +} +==SCOPE::namespace B== +namespace A { + export interface I { x: number }; + namespace B { + function a() { + return newFunction(); + } + + function newFunction() { + let a1: I = { x: 1 }; + return a1.x + 10; + } + } +} +==SCOPE::namespace A== +namespace A { + export interface I { x: number }; + namespace B { + function a() { + return newFunction(); + } + } + + function newFunction() { + let a1: I = { x: 1 }; + return a1.x + 10; + } +} +==SCOPE::file '/a.ts'== +namespace A { + export interface I { x: number }; + namespace B { + function a() { + return newFunction(); + } + } +} +function newFunction() { + let a1: A.I = { x: 1 }; + return a1.x + 10; +} diff --git a/tests/baselines/reference/extractMethod1.js b/tests/baselines/reference/extractMethod1.js new file mode 100644 index 0000000000000..699916b0dc229 --- /dev/null +++ b/tests/baselines/reference/extractMethod1.js @@ -0,0 +1,98 @@ +==ORIGINAL== +namespace A { + let x = 1; + function foo() { + } + namespace B { + function a() { + let a = 1; + + let y = 5; + let z = x; + a = y; + foo(); + } + } +} +==SCOPE::function a== +namespace A { + let x = 1; + function foo() { + } + namespace B { + function a() { + let a = 1; + + newFunction(); + + function newFunction() { + let y = 5; + let z = x; + a = y; + foo(); + } + } + } +} +==SCOPE::namespace B== +namespace A { + let x = 1; + function foo() { + } + namespace B { + function a() { + let a = 1; + + ({ a } = newFunction(a)); + } + + function newFunction(a: any) { + let y = 5; + let z = x; + a = y; + foo(); + return { a }; + } + } +} +==SCOPE::namespace A== +namespace A { + let x = 1; + function foo() { + } + namespace B { + function a() { + let a = 1; + + ({ a } = newFunction(a)); + } + } + + function newFunction(a: any) { + let y = 5; + let z = x; + a = y; + foo(); + return { a }; + } +} +==SCOPE::file '/a.ts'== +namespace A { + let x = 1; + function foo() { + } + namespace B { + function a() { + let a = 1; + + ({ a } = newFunction(x, a, foo)); + } + } +} +function newFunction(x: any, a: any, foo: any) { + let y = 5; + let z = x; + a = y; + foo(); + return { a }; +} diff --git a/tests/baselines/reference/extractMethod10.js b/tests/baselines/reference/extractMethod10.js new file mode 100644 index 0000000000000..e416a66e262ac --- /dev/null +++ b/tests/baselines/reference/extractMethod10.js @@ -0,0 +1,70 @@ +==ORIGINAL== +namespace A { + export interface I { x: number }; + class C { + a() { + let z = 1; + let a1: I = { x: 1 }; + return a1.x + 10; + } + } +} +==SCOPE::method a== +namespace A { + export interface I { x: number }; + class C { + a() { + let z = 1; + return newFunction(); + + function newFunction() { + let a1: I = { x: 1 }; + return a1.x + 10; + } + } + } +} +==SCOPE::class C== +namespace A { + export interface I { x: number }; + class C { + a() { + let z = 1; + return this.newFunction(); + } + + private newFunction() { + let a1: I = { x: 1 }; + return a1.x + 10; + } + } +} +==SCOPE::namespace A== +namespace A { + export interface I { x: number }; + class C { + a() { + let z = 1; + return newFunction(); + } + } + + function newFunction() { + let a1: I = { x: 1 }; + return a1.x + 10; + } +} +==SCOPE::file '/a.ts'== +namespace A { + export interface I { x: number }; + class C { + a() { + let z = 1; + return newFunction(); + } + } +} +function newFunction() { + let a1: A.I = { x: 1 }; + return a1.x + 10; +} diff --git a/tests/baselines/reference/extractMethod11.js b/tests/baselines/reference/extractMethod11.js new file mode 100644 index 0000000000000..4cbd31d445321 --- /dev/null +++ b/tests/baselines/reference/extractMethod11.js @@ -0,0 +1,86 @@ +==ORIGINAL== +namespace A { + let y = 1; + class C { + a() { + let z = 1; + let a1 = { x: 1 }; + y = 10; + z = 42; + return a1.x + 10; + } + } +} +==SCOPE::method a== +namespace A { + let y = 1; + class C { + a() { + let z = 1; + return newFunction(); + + function newFunction() { + let a1 = { x: 1 }; + y = 10; + z = 42; + return a1.x + 10; + } + } + } +} +==SCOPE::class C== +namespace A { + let y = 1; + class C { + a() { + let z = 1; + var __return: any; + ({ z, __return } = this.newFunction(z)); + return __return; + } + + private newFunction(z: any) { + let a1 = { x: 1 }; + y = 10; + z = 42; + return { z, __return: a1.x + 10 }; + } + } +} +==SCOPE::namespace A== +namespace A { + let y = 1; + class C { + a() { + let z = 1; + var __return: any; + ({ z, __return } = newFunction(z)); + return __return; + } + } + + function newFunction(z: any) { + let a1 = { x: 1 }; + y = 10; + z = 42; + return { z, __return: a1.x + 10 }; + } +} +==SCOPE::file '/a.ts'== +namespace A { + let y = 1; + class C { + a() { + let z = 1; + var __return: any; + ({ y, z, __return } = newFunction(y, z)); + return __return; + } + } +} +function newFunction(y: any, z: any) { + let a1 = { x: 1 }; + y = 10; + z = 42; + return { y, z, __return: a1.x + 10 }; +} diff --git a/tests/baselines/reference/extractMethod12.js b/tests/baselines/reference/extractMethod12.js new file mode 100644 index 0000000000000..da0788a937fcf --- /dev/null +++ b/tests/baselines/reference/extractMethod12.js @@ -0,0 +1,36 @@ +==ORIGINAL== +namespace A { + let y = 1; + class C { + b() {} + a() { + let z = 1; + let a1 = { x: 1 }; + y = 10; + z = 42; + this.b(); + return a1.x + 10; + } + } +} +==SCOPE::class C== +namespace A { + let y = 1; + class C { + b() {} + a() { + let z = 1; + var __return: any; + ({ z, __return } = this.newFunction(z)); + return __return; + } + + private newFunction(z: any) { + let a1 = { x: 1 }; + y = 10; + z = 42; + this.b(); + return { z, __return: a1.x + 10 }; + } + } +} \ No newline at end of file diff --git a/tests/baselines/reference/extractMethod2.js b/tests/baselines/reference/extractMethod2.js new file mode 100644 index 0000000000000..a89fe52f2fb54 --- /dev/null +++ b/tests/baselines/reference/extractMethod2.js @@ -0,0 +1,85 @@ +==ORIGINAL== +namespace A { + let x = 1; + function foo() { + } + namespace B { + function a() { + + let y = 5; + let z = x; + return foo(); + } + } +} +==SCOPE::function a== +namespace A { + let x = 1; + function foo() { + } + namespace B { + function a() { + + return newFunction(); + + function newFunction() { + let y = 5; + let z = x; + return foo(); + } + } + } +} +==SCOPE::namespace B== +namespace A { + let x = 1; + function foo() { + } + namespace B { + function a() { + + return newFunction(); + } + + function newFunction() { + let y = 5; + let z = x; + return foo(); + } + } +} +==SCOPE::namespace A== +namespace A { + let x = 1; + function foo() { + } + namespace B { + function a() { + + return newFunction(); + } + } + + function newFunction() { + let y = 5; + let z = x; + return foo(); + } +} +==SCOPE::file '/a.ts'== +namespace A { + let x = 1; + function foo() { + } + namespace B { + function a() { + + return newFunction(x, foo); + } + } +} +function newFunction(x: any, foo: any) { + let y = 5; + let z = x; + return foo(); +} diff --git a/tests/baselines/reference/extractMethod3.js b/tests/baselines/reference/extractMethod3.js new file mode 100644 index 0000000000000..847e196130ce5 --- /dev/null +++ b/tests/baselines/reference/extractMethod3.js @@ -0,0 +1,80 @@ +==ORIGINAL== +namespace A { + function foo() { + } + namespace B { + function* a(z: number) { + + let y = 5; + yield z; + return foo(); + } + } +} +==SCOPE::function a== +namespace A { + function foo() { + } + namespace B { + function* a(z: number) { + + return yield* newFunction(); + + function* newFunction() { + let y = 5; + yield z; + return foo(); + } + } + } +} +==SCOPE::namespace B== +namespace A { + function foo() { + } + namespace B { + function* a(z: number) { + + return yield* newFunction(z); + } + + function* newFunction(z: any) { + let y = 5; + yield z; + return foo(); + } + } +} +==SCOPE::namespace A== +namespace A { + function foo() { + } + namespace B { + function* a(z: number) { + + return yield* newFunction(z); + } + } + + function* newFunction(z: any) { + let y = 5; + yield z; + return foo(); + } +} +==SCOPE::file '/a.ts'== +namespace A { + function foo() { + } + namespace B { + function* a(z: number) { + + return yield* newFunction(z, foo); + } + } +} +function* newFunction(z: any, foo: any) { + let y = 5; + yield z; + return foo(); +} diff --git a/tests/baselines/reference/extractMethod4.js b/tests/baselines/reference/extractMethod4.js new file mode 100644 index 0000000000000..25f410eeecbde --- /dev/null +++ b/tests/baselines/reference/extractMethod4.js @@ -0,0 +1,90 @@ +==ORIGINAL== +namespace A { + function foo() { + } + namespace B { + async function a(z: number, z1: any) { + + let y = 5; + if (z) { + await z1; + } + return foo(); + } + } +} +==SCOPE::function a== +namespace A { + function foo() { + } + namespace B { + async function a(z: number, z1: any) { + + return await newFunction(); + + async function newFunction() { + let y = 5; + if (z) { + await z1; + } + return foo(); + } + } + } +} +==SCOPE::namespace B== +namespace A { + function foo() { + } + namespace B { + async function a(z: number, z1: any) { + + return await newFunction(z, z1); + } + + async function newFunction(z: any, z1: any) { + let y = 5; + if (z) { + await z1; + } + return foo(); + } + } +} +==SCOPE::namespace A== +namespace A { + function foo() { + } + namespace B { + async function a(z: number, z1: any) { + + return await newFunction(z, z1); + } + } + + async function newFunction(z: any, z1: any) { + let y = 5; + if (z) { + await z1; + } + return foo(); + } +} +==SCOPE::file '/a.ts'== +namespace A { + function foo() { + } + namespace B { + async function a(z: number, z1: any) { + + return await newFunction(z, z1, foo); + } + } +} +async function newFunction(z: any, z1: any, foo: any) { + let y = 5; + if (z) { + await z1; + } + return foo(); +} diff --git a/tests/baselines/reference/extractMethod5.js b/tests/baselines/reference/extractMethod5.js new file mode 100644 index 0000000000000..135c4ed5f621c --- /dev/null +++ b/tests/baselines/reference/extractMethod5.js @@ -0,0 +1,98 @@ +==ORIGINAL== +namespace A { + let x = 1; + export function foo() { + } + namespace B { + function a() { + let a = 1; + + let y = 5; + let z = x; + a = y; + foo(); + } + } +} +==SCOPE::function a== +namespace A { + let x = 1; + export function foo() { + } + namespace B { + function a() { + let a = 1; + + newFunction(); + + function newFunction() { + let y = 5; + let z = x; + a = y; + foo(); + } + } + } +} +==SCOPE::namespace B== +namespace A { + let x = 1; + export function foo() { + } + namespace B { + function a() { + let a = 1; + + ({ a } = newFunction(a)); + } + + function newFunction(a: any) { + let y = 5; + let z = x; + a = y; + foo(); + return { a }; + } + } +} +==SCOPE::namespace A== +namespace A { + let x = 1; + export function foo() { + } + namespace B { + function a() { + let a = 1; + + ({ a } = newFunction(a)); + } + } + + function newFunction(a: any) { + let y = 5; + let z = x; + a = y; + foo(); + return { a }; + } +} +==SCOPE::file '/a.ts'== +namespace A { + let x = 1; + export function foo() { + } + namespace B { + function a() { + let a = 1; + + ({ a } = newFunction(x, a)); + } + } +} +function newFunction(x: any, a: any) { + let y = 5; + let z = x; + a = y; + A.foo(); + return { a }; +} diff --git a/tests/baselines/reference/extractMethod6.js b/tests/baselines/reference/extractMethod6.js new file mode 100644 index 0000000000000..94fcc8e6e310d --- /dev/null +++ b/tests/baselines/reference/extractMethod6.js @@ -0,0 +1,101 @@ +==ORIGINAL== +namespace A { + let x = 1; + export function foo() { + } + namespace B { + function a() { + let a = 1; + + let y = 5; + let z = x; + a = y; + return foo(); + } + } +} +==SCOPE::function a== +namespace A { + let x = 1; + export function foo() { + } + namespace B { + function a() { + let a = 1; + + return newFunction(); + + function newFunction() { + let y = 5; + let z = x; + a = y; + return foo(); + } + } + } +} +==SCOPE::namespace B== +namespace A { + let x = 1; + export function foo() { + } + namespace B { + function a() { + let a = 1; + + var __return: any; + ({ a, __return } = newFunction(a)); + return __return; + } + + function newFunction(a: any) { + let y = 5; + let z = x; + a = y; + return { a, __return: foo() }; + } + } +} +==SCOPE::namespace A== +namespace A { + let x = 1; + export function foo() { + } + namespace B { + function a() { + let a = 1; + + var __return: any; + ({ a, __return } = newFunction(a)); + return __return; + } + } + + function newFunction(a: any) { + let y = 5; + let z = x; + a = y; + return { a, __return: foo() }; + } +} +==SCOPE::file '/a.ts'== +namespace A { + let x = 1; + export function foo() { + } + namespace B { + function a() { + let a = 1; + + var __return: any; + ({ a, __return } = newFunction(x, a)); + return __return; + } + } +} +function newFunction(x: any, a: any) { + let y = 5; + let z = x; + a = y; + return { a, __return: A.foo() }; +} diff --git a/tests/baselines/reference/extractMethod7.js b/tests/baselines/reference/extractMethod7.js new file mode 100644 index 0000000000000..c7c3cc8d77a7c --- /dev/null +++ b/tests/baselines/reference/extractMethod7.js @@ -0,0 +1,111 @@ +==ORIGINAL== +namespace A { + let x = 1; + export namespace C { + export function foo() { + } + } + namespace B { + function a() { + let a = 1; + + let y = 5; + let z = x; + a = y; + return C.foo(); + } + } +} +==SCOPE::function a== +namespace A { + let x = 1; + export namespace C { + export function foo() { + } + } + namespace B { + function a() { + let a = 1; + + return newFunction(); + + function newFunction() { + let y = 5; + let z = x; + a = y; + return C.foo(); + } + } + } +} +==SCOPE::namespace B== +namespace A { + let x = 1; + export namespace C { + export function foo() { + } + } + namespace B { + function a() { + let a = 1; + + var __return: any; + ({ a, __return } = newFunction(a)); + return __return; + } + + function newFunction(a: any) { + let y = 5; + let z = x; + a = y; + return { a, __return: C.foo() }; + } + } +} +==SCOPE::namespace A== +namespace A { + let x = 1; + export namespace C { + export function foo() { + } + } + namespace B { + function a() { + let a = 1; + + var __return: any; + ({ a, __return } = newFunction(a)); + return __return; + } + } + + function newFunction(a: any) { + let y = 5; + let z = x; + a = y; + return { a, __return: C.foo() }; + } +} +==SCOPE::file '/a.ts'== +namespace A { + let x = 1; + export namespace C { + export function foo() { + } + } + namespace B { + function a() { + let a = 1; + + var __return: any; + ({ a, __return } = newFunction(x, a)); + return __return; + } + } +} +function newFunction(x: any, a: any) { + let y = 5; + let z = x; + a = y; + return { a, __return: A.C.foo() }; +} diff --git a/tests/baselines/reference/extractMethod8.js b/tests/baselines/reference/extractMethod8.js new file mode 100644 index 0000000000000..f030b0ea4afac --- /dev/null +++ b/tests/baselines/reference/extractMethod8.js @@ -0,0 +1,57 @@ +==ORIGINAL== +namespace A { + let x = 1; + namespace B { + function a() { + let a1 = 1; + return 1 + a1 + x + 100; + } + } +} +==SCOPE::function a== +namespace A { + let x = 1; + namespace B { + function a() { + let a1 = 1; + return newFunction() + 100; + + function newFunction() { 1 + a1 + x; } + } + } +} +==SCOPE::namespace B== +namespace A { + let x = 1; + namespace B { + function a() { + let a1 = 1; + return newFunction(a1) + 100; + } + + function newFunction(a1: any) { 1 + a1 + x; } + } +} +==SCOPE::namespace A== +namespace A { + let x = 1; + namespace B { + function a() { + let a1 = 1; + return newFunction(a1) + 100; + } + } + + function newFunction(a1: any) { 1 + a1 + x; } +} +==SCOPE::file '/a.ts'== +namespace A { + let x = 1; + namespace B { + function a() { + let a1 = 1; + return newFunction(a1, x) + 100; + } + } +} +function newFunction(a1: any, x: any) { 1 + a1 + x; } diff --git a/tests/baselines/reference/extractMethod9.js b/tests/baselines/reference/extractMethod9.js new file mode 100644 index 0000000000000..fcc5dcd23de07 --- /dev/null +++ b/tests/baselines/reference/extractMethod9.js @@ -0,0 +1,65 @@ +==ORIGINAL== +namespace A { + export interface I { x: number }; + namespace B { + function a() { + let a1: I = { x: 1 }; + return a1.x + 10; + } + } +} +==SCOPE::function a== +namespace A { + export interface I { x: number }; + namespace B { + function a() { + return newFunction(); + + function newFunction() { + let a1: I = { x: 1 }; + return a1.x + 10; + } + } + } +} +==SCOPE::namespace B== +namespace A { + export interface I { x: number }; + namespace B { + function a() { + return newFunction(); + } + + function newFunction() { + let a1: I = { x: 1 }; + return a1.x + 10; + } + } +} +==SCOPE::namespace A== +namespace A { + export interface I { x: number }; + namespace B { + function a() { + return newFunction(); + } + } + + function newFunction() { + let a1: I = { x: 1 }; + return a1.x + 10; + } +} +==SCOPE::file '/a.ts'== +namespace A { + export interface I { x: number }; + namespace B { + function a() { + return newFunction(); + } + } +} +function newFunction() { + let a1: A.I = { x: 1 }; + return a1.x + 10; +} diff --git a/tests/cases/fourslash/completionListIsGlobalCompletion.ts b/tests/cases/fourslash/completionListIsGlobalCompletion.ts index 121ab940d6549..49196bdabd4f6 100644 --- a/tests/cases/fourslash/completionListIsGlobalCompletion.ts +++ b/tests/cases/fourslash/completionListIsGlobalCompletion.ts @@ -47,7 +47,7 @@ verify.completionListIsGlobal(true); goTo.marker("6"); verify.completionListIsGlobal(false); goTo.marker("7"); -verify.completionListIsGlobal(true); +verify.completionListIsGlobal(false); goTo.marker("8"); verify.completionListIsGlobal(false); goTo.marker("9"); diff --git a/tests/cases/fourslash/extract-method1.ts b/tests/cases/fourslash/extract-method1.ts new file mode 100644 index 0000000000000..7f5c2ac2d500a --- /dev/null +++ b/tests/cases/fourslash/extract-method1.ts @@ -0,0 +1,33 @@ +/// + +//// class Foo { +//// someMethod(m: number) { +//// /*start*/var x = m; +//// x = x * 3; +//// var y = 30; +//// var z = y + x; +//// console.log(z);/*end*/ +//// var q = 10; +//// return q; +//// } +//// } + +goTo.select('start', 'end') +verify.refactorAvailable('Extract Method'); +edit.applyRefactor('Extract Method', "scope_0"); +verify.currentFileContentIs( +`class Foo { + someMethod(m: number) { + this.newFunction(m); + var q = 10; + return q; + } + + private newFunction(m: number) { + var x = m; + x = x * 3; + var y = 30; + var z = y + x; + console.log(z); + } +}`); diff --git a/tests/cases/fourslash/extract-method10.ts b/tests/cases/fourslash/extract-method10.ts new file mode 100644 index 0000000000000..1a02bfa00f53e --- /dev/null +++ b/tests/cases/fourslash/extract-method10.ts @@ -0,0 +1,6 @@ +/// + +//// (x => x)(/*1*/x => x/*2*/)(1); + +goTo.select('1', '2'); +edit.applyRefactor('Extract Method', 'scope_0'); diff --git a/tests/cases/fourslash/extract-method11.ts b/tests/cases/fourslash/extract-method11.ts new file mode 100644 index 0000000000000..705f2373104de --- /dev/null +++ b/tests/cases/fourslash/extract-method11.ts @@ -0,0 +1,28 @@ +/// + +// Nonexhaustive list of things it should be illegal to be extract-method on + +// * Import declarations +// * Super calls +// * Function body blocks +// * try/catch blocks + +//// /*1a*/import * as x from 'y';/*1b*/ +//// namespace N { +//// /*oka*/class C extends B { +//// constructor() { +//// /*2a*/super();/*2b*/ +//// } +//// }/*okb*/ +//// } +//// function f() /*3a*/{ return 0 }/*3b*/ +//// try /*4a*/{ console.log }/*4b*/ catch (e) /*5a*/{ console.log; }/*5b*/ + +for (const m of ['1', '2', '3', '4', '5']) { + goTo.select(m + 'a', m + 'b'); + verify.not.refactorAvailable('Extract Method'); +} + +// Verify we can still extract the entire class +goTo.select('oka', 'okb'); +verify.refactorAvailable('Extract Method'); diff --git a/tests/cases/fourslash/extract-method13.ts b/tests/cases/fourslash/extract-method13.ts new file mode 100644 index 0000000000000..b9b7c0096aabc --- /dev/null +++ b/tests/cases/fourslash/extract-method13.ts @@ -0,0 +1,30 @@ +/// + +// Extracting from a static context should make static methods. +// Also checks that we correctly find non-conflicting names in static contexts. + +//// class C { +//// static j = /*c*/100/*d*/; +//// constructor(q: string = /*a*/"hello"/*b*/) { +//// } +//// } + +goTo.select('a', 'b'); +edit.applyRefactor('Extract Method', 'scope_0'); + +goTo.select('c', 'd'); +edit.applyRefactor('Extract Method', 'scope_0'); + +verify.currentFileContentIs(`class C { + static j = C.newFunction_1(); + constructor(q: string = C.newFunction()) { + } + + private static newFunction(): string { + return "hello"; + } + + private static newFunction_1() { + return 100; + } +}`); \ No newline at end of file diff --git a/tests/cases/fourslash/extract-method14.ts b/tests/cases/fourslash/extract-method14.ts new file mode 100644 index 0000000000000..c8bab1b3a56d7 --- /dev/null +++ b/tests/cases/fourslash/extract-method14.ts @@ -0,0 +1,24 @@ +/// + +// Don't emit type annotations in JavaScript files +// Also tests that single-variable return extractions don't get superfluous destructuring + +// @allowNonTsExtensions: true +// @Filename: foo.js +//// function foo() { +//// var i = 10; +//// /*a*/return i++;/*b*/ +//// } + +goTo.select('a', 'b'); +edit.applyRefactor('Extract Method', 'scope_1'); +verify.currentFileContentIs(`function foo() { + var i = 10; + var __return: any; + ({ __return, i } = newFunction(i)); + return __return; +} +function newFunction(i) { + return { __return: i++, i }; +} +`); \ No newline at end of file diff --git a/tests/cases/fourslash/extract-method15.ts b/tests/cases/fourslash/extract-method15.ts new file mode 100644 index 0000000000000..ef62bd3fd3f74 --- /dev/null +++ b/tests/cases/fourslash/extract-method15.ts @@ -0,0 +1,22 @@ +/// + +// Extracting an increment expression (not statement) should do the right thing, +// including not generating extra destructuring unless needed + +//// function foo() { +//// var i = 10; +//// /*a*/i++/*b*/; +//// } + +goTo.select('a', 'b'); +edit.applyRefactor('Extract Method', 'scope_1'); + +verify.currentFileContentIs(`function foo() { + var i = 10; + i = newFunction(i); +} +function newFunction(i: number) { + i++; + return i; +} +`); diff --git a/tests/cases/fourslash/extract-method17.ts b/tests/cases/fourslash/extract-method17.ts new file mode 100644 index 0000000000000..ce54896604dd9 --- /dev/null +++ b/tests/cases/fourslash/extract-method17.ts @@ -0,0 +1,10 @@ +/// + +//// function foo () { +//// var x = 3; +//// var y = /*start*/x++ + 5/*end*/; +//// } + +goTo.select('start', 'end') +verify.refactorAvailable('Extract Method', 'scope_0'); +verify.not.refactorAvailable('Extract Method', 'scope_1'); diff --git a/tests/cases/fourslash/extract-method18.ts b/tests/cases/fourslash/extract-method18.ts new file mode 100644 index 0000000000000..9d87979a1ed1c --- /dev/null +++ b/tests/cases/fourslash/extract-method18.ts @@ -0,0 +1,21 @@ +/// + +// Don't try to propagate property accessed variables back, +// or emit spurious returns when the value is clearly ignored + +//// function fn() { +//// const x = { m: 1 }; +//// /*a*/x.m = 3/*b*/; +//// } + +goTo.select('a', 'b') +verify.refactorAvailable('Extract Method'); +edit.applyRefactor('Extract Method', "scope_1"); +verify.currentFileContentIs(`function fn() { + const x = { m: 1 }; + newFunction(x); +} +function newFunction(x: { m: number; }) { + x.m = 3; +} +`); diff --git a/tests/cases/fourslash/extract-method19.ts b/tests/cases/fourslash/extract-method19.ts new file mode 100644 index 0000000000000..54f79311cc72b --- /dev/null +++ b/tests/cases/fourslash/extract-method19.ts @@ -0,0 +1,22 @@ +/// + +// New function names should be totally new to the file + +//// function fn() { +//// /*a*/console.log("hi");/*b*/ +//// } +//// +//// function newFunction() { } + +goTo.select('a', 'b') +verify.refactorAvailable('Extract Method'); +edit.applyRefactor('Extract Method', "scope_0"); +verify.currentFileContentIs(`function fn() { + newFunction_1(); + + function newFunction_1() { + console.log("hi"); + } +} + +function newFunction() { }`); diff --git a/tests/cases/fourslash/extract-method2.ts b/tests/cases/fourslash/extract-method2.ts new file mode 100644 index 0000000000000..0a4f346307b5f --- /dev/null +++ b/tests/cases/fourslash/extract-method2.ts @@ -0,0 +1,28 @@ +/// + +//// namespace NS { +//// class Q { +//// foo() { +//// console.log('100'); +//// const m = 10, j = "hello", k = {x: "what"}; +//// const q = /*start*/m + j + k/*end*/; +//// } +//// } +//// } +goTo.select('start', 'end') +verify.refactorAvailable('Extract Method'); +edit.applyRefactor('Extract Method', "scope_2"); +verify.currentFileContentIs( +`namespace NS { + class Q { + foo() { + console.log('100'); + const m = 10, j = "hello", k = {x: "what"}; + const q = newFunction(m, j, k); + } + } +} +function newFunction(m: number, j: string, k: { x: string; }) { + return m + j + k; +} +`); diff --git a/tests/cases/fourslash/extract-method20.ts b/tests/cases/fourslash/extract-method20.ts new file mode 100644 index 0000000000000..04990001b5f0e --- /dev/null +++ b/tests/cases/fourslash/extract-method20.ts @@ -0,0 +1,14 @@ +/// + +// Shouldn't be able to extract a readonly property initializer outside the constructor + +//// class Foo { +//// readonly prop; +//// constructor() { +//// /*a*/this.prop = 10;/*b*/ +//// } +//// } + +goTo.select('a', 'b') +verify.refactorAvailable('Extract Method', 'scope_0'); +verify.not.refactorAvailable('Extract Method', 'scope_1'); diff --git a/tests/cases/fourslash/extract-method21.ts b/tests/cases/fourslash/extract-method21.ts new file mode 100644 index 0000000000000..0168daf5fcb02 --- /dev/null +++ b/tests/cases/fourslash/extract-method21.ts @@ -0,0 +1,25 @@ +/// + +// Extracting from a static method should create a static method + +//// class Foo { +//// static method() { +//// /*start*/return 1;/*end*/ +//// } +//// } + +goTo.select('start', 'end') + +verify.refactorAvailable('Extract Method'); + +edit.applyRefactor('Extract Method', "scope_0"); + +verify.currentFileContentIs(`class Foo { + static method() { + return Foo.newFunction(); + } + + private static newFunction() { + return 1; + } +}`); \ No newline at end of file diff --git a/tests/cases/fourslash/extract-method22.ts b/tests/cases/fourslash/extract-method22.ts new file mode 100644 index 0000000000000..7486520e700f7 --- /dev/null +++ b/tests/cases/fourslash/extract-method22.ts @@ -0,0 +1,10 @@ +/// + +// You may not extract variable declarations with the export modifier + +//// namespace NS { +//// /*start*/export var x = 10;/*end*/ +//// } + +goTo.select('start', 'end') +verify.not.refactorAvailable('Extract Method'); diff --git a/tests/cases/fourslash/extract-method23.ts b/tests/cases/fourslash/extract-method23.ts new file mode 100644 index 0000000000000..7da8f175f1353 --- /dev/null +++ b/tests/cases/fourslash/extract-method23.ts @@ -0,0 +1,8 @@ +/// + +//// declare namespace Foo { +//// const x = /*start*/3/*end*/; +//// } + +goTo.select('start', 'end') +verify.not.refactorAvailable('Extract Method'); diff --git a/tests/cases/fourslash/extract-method24.ts b/tests/cases/fourslash/extract-method24.ts new file mode 100644 index 0000000000000..9eebc00316bd6 --- /dev/null +++ b/tests/cases/fourslash/extract-method24.ts @@ -0,0 +1,19 @@ +/// + +//// function M() { +//// let a = [1,2,3]; +//// let x = 0; +//// console.log(/*a*/a[x]/*b*/); +//// } + +goTo.select('a', 'b') +edit.applyRefactor('Extract Method', 'scope_1'); +verify.currentFileContentIs(`function M() { + let a = [1,2,3]; + let x = 0; + console.log(newFunction(a, x)); +} +function newFunction(a: number[], x: number): any { + return a[x]; +} +`); \ No newline at end of file diff --git a/tests/cases/fourslash/extract-method3.ts b/tests/cases/fourslash/extract-method3.ts new file mode 100644 index 0000000000000..af543121eebf2 --- /dev/null +++ b/tests/cases/fourslash/extract-method3.ts @@ -0,0 +1,18 @@ +/// + +//// namespace NS { +//// class Q { +//// foo() { +//// console.log('100'); +//// const m = 10, j = "hello", k = {x: "what"}; +//// const q = /*a*/m/*b*/; +//// } +//// } +//// } + +// Don't offer to to 'extract method' a single identifier + +goTo.marker('a'); +verify.not.refactorAvailable('Extract Method'); +goTo.select('a', 'b'); +verify.not.refactorAvailable('Extract Method'); diff --git a/tests/cases/fourslash/extract-method4.ts b/tests/cases/fourslash/extract-method4.ts new file mode 100644 index 0000000000000..ec8f39f354189 --- /dev/null +++ b/tests/cases/fourslash/extract-method4.ts @@ -0,0 +1,14 @@ +/// + +//// let a = 1, b = 2, c = 3, d = 4; +//// namespace NS { +//// class Q { +//// foo() { +//// a = /*1*/b = c/*2*/ = d; +//// } +//// } +//// } + +// Should rewrite to a = newFunc(); function() { return b = c = d; } +goTo.select('1', '2'); +verify.not.refactorAvailable('Extract Method'); diff --git a/tests/cases/fourslash/extract-method5.ts b/tests/cases/fourslash/extract-method5.ts new file mode 100644 index 0000000000000..ac09f92cc0520 --- /dev/null +++ b/tests/cases/fourslash/extract-method5.ts @@ -0,0 +1,20 @@ +/// + +// Extraction in the context of a contextual +// type needs to produce an explicit return type +// annotation in the extracted function + +//// function f() { +//// var x: 1 | 2 | 3 = /*start*/2/*end*/; +//// } + +goTo.select('start', 'end'); +edit.applyRefactor('Extract Method', 'scope_0'); +verify.currentFileContentIs( +`function f() { + var x: 1 | 2 | 3 = newFunction(); + + function newFunction(): 1 | 2 | 3 { + return 2; + } +}`); \ No newline at end of file diff --git a/tests/cases/fourslash/extract-method6.ts b/tests/cases/fourslash/extract-method6.ts new file mode 100644 index 0000000000000..f188ab319e9b8 --- /dev/null +++ b/tests/cases/fourslash/extract-method6.ts @@ -0,0 +1,16 @@ +/// + +// Cannot extract globally-declared functions or +// those with non-selected local references + +//// /*f1a*/function f() { +//// /*g1a*/function g() { } +//// g();/*g1b*/ +//// g(); +//// }/*f1b*/ + +goTo.select('f1a', 'f1b'); +verify.not.refactorAvailable('Extract Method'); +goTo.select('g1a', 'g1b'); +verify.not.refactorAvailable('Extract Method'); + diff --git a/tests/cases/fourslash/extract-method7.ts b/tests/cases/fourslash/extract-method7.ts new file mode 100644 index 0000000000000..4c95c6a551d57 --- /dev/null +++ b/tests/cases/fourslash/extract-method7.ts @@ -0,0 +1,16 @@ +/// + +// You cannot extract a function initializer into the function's body. +// The innermost scope (scope_0) is the sibling of the function, not the function itself. + +//// function fn(x = /*a*/3/*b*/) { +//// } + +goTo.select('a', 'b'); +edit.applyRefactor('Extract Method', 'scope_0'); +verify.currentFileContentIs(`function fn(x = newFunction()) { +} +function newFunction() { + return 3; +} +`); diff --git a/tests/cases/fourslash/extract-method8.ts b/tests/cases/fourslash/extract-method8.ts new file mode 100644 index 0000000000000..28068dd7c783b --- /dev/null +++ b/tests/cases/fourslash/extract-method8.ts @@ -0,0 +1,17 @@ +/// + +// You cannot extract an exported function declaration + +//// namespace ns { +//// /*a*/export function fn() { +//// +//// } +//// fn(); +//// /*b*/ +//// } + +goTo.select('a', 'b'); +verify.not.refactorAvailable("Extract Method"); +edit.deleteAtCaret('export'.length); +goTo.select('a', 'b'); +verify.refactorAvailable("Extract Method"); diff --git a/tests/cases/fourslash/extract-method9.ts b/tests/cases/fourslash/extract-method9.ts new file mode 100644 index 0000000000000..f70ef20a87b56 --- /dev/null +++ b/tests/cases/fourslash/extract-method9.ts @@ -0,0 +1,11 @@ +/// + +//// function f() { +//// /*a*/function q() { } +//// q();/*b*/ +//// q(); +//// } + +goTo.select('a', 'b'); +verify.not.refactorAvailable("Extract Method"); + diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index 59cb881ab7b09..5175b52df6c9f 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -130,6 +130,7 @@ declare namespace FourSlashInterface { position(position: number, fileName?: string): any; file(index: number, content?: string, scriptKindName?: string): any; file(name: string, content?: string, scriptKindName?: string): any; + select(startMarker: string, endMarker: string): void; } class verifyNegatable { private negative; @@ -156,6 +157,8 @@ declare namespace FourSlashInterface { applicableRefactorAvailableAtMarker(markerName: string): void; codeFixDiagnosticsAvailableAtMarkers(markerNames: string[], diagnosticCode?: number): void; applicableRefactorAvailableForRange(): void; + + refactorAvailable(name?: string, subName?: string); } class verify extends verifyNegatable { assertHasRanges(ranges: Range[]): void; @@ -305,6 +308,8 @@ declare namespace FourSlashInterface { moveLeft(count?: number): void; enableFormatting(): void; disableFormatting(): void; + + applyRefactor(refactorName: string, actionName: string): void; } class debug { printCurrentParameterHelp(): void;