diff --git a/Jakefile.js b/Jakefile.js
index 7b4991b74b6..b1823f4ea0e 100644
--- a/Jakefile.js
+++ b/Jakefile.js
@@ -138,6 +138,7 @@ var harnessSources = harnessCoreSources.concat([
"projectErrors.ts",
"matchFiles.ts",
"initializeTSConfig.ts",
+ "extractConstants.ts",
"extractMethods.ts",
"printer.ts",
"textChanges.ts",
diff --git a/src/harness/tsconfig.json b/src/harness/tsconfig.json
index c6e78138638..53654f7365f 100644
--- a/src/harness/tsconfig.json
+++ b/src/harness/tsconfig.json
@@ -128,6 +128,7 @@
"./unittests/printer.ts",
"./unittests/transform.ts",
"./unittests/customTransforms.ts",
+ "./unittests/extractConstants.ts",
"./unittests/extractMethods.ts",
"./unittests/textChanges.ts",
"./unittests/telemetry.ts",
diff --git a/src/harness/unittests/extractConstants.ts b/src/harness/unittests/extractConstants.ts
new file mode 100644
index 00000000000..f5018811b1a
--- /dev/null
+++ b/src/harness/unittests/extractConstants.ts
@@ -0,0 +1,250 @@
+///
+///
+
+namespace ts {
+ interface Range {
+ start: number;
+ end: number;
+ name: string;
+ }
+
+ interface Test {
+ source: string;
+ ranges: Map;
+ }
+
+ // TODO (acasey): share
+ 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 };
+ }
+
+ // TODO (acasey): share
+ 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;
+ }
+
+ describe("extractConstants", () => {
+ testExtractConstant("extractConstant_TopLevel",
+ `let x = [#|1|];`);
+
+ testExtractConstant("extractConstant_Namespace",
+ `namespace N {
+ let x = [#|1|];
+}`);
+
+ testExtractConstant("extractConstant_Class",
+ `class C {
+ x = [#|1|];
+}`);
+
+ testExtractConstant("extractConstant_Method",
+ `class C {
+ M() {
+ let x = [#|1|];
+ }
+}`);
+
+ testExtractConstant("extractConstant_Function",
+ `function F() {
+ let x = [#|1|];
+}`);
+
+ testExtractConstant("extractConstant_ExpressionStatement",
+ `[#|"hello";|]`);
+
+ testExtractConstant("extractConstant_ExpressionStatementExpression",
+ `[#|"hello"|];`);
+
+ testExtractConstant("extractConstant_BlockScopes_NoDependencies",
+ `for (let i = 0; i < 10; i++) {
+ for (let j = 0; j < 10; j++) {
+ let x = [#|1|];
+ }
+}`);
+
+ testExtractConstant("extractConstant_ClassInsertionPosition",
+ `class C {
+ a = 1;
+ b = 2;
+ M1() { }
+ M2() { }
+ M3() {
+ let x = [#|1|];
+ }
+}`);
+
+ testExtractConstantFailed("extractConstant_Parameters",
+ `function F() {
+ let w = 1;
+ let x = [#|w + 1|];
+}`);
+
+ testExtractConstantFailed("extractConstant_TypeParameters",
+ `function F(t: T) {
+ let x = [#|t + 1|];
+}`);
+
+ testExtractConstantFailed("extractConstant_BlockScopes_Dependencies",
+ `for (let i = 0; i < 10; i++) {
+ for (let j = 0; j < 10; j++) {
+ let x = [#|i + 1|];
+ }
+}`);
+ });
+
+ // TODO (acasey): share?
+ function testExtractConstant(caption: string, text: string) {
+ it(caption, () => {
+ Harness.Baseline.runBaseline(`extractConstant/${caption}.ts`, () => {
+ 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, projectSystem.libFile]);
+ 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: selectionRange.start,
+ endPosition: selectionRange.end,
+ rulesProvider: getRuleProvider()
+ };
+ const rangeToExtract = refactor.extractSymbol.getRangeToExtract(sourceFile, createTextSpanFromBounds(selectionRange.start, selectionRange.end));
+ assert.equal(rangeToExtract.errors, undefined, rangeToExtract.errors && "Range error: " + rangeToExtract.errors[0].messageText);
+ const infos = refactor.extractSymbol.getAvailableActions(context);
+ const actions = find(infos, info => info.description === Diagnostics.Extract_constant.message).actions;
+ const data: string[] = [];
+ data.push(`// ==ORIGINAL==`);
+ data.push(sourceFile.text);
+ for (const action of actions) {
+ const { renameLocation, edits } = refactor.extractSymbol.getEditsForAction(context, action.name);
+ assert.lengthOf(edits, 1);
+ data.push(`// ==SCOPE::${action.description}==`);
+ const newText = textChanges.applyChanges(sourceFile.text, edits[0].textChanges);
+ const newTextWithRename = newText.slice(0, renameLocation) + "/*RENAME*/" + newText.slice(renameLocation);
+ data.push(newTextWithRename);
+ }
+ return data.join(newLineCharacter);
+ });
+ });
+ }
+
+ // TODO (acasey): share?
+ function testExtractConstantFailed(caption: string, text: string) {
+ it(caption, () => {
+ 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, projectSystem.libFile]);
+ 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: selectionRange.start,
+ endPosition: selectionRange.end,
+ rulesProvider: getRuleProvider()
+ };
+ const rangeToExtract = refactor.extractSymbol.getRangeToExtract(sourceFile, createTextSpanFromBounds(selectionRange.start, selectionRange.end));
+ assert.isUndefined(rangeToExtract.errors, rangeToExtract.errors && "Range error: " + rangeToExtract.errors[0].messageText);
+ const infos = refactor.extractSymbol.getAvailableActions(context);
+ assert.isUndefined(find(infos, info => info.description === Diagnostics.Extract_constant.message));
+ });
+ }
+}
diff --git a/src/harness/unittests/extractMethods.ts b/src/harness/unittests/extractMethods.ts
index 3a161baa8e7..be5d21a80ae 100644
--- a/src/harness/unittests/extractMethods.ts
+++ b/src/harness/unittests/extractMethods.ts
@@ -805,7 +805,8 @@ function parsePrimaryExpression(): any {
};
const rangeToExtract = refactor.extractSymbol.getRangeToExtract(sourceFile, createTextSpanFromBounds(selectionRange.start, selectionRange.end));
assert.equal(rangeToExtract.errors, undefined, "expect no errors");
- const actions = refactor.extractSymbol.getAvailableActions(context)[0].actions; // TODO (acasey): smarter index
+ const infos = refactor.extractSymbol.getAvailableActions(context);
+ const actions = find(infos, info => info.description === Diagnostics.Extract_function.message).actions;
const data: string[] = [];
data.push(`// ==ORIGINAL==`);
data.push(sourceFile.text);
diff --git a/src/services/refactors/extractSymbol.ts b/src/services/refactors/extractSymbol.ts
index a1212e7260c..0c3b3726312 100644
--- a/src/services/refactors/extractSymbol.ts
+++ b/src/services/refactors/extractSymbol.ts
@@ -262,10 +262,6 @@ namespace ts.refactor.extractSymbol {
return { targetRange: { range: statements, facts: rangeFacts, declarations } };
}
- if (isExpressionStatement(start)) {
- start = start.expression;
- }
-
// We have a single node (start)
const errors = checkRootNode(start) || checkNode(start);
if (errors) {
@@ -274,7 +270,7 @@ namespace ts.refactor.extractSymbol {
return { targetRange: { range: getStatementOrExpressionRange(start), facts: rangeFacts, declarations } };
function checkRootNode(node: Node): Diagnostic[] | undefined {
- if (isIdentifier(node)) {
+ if (isIdentifier(isExpressionStatement(node) ? node.expression : node)) {
return [createDiagnosticForNode(node, Messages.CannotExtractIdentifier)];
}
return undefined;
@@ -539,8 +535,10 @@ namespace ts.refactor.extractSymbol {
const { scopes, readsAndWrites: { target, usagesPerScope, constantErrorsPerScope } } = getPossibleExtractionsWorker(targetRange, context);
Debug.assert(!constantErrorsPerScope[requestedChangesIndex].length, "The extraction went missing? How?");
context.cancellationToken.throwIfCancellationRequested();
- Debug.assert(target === targetRange.range);
- return extractConstantInScope(target as Expression, scopes[requestedChangesIndex], usagesPerScope[requestedChangesIndex], targetRange.facts, context);
+ const expression = isExpression(target)
+ ? target
+ : (target.statements[0] as ExpressionStatement).expression;
+ return extractConstantInScope(expression, scopes[requestedChangesIndex], usagesPerScope[requestedChangesIndex], targetRange.facts, context);
}
interface PossibleExtraction {
@@ -1114,18 +1112,24 @@ namespace ts.refactor.extractSymbol {
child.pos >= minPos && isFunctionLikeDeclaration(child) && !isConstructorDeclaration(child));
}
- function getNodeToInsertConstantBefore(minPos: number, scope: Scope): Node {
- const isClassLikeScope = isClassLike(scope);
+ // TODO (acasey): need to dig into nested statements
+ function getNodeToInsertConstantBefore(maxPos: number, scope: Scope): Node {
const children = getStatementsOrClassElements(scope);
+ Debug.assert(children.length > 0); // There must be at least one child, since we extracted from one.
+
+ const isClassLikeScope = isClassLike(scope);
let prevChild: Statement | ClassElement | undefined = undefined;
for (const child of children) {
- if (child.pos >= minPos || (isClassLikeScope && !isPropertyDeclaration(child))) {
+ if (child.pos >= maxPos) {
break;
}
prevChild = child;
+ if (isClassLikeScope && !isPropertyDeclaration(child)) {
+ break;
+ }
}
- return prevChild || children[0]; // There must be one - minPos is in one.
+ return prevChild;
}
function getPropertyAssignmentsForWrites(writes: ReadonlyArray): ShorthandPropertyAssignment[] {
@@ -1192,7 +1196,7 @@ namespace ts.refactor.extractSymbol {
const visibleDeclarationsInExtractedRange: Symbol[] = [];
const expressionDiagnostics =
- isReadonlyArray(targetRange.range)
+ isReadonlyArray(targetRange.range) && !(targetRange.range.length === 1 && isExpressionStatement(targetRange.range[0]))
? ((start, end) => [createFileDiagnostic(sourceFile, start, end - start, Messages.ExpressionExpected)])(firstOrUndefined(targetRange.range).getStart(), lastOrUndefined(targetRange.range).end)
: [];
diff --git a/tests/baselines/reference/extractConstant/extractConstant_BlockScopes_NoDependencies.ts b/tests/baselines/reference/extractConstant/extractConstant_BlockScopes_NoDependencies.ts
new file mode 100644
index 00000000000..25609a3a801
--- /dev/null
+++ b/tests/baselines/reference/extractConstant/extractConstant_BlockScopes_NoDependencies.ts
@@ -0,0 +1,14 @@
+// ==ORIGINAL==
+for (let i = 0; i < 10; i++) {
+ for (let j = 0; j < 10; j++) {
+ let x = 1;
+ }
+}
+// ==SCOPE::Extract to constant in global scope==
+const newLocal = 1;
+
+for (let i = 0; i < 10; i++) {
+ for (let j = 0; j < 10; j++) {
+ let x = /*RENAME*/newLocal;
+ }
+}
\ No newline at end of file
diff --git a/tests/baselines/reference/extractConstant/extractConstant_Class.ts b/tests/baselines/reference/extractConstant/extractConstant_Class.ts
new file mode 100644
index 00000000000..eb06cf6c0cf
--- /dev/null
+++ b/tests/baselines/reference/extractConstant/extractConstant_Class.ts
@@ -0,0 +1,16 @@
+// ==ORIGINAL==
+class C {
+ x = 1;
+}
+// ==SCOPE::Extract to readonly field in class 'C'==
+class C {
+ private readonly newProperty = 1;
+
+ x = this./*RENAME*/newProperty;
+}
+// ==SCOPE::Extract to constant in global scope==
+const newLocal = 1;
+
+class C {
+ x = /*RENAME*/newLocal;
+}
\ No newline at end of file
diff --git a/tests/baselines/reference/extractConstant/extractConstant_ClassInsertionPosition.ts b/tests/baselines/reference/extractConstant/extractConstant_ClassInsertionPosition.ts
new file mode 100644
index 00000000000..e024fda34bc
--- /dev/null
+++ b/tests/baselines/reference/extractConstant/extractConstant_ClassInsertionPosition.ts
@@ -0,0 +1,46 @@
+// ==ORIGINAL==
+class C {
+ a = 1;
+ b = 2;
+ M1() { }
+ M2() { }
+ M3() {
+ let x = 1;
+ }
+}
+// ==SCOPE::Extract to constant in method 'M3==
+class C {
+ a = 1;
+ b = 2;
+ M1() { }
+ M2() { }
+ M3() {
+ const newLocal = 1;
+
+ let x = /*RENAME*/newLocal;
+ }
+}
+// ==SCOPE::Extract to readonly field in class 'C'==
+class C {
+ a = 1;
+ b = 2;
+ private readonly newProperty = 1;
+
+ M1() { }
+ M2() { }
+ M3() {
+ let x = this./*RENAME*/newProperty;
+ }
+}
+// ==SCOPE::Extract to constant in global scope==
+const newLocal = 1;
+
+class C {
+ a = 1;
+ b = 2;
+ M1() { }
+ M2() { }
+ M3() {
+ let x = /*RENAME*/newLocal;
+ }
+}
\ No newline at end of file
diff --git a/tests/baselines/reference/extractConstant/extractConstant_ExpressionStatement.ts b/tests/baselines/reference/extractConstant/extractConstant_ExpressionStatement.ts
new file mode 100644
index 00000000000..6bf35cd17e1
--- /dev/null
+++ b/tests/baselines/reference/extractConstant/extractConstant_ExpressionStatement.ts
@@ -0,0 +1,4 @@
+// ==ORIGINAL==
+"hello";
+// ==SCOPE::Extract to constant in global scope==
+const /*RENAME*/newLocal = "hello";
\ No newline at end of file
diff --git a/tests/baselines/reference/extractConstant/extractConstant_ExpressionStatementExpression.ts b/tests/baselines/reference/extractConstant/extractConstant_ExpressionStatementExpression.ts
new file mode 100644
index 00000000000..6bf35cd17e1
--- /dev/null
+++ b/tests/baselines/reference/extractConstant/extractConstant_ExpressionStatementExpression.ts
@@ -0,0 +1,4 @@
+// ==ORIGINAL==
+"hello";
+// ==SCOPE::Extract to constant in global scope==
+const /*RENAME*/newLocal = "hello";
\ No newline at end of file
diff --git a/tests/baselines/reference/extractConstant/extractConstant_Function.ts b/tests/baselines/reference/extractConstant/extractConstant_Function.ts
new file mode 100644
index 00000000000..67c8255c4b4
--- /dev/null
+++ b/tests/baselines/reference/extractConstant/extractConstant_Function.ts
@@ -0,0 +1,16 @@
+// ==ORIGINAL==
+function F() {
+ let x = 1;
+}
+// ==SCOPE::Extract to constant in function 'F'==
+function F() {
+ const newLocal = 1;
+
+ let x = /*RENAME*/newLocal;
+}
+// ==SCOPE::Extract to constant in global scope==
+const newLocal = 1;
+
+function F() {
+ let x = /*RENAME*/newLocal;
+}
\ No newline at end of file
diff --git a/tests/baselines/reference/extractConstant/extractConstant_Method.ts b/tests/baselines/reference/extractConstant/extractConstant_Method.ts
new file mode 100644
index 00000000000..1ae4c8b1cb5
--- /dev/null
+++ b/tests/baselines/reference/extractConstant/extractConstant_Method.ts
@@ -0,0 +1,30 @@
+// ==ORIGINAL==
+class C {
+ M() {
+ let x = 1;
+ }
+}
+// ==SCOPE::Extract to constant in method 'M==
+class C {
+ M() {
+ const newLocal = 1;
+
+ let x = /*RENAME*/newLocal;
+ }
+}
+// ==SCOPE::Extract to readonly field in class 'C'==
+class C {
+ private readonly newProperty = 1;
+
+ M() {
+ let x = this./*RENAME*/newProperty;
+ }
+}
+// ==SCOPE::Extract to constant in global scope==
+const newLocal = 1;
+
+class C {
+ M() {
+ let x = /*RENAME*/newLocal;
+ }
+}
\ No newline at end of file
diff --git a/tests/baselines/reference/extractConstant/extractConstant_Namespace.ts b/tests/baselines/reference/extractConstant/extractConstant_Namespace.ts
new file mode 100644
index 00000000000..8f25847165f
--- /dev/null
+++ b/tests/baselines/reference/extractConstant/extractConstant_Namespace.ts
@@ -0,0 +1,16 @@
+// ==ORIGINAL==
+namespace N {
+ let x = 1;
+}
+// ==SCOPE::Extract to constant in namespace 'N'==
+namespace N {
+ const newLocal = 1;
+
+ let x = /*RENAME*/newLocal;
+}
+// ==SCOPE::Extract to constant in global scope==
+const newLocal = 1;
+
+namespace N {
+ let x = /*RENAME*/newLocal;
+}
\ No newline at end of file
diff --git a/tests/baselines/reference/extractConstant/extractConstant_TopLevel.ts b/tests/baselines/reference/extractConstant/extractConstant_TopLevel.ts
new file mode 100644
index 00000000000..fb0447583ff
--- /dev/null
+++ b/tests/baselines/reference/extractConstant/extractConstant_TopLevel.ts
@@ -0,0 +1,6 @@
+// ==ORIGINAL==
+let x = 1;
+// ==SCOPE::Extract to constant in global scope==
+const newLocal = 1;
+
+let x = /*RENAME*/newLocal;
\ No newline at end of file