Skip to content

Commit 3eea1a9

Browse files
committed
Generalize extract method to handle constants as well
Major changes: 1) Instead of skipping undesirable scopes, include them and mark them with errors. Constants can be extracted into more scopes. 2) Update the tests to call through the "public" API. This caused some baseline changes. 3) Rename refactoring to "Extract Symbol" for generality. 4) Return a second ApplicableRefactorInfo for constants. Distinguish the two by splitting the action name.
1 parent 6ffee10 commit 3eea1a9

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

65 files changed

+539
-305
lines changed

src/compiler/diagnosticMessages.json

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3703,13 +3703,23 @@
37033703
"code": 95002
37043704
},
37053705

3706-
"Extract function": {
3706+
"Extract symbol": {
37073707
"category": "Message",
37083708
"code": 95003
37093709
},
37103710

37113711
"Extract to {0}": {
37123712
"category": "Message",
37133713
"code": 95004
3714+
},
3715+
3716+
"Extract function": {
3717+
"category": "Message",
3718+
"code": 95005
3719+
},
3720+
3721+
"Extract constant": {
3722+
"category": "Message",
3723+
"code": 95006
37143724
}
37153725
}

src/harness/unittests/extractMethods.ts

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ namespace ts {
105105
if (!selectionRange) {
106106
throw new Error(`Test ${s} does not specify selection range`);
107107
}
108-
const result = refactor.extractMethod.getRangeToExtract(file, createTextSpanFromBounds(selectionRange.start, selectionRange.end));
108+
const result = refactor.extractSymbol.getRangeToExtract(file, createTextSpanFromBounds(selectionRange.start, selectionRange.end));
109109
assert(result.targetRange === undefined, "failure expected");
110110
const sortedErrors = result.errors.map(e => <string>e.messageText).sort();
111111
assert.deepEqual(sortedErrors, expectedErrors.sort(), "unexpected errors");
@@ -119,7 +119,7 @@ namespace ts {
119119
if (!selectionRange) {
120120
throw new Error(`Test ${s} does not specify selection range`);
121121
}
122-
const result = refactor.extractMethod.getRangeToExtract(f, createTextSpanFromBounds(selectionRange.start, selectionRange.end));
122+
const result = refactor.extractSymbol.getRangeToExtract(f, createTextSpanFromBounds(selectionRange.start, selectionRange.end));
123123
const expectedRange = t.ranges.get("extracted");
124124
if (expectedRange) {
125125
let start: number, end: number;
@@ -407,7 +407,7 @@ function test(x: number) {
407407
testExtractRangeFailed("extractRangeFailed9",
408408
`var x = ([#||]1 + 2);`,
409409
[
410-
"Statement or expression expected."
410+
"Cannot extract empty range."
411411
]);
412412

413413
testExtractRangeFailed("extract-method-not-for-token-expression-statement", `[#|a|]`, ["Select more than a single identifier."]);
@@ -604,15 +604,15 @@ function test(x: number) {
604604
// doesn't handle type parameter shadowing.
605605
testExtractMethod("extractMethod14",
606606
`function F<T>(t1: T) {
607-
function F<T>(t2: T) {
607+
function G<T>(t2: T) {
608608
[#|t1.toString();
609609
t2.toString();|]
610610
}
611611
}`);
612612
// Confirm that the constraint is preserved.
613613
testExtractMethod("extractMethod15",
614614
`function F<T>(t1: T) {
615-
function F<U extends T[]>(t2: U) {
615+
function G<U extends T[]>(t2: U) {
616616
[#|t2.toString();|]
617617
}
618618
}`);
@@ -799,19 +799,20 @@ function parsePrimaryExpression(): any {
799799
newLineCharacter,
800800
program,
801801
file: sourceFile,
802-
startPosition: -1,
802+
startPosition: selectionRange.start,
803+
endPosition: selectionRange.end,
803804
rulesProvider: getRuleProvider()
804805
};
805-
const result = refactor.extractMethod.getRangeToExtract(sourceFile, createTextSpanFromBounds(selectionRange.start, selectionRange.end));
806-
assert.equal(result.errors, undefined, "expect no errors");
807-
const results = refactor.extractMethod.getPossibleExtractions(result.targetRange, context);
806+
const rangeToExtract = refactor.extractSymbol.getRangeToExtract(sourceFile, createTextSpanFromBounds(selectionRange.start, selectionRange.end));
807+
assert.equal(rangeToExtract.errors, undefined, "expect no errors");
808+
const actions = refactor.extractSymbol.getAvailableActions(context)[0].actions; // TODO (acasey): smarter index
808809
const data: string[] = [];
809810
data.push(`// ==ORIGINAL==`);
810811
data.push(sourceFile.text);
811-
for (const r of results) {
812-
const { renameLocation, edits } = refactor.extractMethod.getExtractionAtIndex(result.targetRange, context, results.indexOf(r));
812+
for (const action of actions) {
813+
const { renameLocation, edits } = refactor.extractSymbol.getEditsForAction(context, action.name);
813814
assert.lengthOf(edits, 1);
814-
data.push(`// ==SCOPE::${r.scopeDescription}==`);
815+
data.push(`// ==SCOPE::${action.description}==`);
815816
const newText = textChanges.applyChanges(sourceFile.text, edits[0].textChanges);
816817
const newTextWithRename = newText.slice(0, renameLocation) + "/*RENAME*/" + newText.slice(renameLocation);
817818
data.push(newTextWithRename);

0 commit comments

Comments
 (0)