From 7b0bd090e28c5e19143c2823dfcc10c5ea4e392e Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Tue, 23 May 2017 08:52:55 -0700 Subject: [PATCH] findAllReferences: Make "isWriteAccess" handle special declaration kinds --- src/compiler/checker.ts | 4 ++-- src/compiler/utilities.ts | 17 ++++++++++++++ src/services/findAllReferences.ts | 22 +++++++++---------- .../fourslash/findAllRefsForModuleGlobal.ts | 2 +- ...rencesIsDefinitionOfNumberNamedProperty.ts | 2 +- ...rencesIsDefinitionOfStringNamedProperty.ts | 2 +- .../cases/fourslash/referencesBloomFilters.ts | 2 +- .../fourslash/referencesBloomFilters2.ts | 4 ++-- .../fourslash/referencesBloomFilters3.ts | 2 +- .../cases/fourslash/referencesForAmbients.ts | 4 ++-- tests/cases/fourslash/referencesForEnums.ts | 4 ++-- .../referencesForExternalModuleNames.ts | 2 +- ...eferencesForNumericLiteralPropertyNames.ts | 6 ++--- ...referencesForStringLiteralPropertyNames.ts | 4 ++-- ...eferencesForStringLiteralPropertyNames2.ts | 2 +- ...eferencesForStringLiteralPropertyNames3.ts | 4 ++-- ...eferencesForStringLiteralPropertyNames4.ts | 2 +- tests/cases/fourslash/renameJsExports01.ts | 2 +- 18 files changed, 51 insertions(+), 36 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 8551652c176..46f56d99b39 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -12750,7 +12750,7 @@ namespace ts { function getContextualTypeForBinaryOperand(node: Expression): Type { const binaryExpression = node.parent; const operator = binaryExpression.operatorToken.kind; - if (operator >= SyntaxKind.FirstAssignment && operator <= SyntaxKind.LastAssignment) { + if (isAssignmentOperator(operator)) { // Don't do this for special property assignments to avoid circularity if (getSpecialPropertyAssignmentKind(binaryExpression) !== SpecialPropertyAssignmentKind.None) { return undefined; @@ -17305,7 +17305,7 @@ namespace ts { } function checkAssignmentOperator(valueType: Type): void { - if (produceDiagnostics && operator >= SyntaxKind.FirstAssignment && operator <= SyntaxKind.LastAssignment) { + if (produceDiagnostics && isAssignmentOperator(operator)) { // TypeScript 1.0 spec (April 2014): 4.17 // An assignment of the form // VarExpr = ValueExpr diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index a5750248707..3a8b8b59f7d 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -1785,6 +1785,23 @@ namespace ts { } } + /* @internal */ + // See GH#16030 + export function isAnyDeclarationName(name: Node): boolean { + switch (name.kind) { + case SyntaxKind.Identifier: + case SyntaxKind.StringLiteral: + case SyntaxKind.NumericLiteral: + if (isDeclaration(name.parent)) { + return name.parent.name === name; + } + const binExp = name.parent.parent; + return isBinaryExpression(binExp) && getSpecialPropertyAssignmentKind(binExp) !== SpecialPropertyAssignmentKind.None && getNameOfDeclaration(binExp) === name; + default: + return false; + } + } + export function isLiteralComputedPropertyDeclarationName(node: Node) { return (node.kind === SyntaxKind.StringLiteral || node.kind === SyntaxKind.NumericLiteral) && node.parent.kind === SyntaxKind.ComputedPropertyName && diff --git a/src/services/findAllReferences.ts b/src/services/findAllReferences.ts index fd564cac13d..efc6a6f6ed1 100644 --- a/src/services/findAllReferences.ts +++ b/src/services/findAllReferences.ts @@ -176,7 +176,7 @@ namespace ts.FindAllReferences { fileName: node.getSourceFile().fileName, textSpan: getTextSpan(node), isWriteAccess: isWriteAccess(node), - isDefinition: isDeclarationName(node) || isLiteralComputedPropertyDeclarationName(node), + isDefinition: isAnyDeclarationName(node) || isLiteralComputedPropertyDeclarationName(node), isInString }; } @@ -242,22 +242,20 @@ namespace ts.FindAllReferences { /** A node is considered a writeAccess iff it is a name of a declaration or a target of an assignment */ function isWriteAccess(node: Node): boolean { - if (node.kind === SyntaxKind.Identifier && isDeclarationName(node)) { + if (isAnyDeclarationName(node)) { return true; } - const parent = node.parent; - if (parent) { - if (parent.kind === SyntaxKind.PostfixUnaryExpression || parent.kind === SyntaxKind.PrefixUnaryExpression) { + const { parent } = node; + switch (parent && parent.kind) { + case SyntaxKind.PostfixUnaryExpression: + case SyntaxKind.PrefixUnaryExpression: return true; - } - else if (parent.kind === SyntaxKind.BinaryExpression && (parent).left === node) { - const operator = (parent).operatorToken.kind; - return SyntaxKind.FirstAssignment <= operator && operator <= SyntaxKind.LastAssignment; - } + case SyntaxKind.BinaryExpression: + return (parent).left === node && isAssignmentOperator((parent).operatorToken.kind); + default: + return false; } - - return false; } } diff --git a/tests/cases/fourslash/findAllRefsForModuleGlobal.ts b/tests/cases/fourslash/findAllRefsForModuleGlobal.ts index 25c23faeae5..4c1b39cdb0f 100644 --- a/tests/cases/fourslash/findAllRefsForModuleGlobal.ts +++ b/tests/cases/fourslash/findAllRefsForModuleGlobal.ts @@ -6,7 +6,7 @@ // @Filename: /b.ts /////// ////import { x } from "[|foo|]"; -////declare module "[|{| "isDefinition": true |}foo|]" {} +////declare module "[|{| "isWriteAccess": true, "isDefinition": true |}foo|]" {} verify.noErrors(); diff --git a/tests/cases/fourslash/getOccurrencesIsDefinitionOfNumberNamedProperty.ts b/tests/cases/fourslash/getOccurrencesIsDefinitionOfNumberNamedProperty.ts index c603f398e5e..b391deec9cc 100644 --- a/tests/cases/fourslash/getOccurrencesIsDefinitionOfNumberNamedProperty.ts +++ b/tests/cases/fourslash/getOccurrencesIsDefinitionOfNumberNamedProperty.ts @@ -1,5 +1,5 @@ /// -////let o = { [|{| "isDefinition": true |}1|]: 12 }; +////let o = { [|{| "isWriteAccess": true, "isDefinition": true |}1|]: 12 }; ////let y = o[[|1|]]; const ranges = test.ranges(); diff --git a/tests/cases/fourslash/getOccurrencesIsDefinitionOfStringNamedProperty.ts b/tests/cases/fourslash/getOccurrencesIsDefinitionOfStringNamedProperty.ts index 43fe0c571fc..4b842da6f59 100644 --- a/tests/cases/fourslash/getOccurrencesIsDefinitionOfStringNamedProperty.ts +++ b/tests/cases/fourslash/getOccurrencesIsDefinitionOfStringNamedProperty.ts @@ -1,5 +1,5 @@ /// -////let o = { "[|{| "isDefinition": true |}x|]": 12 }; +////let o = { "[|{| "isWriteAccess": true, "isDefinition": true |}x|]": 12 }; ////let y = o.[|x|]; const ranges = test.ranges(); diff --git a/tests/cases/fourslash/referencesBloomFilters.ts b/tests/cases/fourslash/referencesBloomFilters.ts index c9f290db0db..2745f6144ca 100644 --- a/tests/cases/fourslash/referencesBloomFilters.ts +++ b/tests/cases/fourslash/referencesBloomFilters.ts @@ -12,7 +12,7 @@ ////function blah2() { container["[|searchProp|]"] }; // @Filename: redeclaration.ts -////container = { "[|{| "isDefinition": true |}searchProp|]" : 18 }; +////container = { "[|{| "isWriteAccess": true, "isDefinition": true |}searchProp|]" : 18 }; const ranges = test.ranges(); const [r0, r1, r2, r3] = ranges; diff --git a/tests/cases/fourslash/referencesBloomFilters2.ts b/tests/cases/fourslash/referencesBloomFilters2.ts index daf60d0dd01..26690e437ee 100644 --- a/tests/cases/fourslash/referencesBloomFilters2.ts +++ b/tests/cases/fourslash/referencesBloomFilters2.ts @@ -3,7 +3,7 @@ // Ensure BloomFilter building logic is correct, by having one reference per file // @Filename: declaration.ts -////var container = { [|{| "isDefinition": true |}42|]: 1 }; +////var container = { [|{| "isWriteAccess": true, "isDefinition": true |}42|]: 1 }; // @Filename: expression.ts ////function blah() { return (container[[|42|]]) === 2; }; @@ -12,7 +12,7 @@ ////function blah2() { container["[|42|]"] }; // @Filename: redeclaration.ts -////container = { "[|{| "isDefinition": true |}42|]" : 18 }; +////container = { "[|{| "isWriteAccess": true, "isDefinition": true |}42|]" : 18 }; const ranges = test.ranges(); const [r0, r1, r2, r3] = ranges; diff --git a/tests/cases/fourslash/referencesBloomFilters3.ts b/tests/cases/fourslash/referencesBloomFilters3.ts index 0dd5c645264..d7d335cde02 100644 --- a/tests/cases/fourslash/referencesBloomFilters3.ts +++ b/tests/cases/fourslash/referencesBloomFilters3.ts @@ -4,7 +4,7 @@ // @Filename: declaration.ts -////enum Test { "[|{| "isDefinition": true |}42|]" = 1 }; +////enum Test { "[|{| "isWriteAccess": true, "isDefinition": true |}42|]" = 1 }; // @Filename: expression.ts ////(Test[[|42|]]); diff --git a/tests/cases/fourslash/referencesForAmbients.ts b/tests/cases/fourslash/referencesForAmbients.ts index f5789716155..2397738581e 100644 --- a/tests/cases/fourslash/referencesForAmbients.ts +++ b/tests/cases/fourslash/referencesForAmbients.ts @@ -1,10 +1,10 @@ /// -////declare module "[|{| "isDefinition": true |}foo|]" { +////declare module "[|{| "isWriteAccess": true, "isDefinition": true |}foo|]" { //// var [|{| "isWriteAccess": true, "isDefinition": true |}f|]: number; ////} //// -////declare module "[|{| "isDefinition": true |}bar|]" { +////declare module "[|{| "isWriteAccess": true, "isDefinition": true |}bar|]" { //// export import [|{| "isWriteAccess": true, "isDefinition": true |}foo|] = require("[|foo|]"); //// var f2: typeof [|foo|].[|f|]; ////} diff --git a/tests/cases/fourslash/referencesForEnums.ts b/tests/cases/fourslash/referencesForEnums.ts index 68face36d04..af4fec8aae8 100644 --- a/tests/cases/fourslash/referencesForEnums.ts +++ b/tests/cases/fourslash/referencesForEnums.ts @@ -2,8 +2,8 @@ ////enum E { //// [|{| "isWriteAccess": true, "isDefinition": true |}value1|] = 1, -//// "[|{| "isDefinition": true |}value2|]" = [|value1|], -//// [|{| "isDefinition": true |}111|] = 11 +//// "[|{| "isWriteAccess": true, "isDefinition": true |}value2|]" = [|value1|], +//// [|{| "isWriteAccess": true, "isDefinition": true |}111|] = 11 ////} //// ////E.[|value1|]; diff --git a/tests/cases/fourslash/referencesForExternalModuleNames.ts b/tests/cases/fourslash/referencesForExternalModuleNames.ts index 75ea9e908cf..127aa9fd657 100644 --- a/tests/cases/fourslash/referencesForExternalModuleNames.ts +++ b/tests/cases/fourslash/referencesForExternalModuleNames.ts @@ -3,7 +3,7 @@ // Global interface reference. // @Filename: referencesForGlobals_1.ts -////declare module "[|{| "isDefinition": true |}foo|]" { +////declare module "[|{| "isWriteAccess": true, "isDefinition": true |}foo|]" { //// var f: number; ////} diff --git a/tests/cases/fourslash/referencesForNumericLiteralPropertyNames.ts b/tests/cases/fourslash/referencesForNumericLiteralPropertyNames.ts index effa4abc164..a869ba8bcc1 100644 --- a/tests/cases/fourslash/referencesForNumericLiteralPropertyNames.ts +++ b/tests/cases/fourslash/referencesForNumericLiteralPropertyNames.ts @@ -1,13 +1,13 @@ /// ////class Foo { -//// public [|{| "isDefinition": true |}12|]: any; +//// public [|{| "isWriteAccess": true, "isDefinition": true |}12|]: any; ////} //// ////var x: Foo; ////x[[|12|]]; -////x = { "[|{| "isDefinition": true |}12|]": 0 }; -////x = { [|{| "isDefinition": true |}12|]: 0 }; +////x = { "[|{| "isWriteAccess": true, "isDefinition": true |}12|]": 0 }; +////x = { [|{| "isWriteAccess": true, "isDefinition": true |}12|]: 0 }; //verify.singleReferenceGroup("(property) Foo[12]: any"); const ranges = test.ranges(); diff --git a/tests/cases/fourslash/referencesForStringLiteralPropertyNames.ts b/tests/cases/fourslash/referencesForStringLiteralPropertyNames.ts index e9be352f819..c5ae2f0c586 100644 --- a/tests/cases/fourslash/referencesForStringLiteralPropertyNames.ts +++ b/tests/cases/fourslash/referencesForStringLiteralPropertyNames.ts @@ -1,13 +1,13 @@ /// ////class Foo { -//// public "[|{| "isDefinition": true |}ss|]": any; +//// public "[|{| "isWriteAccess": true, "isDefinition": true |}ss|]": any; ////} //// ////var x: Foo; ////x.[|ss|]; ////x["[|ss|]"]; -////x = { "[|{| "isDefinition": true |}ss|]": 0 }; +////x = { "[|{| "isWriteAccess": true, "isDefinition": true |}ss|]": 0 }; ////x = { [|{| "isWriteAccess": true, "isDefinition": true |}ss|]: 0 }; const ranges = test.ranges(); diff --git a/tests/cases/fourslash/referencesForStringLiteralPropertyNames2.ts b/tests/cases/fourslash/referencesForStringLiteralPropertyNames2.ts index 6fbb0b85072..89b6c7c755e 100644 --- a/tests/cases/fourslash/referencesForStringLiteralPropertyNames2.ts +++ b/tests/cases/fourslash/referencesForStringLiteralPropertyNames2.ts @@ -1,7 +1,7 @@ /// ////class Foo { -//// "[|{| "isDefinition": true |}blah|]"() { return 0; } +//// "[|{| "isWriteAccess": true, "isDefinition": true |}blah|]"() { return 0; } ////} //// ////var x: Foo; diff --git a/tests/cases/fourslash/referencesForStringLiteralPropertyNames3.ts b/tests/cases/fourslash/referencesForStringLiteralPropertyNames3.ts index d4e12a67afc..aae15afd0c7 100644 --- a/tests/cases/fourslash/referencesForStringLiteralPropertyNames3.ts +++ b/tests/cases/fourslash/referencesForStringLiteralPropertyNames3.ts @@ -1,8 +1,8 @@ /// ////class Foo2 { -//// get "[|{| "isDefinition": true |}42|]"() { return 0; } -//// set [|{| "isDefinition": true |}42|](n) { } +//// get "[|{| "isWriteAccess": true, "isDefinition": true |}42|]"() { return 0; } +//// set [|{| "isWriteAccess": true, "isDefinition": true |}42|](n) { } ////} //// ////var y: Foo2; diff --git a/tests/cases/fourslash/referencesForStringLiteralPropertyNames4.ts b/tests/cases/fourslash/referencesForStringLiteralPropertyNames4.ts index efe8972f2c7..4293435de1d 100644 --- a/tests/cases/fourslash/referencesForStringLiteralPropertyNames4.ts +++ b/tests/cases/fourslash/referencesForStringLiteralPropertyNames4.ts @@ -1,6 +1,6 @@ /// -////var x = { "[|{| "isDefinition": true |}someProperty|]": 0 } +////var x = { "[|{| "isWriteAccess": true, "isDefinition": true |}someProperty|]": 0 } ////x["[|someProperty|]"] = 3; ////x.[|someProperty|] = 5; diff --git a/tests/cases/fourslash/renameJsExports01.ts b/tests/cases/fourslash/renameJsExports01.ts index b731ece63f3..e1b71d232a6 100644 --- a/tests/cases/fourslash/renameJsExports01.ts +++ b/tests/cases/fourslash/renameJsExports01.ts @@ -2,7 +2,7 @@ // @allowJs: true // @Filename: a.js -////exports.[|area|] = function (r) { return r * r; } +////exports.[|{| "isWriteAccess": true, "isDefinition": true |}area|] = function (r) { return r * r; } // @Filename: b.js ////var mod = require('./a');