Style fixups

This commit is contained in:
Ryan Cavanaugh
2017-08-10 16:23:17 -07:00
parent 1ad411e13e
commit f957429efd
5 changed files with 51 additions and 58 deletions

View File

@@ -394,7 +394,7 @@ namespace ts {
function shouldVisitNode(node: Node): boolean {
return (node.transformFlags & TransformFlags.ContainsES2015) !== 0
|| convertedLoopState !== undefined
|| (hierarchyFacts & HierarchyFacts.ConstructorWithCapturedSuper && isStatementOrBlock(node))
|| (hierarchyFacts & HierarchyFacts.ConstructorWithCapturedSuper && (isStatement(node) || (node.kind === SyntaxKind.Block)))
|| (isIterationStatement(node, /*lookInLabeledStatements*/ false) && shouldConvertIterationStatementBody(node))
|| isTypeScriptClassWrapper(node);
}

View File

@@ -5353,14 +5353,6 @@ namespace ts {
|| isBlockStatement(node);
}
/* @internal */
export function isStatementOrBlock(node: Node): node is Statement {
const kind = node.kind;
return isStatementKindButNotDeclarationKind(kind)
|| isDeclarationStatementKind(kind)
|| node.kind === SyntaxKind.Block;
}
function isBlockStatement(node: Node): node is Block {
if (node.kind !== SyntaxKind.Block) return false;
if (node.parent !== undefined) {

View File

@@ -2766,7 +2766,7 @@ namespace FourSlash {
const isAvailable = refactors.length > 0;
if (negative && isAvailable) {
this.raiseError(`verifyApplicableRefactorAvailableForRange failed - expected no refactor but found some.`);
this.raiseError(`verifyApplicableRefactorAvailableForRange failed - expected no refactor but found some: ${refactors.map(r => r.name).join(", ")}`);
}
else if (!negative && !isAvailable) {
this.raiseError(`verifyApplicableRefactorAvailableForRange failed - expected a refactor but found none.`);

View File

@@ -575,12 +575,12 @@ namespace A {
};
const result = refactor.extractMethod.getRangeToExtract(sourceFile, createTextSpanFromBounds(selectionRange.start, selectionRange.end));
assert.equal(result.errors, undefined, "expect no errors");
const results = refactor.extractMethod.extractRange(result.targetRange, context);
const results = refactor.extractMethod.getPossibleExtractions(result.targetRange, context);
const data: string[] = [];
data.push(`==ORIGINAL==`);
data.push(sourceFile.text);
for (const r of results) {
const changes = refactor.extractMethod.extractRange(result.targetRange, context, results.indexOf(r))[0].changes;
const changes = refactor.extractMethod.getPossibleExtractions(result.targetRange, context, results.indexOf(r))[0].changes;
data.push(`==SCOPE::${r.scopeDescription}==`);
data.push(textChanges.applyChanges(sourceFile.text, changes[0].textChanges));
}

View File

@@ -21,7 +21,7 @@ namespace ts.refactor.extractMethod {
return undefined;
}
const extractions = extractRange(targetRange, context);
const extractions = getPossibleExtractions(targetRange, context);
if (extractions === undefined) {
// No extractions possible
return undefined;
@@ -37,15 +37,19 @@ namespace ts.refactor.extractMethod {
continue;
}
// Don't issue refactorings with duplicated names
// 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_function_into_0.message, [extr.scopeDescription]);
if (!usedNames.get(description)) {
if (!usedNames.has(description)) {
usedNames.set(description, true);
actions.push({
description,
name: `scope_${i}`
});
}
// *do* increment i anyway because we'll look for the i-th scope
// later when actually doing the refactoring if the user requests it
i++;
}
@@ -66,21 +70,14 @@ namespace ts.refactor.extractMethod {
const rangeToExtract = getRangeToExtract(context.file, { start: context.startPosition, length });
const targetRange: TargetRange = rangeToExtract.targetRange;
const parsedIndexMatch = /scope_(\d+)/.exec(actionName);
if (!parsedIndexMatch) {
throw new Error("Expected to match the regexp");
}
const parsedIndexMatch = /^scope_(\d+)$/.exec(actionName);
Debug.assert(!!parsedIndexMatch, "Scope name should have matched the regexp");
const index = +parsedIndexMatch[1];
if (!isFinite(index)) {
throw new Error("Expected to parse a number from the scope index");
}
Debug.assert(isFinite(index), "Expected to parse a finite number from the scope index");
const extractions = extractRange(targetRange, context, index);
if (extractions === undefined) {
// Scope is no longer valid from when the user issued the refactor
// return undefined;
return undefined;
}
const extractions = getPossibleExtractions(targetRange, context, index);
// Scope is no longer valid from when the user issued the refactor (??)
Debug.assert(extractions !== undefined, "The extraction went missing? How?");
return ({ edits: extractions[0].changes });
}
@@ -229,7 +226,7 @@ namespace ts.refactor.extractMethod {
return { targetRange: { range: statements, facts: rangeFacts, declarations } };
}
else {
// We have a single expression (start)
// We have a single node (start)
const errors = checkRootNode(start) || checkNode(start);
if (errors) {
return { errors };
@@ -243,7 +240,7 @@ namespace ts.refactor.extractMethod {
? [start]
: start.parent && start.parent.kind === SyntaxKind.ExpressionStatement
? [start.parent as Statement]
: <Expression>start;
: start as Expression;
return { targetRange: { range, facts: rangeFacts, declarations } };
}
@@ -259,6 +256,31 @@ namespace ts.refactor.extractMethod {
return undefined;
}
function checkForStaticContext(nodeToCheck: Node, containingClass: Node) {
let current: Node = nodeToCheck;
while (current !== containingClass) {
if (current.kind === SyntaxKind.PropertyDeclaration) {
if (hasModifier(current, ModifierFlags.Static)) {
rangeFacts |= RangeFacts.InStaticRegion;
}
break;
}
else if (current.kind === SyntaxKind.Parameter) {
const ctorOrMethod = getContainingFunction(current);
if (ctorOrMethod.kind === SyntaxKind.Constructor) {
rangeFacts |= RangeFacts.InStaticRegion;
}
break;
}
else if (current.kind === SyntaxKind.MethodDeclaration) {
if (hasModifier(current, ModifierFlags.Static)) {
rangeFacts |= RangeFacts.InStaticRegion;
}
}
current = current.parent;
}
}
// Verifies whether we can actually extract this node or not.
function checkNode(nodeToCheck: Node): Diagnostic[] | undefined {
const enum PermittedJumps {
@@ -275,31 +297,10 @@ namespace ts.refactor.extractMethod {
return [createDiagnosticForNode(nodeToCheck, Messages.CannotExtractAmbientBlock)];
}
// If we're in a class, see if we're in a static region (static property initializer, static method, class constructor parameter default) or not
const stoppingPoint: Node = getContainingClass(nodeToCheck);
if (stoppingPoint) {
let current: Node = nodeToCheck;
while (current !== stoppingPoint) {
if (current.kind === SyntaxKind.PropertyDeclaration) {
if (hasModifier(current, ModifierFlags.Static)) {
rangeFacts |= RangeFacts.InStaticRegion;
}
break;
}
else if (current.kind === SyntaxKind.Parameter) {
const ctorOrMethod = getContainingFunction(current);
if (ctorOrMethod.kind === SyntaxKind.Constructor) {
rangeFacts |= RangeFacts.InStaticRegion;
}
break;
}
else if (current.kind === SyntaxKind.MethodDeclaration) {
if (hasModifier(current, ModifierFlags.Static)) {
rangeFacts |= RangeFacts.InStaticRegion;
}
}
current = current.parent;
}
// If we're in a class, see whether we're in a static region (static property initializer, static method, class constructor parameter default)
const containingClass: Node = getContainingClass(nodeToCheck);
if (containingClass) {
checkForStaticContext(nodeToCheck, containingClass);
}
let errors: Diagnostic[];
@@ -319,7 +320,7 @@ namespace ts.refactor.extractMethod {
if (isDeclaration(node)) {
const declaringNode = (node.kind === SyntaxKind.VariableDeclaration) ? node.parent.parent : node;
if (hasModifier(declaringNode, ModifierFlags.Export)) {
(errors || (errors = []).push(createDiagnosticForNode(node, Messages.CannotExtractExportedEntity)));
(errors || (errors = [])).push(createDiagnosticForNode(node, Messages.CannotExtractExportedEntity));
return true;
}
declarations.push(node.symbol);
@@ -494,7 +495,7 @@ namespace ts.refactor.extractMethod {
// A function parameter's initializer is actually in the outer scope, not the function declaration
if (current && current.parent && current.parent.kind === SyntaxKind.Parameter) {
// Skip all the way to the outer scope of the function that declared this parameter
current = current.parent.parent.parent;
current = findAncestor(current, parent => isFunctionLike(parent)).parent;
}
else {
current = current.parent;
@@ -509,7 +510,7 @@ namespace ts.refactor.extractMethod {
* Each returned ExtractResultForScope corresponds to a possible target scope and is either a set of changes
* or an error explaining why we can't extract into that scope.
*/
export function extractRange(targetRange: TargetRange, context: RefactorContext, requestedChangesIndex: number = undefined): ReadonlyArray<ExtractResultForScope> | undefined {
export function getPossibleExtractions(targetRange: TargetRange, context: RefactorContext, requestedChangesIndex: number = undefined): ReadonlyArray<ExtractResultForScope> | undefined {
const { file: sourceFile } = context;
if (targetRange === undefined) {