Constructor Visibility #6885
Next
Next commit
Added private and protected modifiers to constructors
- Loading branch information
commit f869b41f144be3cbed1aa0ad3c66e125d57f9752
There are no files selected for viewing
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 |
|---|---|---|
|
|
@@ -5889,16 +5889,20 @@ namespace ts { | |
|
|
||
| const sourceSignatures = getSignaturesOfType(source, kind); | ||
| const targetSignatures = getSignaturesOfType(target, kind); | ||
| if (kind === SignatureKind.Construct && sourceSignatures.length && targetSignatures.length) { | ||
| if (isAbstractConstructorType(source) && !isAbstractConstructorType(target)) { | ||
| // An abstract constructor type is not assignable to a non-abstract constructor type | ||
| // as it would otherwise be possible to new an abstract class. Note that the assignablity | ||
| // check we perform for an extends clause excludes construct signatures from the target, | ||
| // so this check never proceeds. | ||
| if (reportErrors) { | ||
| reportError(Diagnostics.Cannot_assign_an_abstract_constructor_type_to_a_non_abstract_constructor_type); | ||
| } | ||
| return Ternary.False; | ||
| } | ||
| if (!constructorRelatedTo(sourceSignatures[0], targetSignatures[0], reportErrors)) { | ||
| return Ternary.False; | ||
| } | ||
| } | ||
|
|
||
| let result = Ternary.True; | ||
|
|
@@ -6052,6 +6056,32 @@ namespace ts { | |
| } | ||
| return Ternary.True; | ||
| } | ||
|
|
||
| function constructorRelatedTo(sourceSignature: Signature, targetSignature: Signature, reportErrors: boolean) { | ||
|
Member
There was a problem hiding this comment. It looks like a better name would be |
||
| if (sourceSignature && targetSignature && sourceSignature.declaration && targetSignature.declaration) { | ||
|
Member
There was a problem hiding this comment. When would
Member
There was a problem hiding this comment. Same with
Contributor
Author
There was a problem hiding this comment. There are cases where the declarations are |
||
| const sourceAccessibility = sourceSignature.declaration.flags & (NodeFlags.Private | NodeFlags.Protected); | ||
| const targetAccessibility = targetSignature.declaration.flags & (NodeFlags.Private | NodeFlags.Protected); | ||
|
|
||
| const isRelated = sourceAccessibility === targetAccessibility; | ||
| if (!isRelated && reportErrors) { | ||
| reportError(Diagnostics.Cannot_assign_a_0_constructor_type_to_a_1_constructor_type, flagsToString(sourceAccessibility), flagsToString(targetAccessibility)); | ||
| } | ||
|
|
||
| return isRelated; | ||
| } | ||
|
|
||
| return true; | ||
|
|
||
| function flagsToString(flags: NodeFlags) { | ||
|
Member
There was a problem hiding this comment. Move this outside the function and rename ( |
||
| if (flags === NodeFlags.Private) { | ||
| return "private"; | ||
| } | ||
| if (flags === NodeFlags.Protected) { | ||
| return "protected"; | ||
| } | ||
| return "public"; | ||
| } | ||
| } | ||
|
Member
There was a problem hiding this comment. If you don't get rid of the |
||
| } | ||
|
|
||
| // Return true if the given type is the constructor type for an abstract class | ||
|
|
@@ -10103,6 +10133,9 @@ namespace ts { | |
| // that the user will not add any. | ||
| const constructSignatures = getSignaturesOfType(expressionType, SignatureKind.Construct); | ||
| if (constructSignatures.length) { | ||
| if (!isConstructorAccessible(node, constructSignatures[0])) { | ||
| return resolveErrorCall(node); | ||
| } | ||
| return resolveCall(node, constructSignatures, candidatesOutArray); | ||
| } | ||
|
|
||
|
|
@@ -10123,6 +10156,37 @@ namespace ts { | |
| return resolveErrorCall(node); | ||
| } | ||
|
|
||
| function isConstructorAccessible(node: NewExpression, signature: Signature) { | ||
| if (!signature || !signature.declaration) { | ||
| return true; | ||
| } | ||
|
|
||
| const declaration = signature.declaration; | ||
| const flags = declaration.flags; | ||
|
|
||
| // Public constructor is accessible. | ||
| if (!(flags & (NodeFlags.Private | NodeFlags.Protected))) { | ||
| return true; | ||
| } | ||
|
|
||
| const declaringClass = <InterfaceType>getDeclaredTypeOfSymbol(declaration.parent.symbol); | ||
| const enclosingClassDeclaration = getContainingClass(node); | ||
| 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) { | ||
|
Contributor
There was a problem hiding this comment. 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
}
}
}
} |
||
| if (flags & NodeFlags.Private) { | ||
| error(node, Diagnostics.Constructor_of_type_0_is_private_and_only_accessible_within_class_1, signatureToString(signature), typeToString(declaringClass)); | ||
| } | ||
| if (flags & NodeFlags.Protected) { | ||
| error(node, Diagnostics.Constructor_of_type_0_is_protected_and_only_accessible_within_class_1, signatureToString(signature), typeToString(declaringClass)); | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| function resolveTaggedTemplateExpression(node: TaggedTemplateExpression, candidatesOutArray: Signature[]): Signature { | ||
| const tagType = checkExpression(node.tag); | ||
| const apparentType = getApparentType(tagType); | ||
|
|
@@ -12059,7 +12123,7 @@ namespace ts { | |
| error(o.name, Diagnostics.Overload_signatures_must_all_be_ambient_or_non_ambient); | ||
| } | ||
| else if (deviation & (NodeFlags.Private | NodeFlags.Protected)) { | ||
| error(o.name || o, Diagnostics.Overload_signatures_must_all_be_public_private_or_protected); | ||
| } | ||
| else if (deviation & NodeFlags.Abstract) { | ||
| error(o.name, Diagnostics.Overload_signatures_must_all_be_abstract_or_not_abstract); | ||
|
|
@@ -13928,6 +13992,7 @@ namespace ts { | |
| if (baseTypes.length && produceDiagnostics) { | ||
| const baseType = baseTypes[0]; | ||
| const staticBaseType = getBaseConstructorTypeOfClass(type); | ||
| checkBaseTypeAccessibility(staticBaseType, baseTypeNode); | ||
| checkSourceElement(baseTypeNode.expression); | ||
| if (baseTypeNode.typeArguments) { | ||
| forEach(baseTypeNode.typeArguments, checkSourceElement); | ||
|
|
@@ -13983,6 +14048,16 @@ namespace ts { | |
| } | ||
| } | ||
|
|
||
| function checkBaseTypeAccessibility(type: ObjectType, node: ExpressionWithTypeArguments) { | ||
| const signatures = getSignaturesOfType(type, SignatureKind.Construct); | ||
| if (signatures.length) { | ||
| const declaration = signatures[0].declaration; | ||
| if (declaration && declaration.flags & NodeFlags.Private) { | ||
| error(node, Diagnostics.Cannot_extend_private_class_0, (<Identifier>node.expression).text); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function getTargetSymbol(s: Symbol) { | ||
| // if symbol is instantiated its flags are not copied from the 'target' | ||
| // so we'll need to get back original 'target' symbol to work with correct set of flags | ||
|
|
@@ -16348,12 +16423,6 @@ namespace ts { | |
| if (flags & NodeFlags.Abstract) { | ||
| return grammarErrorOnNode(lastStatic, Diagnostics._0_modifier_cannot_appear_on_a_constructor_declaration, "abstract"); | ||
| } | ||
| else if (flags & NodeFlags.Async) { | ||
| return grammarErrorOnNode(lastAsync, Diagnostics._0_modifier_cannot_appear_on_a_constructor_declaration, "async"); | ||
| } | ||
|
|
||
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
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.
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?
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)