Improve Extract* unit test coverage and correctness

src/harness/unittests/extractTestHelpers.ts

@@ -98,35 +98,41 @@ namespace ts {
     }
 
     export function testExtractSymbol(caption: string, text: string, baselineFolder: string, description: DiagnosticMessage) {
-        it(caption, () => {
-            Harness.Baseline.runBaseline(`${baselineFolder}/${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 === description.message).actions;
+        const t = extractTest(text);
+        const selectionRange = t.ranges.get("selection");
+        if (!selectionRange) {
+            throw new Error(`Test ${caption} does not specify selection range`);
+        }
+
+        [Extension.Ts, Extension.Js].forEach(extension =>
+            it(`${caption} [${extension}]`, () => runBaseline(extension)));
+
+        function runBaseline(extension: Extension) {
+            const path = "/a" + extension;
+            const program = makeProgram({ path, content: t.source });
+
+            if (hasSyntacticDiagnostics(program)) {
+                // Don't bother generating JS baselines for inputs that aren't valid JS.
+                assert.equal(Extension.Js, extension);
+                return;
+            }
+
+            const sourceFile = program.getSourceFile(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 === description.message).actions;
+
+            Harness.Baseline.runBaseline(`${baselineFolder}/${caption}${extension}`, () => {
                 const data: string[] = [];
                 data.push(`// ==ORIGINAL==`);
                 data.push(sourceFile.text);
@@ -137,10 +143,26 @@ namespace ts {
                     const newText = textChanges.applyChanges(sourceFile.text, edits[0].textChanges);
                     const newTextWithRename = newText.slice(0, renameLocation) + "/*RENAME*/" + newText.slice(renameLocation);
                     data.push(newTextWithRename);
+
+                    const diagProgram = makeProgram({ path, content: newText });
+                    assert.isFalse(hasSyntacticDiagnostics(diagProgram));
                 }
                 return data.join(newLineCharacter);
             });
-        });
+        }
+
+        function makeProgram(f: {path: string, content: string }) {
+            const host = projectSystem.createServerHost([f, projectSystem.libFile]);
+            const projectService = projectSystem.createProjectService(host);
+            projectService.openClientFile(f.path);
+            const program = projectService.inferredProjects[0].getLanguageService().getProgram();
+            return program;
+        }
+
+        function hasSyntacticDiagnostics(program: Program) {
+            const diags = program.getSyntacticDiagnostics();
+            return length(diags) > 0;
+        }
     }
 
     export function testExtractSymbolFailed(caption: string, text: string, description: DiagnosticMessage) {

tests/baselines/reference/extractConstant/extractConstant_BlockScopes_NoDependencies.js

@@ -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;
+    }
+}

tests/baselines/reference/extractConstant/extractConstant_Class.js

@@ -0,0 +1,10 @@
+// ==ORIGINAL==
+class C {
+    x = 1;
+}
+// ==SCOPE::Extract to constant in global scope==
+const newLocal = 1;
+
+class C {
+    x = /*RENAME*/newLocal;
+}

tests/baselines/reference/extractConstant/extractConstant_ClassInsertionPosition.js

@@ -0,0 +1,34 @@
+// ==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 constant in global scope==
+const newLocal = 1;
+
+class C {
+    a = 1;
+    b = 2;
+    M1() { }
+    M2() { }
+    M3() {
+        let x = /*RENAME*/newLocal;
+    }
+}

tests/baselines/reference/extractConstant/extractConstant_ExpressionStatement.js

@@ -0,0 +1,4 @@
+// ==ORIGINAL==
+"hello";
+// ==SCOPE::Extract to constant in global scope==
+const /*RENAME*/newLocal = "hello";

tests/baselines/reference/extractConstant/extractConstant_ExpressionStatementExpression.js

@@ -0,0 +1,4 @@
+// ==ORIGINAL==
+"hello";
+// ==SCOPE::Extract to constant in global scope==
+const /*RENAME*/newLocal = "hello";

tests/baselines/reference/extractConstant/extractConstant_Function.js

@@ -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;
+}

tests/baselines/reference/extractConstant/extractConstant_Method.js

@@ -0,0 +1,22 @@
+// ==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 constant in global scope==
+const newLocal = 1;
+
+class C {
+    M() {
+        let x = /*RENAME*/newLocal;
+    }
+}

