- Notifications
You must be signed in to change notification settings - Fork13.2k
ES private class elements#42458
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Merged
rbuckton merged 87 commits intomicrosoft:masterfrombloomberg:es-private-methods-and-accessorsMar 25, 2021
Uh oh!
There was an error while loading.Please reload this page.
Merged
Changes from1 commit
Commits
Show all changes
87 commits Select commitHold shift + click to select a range
e3a3fb8 Added support for private identifier methods.
dragomirtitian4ab27a2 Added tests for private methods.
dragomirtitian6977b9f Added check to only not allow private name method signatures in anyth…
dragomirtitianbc8c07e Added private accessors tests
dragomirtitian2760ed8 Transform private methods
mkubilayk04af85d Rename shouldTransformPrivateFields
mkubilaykf048d72 Accept baseline
mkubilaykf8cfb81 Use a single WeakSet for brand-check
mkubilayk4aa4c3b Accept baseline
mkubilayk0ac1271 Add a test for using private methods in static field initializers
mkubilayk7da5bd9 Add breaking checker test
mkubilayke133f60 Add to instances once per-instance
mkubilaykaa5dd8a Accept baseline
mkubilayka3889a7 fix: evaluate receiver and rhs expressions before throwing on readonl…
mkubilayk8952caa Add a test for evaluating rhs before readonly assignment
mkubilaykdee87bb Transpile private accessors
mkubilaykaec7e9b Accept baseline
mkubilaykf3bbf0f fix: handle readonly/writeonly accessors
mkubilayka1b59cd accept baseline
mkubilayk6b74498 add a test for private setter without a getter
mkubilaykd51f6cd fix: getAllUnscopedEmitHelpers
mkubilaykc175770 fix: better handling of duplicate names
mkubilayk4301af5 Fixed wrong error message for private methods in class expressions.
dragomirtitian3a056b2 change error message
mkubilayk95df2d1 add a test for async private methods with a higher target
mkubilayk590124e fix: setter assignment returns rhs value
mkubilayk219ea83 add a test for setter assignment return value
mkubilayk25da8ca fix: handle duplicate accessors
mkubilayka2228be add tests for duplicate accessors
mkubilaykff4f737 docs: add missing parameter docs
mkubilayk704727c Fixed failing test.
dragomirtitian73925dc baseline-accept: ordering changes
mkubilayk67fd38d fix: attach weakSetName to property declaration
mkubilayk45c2a41 add a test for nested private methods
mkubilayk356b91d add a test with any
mkubilaykb391464 Merge pull request #51 from bloomberg/fix/nested-private-methods
dragomirtitian5ebb3d6 Added support for static private fields accessors and methods.
dragomirtitian7006c09 Added error message for private identifiers used with static decorato…
dragomirtitianb876613 Fixed emit static bug that used private names outside of classes for …
dragomirtitian31a8c00 Added tests for private static fields methods and accessors.
dragomirtitian9219e99 Merge remote-tracking branch 'remotes/origin/master' into es-private-…
dragomirtitian5a27a32 Merge remote-tracking branch 'remotes/bloomberg/es-private-methods-an…
dragomirtitian964fc27 Fixed error messages and tests after merge.
dragomirtitiand2ab6fc Merge branch 'es-private-methods-and-accessors' into es-private-stati…
dragomirtitian23f6c67 Accept new baseline.
dragomirtitian374e6f3 Improved duplicate identifier checks for static private class elements.
dragomirtitian62b4f2f Added error when using initializers with private static fields when u…
dragomirtitianb2a692d Fixed code review issues.
dragomirtitian363041e Removed semantically wrong emit on `useDefineForClassFields:true` wit…
dragomirtitian64757e0 Changed emit for uninitialized private static fields.
dragomirtitian68a7f80 Added runtime error in helper if a static private field is accessed b…
dragomirtitian7e7b0b1 Fixed code review comments for private identifier static class elements.
dragomirtitian30c7f69 add debug.assertNever for unknown node type (#53)
mkubilaykc924058 Merge remote-tracking branch 'upstream/master' into es-private-method…
mkubilayke65116f Merge remote-tracking branch 'upstream/master' into es-private-method…
mkubilayk4748c87 Fixed code review issues.
dragomirtitian09a5aae Merge branch 'es-private-methods-and-accessors' into es-private-stati…
dragomirtitian497e47f Fixed code review issues for private class elements.
dragomirtitian294cc48 Fixes class shadowing when checking access to a private static class …
dragomirtitian027bdb3 Merge remote-tracking branch 'remotes/origin/master' into es-private-…
dragomirtitian7bbc944 Merge remote-tracking branch 'upstream/master'
mkubilaykdd526e7 fix private methods/accessors in class expr inside a loop
mkubilaykf6ecfab collapse switch case
mkubilayk6c3f661 fix class name
mkubilaykef69c6c simplify getPrivateMethodsAndAccessors
mkubilayk2a646ed remove findPreviousAccessorInfo
mkubilaykd8136f7 lazily create weakSetName identifier
mkubilaykfbd749c Merge remote-tracking branch 'remotes/bloomberg/es-private-static-fie…
dragomirtitianf96eafe Merge remote-tracking branch 'remotes/bloomberg/es-private-static-fie…
dragomirtitian2de8a43 Merge branch 'es-private-methods-and-accessors' of https://github.com…
dragomirtitian7490a36 do not allocate a node if not needed in visitMehodDeclaration (#55)
mkubilaykc273258 Removed all the emit helpers for private identifier methods accessors…
dragomirtitian1addbb3 Simplified emit for private identifier class elements.
dragomirtitianc461b92 do not clone the receiver (#57)
mkubilayka61fd5f leave bad code in for #constructor and duplicate private names (#58)
mkubilayk89f746a Added check for WeakSet collision.
dragomirtitianff24636 Added error for using a set only accessor.
dragomirtitiana60e429 Merge pull request #61 from bloomberg/es-private-methods-and-accessor…
dragomirtitianf5816ab update keyof tests and ?? (#62)
mkubilayk3b62d3c fix emit helpers comments
mkubilayk7d8a2b9 produce an error if private field helpers are not up to date
mkubilaykd0d2055 add tests
mkubilayk93a7ca3 fix setter-only compound assignment
mkubilayk6da2cdd Merge pull request #63 from bloomberg/helper-parameter-count
mkubilayk30805b7 fix tests
mkubilayk8d77a8e fix duplicated trailing comments (#64)
mkubilayk8078ae0 clear receiver pos and setTextRange on helper calls
mkubilaykFile filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
leave bad code in for #constructor and duplicate private names (#58)
- Loading branch information
Uh oh!
There was an error while loading.Please reload this page.
commita61fd5f7d48d9141615e7b16a7c537de6bd8738c
There are no files selected for viewing
90 changes: 63 additions & 27 deletionssrc/compiler/transformers/classFields.ts
This file contains hidden or 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 |
|---|---|---|
| @@ -24,6 +24,11 @@ namespace ts { | ||
| * Stores if the identifier is static or not | ||
| */ | ||
| isStatic: boolean; | ||
| /** | ||
| * Stores if the identifier declaration is valid or not. Reserved names (e.g. #constructor) | ||
| * or duplicate identifiers are considered invalid. | ||
| */ | ||
| isValid: boolean; | ||
| } | ||
| interface PrivateIdentifierAccessorInfo extends PrivateIdentifierInfoBase { | ||
| kind: PrivateIdentifierKind.Accessor; | ||
| @@ -260,6 +265,13 @@ namespace ts { | ||
| return visitEachChild(node, classElementVisitor, context); | ||
| } | ||
| // leave invalid code untransformed | ||
| const info = accessPrivateIdentifier(node.name); | ||
| Debug.assert(info, "Undeclared private name for property declaration."); | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Can a user write source text that would fail this assertion? Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. I don't think so. That would indicate a bug in | ||
| if (!info.isValid) { | ||
| return node; | ||
| } | ||
| const functionName = getHoistedFunctionName(node); | ||
| if (functionName) { | ||
| getPendingExpressions().push( | ||
| @@ -303,17 +315,27 @@ namespace ts { | ||
| function visitPropertyDeclaration(node: PropertyDeclaration) { | ||
| Debug.assert(!some(node.decorators)); | ||
| if (isPrivateIdentifier(node.name)) { | ||
| if (!shouldTransformPrivateElements) { | ||
| // Initializer is elided as the field is initialized in transformConstructor. | ||
| return factory.updatePropertyDeclaration( | ||
| node, | ||
| /*decorators*/ undefined, | ||
| visitNodes(node.modifiers, visitor, isModifier), | ||
| node.name, | ||
| /*questionOrExclamationToken*/ undefined, | ||
| /*type*/ undefined, | ||
| /*initializer*/ undefined | ||
| ); | ||
| } | ||
| // leave invalid code untransformed | ||
| const info = accessPrivateIdentifier(node.name); | ||
| Debug.assert(info, "Undeclared private name for property declaration."); | ||
| if (!info.isValid) { | ||
| return node; | ||
| } | ||
| } | ||
| // Create a temporary variable to store a computed property name (if necessary). | ||
| // If it's not inlineable, then we emit an expression after the class which assigns | ||
| @@ -1162,55 +1184,62 @@ namespace ts { | ||
| const env = getPrivateIdentifierEnvironment(); | ||
| const { weakSetName, classConstructor } = env; | ||
| const assignmentExpressions: Expression[] = []; | ||
| const privateName = node.name.escapedText; | ||
| const previousInfo = env.identifiers.get(privateName); | ||
| const isValid = !isReservedPrivateName(node.name) && previousInfo === undefined; | ||
| if (hasStaticModifier(node)) { | ||
| Debug.assert(classConstructor, "weakSetName should be set in private identifier environment"); | ||
| if (isPropertyDeclaration(node)) { | ||
| const variableName = createHoistedVariableForPrivateName(text, node); | ||
| env.identifiers.set(privateName, { | ||
| kind: PrivateIdentifierKind.Field, | ||
| variableName, | ||
| brandCheckIdentifier: classConstructor, | ||
| isStatic: true, | ||
| isValid, | ||
| }); | ||
| } | ||
| else if (isMethodDeclaration(node)) { | ||
| const functionName = createHoistedVariableForPrivateName(text, node); | ||
| env.identifiers.set(privateName, { | ||
| kind: PrivateIdentifierKind.Method, | ||
| methodName: functionName, | ||
| brandCheckIdentifier: classConstructor, | ||
| isStatic: true, | ||
| isValid, | ||
| }); | ||
| } | ||
| else if (isGetAccessorDeclaration(node)) { | ||
| const getterName = createHoistedVariableForPrivateName(text + "_get", node); | ||
| if (previousInfo?.kind === PrivateIdentifierKind.Accessor && previousInfo.isStatic && !previousInfo.getterName) { | ||
| previousInfo.getterName = getterName; | ||
| } | ||
| else { | ||
| env.identifiers.set(privateName, { | ||
| kind: PrivateIdentifierKind.Accessor, | ||
| getterName, | ||
| setterName: undefined, | ||
| brandCheckIdentifier: classConstructor, | ||
| isStatic: true, | ||
| isValid, | ||
| }); | ||
| } | ||
| } | ||
| else if (isSetAccessorDeclaration(node)) { | ||
| const setterName = createHoistedVariableForPrivateName(text + "_set", node); | ||
| if (previousInfo?.kind === PrivateIdentifierKind.Accessor && previousInfo.isStatic && !previousInfo.setterName) { | ||
| previousInfo.setterName = setterName; | ||
| } | ||
| else { | ||
| env.identifiers.set(privateName, { | ||
| kind: PrivateIdentifierKind.Accessor, | ||
| getterName: undefined, | ||
| setterName, | ||
| brandCheckIdentifier: classConstructor, | ||
| isStatic: true, | ||
| isValid, | ||
| }); | ||
| } | ||
| } | ||
| @@ -1220,11 +1249,12 @@ namespace ts { | ||
| } | ||
| else if (isPropertyDeclaration(node)) { | ||
| const weakMapName = createHoistedVariableForPrivateName(text, node); | ||
| env.identifiers.set(privateName, { | ||
| kind: PrivateIdentifierKind.Field, | ||
| brandCheckIdentifier: weakMapName, | ||
| isStatic: false, | ||
| variableName: undefined, | ||
| isValid, | ||
| }); | ||
| assignmentExpressions.push(factory.createAssignment( | ||
| @@ -1239,46 +1269,48 @@ namespace ts { | ||
| else if (isMethodDeclaration(node)) { | ||
| Debug.assert(weakSetName, "weakSetName should be set in private identifier environment"); | ||
| env.identifiers.set(privateName, { | ||
| kind: PrivateIdentifierKind.Method, | ||
| methodName: createHoistedVariableForPrivateName(text, node), | ||
| brandCheckIdentifier: weakSetName, | ||
| isStatic: false, | ||
| isValid, | ||
| }); | ||
| } | ||
| else if (isAccessor(node)) { | ||
| Debug.assert(weakSetName, "weakSetName should be set in private identifier environment"); | ||
| if (isGetAccessor(node)) { | ||
| const getterName = createHoistedVariableForPrivateName(text + "_get", node); | ||
| if (previousInfo?.kind === PrivateIdentifierKind.Accessor && !previousInfo.isStatic && !previousInfo.getterName) { | ||
| previousInfo.getterName = getterName; | ||
| } | ||
| else { | ||
| env.identifiers.set(privateName, { | ||
| kind: PrivateIdentifierKind.Accessor, | ||
| getterName, | ||
| setterName: undefined, | ||
| brandCheckIdentifier: weakSetName, | ||
| isStatic: false, | ||
| isValid, | ||
| }); | ||
| } | ||
| } | ||
| else { | ||
| const setterName = createHoistedVariableForPrivateName(text + "_set", node); | ||
| if (previousInfo?.kind === PrivateIdentifierKind.Accessor && !previousInfo.isStatic && !previousInfo.setterName) { | ||
| previousInfo.setterName = setterName; | ||
| } | ||
| else { | ||
| env.identifiers.set(privateName, { | ||
| kind: PrivateIdentifierKind.Accessor, | ||
| getterName: undefined, | ||
| setterName, | ||
| brandCheckIdentifier: weakSetName, | ||
| isStatic: false, | ||
| isValid, | ||
| }); | ||
| } | ||
| } | ||
| @@ -1475,4 +1507,8 @@ namespace ts { | ||
| [receiver] | ||
| ); | ||
| } | ||
| function isReservedPrivateName(node: PrivateIdentifier) { | ||
| return node.escapedText === "#constructor"; | ||
| } | ||
| } | ||
3 changes: 2 additions & 1 deletiontests/baselines/reference/privateNameConstructorReserved.js
This file contains hidden or 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
Oops, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.