GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Constructor Visibility by AbubakerB · Pull Request #6885 · microsoft/TypeScript

Conversation

@AbubakerB

@AbubakerB Abubaker Bashir (AbubakerB) commented Feb 3, 2016

Copy link
Copy Markdown
Contributor

Constructor Visibility

(This is a updated PR from #4174)

There wasn't any detailed proposal, so I'm just going to list what this PR implements.

1) Constructor Visibility

  1. A constructor can now have the private or protected keyword on its signature.
  2. All constructor overloads must be of the same visibility (i.e. all private, protected, or public)

2) Private Constructors

A class with a private constructor:

  1. Can only be instantiated within it's own class.
  2. Can only be extended within it's own class.

3) Protected Constructors

A class with a protected constructor:

  1. Can only be instantiated within it's own class.
  2. Can be extended.

4) Signature Assignability

  • A public, protected and private signature is assignable to a private signature.
  • A public and protected signature is assignable to a protected signature.
  • A public signature is assignable to public signature.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ulrichb, Zorgatone, Taytay, basarat, ullalaaron, and yogeshgadge reacted with heart emoji
All reactions
  • ❤️ 6 reactions
Abubaker Bashir (AbubakerB) added 2 commits February 3, 2016 22:47
@AbubakerB
Added private and protected modifiers to constructors
f869b41
@AbubakerB
Added and updated tests for constructor visibility
c351ffc
@msftclas

Microsoft Contribution License Agreements (msftclas) commented Feb 3, 2016

Copy link
Copy Markdown

Hi Abubaker Bashir (@AbubakerB), I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@msftclas Microsoft Contribution License Agreements (msftclas) added the cla-already-signed label Feb 3, 2016
RyanCavanaugh
RyanCavanaugh reviewed Feb 3, 2016
View reviewed changes
Comment thread tests/baselines/reference/classConstructorAccessibility3.errors.txt Outdated

@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) Feb 3, 2016

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a one-directional relationship: public is assignable to protected and private, and protected is assignable to private (but none of the reverse)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@AbubakerB Abubaker Bashir (AbubakerB) Feb 4, 2016

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Abubaker Bashir (AbubakerB) added 5 commits February 4, 2016 18:32
@AbubakerB
Added constructor visibility in the declaration emitter
bbf92ce
@AbubakerB
Update baselines
3d529a8
@AbubakerB
Addressed PR feedback.
4200051
@AbubakerB
Accept baselines
4c93eb2
@AbubakerB
Refactored visibility text on modifier grammar check
5ce0202
mhegazy
mhegazy reviewed Feb 4, 2016
View reviewed changes
Comment thread src/compiler/diagnosticMessages.json Outdated
"category": "Error",
"code": 2674
},
"Cannot extend private class '{0}'.": {

@mhegazy Mohamed Hegazy (mhegazy) Feb 4, 2016

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about:
Cannot extend a class '{0}'. Class constructor is marked as private.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@AbubakerB Abubaker Bashir (AbubakerB) Feb 4, 2016

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Abubaker Bashir (AbubakerB) added 2 commits February 4, 2016 22:52
@AbubakerB
Addressed PR. Diagnostic messages
16b54e0
@AbubakerB
Accept baselines
037b657
DanielRosenwasser
DanielRosenwasser reviewed Feb 4, 2016
View reviewed changes
Comment thread src/compiler/checker.ts Outdated
}
return Ternary.False;
}
if (!constructorRelatedTo(sourceSignatures[0], targetSignatures[0], reportErrors)) {

@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) Feb 4, 2016

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it just comparing the first of each? Why was this change made?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) Feb 4, 2016

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only the first has to be checked because the rest must have identical visibility (it's an error for this not to be the case)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@AbubakerB
Addressed PR
a6a5a22
@AbubakerB

Abubaker Bashir (AbubakerB) commented Feb 9, 2016

Copy link
Copy Markdown
Contributor Author

Is there anything left for me to do on this?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@RyanCavanaugh

Ryan Cavanaugh (RyanCavanaugh) commented Feb 9, 2016

Copy link
Copy Markdown
Member

Anders Hejlsberg (@ahejlsberg) - take a look?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mhegazy

Mohamed Hegazy (mhegazy) commented Feb 11, 2016

Copy link
Copy Markdown
Contributor

Sorry for the delay, we will be discussing this in the TS design meeting tomorrow. should get back to you shortly after.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) mentioned this pull request Feb 12, 2016
Design Meeting Notes, 2/12/2016 #7059
Closed
@mhegazy

Mohamed Hegazy (mhegazy) commented Feb 12, 2016

Copy link
Copy Markdown
Contributor

Abubaker Bashir (@AbubakerB), we have no more comments on the design. Please update for merge conflicts.

