From fe1242c8a902a2f530e5fb91b8f8f3e90bd4f4a6 Mon Sep 17 00:00:00 2001 From: Andy Date: Fri, 25 Aug 2017 09:53:56 -0700 Subject: [PATCH] Don't try to extract `import` to a method (#18025) --- src/compiler/utilities.ts | 117 ++++++++++-------- src/services/refactors/extractMethod.ts | 27 ++-- .../extract-method-not-for-import.ts | 10 ++ 3 files changed, 87 insertions(+), 67 deletions(-) create mode 100644 tests/cases/fourslash/extract-method-not-for-import.ts diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 7462bf02adccd..9f679011dd02b 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -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 */ @@ -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 { diff --git a/src/services/refactors/extractMethod.ts b/src/services/refactors/extractMethod.ts index f32321aff2a17..820212011a964 100644 --- a/src/services/refactors/extractMethod.ts +++ b/src/services/refactors/extractMethod.ts @@ -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 { @@ -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); diff --git a/tests/cases/fourslash/extract-method-not-for-import.ts b/tests/cases/fourslash/extract-method-not-for-import.ts new file mode 100644 index 0000000000000..a72d793611d62 --- /dev/null +++ b/tests/cases/fourslash/extract-method-not-for-import.ts @@ -0,0 +1,10 @@ +/// + +// @Filename: /a.ts +////i/**/mport _ from "./b"; + +// @Filename: /b.ts +////export default function f() {} + +goTo.marker(""); +verify.not.refactorAvailable('Extract Method');