-
Notifications
You must be signed in to change notification settings - Fork 109
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Quote all property accesses on types with index types.
TypeScript 2.3 allows users to use `dotted.access` for types that (only) have a
string index type declared, e.g. `{[k: string]: ...}`. This breaks renaming in
tools such as Closure Compiler (which tsickle targets), as some locations might
access the property with quotes, some without.
This is an incomplete fix: TypeScript allows assigning values with properties of
the matching type into types with index signatures. Users can then access the
original value with dots, but the aliased value with quotes, which breaks, too.
This is probably not fixable without a global aliasing analysis of the program.
See also:
microsoft/TypeScript#14267
microsoft/TypeScript#15206- Loading branch information
Showing
4 changed files
with
128 additions
and
25 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| goog.module('test_files.quote_props.quote');var module = module || {id: 'test_files/quote_props/quote.js'};/** | ||
| * @record | ||
| */ | ||
| function Quoted() { } | ||
| let /** @type {!Quoted} */ quoted = {}; | ||
| console.log(quoted['hello']); | ||
| quoted['hello'] = 1; | ||
| quoted['hello'] = 1; | ||
| /** | ||
| * @record | ||
| * @extends {Quoted} | ||
| */ | ||
| function QuotedMixed() { } | ||
| /** @type {number} */ | ||
| QuotedMixed.prototype.foo; | ||
| // TODO(martinprobst): should 'foo: 1' below be quoted? | ||
| let /** @type {!QuotedMixed} */ quotedMixed = { foo: 1 }; | ||
| console.log(quotedMixed['foo']); | ||
| // TODO(martinprobst): should this access to a declared property be quoted? | ||
| quotedMixed['foo'] = 1; | ||
| // TODO(martinprobst): should this access to a declared property be un-quoted? | ||
| quotedMixed['foo'] = 1; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| interface Quoted { | ||
| [k: string]: number; | ||
| } | ||
| let quoted: Quoted = {}; | ||
|
|
||
| console.log(quoted.hello); | ||
| quoted.hello = 1; | ||
| quoted['hello'] = 1; | ||
|
|
||
| interface QuotedMixed extends Quoted { | ||
| // Assume that foo should be renamed, as it is explicitly declared. | ||
| // It's unclear whether it's the right thing to do, user code might | ||
| // access this field in a mixed fashion. | ||
| foo: number; | ||
| } | ||
| // TODO(martinprobst): should 'foo: 1' below be quoted? | ||
| let quotedMixed: QuotedMixed = {foo: 1}; | ||
| console.log(quotedMixed.foo); | ||
|
|
||
| // TODO(martinprobst): should this access to a declared property be quoted? | ||
| quotedMixed.foo = 1; | ||
| // TODO(martinprobst): should this access to a declared property be un-quoted? | ||
| quotedMixed['foo'] = 1; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| Warning at test_files/quote_props/quote.ts:6:13: Quoted has a string index type but is accessed using dotted access. Quoting the access. | ||
| Warning at test_files/quote_props/quote.ts:7:1: Quoted has a string index type but is accessed using dotted access. Quoting the access. | ||
| Warning at test_files/quote_props/quote.ts:18:13: QuotedMixed has a string index type but is accessed using dotted access. Quoting the access. | ||
| Warning at test_files/quote_props/quote.ts:21:1: QuotedMixed has a string index type but is accessed using dotted access. Quoting the access. | ||
| ==== | ||
|
|
||
| /** | ||
| * @record | ||
| */ | ||
| function Quoted() {} | ||
| /* TODO: handle strange member: | ||
| [k: string]: number; | ||
| */ | ||
| interface Quoted { | ||
| [k: string]: number; | ||
| } | ||
| let /** @type {!Quoted} */ quoted: Quoted = {}; | ||
|
|
||
| console.log(quoted['hello']); | ||
| quoted['hello'] = 1; | ||
| quoted['hello'] = 1; | ||
| /** | ||
| * @record | ||
| * @extends {Quoted} | ||
| */ | ||
| function QuotedMixed() {} | ||
| /** @type {number} */ | ||
| QuotedMixed.prototype.foo; | ||
|
|
||
|
|
||
| interface QuotedMixed extends Quoted { | ||
| // Assume that foo should be renamed, as it is explicitly declared. | ||
| // It's unclear whether it's the right thing to do, user code might | ||
| // access this field in a mixed fashion. | ||
| foo: number; | ||
| } | ||
| // TODO(martinprobst): should 'foo: 1' below be quoted? | ||
| let /** @type {!QuotedMixed} */ quotedMixed: QuotedMixed = {foo: 1}; | ||
| console.log(quotedMixed['foo']); | ||
|
|
||
| // TODO(martinprobst): should this access to a declared property be quoted? | ||
| quotedMixed['foo'] = 1; | ||
| // TODO(martinprobst): should this access to a declared property be un-quoted? | ||
| quotedMixed['foo'] = 1; |