Just one comment a private constrictor should be visible within nested classes, we do not handle this for private and protected members today (see #7058).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
mhegazy
mhegazy reviewed Feb 12, 2016
View reviewed changes
Comment thread src/compiler/checker.ts Outdated
const enclosingClass = enclosingClassDeclaration ? <InterfaceType>getDeclaredTypeOfSymbol(getSymbolOfNode(enclosingClassDeclaration)) : undefined;

// A private or protected constructor can only be instantiated within it's own class
if (declaringClass !== enclosingClass) {

@mhegazy Mohamed Hegazy (mhegazy) Feb 12, 2016

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as noted in , we #7059, if here is not sufficient, it should be a while loop walking up, looking at all enclosing classes, so that something like this should be legal:

class B {
    private constructor() { }

    method() {
        class C {
            method() {
                new B(); // should be fine
            }
        }
    }
}

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@AbubakerB Abubaker Bashir (AbubakerB) Feb 13, 2016

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Abubaker Bashir (AbubakerB) added 2 commits February 13, 2016 01:07
@AbubakerB
Allow extending and instantiating a private or protected class within…
8ef9599 
… itself
@AbubakerB
Added tests and accept baselines
ec7e80e
@AbubakerB

Abubaker Bashir (AbubakerB) commented Feb 13, 2016

Copy link
Copy Markdown
Contributor Author

So now, a private and protected constructor can be instantiated and extended (already the case for protected) within the scope of its own class.

I also, hopefully, implemented it such that it'll be easy for #7058 😃
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Abubaker Bashir (AbubakerB) added 2 commits February 13, 2016 02:51
@AbubakerB
Addressed PR
91ea83c
@AbubakerB
Updated tests and accepted baseline
208830e
@AbubakerB

Abubaker Bashir (AbubakerB) commented Feb 13, 2016

Copy link
Copy Markdown
Contributor Author

Build is failing due to gitignore ignoring the tests/**/*.js.
There's an open PR to fix this - awaiting it's merge.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Abubaker Bashir (AbubakerB) added 2 commits February 14, 2016 20:58
@AbubakerB
Merge branch 'master' into constructorAccessibility
2d7a0f4
@AbubakerB
Included previously ignored baseline .js file and slight refactoring
ba8b168
@RyanCavanaugh

Ryan Cavanaugh (RyanCavanaugh) commented Feb 16, 2016

Copy link
Copy Markdown
Member

Abubaker Bashir (@AbubakerB) thanks! This was a three-day weekend but we're back on Tuesday and should be able to take a last look.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mhegazy

Mohamed Hegazy (mhegazy) commented Feb 16, 2016

Copy link
Copy Markdown
Contributor

Abubaker Bashir (@AbubakerB), one last merge conflict. but we should be good to go after that.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@AbubakerB
Merge branch 'master' into constructorAccessibility
de23e2f 
Conflicts:
	src/compiler/checker.ts
@AbubakerB

Abubaker Bashir (AbubakerB) commented Feb 16, 2016

Copy link
Copy Markdown
Contributor Author

Sure :)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mhegazy

Mohamed Hegazy (mhegazy) commented Feb 17, 2016

Copy link
Copy Markdown
Contributor

thanks!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Mohamed Hegazy (mhegazy) added a commit that referenced this pull request Feb 17, 2016
@mhegazy
Merge pull request #6885 from AbubakerB/constructorAccessibility
bde20c4 
Constructor Visibility
@mhegazy
Mohamed Hegazy (mhegazy) merged commit bde20c4 into microsoft:master Feb 17, 2016
@mhegazy

Mohamed Hegazy (mhegazy) commented Feb 17, 2016

Copy link
Copy Markdown
Contributor

#7058 should be a 1-line fix as well if you are interested.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mhegazy

Mohamed Hegazy (mhegazy) commented Feb 17, 2016

Copy link
Copy Markdown
Contributor

Abubaker Bashir (@AbubakerB), Since you implemented this feature, do you mind adding some documentation at https://github.com/Microsoft/TypeScript-Handbook/blob/master/pages/Classes.md?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mhegazy Mohamed Hegazy (mhegazy) mentioned this pull request Feb 17, 2016
Allow visibility on constructors #2341
Closed
@AbubakerB

Abubaker Bashir (AbubakerB) commented Feb 17, 2016

Copy link
Copy Markdown
Contributor Author

Thanks guys ⛄ !

Sure, no problem
RyanQuackenbush reacted with thumbs up emoji jimbasilio reacted with heart emoji
All reactions
  • 👍 1 reaction
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@myitcv

Paul Jolly (myitcv) commented Feb 17, 2016

Copy link
Copy Markdown

This is a great change, thanks very much Abubaker Bashir (@AbubakerB) and all involved
basarat reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@AbubakerB Abubaker Bashir (AbubakerB) mentioned this pull request Feb 18, 2016
Add protected constructor topic microsoft/TypeScript-Handbook#158
Merged
@myitcv Paul Jolly (myitcv) mentioned this pull request Feb 29, 2016
getDocumentHighlights does not return uses of SFCs in JSX elements #7109
Closed
@mhegazy Mohamed Hegazy (mhegazy) mentioned this pull request Mar 18, 2016
error TS2385: Overload signatures must all be public, private or protected. Why? #7577
Closed
@mhegazy Mohamed Hegazy (mhegazy) mentioned this pull request Apr 26, 2016
Support final classes (non-subclassable) #8306
Closed
@jkillian Jason Killian (jkillian) mentioned this pull request May 24, 2016
member-access: modify docs palantir/tslint#1268
Merged
@lambertjamesd lambertjamesd mentioned this pull request Jun 29, 2016
Emit classes with private constructors. angular/clutz#229
Closed
@shiwano Shogo Iwano (shiwano) mentioned this pull request Sep 23, 2016
Bump TypeScript version to 2.0 shiwano/typhen#5
Closed
6 tasks done
@wiredprairie

Aaron (wiredprairie) commented Nov 14, 2016

Copy link
Copy Markdown

Was point 4 regarding signature assignability completed as part of this PR? If so, is there documentation with an example? I couldn't find anything that seemed connected or related in the TypeScript Handbook.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mtaran-google mtaran-google mentioned this pull request May 18, 2018
Support making a constructor private (while not making the whole class private) google/closure-compiler#2761
Open
@microsoft Microsoft (microsoft) locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

7 participants

@AbubakerB @msftclas @RyanCavanaugh @mhegazy @DanielRosenwasser @myitcv @wiredprairie
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.