tests/baselines/reference/extractConstant/extractConstant_Parameters.js

@@ -0,0 +1,12 @@
+// ==ORIGINAL==
+function F() {
+    let w = 1;
+    let x = w + 1;
+}
+// ==SCOPE::Extract to constant in function 'F'==
+function F() {
+    let w = 1;
+    const newLocal = w + 1;
+
+    let x = /*RENAME*/newLocal;
+}

tests/baselines/reference/extractConstant/extractConstant_TopLevel.js

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

tests/baselines/reference/extractFunction/extractFunction20.js

@@ -0,0 +1,28 @@
+// ==ORIGINAL==
+const _ = class {
+    a() {
+        let a1 = { x: 1 };
+        return a1.x + 10;
+    }
+}
+// ==SCOPE::Extract to method in anonymous class expression==
+const _ = class {
+    a() {
+        return this./*RENAME*/newMethod();
+    }
+
+    newMethod() {
+        let a1 = { x: 1 };
+        return a1.x + 10;
+    }
+}
+// ==SCOPE::Extract to function in global scope==
+const _ = class {
+    a() {
+        return /*RENAME*/newFunction();
+    }
+}
+function newFunction() {
+    let a1 = { x: 1 };
+    return a1.x + 10;
+}

tests/baselines/reference/extractFunction/extractFunction21.js

@@ -0,0 +1,26 @@
+// ==ORIGINAL==
+function foo() {
+    let x = 10;
+    x++;
+    return;
+}
+// ==SCOPE::Extract to inner function in function 'foo'==
+function foo() {
+    let x = 10;
+    return /*RENAME*/newFunction();
+
+    function newFunction() {
+        x++;
+        return;
+    }
+}
+// ==SCOPE::Extract to function in global scope==
+function foo() {
+    let x = 10;
+    x = /*RENAME*/newFunction(x);
+    return;
+}
+function newFunction(x) {
+    x++;
+    return x;
+}

tests/baselines/reference/extractFunction/extractFunction22.js

@@ -0,0 +1,31 @@
+// ==ORIGINAL==
+function test() {
+    try {
+    }
+    finally {
+        return 1;
+    }
+}
+// ==SCOPE::Extract to inner function in function 'test'==
+function test() {
+    try {
+    }
+    finally {
+        return /*RENAME*/newFunction();
+    }
+
+    function newFunction() {
+        return 1;
+    }
+}
+// ==SCOPE::Extract to function in global scope==
+function test() {
+    try {
+    }
+    finally {
+        return /*RENAME*/newFunction();
+    }
+}
+function newFunction() {
+    return 1;
+}

tests/baselines/reference/extractFunction/extractFunction24.js

@@ -0,0 +1,43 @@
+// ==ORIGINAL==
+function Outer() {
+    function M1() { }
+    function M2() {
+        return 1;
+    }
+    function M3() { }
+}
+// ==SCOPE::Extract to inner function in function 'M2'==
+function Outer() {
+    function M1() { }
+    function M2() {
+        return /*RENAME*/newFunction();
+
+        function newFunction() {
+            return 1;
+        }
+    }
+    function M3() { }
+}
+// ==SCOPE::Extract to inner function in function 'Outer'==
+function Outer() {
+    function M1() { }
+    function M2() {
+        return /*RENAME*/newFunction();
+    }
+    function newFunction() {
+        return 1;
+    }
+
+    function M3() { }
+}
+// ==SCOPE::Extract to function in global scope==
+function Outer() {
+    function M1() { }
+    function M2() {
+        return /*RENAME*/newFunction();
+    }
+    function M3() { }
+}
+function newFunction() {
+    return 1;
+}

tests/baselines/reference/extractFunction/extractFunction25.js

@@ -0,0 +1,26 @@
+// ==ORIGINAL==
+function M1() { }
+function M2() {
+    return 1;
+}
+function M3() { }
+// ==SCOPE::Extract to inner function in function 'M2'==
+function M1() { }
+function M2() {
+    return /*RENAME*/newFunction();
+
+    function newFunction() {
+        return 1;
+    }
+}
+function M3() { }
+// ==SCOPE::Extract to function in global scope==
+function M1() { }
+function M2() {
+    return /*RENAME*/newFunction();
+}
+function newFunction() {
+    return 1;
+}
+
+function M3() { }