Private-named instance fields by mheiber · Pull Request #32 · bloomberg/TypeScript

mheiber · 2019-03-27T20:16:31Z

@bloomberg teammates: The target for this PR is a branch that is currently up to date with Microsoft/TypeScript master.
Once this PR is approved, we can open a PR with the same changes from Bloomberg/private-named-instance-fields to Microsoft/private-named-instance-fields.

Private-Named Instance Fields

This PR implements part of the tc39 class fields proposal for TypeScript. It includes:

parse and check private-named fields, methods, and accessors
displayprivate names in the language server
transform private-named instance fields

Example:

ts

class Greeter {
    #name: string;
    constructor(name: string) {
        this.#name = name;
    }
    greet() {
        console.log(`hello ${this.#name}`);
    }
}

const greeter = new Greeter("world");
console.log(greeter);                 // logs 'Greeter {}'
console.log(Object.keys(greeter));    // logs '[]'
greeter.greet();                      // logs 'hello world'

js

var _classPrivateFieldSet = function (receiver, privateMap, value) { if (!privateMap.has(receiver)) { throw new TypeError("attempted to set private field on non-instance"); } privateMap.set(receiver, value); return value; };
var _classPrivateFieldGet = function (receiver, privateMap) { if (!privateMap.has(receiver)) { throw new TypeError("attempted to get private field on non-instance"); } return privateMap.get(receiver); };
var _nameWeakMap_1;
"use strict";
class Greeter {
    constructor(name) {
        _nameWeakMap_1.set(this, void 0);
        _classPrivateFieldSet(this, _nameWeakMap_1, name);
    }
    greet() {
        console.log(`hello ${_classPrivateFieldGet(this, _nameWeakMap_1)}`);
    }
}
_nameWeakMap_1 = new WeakMap();
const greeter = new Greeter("world");
console.log(greeter); // logs 'Greeter {}'
console.log(Object.keys(greeter)); // logs '[]'
greeter.greet(); // logs 'hello world'

We plan to open this PR against Microsoft/TypeScript as soon as this dependency is merged: Move class property transformation into new transformer file microsoft/TypeScript#30467
Related issue
Chrome 74 announcement (includes private class fields)
tc39 class fields proposal

This PR includes work by the following engineers:

Joey Watts @joeywatts (Bloomberg): private fields transformation
Max Heiber @mheiber (Bloomberg): parser and checker, pr review
Michael Gunter @Neuroboy23 (Bloomberg): pr review
Rob Palmer @robpalme (Bloomberg): leadership, spec expertise
Jaideep Bhoosreddy, Michael Molisani, Ravi Amin (Bloomberg)
Ron Buckton @rbuckton (Microsoft): designed the checker implementation, pr review
Daniel Rosenwasser @DanielRosenwasser (Microsoft): pr review

Signed-off-by: Joseph Watts <jwatts43@bloomberg.net>

and check that private names not used in parameters Signed-off-by: Max Heiber <max.heiber@gmail.com>

Signed-off-by: Joseph Watts <jwatts43@bloomberg.net>

