Merge pull request #18861 from amcasey/ConstantInsertionPosition

Improve insertion positions of extracted constants
2025-11-18 17:21:48 +00:00 · 2017-10-02 16:39:32 -07:00
parent 884c72efb4 724f71cae1
commit 2df7058af7
61 changed files with 1083 additions and 105 deletions
@@ -3724,7 +3724,7 @@
        "code": 95003
    },

-    "Extract to {0}": {
+    "Extract to {0} in {1}": {
        "category": "Message",
        "code": 95004
    },
@@ -40,7 +40,7 @@ namespace ts {
    }
 }`);

-        testExtractConstant("extractConstant_ClassInsertionPosition",
+        testExtractConstant("extractConstant_ClassInsertionPosition1",
            `class C {
    a = 1;
    b = 2;
@@ -51,6 +51,28 @@ namespace ts {
    }
 }`);

+        testExtractConstant("extractConstant_ClassInsertionPosition2",
+            `class C {
+    a = 1;
+    M1() { }
+    b = 2;
+    M2() { }
+    M3() {
+        let x = [#|1|];
+    }
+}`);
+
+        testExtractConstant("extractConstant_ClassInsertionPosition3",
+            `class C {
+    M1() { }
+    a = 1;
+    b = 2;
+    M2() { }
+    M3() {
+        let x = [#|1|];
+    }
+}`);
+
        testExtractConstant("extractConstant_Parameters",
            `function F() {
    let w = 1;
@@ -62,26 +84,126 @@ namespace ts {
    let x = [#|t + 1|];
 }`);

-// TODO (acasey): handle repeated substitution
+// TODO (18857): handle repeated substitution
 //         testExtractConstant("extractConstant_RepeatedSubstitution",
 //             `namespace X {
 //     export const j = 10;
 //     export const y = [#|j * j|];
 // }`);

-        testExtractConstantFailed("extractConstant_BlockScopes_Dependencies",
-            `for (let i = 0; i < 10; i++) {
+        testExtractConstant("extractConstant_VariableList_const",
+            `const a = 1, b = [#|a + 1|];`);
+
+        // NOTE: this test isn't normative - it just documents our sub-optimal behavior.
+        testExtractConstant("extractConstant_VariableList_let",
+            `let a = 1, b = [#|a + 1|];`);
+
+        // NOTE: this test isn't normative - it just documents our sub-optimal behavior.
+        testExtractConstant("extractConstant_VariableList_MultipleLines",
+            `const /*About A*/a = 1,
+    /*About B*/b = [#|a + 1|];`);
+
+        testExtractConstant("extractConstant_BlockScopeMismatch", `
+for (let i = 0; i < 10; i++) {
    for (let j = 0; j < 10; j++) {
-        let x = [#|i + 1|];
+        const x = [#|i + 1|];
    }
-}`);
+}
+        `);
+
+        testExtractConstant("extractConstant_StatementInsertionPosition1", `
+const i = 0;
+for (let j = 0; j < 10; j++) {
+    const x = [#|i + 1|];
+}
+        `);
+
+        testExtractConstant("extractConstant_StatementInsertionPosition2", `
+const i = 0;
+function F() {
+    for (let j = 0; j < 10; j++) {
+        const x = [#|i + 1|];
+    }
+}
+        `);
+
+        testExtractConstant("extractConstant_StatementInsertionPosition3", `
+for (let j = 0; j < 10; j++) {
+    const x = [#|2 + 1|];
+}
+        `);
+
+        testExtractConstant("extractConstant_StatementInsertionPosition4", `
+function F() {
+    for (let j = 0; j < 10; j++) {
+        const x = [#|2 + 1|];
+    }
+}
+        `);
+
+        testExtractConstant("extractConstant_StatementInsertionPosition5", `
+function F0() {
+    function F1() {
+        function F2(x = [#|2 + 1|]) {
+        }
+    }
+}
+        `);
+
+        testExtractConstant("extractConstant_StatementInsertionPosition6", `
+class C {
+    x = [#|2 + 1|];
+}
+        `);
+
+        testExtractConstant("extractConstant_StatementInsertionPosition7", `
+const i = 0;
+class C {
+    M() {
+        for (let j = 0; j < 10; j++) {
+            x = [#|i + 1|];
+        }
+    }
+}
+        `);
+
+        testExtractConstant("extractConstant_TripleSlash", `
+/// <reference path="path.js"/>
+
+const x = [#|2 + 1|];
+        `);
+
+        testExtractConstant("extractConstant_PinnedComment", `
+/*! Copyright */
+
+const x = [#|2 + 1|];
+        `);
+
+        testExtractConstant("extractConstant_Directive", `
+"strict";
+
+const x = [#|2 + 1|];
+        `);
+
+        testExtractConstant("extractConstant_MultipleHeaders", `
+/*! Copyright */
+
+/// <reference path="path.js"/>
+
+"strict";
+
+const x = [#|2 + 1|];
+        `);
+
+        testExtractConstant("extractConstant_PinnedCommentAndDocComment", `
+/*! Copyright */
+
+/* About x */
+const x = [#|2 + 1|];
+        `);
    });

    function testExtractConstant(caption: string, text: string) {
        testExtractSymbol(caption, text, "extractConstant", Diagnostics.Extract_constant);
    }
-
-    function testExtractConstantFailed(caption: string, text: string) {
-        testExtractSymbolFailed(caption, text, Diagnostics.Extract_constant);
-    }
 }
@@ -355,7 +355,7 @@ function parsePrimaryExpression(): any {
    [#|function G() { }|]
 }`);

-// TODO (acasey): handle repeated substitution
+// TODO (18857): handle repeated substitution
 //         testExtractFunction("extractFunction_RepeatedSubstitution",
 //             `namespace X {
 //     export const j = 10;
@@ -113,7 +113,7 @@ namespace ts {

            if (hasSyntacticDiagnostics(program)) {
                // Don't bother generating JS baselines for inputs that aren't valid JS.
-                assert.equal(Extension.Js, extension);
+                assert.equal(Extension.Js, extension, "Syntactic diagnostics found in non-JS file");
                return;
            }

@@ -407,28 +407,6 @@ namespace ts.codefix {
                    moduleSpecifierWithoutQuotes
                );

-                function getSourceFileImportLocation(node: SourceFile) {
-                    // For a source file, it is possible there are detached comments we should not skip
-                    const text = node.text;
-                    let ranges = getLeadingCommentRanges(text, 0);
-                    if (!ranges) return 0;
-                    let position = 0;
-                    // However we should still skip a pinned comment at the top
-                    if (ranges.length && ranges[0].kind === SyntaxKind.MultiLineCommentTrivia && isPinnedComment(text, ranges[0])) {
-                        position = ranges[0].end + 1;
-                        ranges = ranges.slice(1);
-                    }
-                    // As well as any triple slash references
-                    for (const range of ranges) {
-                        if (range.kind === SyntaxKind.SingleLineCommentTrivia && isRecognizedTripleSlashComment(node.text, range.pos, range.end)) {
-                            position = range.end + 1;
-                            continue;
-                        }
-                        break;
-                    }
-                    return position;
-                }
-
                function getSingleQuoteStyleFromExistingImports() {
                    const firstModuleSpecifier = forEach(sourceFile.statements, node => {
                        if (isImportDeclaration(node) || isExportDeclaration(node)) {
@@ -43,7 +43,7 @@ namespace ts.refactor.extractSymbol {
                // Don't issue refactorings with duplicated names.
                // Scopes come back in "innermost first" order, so extractions will
                // preferentially go into nearer scopes
-                const description = formatStringFromArgs(Diagnostics.Extract_to_0.message, [extraction.functionDescription]);
+                const description = formatStringFromArgs(Diagnostics.Extract_to_0_in_1.message, [extraction.functionDescription, extraction.functionScopeDescription]);
                if (!usedFunctionNames.has(description)) {
                    usedFunctionNames.set(description, true);
                    functionActions.push({
@@ -58,7 +58,7 @@ namespace ts.refactor.extractSymbol {
                // Don't issue refactorings with duplicated names.
                // Scopes come back in "innermost first" order, so extractions will
                // preferentially go into nearer scopes
-                const description = formatStringFromArgs(Diagnostics.Extract_to_0.message, [extraction.constantDescription]);
+                const description = formatStringFromArgs(Diagnostics.Extract_to_0_in_1.message, [extraction.constantDescription, extraction.constantScopeDescription]);
                if (!usedConstantNames.has(description)) {
                    usedConstantNames.set(description, true);
                    constantActions.push({
@@ -523,8 +523,10 @@ namespace ts.refactor.extractSymbol {

    interface PossibleExtraction {
        readonly functionDescription: string;
+        readonly functionScopeDescription: string;
        readonly functionErrors: ReadonlyArray<Diagnostic>;
        readonly constantDescription: string;
+        readonly constantScopeDescription: string;
        readonly constantErrors: ReadonlyArray<Diagnostic>;
    }
    /**
@@ -535,12 +537,23 @@ namespace ts.refactor.extractSymbol {
    function getPossibleExtractions(targetRange: TargetRange, context: RefactorContext): ReadonlyArray<PossibleExtraction> | undefined {
        const { scopes, readsAndWrites: { functionErrorsPerScope, constantErrorsPerScope } } = getPossibleExtractionsWorker(targetRange, context);
        // Need the inner type annotation to avoid https://github.com/Microsoft/TypeScript/issues/7547
-        const extractions = scopes.map((scope, i): PossibleExtraction => ({
-            functionDescription: getDescriptionForFunctionInScope(scope),
-            functionErrors: functionErrorsPerScope[i],
-            constantDescription: getDescriptionForConstantInScope(scope),
-            constantErrors: constantErrorsPerScope[i],
-        }));
+        const extractions = scopes.map((scope, i): PossibleExtraction => {
+            const scopeDescription = isFunctionLikeDeclaration(scope)
+                ? getDescriptionForFunctionLikeDeclaration(scope)
+                : isClassLike(scope)
+                    ? getDescriptionForClassLikeDeclaration(scope)
+                    : getDescriptionForModuleLikeDeclaration(scope);
+            return {
+                functionDescription: getDescriptionForFunctionInScope(scope),
+                functionErrors: functionErrorsPerScope[i],
+                functionScopeDescription: scopeDescription,
+                constantDescription: getDescriptionForConstantInScope(scope),
+                constantErrors: constantErrorsPerScope[i],
+                constantScopeDescription: (i === 0 && !isClassLike(scope))
+                    ? "enclosing scope" // Like "global scope" and "module scope", this is not localized.
+                    : scopeDescription,
+            };
+        });
        return extractions;
    }

@@ -569,17 +582,15 @@ namespace ts.refactor.extractSymbol {

    function getDescriptionForFunctionInScope(scope: Scope): string {
        return isFunctionLikeDeclaration(scope)
-            ? `inner function in ${getDescriptionForFunctionLikeDeclaration(scope)}`
+            ? "inner function"
            : isClassLike(scope)
-                ? `method in ${getDescriptionForClassLikeDeclaration(scope)}`
-                : `function in ${getDescriptionForModuleLikeDeclaration(scope)}`;
+                ? "method"
+                : "function";
    }
    function getDescriptionForConstantInScope(scope: Scope): string {
-        return isFunctionLikeDeclaration(scope)
-            ? `constant in ${getDescriptionForFunctionLikeDeclaration(scope)}`
-            : isClassLike(scope)
-                ? `readonly field in ${getDescriptionForClassLikeDeclaration(scope)}`
-                : `constant in ${getDescriptionForModuleLikeDeclaration(scope)}`;
+        return isClassLike(scope)
+            ? "readonly field"
+            : "constant";
    }
    function getDescriptionForFunctionLikeDeclaration(scope: FunctionLikeDeclaration): string {
        switch (scope.kind) {
@@ -869,33 +880,61 @@ namespace ts.refactor.extractSymbol {
                createIdentifier(localNameText));

            // Declare
-            const minInsertionPos = node.end;
-            const nodeToInsertBefore = getNodeToInsertConstantBefore(minInsertionPos, scope);
+            const maxInsertionPos = node.pos;
+            const nodeToInsertBefore = getNodeToInsertPropertyBefore(maxInsertionPos, scope);
            changeTracker.insertNodeBefore(context.file, nodeToInsertBefore, newVariable, { suffix: context.newLineCharacter + context.newLineCharacter });

            // Consume
            changeTracker.replaceNodeWithNodes(context.file, node, [localReference], { nodeSeparator: context.newLineCharacter });
        }
        else {
-            const newVariable = createVariableStatement(
-                /*modifiers*/ undefined,
-                createVariableDeclarationList(
-                    [createVariableDeclaration(localNameText, variableType, initializer)],
-                    NodeFlags.Const));
+            const newVariableDeclaration = createVariableDeclaration(localNameText, variableType, initializer);

-            // If the parent is an expression statement, replace the statement with the declaration
-            if (node.parent.kind === SyntaxKind.ExpressionStatement) {
-                changeTracker.replaceNodeWithNodes(context.file, node.parent, [newVariable], { nodeSeparator: context.newLineCharacter });
-            }
-            else {
+            // If the node is part of an initializer in a list of variable declarations, insert a new
+            // variable declaration into the list (in case it depends on earlier ones).
+            // CONSIDER: If the declaration list isn't const, we might want to split it into multiple
+            // lists so that the newly extracted one can be const.
+            const oldVariableDeclaration = getContainingVariableDeclarationIfInList(node, scope);
+            if (oldVariableDeclaration) {
                // Declare
-                const minInsertionPos = node.end;
-                const nodeToInsertBefore = getNodeToInsertConstantBefore(minInsertionPos, scope);
-                changeTracker.insertNodeBefore(context.file, nodeToInsertBefore, newVariable, { suffix: context.newLineCharacter + context.newLineCharacter });
+                // CONSIDER: could detect that each is on a separate line
+                changeTracker.insertNodeAt(context.file, oldVariableDeclaration.getStart(), newVariableDeclaration, { suffix: ", " });

                // Consume
                const localReference = createIdentifier(localNameText);
-                changeTracker.replaceNodeWithNodes(context.file, node, [localReference], { nodeSeparator: context.newLineCharacter });
+                changeTracker.replaceRange(context.file, { pos: node.getStart(), end: node.end }, localReference);
+            }
+            else if (node.parent.kind === SyntaxKind.ExpressionStatement) {
+                // If the parent is an expression statement, replace the statement with the declaration.
+                const newVariableStatement = createVariableStatement(
+                    /*modifiers*/ undefined,
+                    createVariableDeclarationList([newVariableDeclaration], NodeFlags.Const));
+                changeTracker.replaceNodeWithNodes(context.file, node.parent, [newVariableStatement], { nodeSeparator: context.newLineCharacter });
+            }
+            else {
+                const newVariableStatement = createVariableStatement(
+                    /*modifiers*/ undefined,
+                    createVariableDeclarationList([newVariableDeclaration], NodeFlags.Const));
+
+                // Declare
+                const nodeToInsertBefore = getNodeToInsertConstantBefore(node, scope);
+                if (nodeToInsertBefore.pos === 0) {
+                    // If we're at the beginning of the file, we need to take care not to insert before header comments
+                    // (e.g. copyright, triple-slash references).  Fortunately, this problem has already been solved
+                    // for imports.
+                    const insertionPos = getSourceFileImportLocation(file);
+                    changeTracker.insertNodeAt(context.file, insertionPos, newVariableStatement, {
+                        prefix: insertionPos === 0 ? undefined : context.newLineCharacter,
+                        suffix: isLineBreak(file.text.charCodeAt(insertionPos)) ? context.newLineCharacter : context.newLineCharacter + context.newLineCharacter
+                    });
+                }
+                else {
+                    changeTracker.insertNodeBefore(context.file, nodeToInsertBefore, newVariableStatement, { suffix: context.newLineCharacter + context.newLineCharacter });
+                }
+
+                // Consume
+                const localReference = createIdentifier(localNameText);
+                changeTracker.replaceRange(context.file, { pos: node.getStart(), end: node.end }, localReference);
            }
        }

@@ -906,6 +945,22 @@ namespace ts.refactor.extractSymbol {
        return { renameFilename, renameLocation, edits };
    }

+    function getContainingVariableDeclarationIfInList(node: Node, scope: Scope) {
+        let prevNode = undefined;
+        while (node !== undefined && node !== scope) {
+            if (isVariableDeclaration(node) &&
+                node.initializer === prevNode &&
+                isVariableDeclarationList(node.parent) &&
+                node.parent.declarations.length > 1) {
+
+                return node;
+            }
+
+            prevNode = node;
+            node = node.parent;
+        }
+    }
+
    /**
     * @return The index of the (only) reference to the extracted symbol.  We want the cursor
     * to be on the reference, rather than the declaration, because it's closer to where the
@@ -1088,26 +1143,61 @@ namespace ts.refactor.extractSymbol {
            child.pos >= minPos && isFunctionLikeDeclaration(child) && !isConstructorDeclaration(child));
    }

-    // TODO (acasey): need to dig into nested statements
-    // TODO (acasey): don't insert before pinned comments, directives, or triple-slash references
-    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.
+    function getNodeToInsertPropertyBefore(maxPos: number, scope: ClassLikeDeclaration): Node {
+        const members = scope.members;
+        Debug.assert(members.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 >= maxPos) {
-                break;
+        let prevMember: ClassElement | undefined = undefined;
+        let allProperties = true;
+        for (const member of members) {
+            if (member.pos > maxPos) {
+                return prevMember || members[0];
            }
-            prevChild = child;
-            if (isClassLikeScope && !isPropertyDeclaration(child)) {
-                break;
+            if (allProperties && !isPropertyDeclaration(member)) {
+                // If it is non-vacuously true that all preceding members are properties,
+                // insert before the current member (i.e. at the end of the list of properties).
+                if (prevMember !== undefined) {
+                    return member;
+                }
+
+                allProperties = false;
+            }
+            prevMember = member;
+        }
+
+        Debug.assert(prevMember !== undefined); // If the loop didn't return, then it did set prevMember.
+        return prevMember;
+    }
+
+    function getNodeToInsertConstantBefore(node: Node, scope: Scope): Node {
+        Debug.assert(!isClassLike(scope));
+
+        let prevScope: Scope | undefined = undefined;
+        for (let curr = node; curr !== scope; curr = curr.parent) {
+            if (isScope(curr)) {
+                prevScope = curr;
            }
        }

-        Debug.assert(prevChild !== undefined);
-        return prevChild;
+        for (let curr = (prevScope || node).parent; ; curr = curr.parent) {
+            if (isBlockLike(curr)) {
+                let prevStatement = undefined;
+                for (const statement of curr.statements) {
+                    if (statement.pos > node.pos) {
+                        break;
+                    }
+                    prevStatement = statement;
+                }
+                // There must be at least one statement since we started in one.
+                Debug.assert(prevStatement !== undefined);
+                return prevStatement;
+            }
+
+            if (curr === scope) {
+                Debug.fail("Didn't encounter a block-like before encountering scope");
+                break;
+            }
+        }
    }

    function getPropertyAssignmentsForWrites(writes: ReadonlyArray<UsageEntry>): ShorthandPropertyAssignment[] {
@@ -1249,7 +1339,10 @@ namespace ts.refactor.extractSymbol {
        for (let i = 0; i < scopes.length; i++) {
            if (!isReadonlyArray(targetRange.range)) {
                const scopeUsages = usagesPerScope[i];
-                if (scopeUsages.usages.size > 0 || scopeUsages.typeParameterUsages.size > 0) {
+                // Special case: in the innermost scope, all usages are available.
+                // (The computed value reflects the value at the top-level of the scope, but the
+                // local will actually be declared at the same level as the extracted expression).
+                if (i > 0 && (scopeUsages.usages.size > 0 || scopeUsages.typeParameterUsages.size > 0)) {
                    constantErrorsPerScope[i].push(createDiagnosticForNode(targetRange.range, Messages.CannotAccessVariablesFromNestedScopes));
                }
            }
@@ -1332,4 +1332,26 @@ namespace ts {
    export function getOpenBraceOfClassLike(declaration: ClassLikeDeclaration, sourceFile: SourceFile) {
        return getTokenAtPosition(sourceFile, declaration.members.pos - 1, /*includeJsDocComment*/ false);
    }
+
+    export function getSourceFileImportLocation(node: SourceFile) {
+        // For a source file, it is possible there are detached comments we should not skip
+        const text = node.text;
+        let ranges = getLeadingCommentRanges(text, 0);
+        if (!ranges) return 0;
+        let position = 0;
+        // However we should still skip a pinned comment at the top
+        if (ranges.length && ranges[0].kind === SyntaxKind.MultiLineCommentTrivia && isPinnedComment(text, ranges[0])) {
+            position = ranges[0].end + 1;
+            ranges = ranges.slice(1);
+        }
+        // As well as any triple slash references
+        for (const range of ranges) {
+            if (range.kind === SyntaxKind.SingleLineCommentTrivia && isRecognizedTripleSlashComment(node.text, range.pos, range.end)) {
+                position = range.end + 1;
+                continue;
+            }
+            break;
+        }
+        return position;
+    }
 }
@@ -0,0 +1,18 @@
+// ==ORIGINAL==
+
+for (let i = 0; i < 10; i++) {
+    for (let j = 0; j < 10; j++) {
+        const x = i + 1;
+    }
+}
+        
+// ==SCOPE::Extract to constant in enclosing scope==
+
+for (let i = 0; i < 10; i++) {
+    for (let j = 0; j < 10; j++) {
+        const newLocal = i + 1;
+
+        const x = /*RENAME*/newLocal;
+    }
+}
+        
@@ -0,0 +1,18 @@
+// ==ORIGINAL==
+
+for (let i = 0; i < 10; i++) {
+    for (let j = 0; j < 10; j++) {
+        const x = i + 1;
+    }
+}
+        
+// ==SCOPE::Extract to constant in enclosing scope==
+
+for (let i = 0; i < 10; i++) {
+    for (let j = 0; j < 10; j++) {
+        const newLocal = i + 1;
+
+        const x = /*RENAME*/newLocal;
+    }
+}
+        
@@ -4,11 +4,11 @@ for (let i = 0; i < 10; i++) {
        let x = 1;
    }
 }
-// ==SCOPE::Extract to constant in global scope==
-const newLocal = 1;
-
+// ==SCOPE::Extract to constant in enclosing scope==
 for (let i = 0; i < 10; i++) {
    for (let j = 0; j < 10; j++) {
+        const newLocal = 1;
+
        let x = /*RENAME*/newLocal;
    }
 }
@@ -4,11 +4,11 @@ for (let i = 0; i < 10; i++) {
        let x = 1;
    }
 }
-// ==SCOPE::Extract to constant in global scope==
-const newLocal = 1;
-
+// ==SCOPE::Extract to constant in enclosing scope==
 for (let i = 0; i < 10; i++) {
    for (let j = 0; j < 10; j++) {
+        const newLocal = 1;
+
        let x = /*RENAME*/newLocal;
    }
 }
@@ -8,7 +8,7 @@ class C {
        let x = 1;
    }
 }
-// ==SCOPE::Extract to constant in method 'M3==
+// ==SCOPE::Extract to constant in enclosing scope==
 class C {
    a = 1;
    b = 2;
@@ -8,7 +8,7 @@ class C {
        let x = 1;
    }
 }
-// ==SCOPE::Extract to constant in method 'M3==
+// ==SCOPE::Extract to constant in enclosing scope==
 class C {
    a = 1;
    b = 2;
@@ -0,0 +1,34 @@
+// ==ORIGINAL==
+class C {
+    a = 1;
+    M1() { }
+    b = 2;
+    M2() { }
+    M3() {
+        let x = 1;
+    }
+}
+// ==SCOPE::Extract to constant in enclosing scope==
+class C {
+    a = 1;
+    M1() { }
+    b = 2;
+    M2() { }
+    M3() {
+        const newLocal = 1;
+
+        let x = /*RENAME*/newLocal;
+    }
+}
+// ==SCOPE::Extract to constant in global scope==
+const newLocal = 1;
+
+class C {
+    a = 1;
+    M1() { }
+    b = 2;
+    M2() { }
+    M3() {
+        let x = /*RENAME*/newLocal;
+    }
+}
@@ -0,0 +1,46 @@
+// ==ORIGINAL==
+class C {
+    a = 1;
+    M1() { }
+    b = 2;
+    M2() { }
+    M3() {
+        let x = 1;
+    }
+}
+// ==SCOPE::Extract to constant in enclosing scope==
+class C {
+    a = 1;
+    M1() { }
+    b = 2;
+    M2() { }
+    M3() {
+        const newLocal = 1;
+
+        let x = /*RENAME*/newLocal;
+    }
+}
+// ==SCOPE::Extract to readonly field in class 'C'==
+class C {
+    a = 1;
+    private readonly newProperty = 1;
+
+    M1() { }
+    b = 2;
+    M2() { }
+    M3() {
+        let x = this./*RENAME*/newProperty;
+    }
+}
+// ==SCOPE::Extract to constant in global scope==
+const newLocal = 1;
+
+class C {
+    a = 1;
+    M1() { }
+    b = 2;
+    M2() { }
+    M3() {
+        let x = /*RENAME*/newLocal;
+    }
+}
@@ -0,0 +1,34 @@
+// ==ORIGINAL==
+class C {
+    M1() { }
+    a = 1;
+    b = 2;
+    M2() { }
+    M3() {
+        let x = 1;
+    }
+}
+// ==SCOPE::Extract to constant in enclosing scope==
+class C {
+    M1() { }
+    a = 1;
+    b = 2;
+    M2() { }
+    M3() {
+        const newLocal = 1;
+
+        let x = /*RENAME*/newLocal;
+    }
+}
+// ==SCOPE::Extract to constant in global scope==
+const newLocal = 1;
+
+class C {
+    M1() { }
+    a = 1;
+    b = 2;
+    M2() { }
+    M3() {
+        let x = /*RENAME*/newLocal;
+    }
+}
@@ -0,0 +1,46 @@
+// ==ORIGINAL==
+class C {
+    M1() { }
+    a = 1;
+    b = 2;
+    M2() { }
+    M3() {
+        let x = 1;
+    }
+}
+// ==SCOPE::Extract to constant in enclosing scope==
+class C {
+    M1() { }
+    a = 1;
+    b = 2;
+    M2() { }
+    M3() {
+        const newLocal = 1;
+
+        let x = /*RENAME*/newLocal;
+    }
+}
+// ==SCOPE::Extract to readonly field in class 'C'==
+class C {
+    M1() { }
+    a = 1;
+    b = 2;
+    M2() { }
+    private readonly newProperty = 1;
+
+    M3() {
+        let x = this./*RENAME*/newProperty;
+    }
+}
+// ==SCOPE::Extract to constant in global scope==
+const newLocal = 1;
+
+class C {
+    M1() { }
+    a = 1;
+    b = 2;
+    M2() { }
+    M3() {
+        let x = /*RENAME*/newLocal;
+    }
+}
@@ -0,0 +1,14 @@
+// ==ORIGINAL==
+
+"strict";
+
+const x = 2 + 1;
+        
+// ==SCOPE::Extract to constant in enclosing scope==
+
+"strict";
+
+const newLocal = 2 + 1;
+
+const x = /*RENAME*/newLocal;
+        
@@ -0,0 +1,14 @@
+// ==ORIGINAL==
+
+"strict";
+
+const x = 2 + 1;
+        
+// ==SCOPE::Extract to constant in enclosing scope==
+
+"strict";
+
+const newLocal = 2 + 1;
+
+const x = /*RENAME*/newLocal;
+        
@@ -1,4 +1,4 @@
 // ==ORIGINAL==
 "hello";
-// ==SCOPE::Extract to constant in global scope==
+// ==SCOPE::Extract to constant in enclosing scope==
 const /*RENAME*/newLocal = "hello";
@@ -1,4 +1,4 @@
 // ==ORIGINAL==
 "hello";
-// ==SCOPE::Extract to constant in global scope==
+// ==SCOPE::Extract to constant in enclosing scope==
 const /*RENAME*/newLocal = "hello";
@@ -1,4 +1,4 @@
 // ==ORIGINAL==
 "hello";
-// ==SCOPE::Extract to constant in global scope==
+// ==SCOPE::Extract to constant in enclosing scope==
 const /*RENAME*/newLocal = "hello";
@@ -1,4 +1,4 @@
 // ==ORIGINAL==
 "hello";
-// ==SCOPE::Extract to constant in global scope==
+// ==SCOPE::Extract to constant in enclosing scope==
 const /*RENAME*/newLocal = "hello";
@@ -2,7 +2,7 @@
 function F() {
    let x = 1;
 }
-// ==SCOPE::Extract to constant in function 'F'==
+// ==SCOPE::Extract to constant in enclosing scope==
 function F() {
    const newLocal = 1;

@@ -2,7 +2,7 @@
 function F() {
    let x = 1;
 }
-// ==SCOPE::Extract to constant in function 'F'==
+// ==SCOPE::Extract to constant in enclosing scope==
 function F() {
    const newLocal = 1;

@@ -4,7 +4,7 @@ class C {
        let x = 1;
    }
 }
-// ==SCOPE::Extract to constant in method 'M==
+// ==SCOPE::Extract to constant in enclosing scope==
 class C {
    M() {
        const newLocal = 1;
@@ -4,7 +4,7 @@ class C {
        let x = 1;
    }
 }
-// ==SCOPE::Extract to constant in method 'M==
+// ==SCOPE::Extract to constant in enclosing scope==
 class C {
    M() {
        const newLocal = 1;
@@ -0,0 +1,22 @@
+// ==ORIGINAL==
+
+/*! Copyright */
+
+/// <reference path="path.js"/>
+
+"strict";
+
+const x = 2 + 1;
+        
+// ==SCOPE::Extract to constant in enclosing scope==
+
+/*! Copyright */
+
+/// <reference path="path.js"/>
+
+"strict";
+
+const newLocal = 2 + 1;
+
+const x = /*RENAME*/newLocal;
+        
@@ -0,0 +1,22 @@
+// ==ORIGINAL==
+
+/*! Copyright */
+
+/// <reference path="path.js"/>
+
+"strict";
+
+const x = 2 + 1;
+        
+// ==SCOPE::Extract to constant in enclosing scope==
+
+/*! Copyright */
+
+/// <reference path="path.js"/>
+
+"strict";
+
+const newLocal = 2 + 1;
+
+const x = /*RENAME*/newLocal;
+        
@@ -2,7 +2,7 @@
 namespace N {
    let x = 1;
 }
-// ==SCOPE::Extract to constant in namespace 'N'==
+// ==SCOPE::Extract to constant in enclosing scope==
 namespace N {
    const newLocal = 1;

@@ -3,7 +3,7 @@ function F() {
    let w = 1;
    let x = w + 1;
 }
-// ==SCOPE::Extract to constant in function 'F'==
+// ==SCOPE::Extract to constant in enclosing scope==
 function F() {
    let w = 1;
    const newLocal = w + 1;
@@ -3,7 +3,7 @@ function F() {
    let w = 1;
    let x = w + 1;
 }
-// ==SCOPE::Extract to constant in function 'F'==
+// ==SCOPE::Extract to constant in enclosing scope==
 function F() {
    let w = 1;
    const newLocal = w + 1;
@@ -0,0 +1,14 @@
+// ==ORIGINAL==
+
+/*! Copyright */
+
+const x = 2 + 1;
+        
+// ==SCOPE::Extract to constant in enclosing scope==
+
+/*! Copyright */
+
+const newLocal = 2 + 1;
+
+const x = /*RENAME*/newLocal;
+        
@@ -0,0 +1,14 @@
+// ==ORIGINAL==
+
+/*! Copyright */
+
+const x = 2 + 1;
+        
+// ==SCOPE::Extract to constant in enclosing scope==
+
+/*! Copyright */
+
+const newLocal = 2 + 1;
+
+const x = /*RENAME*/newLocal;
+        
@@ -0,0 +1,16 @@
+// ==ORIGINAL==
+
+/*! Copyright */
+
+/* About x */
+const x = 2 + 1;
+        
+// ==SCOPE::Extract to constant in enclosing scope==
+
+/*! Copyright */
+
+const newLocal = 2 + 1;
+
+/* About x */
+const x = /*RENAME*/newLocal;
+        
@@ -0,0 +1,16 @@
+// ==ORIGINAL==
+
+/*! Copyright */
+
+/* About x */
+const x = 2 + 1;
+        
+// ==SCOPE::Extract to constant in enclosing scope==
+
+/*! Copyright */
+
+const newLocal = 2 + 1;
+
+/* About x */
+const x = /*RENAME*/newLocal;
+        
@@ -0,0 +1,16 @@
+// ==ORIGINAL==
+
+const i = 0;
+for (let j = 0; j < 10; j++) {
+    const x = i + 1;
+}
+        
+// ==SCOPE::Extract to constant in enclosing scope==
+
+const i = 0;
+for (let j = 0; j < 10; j++) {
+    const newLocal = i + 1;
+
+    const x = /*RENAME*/newLocal;
+}
+        
@@ -0,0 +1,16 @@
+// ==ORIGINAL==
+
+const i = 0;
+for (let j = 0; j < 10; j++) {
+    const x = i + 1;
+}
+        
+// ==SCOPE::Extract to constant in enclosing scope==
+
+const i = 0;
+for (let j = 0; j < 10; j++) {
+    const newLocal = i + 1;
+
+    const x = /*RENAME*/newLocal;
+}
+        
@@ -0,0 +1,31 @@
+// ==ORIGINAL==
+
+const i = 0;
+function F() {
+    for (let j = 0; j < 10; j++) {
+        const x = i + 1;
+    }
+}
+        
+// ==SCOPE::Extract to constant in enclosing scope==
+
+const i = 0;
+function F() {
+    for (let j = 0; j < 10; j++) {
+        const newLocal = i + 1;
+
+        const x = /*RENAME*/newLocal;
+    }
+}
+        
+// ==SCOPE::Extract to constant in global scope==
+
+const i = 0;
+const newLocal = i + 1;
+
+function F() {
+    for (let j = 0; j < 10; j++) {
+        const x = /*RENAME*/newLocal;
+    }
+}
+        
@@ -0,0 +1,31 @@
+// ==ORIGINAL==
+
+const i = 0;
+function F() {
+    for (let j = 0; j < 10; j++) {
+        const x = i + 1;
+    }
+}
+        
+// ==SCOPE::Extract to constant in enclosing scope==
+
+const i = 0;
+function F() {
+    for (let j = 0; j < 10; j++) {
+        const newLocal = i + 1;
+
+        const x = /*RENAME*/newLocal;
+    }
+}
+        
+// ==SCOPE::Extract to constant in global scope==
+
+const i = 0;
+const newLocal = i + 1;
+
+function F() {
+    for (let j = 0; j < 10; j++) {
+        const x = /*RENAME*/newLocal;
+    }
+}
+        
@@ -0,0 +1,14 @@
+// ==ORIGINAL==
+
+for (let j = 0; j < 10; j++) {
+    const x = 2 + 1;
+}
+        
+// ==SCOPE::Extract to constant in enclosing scope==
+
+for (let j = 0; j < 10; j++) {
+    const newLocal = 2 + 1;
+
+    const x = /*RENAME*/newLocal;
+}
+        
@@ -0,0 +1,14 @@
+// ==ORIGINAL==
+
+for (let j = 0; j < 10; j++) {
+    const x = 2 + 1;
+}
+        
+// ==SCOPE::Extract to constant in enclosing scope==
+
+for (let j = 0; j < 10; j++) {
+    const newLocal = 2 + 1;
+
+    const x = /*RENAME*/newLocal;
+}
+        
@@ -0,0 +1,27 @@
+// ==ORIGINAL==
+
+function F() {
+    for (let j = 0; j < 10; j++) {
+        const x = 2 + 1;
+    }
+}
+        
+// ==SCOPE::Extract to constant in enclosing scope==
+
+function F() {
+    for (let j = 0; j < 10; j++) {
+        const newLocal = 2 + 1;
+
+        const x = /*RENAME*/newLocal;
+    }
+}
+        
+// ==SCOPE::Extract to constant in global scope==
+const newLocal = 2 + 1;
+
+function F() {
+    for (let j = 0; j < 10; j++) {
+        const x = /*RENAME*/newLocal;
+    }
+}
+        
@@ -0,0 +1,27 @@
+// ==ORIGINAL==
+
+function F() {
+    for (let j = 0; j < 10; j++) {
+        const x = 2 + 1;
+    }
+}
+        
+// ==SCOPE::Extract to constant in enclosing scope==
+
+function F() {
+    for (let j = 0; j < 10; j++) {
+        const newLocal = 2 + 1;
+
+        const x = /*RENAME*/newLocal;
+    }
+}
+        
+// ==SCOPE::Extract to constant in global scope==
+const newLocal = 2 + 1;
+
+function F() {
+    for (let j = 0; j < 10; j++) {
+        const x = /*RENAME*/newLocal;
+    }
+}
+        
@@ -0,0 +1,41 @@
+// ==ORIGINAL==
+
+function F0() {
+    function F1() {
+        function F2(x = 2 + 1) {
+        }
+    }
+}
+        
+// ==SCOPE::Extract to constant in enclosing scope==
+
+function F0() {
+    function F1() {
+        const newLocal = 2 + 1;
+
+        function F2(x = /*RENAME*/newLocal) {
+        }
+    }
+}
+        
+// ==SCOPE::Extract to constant in function 'F0'==
+
+function F0() {
+    const newLocal = 2 + 1;
+
+    function F1() {
+        function F2(x = /*RENAME*/newLocal) {
+        }
+    }
+}
+        
+// ==SCOPE::Extract to constant in global scope==
+const newLocal = 2 + 1;
+
+function F0() {
+    function F1() {
+        function F2(x = /*RENAME*/newLocal) {
+        }
+    }
+}
+        
@@ -0,0 +1,41 @@
+// ==ORIGINAL==
+
+function F0() {
+    function F1() {
+        function F2(x = 2 + 1) {
+        }
+    }
+}
+        
+// ==SCOPE::Extract to constant in enclosing scope==
+
+function F0() {
+    function F1() {
+        const newLocal = 2 + 1;
+
+        function F2(x = /*RENAME*/newLocal) {
+        }
+    }
+}
+        
+// ==SCOPE::Extract to constant in function 'F0'==
+
+function F0() {
+    const newLocal = 2 + 1;
+
+    function F1() {
+        function F2(x = /*RENAME*/newLocal) {
+        }
+    }
+}
+        
+// ==SCOPE::Extract to constant in global scope==
+const newLocal = 2 + 1;
+
+function F0() {
+    function F1() {
+        function F2(x = /*RENAME*/newLocal) {
+        }
+    }
+}
+        
@@ -0,0 +1,13 @@
+// ==ORIGINAL==
+
+class C {
+    x = 2 + 1;
+}
+        
+// ==SCOPE::Extract to constant in global scope==
+const newLocal = 2 + 1;
+
+class C {
+    x = /*RENAME*/newLocal;
+}
+        
@@ -0,0 +1,21 @@
+// ==ORIGINAL==
+
+class C {
+    x = 2 + 1;
+}
+        
+// ==SCOPE::Extract to readonly field in class 'C'==
+
+class C {
+    private readonly newProperty = 2 + 1;
+
+    x = this./*RENAME*/newProperty;
+}
+        
+// ==SCOPE::Extract to constant in global scope==
+const newLocal = 2 + 1;
+
+class C {
+    x = /*RENAME*/newLocal;
+}
+        
@@ -0,0 +1,37 @@
+// ==ORIGINAL==
+
+const i = 0;
+class C {
+    M() {
+        for (let j = 0; j < 10; j++) {
+            x = i + 1;
+        }
+    }
+}
+        
+// ==SCOPE::Extract to constant in enclosing scope==
+
+const i = 0;
+class C {
+    M() {
+        for (let j = 0; j < 10; j++) {
+            const newLocal = i + 1;
+
+            x = /*RENAME*/newLocal;
+        }
+    }
+}
+        
+// ==SCOPE::Extract to constant in global scope==
+
+const i = 0;
+const newLocal = i + 1;
+
+class C {
+    M() {
+        for (let j = 0; j < 10; j++) {
+            x = /*RENAME*/newLocal;
+        }
+    }
+}
+        
@@ -0,0 +1,50 @@
+// ==ORIGINAL==
+
+const i = 0;
+class C {
+    M() {
+        for (let j = 0; j < 10; j++) {
+            x = i + 1;
+        }
+    }
+}
+        
+// ==SCOPE::Extract to constant in enclosing scope==
+
+const i = 0;
+class C {
+    M() {
+        for (let j = 0; j < 10; j++) {
+            const newLocal: any = i + 1;
+
+            x = /*RENAME*/newLocal;
+        }
+    }
+}
+        
+// ==SCOPE::Extract to readonly field in class 'C'==
+
+const i = 0;
+class C {
+    private readonly newProperty: any = i + 1;
+
+    M() {
+        for (let j = 0; j < 10; j++) {
+            x = this./*RENAME*/newProperty;
+        }
+    }
+}
+        
+// ==SCOPE::Extract to constant in global scope==
+
+const i = 0;
+const newLocal: any = i + 1;
+
+class C {
+    M() {
+        for (let j = 0; j < 10; j++) {
+            x = /*RENAME*/newLocal;
+        }
+    }
+}
+        
@@ -1,6 +1,6 @@
 // ==ORIGINAL==
 let x = 1;
-// ==SCOPE::Extract to constant in global scope==
+// ==SCOPE::Extract to constant in enclosing scope==
 const newLocal = 1;

 let x = /*RENAME*/newLocal;
@@ -1,6 +1,6 @@
 // ==ORIGINAL==
 let x = 1;
-// ==SCOPE::Extract to constant in global scope==
+// ==SCOPE::Extract to constant in enclosing scope==
 const newLocal = 1;

 let x = /*RENAME*/newLocal;
@@ -0,0 +1,14 @@
+// ==ORIGINAL==
+
+/// <reference path="path.js"/>
+
+const x = 2 + 1;
+        
+// ==SCOPE::Extract to constant in enclosing scope==
+
+/// <reference path="path.js"/>
+
+const newLocal = 2 + 1;
+
+const x = /*RENAME*/newLocal;
+        
@@ -0,0 +1,14 @@
+// ==ORIGINAL==
+
+/// <reference path="path.js"/>
+
+const x = 2 + 1;
+        
+// ==SCOPE::Extract to constant in enclosing scope==
+
+/// <reference path="path.js"/>
+
+const newLocal = 2 + 1;
+
+const x = /*RENAME*/newLocal;
+        
@@ -2,7 +2,7 @@
 function F<T>(t: T) {
    let x = t + 1;
 }
-// ==SCOPE::Extract to constant in function 'F'==
+// ==SCOPE::Extract to constant in enclosing scope==
 function F<T>(t: T) {
    const newLocal = t + 1;

@@ -0,0 +1,6 @@
+// ==ORIGINAL==
+const /*About A*/a = 1,
+    /*About B*/b = a + 1;
+// ==SCOPE::Extract to constant in enclosing scope==
+const /*About A*/a = 1,
+    /*About B*/newLocal = a + 1, b = /*RENAME*/newLocal;
@@ -0,0 +1,6 @@
+// ==ORIGINAL==
+const /*About A*/a = 1,
+    /*About B*/b = a + 1;
+// ==SCOPE::Extract to constant in enclosing scope==
+const /*About A*/a = 1,
+    /*About B*/newLocal = a + 1, b = /*RENAME*/newLocal;
@@ -0,0 +1,4 @@
+// ==ORIGINAL==
+const a = 1, b = a + 1;
+// ==SCOPE::Extract to constant in enclosing scope==
+const a = 1, newLocal = a + 1, b = /*RENAME*/newLocal;
@@ -0,0 +1,4 @@
+// ==ORIGINAL==
+const a = 1, b = a + 1;
+// ==SCOPE::Extract to constant in enclosing scope==
+const a = 1, newLocal = a + 1, b = /*RENAME*/newLocal;
@@ -0,0 +1,4 @@
+// ==ORIGINAL==
+let a = 1, b = a + 1;
+// ==SCOPE::Extract to constant in enclosing scope==
+let a = 1, newLocal = a + 1, b = /*RENAME*/newLocal;
@@ -0,0 +1,4 @@
+// ==ORIGINAL==
+let a = 1, b = a + 1;
+// ==SCOPE::Extract to constant in enclosing scope==
+let a = 1, newLocal = a + 1, b = /*RENAME*/newLocal;