Skip to content

Commit

Permalink
Don't try to extract import to a method (#18025)
Browse files Browse the repository at this point in the history
  • Loading branch information
Andy authored Aug 25, 2017
1 parent 3a0ab74 commit fe1242c
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 67 deletions.
117 changes: 62 additions & 55 deletions src/compiler/utilities.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4951,54 +4951,60 @@ namespace ts {
|| kind === SyntaxKind.NoSubstitutionTemplateLiteral;
}

function isLeftHandSideExpressionKind(kind: SyntaxKind): boolean {
return kind === SyntaxKind.PropertyAccessExpression
|| kind === SyntaxKind.ElementAccessExpression
|| kind === SyntaxKind.NewExpression
|| kind === SyntaxKind.CallExpression
|| kind === SyntaxKind.JsxElement
|| kind === SyntaxKind.JsxSelfClosingElement
|| kind === SyntaxKind.TaggedTemplateExpression
|| kind === SyntaxKind.ArrayLiteralExpression
|| kind === SyntaxKind.ParenthesizedExpression
|| kind === SyntaxKind.ObjectLiteralExpression
|| kind === SyntaxKind.ClassExpression
|| kind === SyntaxKind.FunctionExpression
|| kind === SyntaxKind.Identifier
|| kind === SyntaxKind.RegularExpressionLiteral
|| kind === SyntaxKind.NumericLiteral
|| kind === SyntaxKind.StringLiteral
|| kind === SyntaxKind.NoSubstitutionTemplateLiteral
|| kind === SyntaxKind.TemplateExpression
|| kind === SyntaxKind.FalseKeyword
|| kind === SyntaxKind.NullKeyword
|| kind === SyntaxKind.ThisKeyword
|| kind === SyntaxKind.TrueKeyword
|| kind === SyntaxKind.SuperKeyword
|| kind === SyntaxKind.ImportKeyword
|| kind === SyntaxKind.NonNullExpression
|| kind === SyntaxKind.MetaProperty;
}

/* @internal */
export function isLeftHandSideExpression(node: Node): node is LeftHandSideExpression {
return isLeftHandSideExpressionKind(skipPartiallyEmittedExpressions(node).kind);
}

function isUnaryExpressionKind(kind: SyntaxKind): boolean {
return kind === SyntaxKind.PrefixUnaryExpression
|| kind === SyntaxKind.PostfixUnaryExpression
|| kind === SyntaxKind.DeleteExpression
|| kind === SyntaxKind.TypeOfExpression
|| kind === SyntaxKind.VoidExpression
|| kind === SyntaxKind.AwaitExpression
|| kind === SyntaxKind.TypeAssertionExpression
|| isLeftHandSideExpressionKind(kind);
switch (node.kind) {
case SyntaxKind.PropertyAccessExpression:
case SyntaxKind.ElementAccessExpression:
case SyntaxKind.NewExpression:
case SyntaxKind.CallExpression:
case SyntaxKind.JsxElement:
case SyntaxKind.JsxSelfClosingElement:
case SyntaxKind.TaggedTemplateExpression:
case SyntaxKind.ArrayLiteralExpression:
case SyntaxKind.ParenthesizedExpression:
case SyntaxKind.ObjectLiteralExpression:
case SyntaxKind.ClassExpression:
case SyntaxKind.FunctionExpression:
case SyntaxKind.Identifier:
case SyntaxKind.RegularExpressionLiteral:
case SyntaxKind.NumericLiteral:
case SyntaxKind.StringLiteral:
case SyntaxKind.NoSubstitutionTemplateLiteral:
case SyntaxKind.TemplateExpression:
case SyntaxKind.FalseKeyword:
case SyntaxKind.NullKeyword:
case SyntaxKind.ThisKeyword:
case SyntaxKind.TrueKeyword:
case SyntaxKind.SuperKeyword:
case SyntaxKind.NonNullExpression:
case SyntaxKind.MetaProperty:
return true;
case SyntaxKind.PartiallyEmittedExpression:
return isLeftHandSideExpression((node as PartiallyEmittedExpression).expression);
case SyntaxKind.ImportKeyword:
return node.parent.kind === SyntaxKind.CallExpression;
default:
return false;
}
}