- [x] treat private names as unique: - case 1: cannot say that a variable is of a class type unless the variable points to an instance of the class - see [test](https://github.com/mheiber/TypeScript/tree/checker/tests/cases/conformance/classes/members/privateNames/privateNamesUnique.ts) - case 2: private names in class hierarchies do not conflict - see [test](https://github.com/mheiber/TypeScript/tree/checker/tests/cases/conformance/classes/members/privateNames/privateNamesNoConflictWhenInheriting.ts) - [x] `#constructor` is reserved - see [test](https://github.com/mheiber/TypeScript/tree/checker/tests/cases/conformance/classes/members/privateNames/privateNameConstructorReserved.ts) - check in `bindWorker`, where every node is visited - [x] Accessibility modifiers can't be used with private names - see [test](https://github.com/mheiber/TypeScript/tree/checker/tests/cases/conformance/classes/members/privateNames/privateNamesNoAccessibilityModifiers.ts) - implemented in `checkAccessibilityModifiers`, using `ModifierFlags.AccessibilityModifier` - [x] `delete #foo` not allowed - [x] Private name accesses not allowed outside of the defining class - see test: https://github.com/mheiber/TypeScript/tree/checker/tests/cases/conformance/classes/members/privateNames/privateNameNotAccessibleOutsideDefiningClass.ts - see [test](https://github.com/mheiber/TypeScript/tree/checker/tests/cases/conformance/classes/members/privateNames/privateNamesNoDelete.ts) - implemented in `checkDeleteExpression` - [x] Do [the right thing](https://gist.github.com/mheiber/b6fc7adb426c2e1cdaceb5d7786fc630) for nested classes mv private name tests together more checker tests for private names update naming and cleanup for check private names for private name support in the checker: - make names more consistent - remove unnecessary checks - add utility function to public API - other small cleanup Move getPropertyNameForUniqueESSymbol to utility for consistency with other calculation of special property names (starting with __), move the calculation of property names for unique es symbols to `utilities.ts`. private name tests strict+es6 Update private name tests to use 'strict' type checking and to target es6 instead of default. Makes the js output easier to read and tests more surface area with other checker features. error message for private names in obj literals Disallow decorating private-named properties because the spec is still in flux. Signed-off-by: Max Heiber <max.heiber@gmail.com>

Implements private instance fields on top of the class properties refactor. Signed-off-by: Joseph Watts <jwatts43@bloomberg.net> Signed-off-by: Max Heiber <max.heiber@gmail.com>

Neuroboy23 · 2019-04-01T17:33:31Z

src/compiler/binder.ts

+                    const containingClass = getContainingClass(name.parent);
+                    if (!containingClass) {
+                        // we're in a case where there's a private name outside a class (invalid)
+                        return undefined;


I see that this is the only explicit return value of undefined. Any other undefined would have to come from an inner method call. I see some existing calling code that does things like !getDeclarationName(...). Does this undefined value here mean the same thing as any other potential undefined values? If not, maybe it's better to throw here (explicitly or implicitly) instead?

@Neuroboy23 these changes were reviewed internally months ago and also given a peruse by Daniel and Ron. I'm open to revisiting this after we open the PR against the MS TS repo.

Re this specific question:

a private name outside a class body is a parse error and will be caught by the parser. If the user has noEmitOnError: false in their compilerOptions then we have to do best-effort on the emit, even though the emit almost certainly bad. In this case, I'm not sure whether it would be better to return node or undefined.

mheiber force-pushed the private-named-instance-fields branch 2 times, most recently from a97aa6b to ecd1529 Compare March 27, 2019 20:22

mheiber changed the base branch from master to private-named-instance-fields March 27, 2019 20:32

mheiber changed the title ~~WIP Private named instance fields~~ Private-named instance fields Mar 27, 2019

This was referenced Mar 27, 2019

STALE: transform private-named instance methods #19

Closed

Private named static fields #33

Closed

Joseph Watts and others added 6 commits March 28, 2019 15:46

Move class property transformation into new transformer.

b0b5d44

Signed-off-by: Joseph Watts <jwatts43@bloomberg.net>

Parse Private Names

256bb91

and check that private names not used in parameters Signed-off-by: Max Heiber <max.heiber@gmail.com>

Fix display of private names in language server

159d511

Signed-off-by: Joseph Watts <jwatts43@bloomberg.net>

Accept baseline changes

2902c73

Signed-off-by: Joseph Watts <jwatts43@bloomberg.net>

Add private named instance field transformation (bloomberg#31)

3f6b68f

Implements private instance fields on top of the class properties refactor. Signed-off-by: Joseph Watts <jwatts43@bloomberg.net> Signed-off-by: Max Heiber <max.heiber@gmail.com>

mheiber force-pushed the private-named-instance-fields branch from ecd1529 to 3f6b68f Compare March 28, 2019 15:50

Neuroboy23 reviewed Apr 1, 2019

View reviewed changes

Neuroboy23 approved these changes Apr 3, 2019

View reviewed changes

Neuroboy23 merged commit a04bfff into bloomberg:private-named-instance-fields Apr 3, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Private-named instance fields#32

Private-named instance fields#32
Neuroboy23 merged 6 commits intobloomberg:private-named-instance-fieldsfrom
mheiber:private-named-instance-fields

mheiber commented Mar 27, 2019 •

edited

Loading

Uh oh!

Neuroboy23 Apr 1, 2019

Uh oh!

mheiber Apr 2, 2019 •

edited

Loading

Uh oh!

mheiber Apr 2, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

mheiber commented Mar 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Private-Named Instance Fields

Uh oh!

Neuroboy23 Apr 1, 2019

Choose a reason for hiding this comment

Uh oh!

mheiber Apr 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mheiber Apr 2, 2019

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mheiber commented Mar 27, 2019 •

edited

Loading

mheiber Apr 2, 2019 •

edited

Loading