microsoft/TypeScript
1
0
Fork 0
mirror of https://github.com/microsoft/TypeScript.git synced 2026-02-05 16:38:05 -06:00
Code Issues Packages Projects Releases 212 Wiki Activity

Expand extractSymbol ranges (#42770)

Browse Source 
* allow partial selections of node ranges

* separate tests for better failure investigation

* gate span expansion behind invoked command

* add invoked test

* comment wording

* for test
This commit is contained in:
Jesse Trinity 2021-02-17 12:55:12 -08:00 committed by GitHub
parent 5cdc87032b
commit d640313ff2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 113 additions and 68 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
src/services/refactors/extractSymbol.ts
View File

@ -70,7 +70,7 @@ namespace ts.refactor.extractSymbol {
let i = 0;
for (const { functionExtraction, constantExtraction } of extractions) {
const description = functionExtraction.description;
if(refactorKindBeginsWith(extractFunctionAction.kind, requestedRefactor)){
if (refactorKindBeginsWith(extractFunctionAction.kind, requestedRefactor)) {
if (functionExtraction.errors.length === 0) {
// Don't issue refactorings with duplicated names.
// Scopes come back in "innermost first" order, so extractions will
@ -94,8 +94,7 @@ namespace ts.refactor.extractSymbol {
}
}
// Skip these since we don't have a way to report errors yet
if(refactorKindBeginsWith(extractConstantAction.kind, requestedRefactor)) {
if (refactorKindBeginsWith(extractConstantAction.kind, requestedRefactor)) {
if (constantExtraction.errors.length === 0) {
// Don't issue refactorings with duplicated names.
// Scopes come back in "innermost first" order, so extractions will
@ -265,24 +264,30 @@ namespace ts.refactor.extractSymbol {
/**
* 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)
* process may fail, in which case a set of errors is returned instead. These errors are shown to
* users if they have the provideRefactorNotApplicableReason option set.
*/
// exported only for tests
export function getRangeToExtract(sourceFile: SourceFile, span: TextSpan, considerEmptySpans = true): RangeToExtract {
export function getRangeToExtract(sourceFile: SourceFile, span: TextSpan, invoked = true): RangeToExtract {
const { length } = span;
if (length === 0 && !considerEmptySpans) {
if (length === 0 && !invoked) {
return { errors: [createFileDiagnostic(sourceFile, span.start, length, Messages.cannotExtractEmpty)] };
}
const cursorRequest = length === 0 && considerEmptySpans;
const cursorRequest = length === 0 && invoked;
const startToken = getTokenAtPosition(sourceFile, span.start);
const endToken = findTokenOnLeftOfPosition(sourceFile, textSpanEnd(span));
/* If the refactoring command is invoked through a keyboard action it's safe to assume that the user is actively looking for
refactoring actions at the span location. As they may not know the exact range that will trigger a refactoring, we expand the
searched span to cover a real node range making it more likely that something useful will show up. */
const adjustedSpan = startToken && endToken && invoked ? getAdjustedSpanFromNodes(startToken, endToken, sourceFile) : span;
// 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)
const startToken = getTokenAtPosition(sourceFile, span.start);
const start = cursorRequest ? getExtractableParent(startToken): getParentNodeInSpan(startToken, sourceFile, span);
const start = cursorRequest ? getExtractableParent(startToken): getParentNodeInSpan(startToken, sourceFile, adjustedSpan);
// Do the same for the ending position
const endToken = findTokenOnLeftOfPosition(sourceFile, textSpanEnd(span));
const end = cursorRequest ? start : getParentNodeInSpan(endToken, sourceFile, span);
const end = cursorRequest ? start : getParentNodeInSpan(endToken, sourceFile, adjustedSpan);
const declarations: Symbol[] = [];
@ -295,6 +300,10 @@ namespace ts.refactor.extractSymbol {
return { errors: [createFileDiagnostic(sourceFile, span.start, length, Messages.cannotExtractRange)] };
}
if (isJSDoc(start)) {
return { errors: [createFileDiagnostic(sourceFile, span.start, length, Messages.cannotExtractJSDoc)] };
}
if (start.parent !== end.parent) {
// start and end nodes belong to different subtrees
return { errors: [createFileDiagnostic(sourceFile, span.start, length, Messages.cannotExtractRange)] };
@ -332,10 +341,6 @@ namespace ts.refactor.extractSymbol {
return { targetRange: { range: statements, facts: rangeFacts, declarations } };
}
if (isJSDoc(start)) {
return { errors: [createFileDiagnostic(sourceFile, span.start, length, Messages.cannotExtractJSDoc)] };
}
if (isReturnStatement(start) && !start.expression) {
// Makes no sense to extract an expression-less return statement.
return { errors: [createFileDiagnostic(sourceFile, span.start, length, Messages.cannotExtractRange)] };
@ -605,6 +610,19 @@ namespace ts.refactor.extractSymbol {
}
}
/**
* Includes the final semicolon so that the span covers statements in cases where it would otherwise
* only cover the declaration list.
*/
function getAdjustedSpanFromNodes(startNode: Node, endNode: Node, sourceFile: SourceFile): TextSpan {
const start = startNode.getStart(sourceFile);
let end = endNode.getEnd();
if (sourceFile.text.charCodeAt(end) === CharacterCodes.semicolon) {
end++;
}
return { start, length: end - start };
}
function getStatementOrExpressionRange(node: Node): Statement[] | Expression | undefined {
if (isStatement(node)) {
return [node];

123
src/testRunner/unittests/services/extract/ranges.ts
View File

@ -14,85 +14,100 @@ namespace ts {
});
}
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.extractSymbol.getRangeToExtract(f, createTextSpanFromRange(selectionRange));
const expectedRange = t.ranges.get("extracted");
if (expectedRange) {
let pos: number, end: number;
const targetRange = result.targetRange!;
if (isArray(targetRange.range)) {
pos = targetRange.range[0].getStart(f);
end = last(targetRange.range).getEnd();
function testExtractRange(caption: string, s: string) {
return it(caption, () => {
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.extractSymbol.getRangeToExtract(f, createTextSpanFromRange(selectionRange));
const expectedRange = t.ranges.get("extracted");
if (expectedRange) {
let pos: number, end: number;
const targetRange = result.targetRange!;
if (isArray(targetRange.range)) {
pos = targetRange.range[0].getStart(f);
end = last(targetRange.range).getEnd();
}
else {
pos = targetRange.range.getStart(f);
end = targetRange.range.getEnd();
}
assert.equal(pos, expectedRange.pos, "incorrect pos of range");
assert.equal(end, expectedRange.end, "incorrect end of range");
}
else {
pos = targetRange.range.getStart(f);
end = targetRange.range.getEnd();
assert.isTrue(!result.targetRange, `expected range to extract to be undefined`);
}
assert.equal(pos, expectedRange.pos, "incorrect pos of range");
assert.equal(end, expectedRange.end, "incorrect end of range");
}
else {
assert.isTrue(!result.targetRange, `expected range to extract to be undefined`);
}
});
}
describe("unittests:: services:: extract:: extractRanges", () => {
it("get extract range from selection", () => {
testExtractRange(`
describe("get extract range from selection", () => {
testExtractRange("extractRange1", `
[#|
[$|var x = 1;
var y = 2;|]|]
`);
testExtractRange(`
[#|
var x = 1;
var y = 2|];
testExtractRange("extractRange2", `
[$|[#|var x = 1;
var y = 2|];|]
`);
testExtractRange(`
[#|var x = 1|];
testExtractRange("extractRange3", `
[#|var x = [$|1|]|];
var y = 2;
`);
testExtractRange(`
testExtractRange("extractRange4", `
var x = [$|10[#|00|]|];
`);
testExtractRange("extractRange5", `
[$|va[#|r foo = 1;
var y = 200|]0;|]
`);
testExtractRange("extractRange6", `
var x = [$|fo[#|o.bar.baz()|]|];
`);
testExtractRange("extractRange7", `
if ([#|[#extracted|a && b && c && d|]|]) {
}
`);
testExtractRange(`
testExtractRange("extractRange8", `
if [#|(a && b && c && d|]) {
}
`);
testExtractRange(`
testExtractRange("extractRange9", `
if ([$|a[#|a && b && c && d|]d|]) {
}
`);
testExtractRange("extractRange10", `
if (a && b && c && d) {
[#| [$|var x = 1;
console.log(x);|] |]
}
`);
testExtractRange(`
testExtractRange("extractRange11", `
[#|
if (a) {
return 100;
} |]
`);
testExtractRange(`
testExtractRange("extractRange12", `
function foo() {
[#| [$|if (a) {
}
return 100|] |]
}
`);
testExtractRange(`
testExtractRange("extractRange13", `
[#|
[$|l1:
if (x) {
break l1;
}|]|]
`);
testExtractRange(`
testExtractRange("extractRange14", `
[#|
[$|l2:
{
@ -101,21 +116,21 @@ namespace ts {
break l2;
}|]|]
`);
testExtractRange(`
testExtractRange("extractRange15", `
while (true) {
[#| if(x) {
}
break; |]
}
`);
testExtractRange(`
testExtractRange("extractRange16", `
while (true) {
[#| if(x) {
}
continue; |]
}
`);
testExtractRange(`
testExtractRange("extractRange17", `
l3:
{
[#|
@ -124,7 +139,7 @@ namespace ts {
break l3; |]
}
`);
testExtractRange(`
testExtractRange("extractRange18", `
function f() {
while (true) {
[#|
@ -134,7 +149,7 @@ namespace ts {
}
}
`);
testExtractRange(`
testExtractRange("extractRange19", `
function f() {
while (true) {
[#|
@ -145,13 +160,13 @@ namespace ts {
}
}
`);
testExtractRange(`
testExtractRange("extractRange20", `
function f() {
return [#| [$|1 + 2|] |]+ 3;
}
}
`);
testExtractRange(`
testExtractRange("extractRange21", `
function f(x: number) {
[#|[$|try {
x++;
@ -163,17 +178,21 @@ namespace ts {
`);
// Variable statements
testExtractRange(`[#|let x = [$|1|];|]`);
testExtractRange(`[#|let x = [$|1|], y;|]`);
testExtractRange(`[#|[$|let x = 1, y = 1;|]|]`);
testExtractRange("extractRange22", `[#|let x = [$|1|];|]`);
testExtractRange("extractRange23", `[#|let x = [$|1|], y;|]`);
testExtractRange("extractRange24", `[#|[$|let x = 1, y = 1;|]|]`);
// Variable declarations
testExtractRange(`let [#|x = [$|1|]|];`);
testExtractRange(`let [#|x = [$|1|]|], y = 2;`);
testExtractRange(`let x = 1, [#|y = [$|2|]|];`);
testExtractRange("extractRange25", `let [#|x = [$|1|]|];`);
testExtractRange("extractRange26", `let [#|x = [$|1|]|], y = 2;`);
testExtractRange("extractRange27", `let x = 1, [#|y = [$|2|]|];`);
// Return statements
testExtractRange(`[#|return [$|1|];|]`);
testExtractRange("extractRange28", `[#|return [$|1|];|]`);
// For statements
testExtractRange("extractRange29", `for ([#|var i = 1|]; i < 2; i++) {}`);
testExtractRange("extractRange30", `for (var i = [#|[$|1|]|]; i < 2; i++) {}`);
});
testExtractRangeFailed("extractRangeFailed1",

0
tests/cases/fourslash/extractSymbolForTriggerReason.ts → tests/cases/fourslash/extractSymbolForTriggerReason1.ts
View File

8
tests/cases/fourslash/extractSymbolForTriggerReason2.ts Normal file
View File

@ -0,0 +1,8 @@
/// <reference path='fourslash.ts' />
////const foo = ba/*a*/r + b/*b*/az;
// Expand selection to fit nodes if refactors are explicitly requested
goTo.select("a", "b");
verify.not.refactorAvailableForTriggerReason("implicit", "Extract Symbol");
verify.refactorAvailableForTriggerReason("invoked", "Extract Symbol", "constant_scope_0");