/* @internal */
export function isUnaryExpression(node: Node): node is UnaryExpression {
return isUnaryExpressionKind(skipPartiallyEmittedExpressions(node).kind);
switch (node.kind) {
case SyntaxKind.PrefixUnaryExpression:
case SyntaxKind.PostfixUnaryExpression:
case SyntaxKind.DeleteExpression:
case SyntaxKind.TypeOfExpression:
case SyntaxKind.VoidExpression:
case SyntaxKind.AwaitExpression:
case SyntaxKind.TypeAssertionExpression:
return true;
case SyntaxKind.PartiallyEmittedExpression:
return isUnaryExpression((node as PartiallyEmittedExpression).expression);
default:
return isLeftHandSideExpression(node);
}
}

/* @internal */
Expand All @@ -5014,21 +5020,22 @@ namespace ts {
}
}

function isExpressionKind(kind: SyntaxKind) {
return kind === SyntaxKind.ConditionalExpression
|| kind === SyntaxKind.YieldExpression
|| kind === SyntaxKind.ArrowFunction
|| kind === SyntaxKind.BinaryExpression
|| kind === SyntaxKind.SpreadElement
|| kind === SyntaxKind.AsExpression
|| kind === SyntaxKind.OmittedExpression
|| kind === SyntaxKind.CommaListExpression
|| isUnaryExpressionKind(kind);
}

/* @internal */
export function isExpression(node: Node): node is Expression {
return isExpressionKind(skipPartiallyEmittedExpressions(node).kind);
switch (node.kind) {
case SyntaxKind.ConditionalExpression:
case SyntaxKind.YieldExpression:
case SyntaxKind.ArrowFunction:
case SyntaxKind.BinaryExpression:
case SyntaxKind.SpreadElement:
case SyntaxKind.AsExpression:
case SyntaxKind.OmittedExpression:
case SyntaxKind.CommaListExpression:
case SyntaxKind.PartiallyEmittedExpression:
return true;
default:
return isUnaryExpression(node);
}
}

export function isAssertionExpression(node: Node): node is AssertionExpression {
Expand Down
27 changes: 15 additions & 12 deletions src/services/refactors/extractMethod.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,18 +231,7 @@ namespace ts.refactor.extractMethod {
if (errors) {
return { errors };
}

// If our selection is the expression in an ExpressionStatement, expand
// the selection to include the enclosing Statement (this stops us
// from trying to care about the return value of the extracted function
// and eliminates double semicolon insertion in certain scenarios)
const range = isStatement(start)
? [start]
: start.parent && start.parent.kind === SyntaxKind.ExpressionStatement
? [start.parent as Statement]
: start as Expression;

return { targetRange: { range, facts: rangeFacts, declarations } };
return { targetRange: { range: getStatementOrExpressionRange(start), facts: rangeFacts, declarations } };
}

function createErrorResult(sourceFile: SourceFile, start: number, length: number, message: DiagnosticMessage): RangeToExtract {
Expand Down Expand Up @@ -459,6 +448,20 @@ namespace ts.refactor.extractMethod {
}
}

function getStatementOrExpressionRange(node: Node): Statement[] | Expression {
if (isStatement(node)) {
return [node];
}
else if (isExpression(node)) {
// If our selection is the expression in an ExpressionStatement, expand
// the selection to include the enclosing Statement (this stops us
// from trying to care about the return value of the extracted function
// and eliminates double semicolon insertion in certain scenarios)
return isExpressionStatement(node.parent) ? [node.parent] : node;
}
return undefined;
}

function isValidExtractionTarget(node: Node): node is Scope {
// Note that we don't use isFunctionLike because we don't want to put the extracted closure *inside* a method
return (node.kind === SyntaxKind.FunctionDeclaration) || isSourceFile(node) || isModuleBlock(node) || isClassLike(node);
Expand Down
10 changes: 10 additions & 0 deletions tests/cases/fourslash/extract-method-not-for-import.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/// <reference path='fourslash.ts' />

// @Filename: /a.ts
////i/**/mport _ from "./b";

// @Filename: /b.ts
////export default function f() {}

goTo.marker("");
verify.not.refactorAvailable('Extract Method');

0 comments on commit fe1242c

Please sign in to comment.