Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Private named instance fields #30829

Merged

Commits on Dec 24, 2019

  1. Parse private names

    Signed-off-by: Max Heiber <max.heiber@gmail.com>
    mheiber committed Dec 24, 2019
    Configuration menu
    Copy the full SHA
    08a9015 View commit details
    Browse the repository at this point in the history
  2. update parser error recovery tests to use ¬ not #

    The intent of the tests seemed to be to
    regiment the behavior of the parser
    when a weird symbol is encountered.
    
    The `#` is now taken by private identifiers,
    so `¬` seems like a good new choice for
    keeping the diff small, since it also fits in
    16 bits (wide emojis would be treated
    as multiple characters, since the scanner
    uses ++).
    
    Signed-off-by: Max Heiber <max.heiber@gmail.com>
    mheiber committed Dec 24, 2019
    Configuration menu
    Copy the full SHA
    1a1cb76 View commit details
    Browse the repository at this point in the history
  3. Fix display of private names in language server

    Signed-off-by: Joseph Watts <jwatts43@bloomberg.net>
    Signed-off-by: Max Heiber <max.heiber@gmail.com>
    Joseph Watts authored and mheiber committed Dec 24, 2019
    Configuration menu
    Copy the full SHA
    be2779e View commit details
    Browse the repository at this point in the history
  4. Private Name Support in the Checker (#5)

    - [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>
    mheiber committed Dec 24, 2019
    Configuration menu
    Copy the full SHA
    7c1827a View commit details
    Browse the repository at this point in the history
  5. Add private instance field transformation, pr feedback

    Implements private instance fields on top of the class properties refactor.
    
    This commit also includes incorporation of PR feedback on the
    checker, parser, and transformer in order to make the rebase
    manageable.
    
    Co-Authored-By: Max Heiber <max.heiber@gmail.com>
    Co-Authored-By: Ron Buckton <ron.buckton@microsoft.com>
    
    Signed-off-by: Joey Watts <jwatts43@bloomberg.net>
    Signed-off-by: Max Heiber <max.heiber@gmail.com>
    
    Incorporate PR feedback
    
    Fix checker crash with new block scoped bindings
    
    Add classExpressionInLoop test
    
    Update baselines for private name errors
    
    Apply suggestions from code review
    
    Fix privateNameFieldCallExpression test
    
    Remove unnecessary comment
    
    Fix PropertyAccessEntityNameExpression type
    
    Remove PrivateName from PropertyNameLiteral
    
    Add createPrivateName
    
    Remove PrivateName type from textSourceNode
    
    Add Debug asserts for invalid syntax kinds
    
    Don't output private name syntax when undeclared
    
    Make PrivateName extend Node
    
    Update baselines for public API
    
    Fix completions in language server
    
    Fix fourslash test crash
    
    intern private name descriptions
    
    undo expensive node.name.parent assignment
    
    Back the way things were on `master`: only
    assign node.name.parent when we need to
    
    Add tests for private names and JSDoc
    
    Patch hoverOverPrivateName fourslash test
    
    Fix goToDefinition for private-named fields
    
    remove Debug.fail for private name outside class
    
    Remove Debug.fail in binder.ts::`getDeclarationName`.
    It turns out this code path *does* get hit (intentionally).
    
    For best error messages, return `undefined` and rely
    on the parser generating a good error message
    
    "Private names are not allowed outside class bodies"
    
    Add rbuckton test cases for private names
    
    These test cases specifically exercise where
    we needed to use name-mangling. They are
    cases where private names have the same
    description.
    
    - private names no conflict when inheriting
    - private names distinct even when
    the two classes have the same name
    
    check dup instance+static private identifiers
    
    class A {
        #foo;
        static #foo;
    }
    
    not allowed because static and private names
    share the same lexical scope
    https://tc39.es/proposal-class-fields/#prod-ClassBody
    
    refactor getPropertyForPrivateName, rel spans
    
    refactor getPropertyForPrivateName so
    it is easier to read (use findAncestor instead
    of loop).
    
    Fix bugs in getPropertyForPrivateName:
    - make it work with deeply-nested classes with
    and without shadowing
    - make it catch shadowing when the conflict is
    between static and instance
    private name descriptions (these can actually
    clash)
    
    And add related spans to diagnostics
    for getPropertyForPrivateName
    
    catch conflicts between static and instance
    private identifiers:
    - cannot have an instance and static private identifier
      with the same spelling, as there is only one namespace
      for private names
    
    rename 'PrivateName' to 'PrivateIdentifier'
    
    to match the change of wording in the spec
    prposal:
    
    https://tc39.es/proposal-class-fields/#sec-syntax
    
    The rename in the spec was to avoid confusion
    between the AST Node PrivateIdentifier
    and the internal spec type PrivateName.
    
    So one uses the [[Description]] of a
    PrivateIdentifier to look up the PrivateName
    for a given scope.
    
    This corresponds closely to our implementation
    in the binder and checker:
    - we mangle PrivateIdentifier.escapedText to
    get a `key` which we use to get the symbol
    for a property
    
    f
    
    getLiteralTypeFromProperty-check privateIdentifier
    
    rename and simplify privateNameAndAny test case
    
    make it clearer that all this test is showing is
    that we allow accessing arbitrary private identifiers
    on `any`.
    
    Note: we could have something more sound here by
    checking that there is a private-named field declaration
    in a class scope above the property access.
    
    Discussion:
    https://github.com/microsoft/TypeScript/pull/30829/files#r302760015
    
    Fix typo in checker
    
    s/identifer/identifier
    
    remove accidental change
    
    patch fourslash test broken by isPrivateIdentifier
    
    just needed to add a check to see if the symbol
    .name is defined
    
    extract reportUnmatchedProperty
    
    per @nsandersn feedback
    
    propertiesRelatedTo was getting to long
    
    pull out the unmatchedProperty reporting into
    a seprate function
    
    fix typo in private names test
    
    Fix type soundness with private names
    
    Remove PrivateIdentifier from emit with Expr hint
    
    Fixup helpers and set function name for privates
    
    remove accidentally-committed baselines
    
    These baselines somehow ended up in this pr,
    though they have nothing to do with the changes
    
    Revert "getLiteralTypeFromProperty-check privateIdentifier"
    
    This reverts commit bd1155c.
    
    reason: the check for isIdentifier in
    getLiteralTypeFromProperty is superfluous because
    we do this check in getLiteralTypeFromPropertyName
    
    Update comment in private name uniqueness test 3
    
    I think the comments were not accurate and that we
    export the error on `this.#x = child.#x` because:
    - private names are lexically scoped: the code in question is not in a
    lexical scope under the definition of Child's #x.
    - private names are private to their containing class: never inherited
    
    This expected behavior matches that of Chrome Canary and
    my understanding of the spec
    
    test private names use before def, circular ref
    
    test private names use before def, circular ref
    
    update diagnosticMessages s/delete/'delete'
    
    per @DanielRosenwasser and @sandersn guidance,
    use this style in diagnostic messages:
    
    "operand of a 'delete' operator" (single quotes)
    
    rather than old style:
    
    "operand of a delete operator" (single quotes)
    
    This is for consistency, as we use the new
    style in the privateIdentifiers error messages
    and it is consistent with our messages about
    other operators
    
    incorporate private names early exit feedback
    
    and code style change to use a ternary
    instead of if so we can 'const'
    
    require private-named fields to be declared in JS
    
    All fields must be declared in TS files.
    
    In JS files, we typically do not have this requirement.
    
    So special-case private fields, which must always
    be declared (even in JS files, per spec)
    
    update debug failure for es2015 private identifier
    
    Co-Authored-By: Ron Buckton <ron.buckton@microsoft.com>
    
    fix checker handling of private name in subclasse
    
    update checker and tests to account for the
    following ts features:
    
    - private names do not participate in
    the prototype chain, but *are* assigned
    in the parent class' constructor. So
    parent can access its *own* private fields
    on instances of the subclass
    
    Add more tests for private-named fields in JS
    
    add test to hit symbolToExpression w private names
    
    symbolToExpression knows about private names
    add a test to exercise this code path
    
    ban private-named static methods (not supported yet)
    
    ban private-named methods (not supported yet)
    
    ban private-named accessors (not supported yet)
    
    fix privateIdentifier fourslash test
    
    change assertion throw to return
    
    Co-Authored-By: Ron Buckton <ron.buckton@microsoft.com>
    
    Update comment in checker.ts re reserved members
    
    Remove case for privateIdentifier in EntityNameExpr
    
    Remove case for privateIdentifier in
    EntityNameExpr. That code path is never hit,
    and privateIdnetifiers cannot be entity names.
    
    remove unnecessary parentheses
    
    Ban private identifier in enum
    
    As the new test, 'privateNameEnumNoEmit',
    shows, the checker now correctly makes
    a diagnostic for private identifiers in enums.
    
    However, when noEmit is false we
    hit this assertion in the transformer.
    
    This assertion will have to be removed
    so that we have a diagnostic here instead
    of an assertion error.
    
    When the assertion is removed,
    the 'privateNameEnum' baseline
    will have to be updated
    
    Fix destructuring assignment, use createCallBinding, remove unneeded helper
    
    Add class private field helpers to external helpers
    
    Remove private identifier enum assertion, use missing identifiers
    
    Fix hash map inefficiency by only using get
    
    Update baselines with empty identifier change
    
    Add privateNameEnum test baselines
    
    Fix private identifier destructuring (array assignment defaults, unique names)
    
    Use createPrivateIdentifierAssignment in destructuring transform
    
    Fix lint error
    
    Separate destructuring target visitor from regular visitor
    
    Fix destructuring assignment with this bindings
    
    Fix destructuring assignment with this bindings
    
    Fix syntax error with destructuring assignment output
    
    Disallow private identifiers in property signatures
    
    remove duplicate test baselines
    
    Add tests for undeclarated private identifiers
    
    remove unnecessary cast
    
    Nicer error message for mis-placed hashbang
    
    Workaround v8 bug with destructured assignments
    
    Optimize private identifier stack lookup
    
    Avoid the cost of performing an array lookup to look at the top of the
    private identifier environment stack.
    
    Change function name to be more specific
    
    Changes "getOperatorForCompoundAssignment" to
    "getNonAssignmentOperatorForCompoundAssignment" now that this
    function is accessible to the entire compiler.
    
    Improve test case for private name assignment
    
    Adds a compound assignment test case for a class with private names
    being declared on the left-hand-side of the assignment expression.
    
    Remove extra non-private-field test
    
    Remove isPrivateIdentifierAssignmentExpression helper
    
    Don't transform private names in ESNext target
    
    Preserve private fields initialized to functions
    
    Move function expressions to outer scope for efficiency
    
    Add WeakMap collision check
    
    Modify potential WeakMap collision condition
    
    Fix this property assignment binding with private names
    
    Add correct error message for WeakMap collision
    
    Add error for statements before super with private identifiers
    
    Refactor getPropertyForPrivateIdentifier
    
    Add test for private identifier fields initialized to class exprs
    
    Fix shebang errors
    
    Fix private errors on index signatures
    
    Add codefix for missing private property
    
    Update error messages for unsupported private features
    
    Fix inheritance-related errors
    
    Fixes inheritance-related errors with private identifiers by resolving
    properties from base classes. Private identifiers do not show up as
    properties on a union type, so those do not type-check.
    
    Add test for interface extending class with private access
    
    Remove debugging log
    
    Remove name assignment from private named functions
    
    Rename to getPrivateIdentifierPropertyOfType
    
    Fix index signature test comment
    
    Fix test target syntax error
    
    Change error messages
    
    patch private identifiers outside class bodies
    
    Add test for private identifier with ooo super
    
    Add test for a class with a private identifier
    with a non-preambly (for example, not a comment)
    statement before 'super':
    
    should error, saying 'super' must come first
    
    Fix nits
    
    incorporate PR feedback
    
    Incorporate checker feedback
    
    - reorganize if statements in checkFunctionOrConstructorSymbol
    - remove overload for getPrivateIdentifierPropertyOfType
    
    reorganize if statements in checkFunctionOrConstructorSymbol
    
    test for private names with JSX
    
    use getPropertyOftype in getPropertyForPrivateIdentifier
    
    getPrivateIdentifierPropertyOfType error on synthetic
    
    make getPrivateIdentifierPropertyOfType  error
    if given a node that is not from the parse tree
    
    Simplify checkPrivateIdentifierPropertyAccess
    
    use getPropertiesOfType instead of
    rehashing that logic
    
    test for private identifiers w decl merging
    
    fix test target for privateNameDeclarationMerging
    
    update baselines
    
    Fix private identifier ++/-- numeric coercion
    
    Remove 'downleveled' from super error
    
    Fix bad comments in helper call emit
    
    Error on private identifiers in JSX tag names
    
    Add more private identifier tests
    
    Add es2015 target for private name destructured binding test
    
    Add privateNameConstructorSignature test
    
    Add test for navigation bar w private identifier
    
    Remove spurious line from test
    
    // in js file
    class A {
        exports.#foo = 3; // should error
    }
    
    The above line failed to produce an error
    when run using the test harness.
    
    When using tsc or the language server,
    we got the expected error message.
    
    Removing the troublesome line, as it
    seems to say more about the test runner
    than about the code it is testing.
    
    Fix inefficient constructor generation
    
    dts: don't emit type for private-identified field
    
    Do not emit types for private-identified fields
    when generating declaration files.
    
    // example.ts
    export class A {
        #foo: number;
    }
    
    // example.d.ts
    
    export declare class A {
        #foo;
    }
    
    **This is not ideal!**
    
    The following would be better:
    
    // example.d.ts
    
    export declare unique class A {
        #foo;
    }
    
    See discussion:
    
    microsoft#33038 (comment)
    
    notice when private-identified field unused
    
    Notice when private-identified fields are unused,
    and implement the same behavior as for unused
    private-modified fields.
    
    This is used in the language server to make things
    grayed out.
    
    This case generates an error when --noUnusedLocals
    flag is passed to tsc:
        - "<name> is declared but never used"
    
    accept baselines
    
    Revert "dts: don't emit type for private-identified field"
    
    This reverts commit e50305d.
    
    Instead of just excluding the type from private identifier
    emit, only emit a single private identifier
    per class.
    
    This accomplishes nominality while
    hiding implementation detail that
    is irrelevant to .d.ts consumers
    
    only emit a single private identifier in dts
    
    In dts emit, emit at most one private identifier,
    and rename it to `#private`.
    
    refactor getPrivateIdentifierPropertyOfType
    
    - safer check for wehther is parse tree node
    - return undefined when not found (instead of
    a Debug.fail)
    
    Incorporate PR feedback
    
    Don't rely on parent pointers in transform
    
    Passes context about whether the postfix unary expression value is
    discarded down the tree into the visitPostfixUnaryExpression function.
    
    Remove orphaned test baseline files
    
    remove unreachable if
    
    Check `any`-typed private identified fields
    
    Update private identifier incompatible modifier checks
    
    - disallow 'abstract' with private identifiers
    - s/private-named-property/private identifier
    
    Add additional call expression test cases
    
    Fix disallow 'abstract' with private identifier
    
    Static private identifiers not inherited
    
    Including this in the PR for private
    instance fields even though static
    private identifiers are banned.
    
    Reason: this change
    improves quality of error messages,
    see test case.
    
    Thanks Ron!
    
    Simplifiy private identifier 'any' type handling
    
    Error on private identifier declarations for ES5/ES3
    
    Bind `this` for private identifier property tagged template literals
    
    Fix target for jsdocPrivateName1 test
    
    Update getPrivateIdentifierPropertyOfType API
    
    Make it easier to use by accepting a string
    and location, rather than a PrivateIdentifier.
    
    Thanks Ron!
    
    remove orphaned tests
    
    rename test
    
    remove duplicate tests
    
    Remove unrelated error from test
    
    update diagnostic message 'private identifier'
    
    The nodes for hash private fields are now
    called 'private identifier'. Update one last
    diagnostic message to use the new terminology.
    
    refine solution for static private identifier fields
    
    - use `continue` instead of `filter` for perf
    - better naming
    - make it clear the current solution will
    need to be altered when we lift the ban on
    static private identifier fields, including
    a test case that should change in the future
    
    Fix display of private identifiers in lang server
    
    Fix bug where private identifiers in completion
    tooltips in the playground were showing up
    as the symbol table entries (with underscores and such).
    
    microsoft#30829 (comment)
    Signed-off-by: Max Heiber <max.heiber@gmail.com>
    joeywatts authored and mheiber committed Dec 24, 2019
    Configuration menu
    Copy the full SHA
    14808d4 View commit details
    Browse the repository at this point in the history
  6. fix privateIdentifier w !useDefineForClassFields

    Signed-off-by: Max Heiber <max.heiber@gmail.com>
    mheiber committed Dec 24, 2019
    Configuration menu
    Copy the full SHA
    deba0e3 View commit details
    Browse the repository at this point in the history
  7. Disallow PrivateIdentifier in Optional Chains

    Signed-off-by: Max Heiber <max.heiber@gmail.com>
    mheiber committed Dec 24, 2019
    Configuration menu
    Copy the full SHA
    3a7fe1d View commit details
    Browse the repository at this point in the history
  8. restrict privateIdentifier completions correctly

    Don't autocomplete privateIdentifiers in
    places where they are not allowed.
    
    Signed-off-by: Max Heiber <max.heiber@gmail.com>
    mheiber committed Dec 24, 2019
    Configuration menu
    Copy the full SHA
    da87430 View commit details
    Browse the repository at this point in the history
  9. make PrivateIdentifier not a PropertyNameLiteral

    Signed-off-by: Max Heiber <max.heiber@gmail.com>
    mheiber committed Dec 24, 2019
    Configuration menu
    Copy the full SHA
    cef62f9 View commit details
    Browse the repository at this point in the history
  10. fix for private field no initializer esnext

    Signed-off-by: Max Heiber <max.heiber@gmail.com>
    mheiber committed Dec 24, 2019
    Configuration menu
    Copy the full SHA
    f3fe338 View commit details
    Browse the repository at this point in the history
  11. Added test.

    DanielRosenwasser authored and mheiber committed Dec 24, 2019
    Configuration menu
    Copy the full SHA
    11965d2 View commit details
    Browse the repository at this point in the history
  12. Accepted baselines.

    DanielRosenwasser authored and mheiber committed Dec 24, 2019
    Configuration menu
    Copy the full SHA
    8b6b77f View commit details
    Browse the repository at this point in the history
  13. Configuration menu
    Copy the full SHA
    3f8b490 View commit details
    Browse the repository at this point in the history
  14. Accepted baselines.

    DanielRosenwasser authored and mheiber committed Dec 24, 2019
    Configuration menu
    Copy the full SHA
    7bda5ac View commit details
    Browse the repository at this point in the history
  15. fix private fields .d.ts emit for JS w expando

    fix bug where the following in a JS file
    would lead to a `#private` in the .d.ts:
    
    ```js
    class C {
        constructor () {
            this.a = 3
        }
    }
    ```
    mheiber committed Dec 24, 2019
    Configuration menu
    Copy the full SHA
    fbb84d1 View commit details
    Browse the repository at this point in the history