Skip to content

Commit 43c49b1

Browse files
ahejlsbergahejlsberg
committed
Properly handle private/protected properties in intersection types
1 parent b830dea commit 43c49b1

File tree

3 files changed

+117
-41
lines changed

3 files changed

+117
-41
lines changed

‎src/compiler/checker.ts

Lines changed: 107 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -3736,9 +3736,9 @@ namespace ts {
37363736
return getObjectFlags(type) & ObjectFlags.Reference ? (<TypeReference>type).target : type;
37373737
}
37383738

3739-
function hasBaseType(type: BaseType, checkBase: BaseType) {
3739+
function hasBaseType(type: Type, checkBase: Type) {
37403740
return check(type);
3741-
function check(type: BaseType): boolean {
3741+
function check(type: Type): boolean {
37423742
if (getObjectFlags(type) & (ObjectFlags.ClassOrInterface | ObjectFlags.Reference)) {
37433743
const target = <InterfaceType>getTargetType(type);
37443744
return target === checkBase || forEach(getBaseTypes(target), check);
@@ -4939,29 +4939,33 @@ namespace ts {
49394939
}
49404940

49414941
function createUnionOrIntersectionProperty(containingType: UnionOrIntersectionType, name: string): Symbol {
4942-
const types = containingType.types;
4943-
const excludeModifiers = containingType.flags & TypeFlags.Union ? ModifierFlags.Private | ModifierFlags.Protected : 0;
49444942
let props: Symbol[];
4943+
const types = containingType.types;
4944+
const isUnion = containingType.flags & TypeFlags.Union;
4945+
const excludeModifiers = isUnion ? ModifierFlags.NonPublicAccessibilityModifier : 0;
49454946
// Flags we want to propagate to the result if they exist in all source symbols
4946-
let commonFlags = (containingType.flags & TypeFlags.Intersection) ? SymbolFlags.Optional : SymbolFlags.None;
4947+
let commonFlags = isUnion ? SymbolFlags.None : SymbolFlags.Optional;
49474948
let checkFlags = CheckFlags.SyntheticProperty;
49484949
for (const current of types) {
49494950
const type = getApparentType(current);
49504951
if (type !== unknownType) {
49514952
const prop = getPropertyOfType(type, name);
4952-
if (prop && !(getDeclarationModifierFlagsFromSymbol(prop) & excludeModifiers)) {
4953+
const modifiers = prop ? getDeclarationModifierFlagsFromSymbol(prop) : 0;
4954+
if (prop && !(modifiers & excludeModifiers)) {
49534955
commonFlags &= prop.flags;
49544956
if (!props) {
49554957
props = [prop];
49564958
}
49574959
else if (!contains(props, prop)) {
49584960
props.push(prop);
49594961
}
4960-
if (isReadonlySymbol(prop)) {
4961-
checkFlags |= CheckFlags.Readonly;
4962-
}
4962+
checkFlags |= (isReadonlySymbol(prop) ? CheckFlags.Readonly : 0) |
4963+
(!(modifiers & ModifierFlags.NonPublicAccessibilityModifier) ? CheckFlags.ContainsPublic : 0) |
4964+
(modifiers & ModifierFlags.Protected ? CheckFlags.ContainsProtected : 0) |
4965+
(modifiers & ModifierFlags.Private ? CheckFlags.ContainsPrivate : 0) |
4966+
(modifiers & ModifierFlags.Static ? CheckFlags.ContainsStatic : 0);
49634967
}
4964-
else if (containingType.flags & TypeFlags.Union) {
4968+
else if (isUnion) {
49654969
checkFlags |= CheckFlags.Partial;
49664970
}
49674971
}
@@ -4992,7 +4996,7 @@ namespace ts {
49924996
result.checkFlags = checkFlags;
49934997
result.containingType = containingType;
49944998
result.declarations = declarations;
4995-
result.type = containingType.flags & TypeFlags.Union ? getUnionType(propTypes) : getIntersectionType(propTypes);
4999+
result.type = isUnion ? getUnionType(propTypes) : getIntersectionType(propTypes);
49965000
return result;
49975001
}
49985002

@@ -6066,7 +6070,10 @@ namespace ts {
60666070
if (type.flags & TypeFlags.Union && typeSet.unionIndex === undefined) {
60676071
typeSet.unionIndex = typeSet.length;
60686072
}
6069-
typeSet.push(type);
6073+
if (!(type.flags & TypeFlags.Object && (<ObjectType>type).objectFlags & ObjectFlags.Anonymous &&
6074+
type.symbol && type.symbol.flags & (SymbolFlags.Function | SymbolFlags.Method) && containsIdenticalType(typeSet, type))) {
6075+
typeSet.push(type);
6076+
}
60706077
}
60716078
}
60726079

@@ -7950,6 +7957,12 @@ namespace ts {
79507957
const sourcePropFlags = getDeclarationModifierFlagsFromSymbol(sourceProp);
79517958
const targetPropFlags = getDeclarationModifierFlagsFromSymbol(targetProp);
79527959
if (sourcePropFlags & ModifierFlags.Private || targetPropFlags & ModifierFlags.Private) {
7960+
if (getCheckFlags(sourceProp) & CheckFlags.ContainsPrivate) {
7961+
if (reportErrors) {
7962+
reportError(Diagnostics.Property_0_has_conflicting_declarations_and_is_inaccessible_in_type_1, symbolToString(sourceProp), typeToString(source));
7963+
}
7964+
return Ternary.False;
7965+
}
79537966
if (sourceProp.valueDeclaration !== targetProp.valueDeclaration) {
79547967
if (reportErrors) {
79557968
if (sourcePropFlags & ModifierFlags.Private && targetPropFlags & ModifierFlags.Private) {
@@ -7965,13 +7978,10 @@ namespace ts {
79657978
}
79667979
}
79677980
else if (targetPropFlags & ModifierFlags.Protected) {
7968-
const sourceDeclaredInClass = sourceProp.parent && sourceProp.parent.flags & SymbolFlags.Class;
7969-
const sourceClass = sourceDeclaredInClass ? <InterfaceType>getDeclaredTypeOfSymbol(getParentOfSymbol(sourceProp)) : undefined;
7970-
const targetClass = <InterfaceType>getDeclaredTypeOfSymbol(getParentOfSymbol(targetProp));
7971-
if (!sourceClass || !hasBaseType(sourceClass, targetClass)) {
7981+
if (!isValidOverrideOf(sourceProp, targetProp)) {
79727982
if (reportErrors) {
7973-
reportError(Diagnostics.Property_0_is_protected_but_type_1_is_not_a_class_derived_from_2,
7974-
symbolToString(targetProp), typeToString(sourceClass || source), typeToString(targetClass));
7983+
reportError(Diagnostics.Property_0_is_protected_but_type_1_is_not_a_class_derived_from_2, symbolToString(targetProp),
7984+
typeToString(getDeclaringClass(sourceProp) || source), typeToString(getDeclaringClass(targetProp) || target));
79757985
}
79767986
return Ternary.False;
79777987
}
@@ -8213,6 +8223,49 @@ namespace ts {
82138223
}
82148224
}
82158225

8226+
// Invoke the callback for each underlying property symbol of the given symbol and return the first
8227+
// value that isn't undefined.
8228+
function forEachProperty<T>(prop: Symbol, callback: (p: Symbol) => T): T {
8229+
if (getCheckFlags(prop) & CheckFlags.SyntheticProperty) {
8230+
for (const t of (<TransientSymbol>prop).containingType.types) {
8231+
const p = getPropertyOfType(t, prop.name);
8232+
const result = p && forEachProperty(p, callback);
8233+
if (result) {
8234+
return result;
8235+
}
8236+
}
8237+
return undefined;
8238+
}
8239+
return callback(prop);
8240+
}
8241+
8242+
// Return the declaring class type of a property or undefined if property not declared in class
8243+
function getDeclaringClass(prop: Symbol) {
8244+
return prop.parent && prop.parent.flags & SymbolFlags.Class ? getDeclaredTypeOfSymbol(getParentOfSymbol(prop)) : undefined;
8245+
}
8246+
8247+
// Return true if some underlying source property is declared in a class that derives
8248+
// from the given base class.
8249+
function isPropertyInClassDerivedFrom(prop: Symbol, baseClass: Type) {
8250+
return forEachProperty(prop, sp => {
8251+
const sourceClass = getDeclaringClass(sp);
8252+
return sourceClass ? hasBaseType(sourceClass, baseClass) : false;
8253+
});
8254+
}
8255+
8256+
// Return true if source property is a valid override of protected parts of target property.
8257+
function isValidOverrideOf(sourceProp: Symbol, targetProp: Symbol) {
8258+
return !forEachProperty(targetProp, tp => getDeclarationModifierFlagsFromSymbol(tp) & ModifierFlags.Protected ?
8259+
!isPropertyInClassDerivedFrom(sourceProp, getDeclaringClass(tp)) : false);
8260+
}
8261+
8262+
// Return true if the given class derives from each of the declaring classes of the protected
8263+
// constituents of the given property.
8264+
function isClassDerivedFromDeclaringClasses(checkClass: Type, prop: Symbol) {
8265+
return forEachProperty(prop, p => getDeclarationModifierFlagsFromSymbol(p) & ModifierFlags.Protected ?
8266+
!hasBaseType(checkClass, getDeclaringClass(p)) : false) ? undefined : checkClass;
8267+
}
8268+
82168269
// Return true if the given type is the constructor type for an abstract class
82178270
function isAbstractConstructorType(type: Type) {
82188271
if (getObjectFlags(type) & ObjectFlags.Anonymous) {
@@ -12369,7 +12422,22 @@ namespace ts {
1236912422
}
1237012423

1237112424
function getDeclarationModifierFlagsFromSymbol(s: Symbol): ModifierFlags {
12372-
return s.valueDeclaration ? getCombinedModifierFlags(s.valueDeclaration) : s.flags & SymbolFlags.Prototype ? ModifierFlags.Public | ModifierFlags.Static : 0;
12425+
if (s.valueDeclaration) {
12426+
const flags = getCombinedModifierFlags(s.valueDeclaration);
12427+
return s.parent && s.parent.flags & SymbolFlags.Class ? flags : flags & ~ModifierFlags.AccessibilityModifier;
12428+
}
12429+
if (getCheckFlags(s) & CheckFlags.SyntheticProperty) {
12430+
const checkFlags = (<TransientSymbol>s).checkFlags;
12431+
const accessModifier = checkFlags & CheckFlags.ContainsPrivate ? ModifierFlags.Private :
12432+
checkFlags & CheckFlags.ContainsPublic ? ModifierFlags.Public :
12433+
ModifierFlags.Protected;
12434+
const staticModifier = checkFlags & CheckFlags.ContainsStatic ? ModifierFlags.Static : 0;
12435+
return accessModifier | staticModifier;
12436+
}
12437+
if (s.flags & SymbolFlags.Prototype) {
12438+
return ModifierFlags.Public | ModifierFlags.Static;
12439+
}
12440+
return 0;
1237312441
}
1237412442

1237512443
function getDeclarationNodeFlagsFromSymbol(s: Symbol): NodeFlags {
@@ -12384,12 +12452,18 @@ namespace ts {
1238412452
* @param type The type of left.
1238512453
* @param prop The symbol for the right hand side of the property access.
1238612454
*/
12387-
function checkClassPropertyAccess(node: PropertyAccessExpression | QualifiedName | VariableLikeDeclaration, left: Expression | QualifiedName, type: Type, prop: Symbol): boolean {
12455+
function checkPropertyAccessibility(node: PropertyAccessExpression | QualifiedName | VariableLikeDeclaration, left: Expression | QualifiedName, type: Type, prop: Symbol): boolean {
1238812456
const flags = getDeclarationModifierFlagsFromSymbol(prop);
12389-
const declaringClass = <InterfaceType>getDeclaredTypeOfSymbol(getParentOfSymbol(prop));
1239012457
const errorNode = node.kind === SyntaxKind.PropertyAccessExpression || node.kind === SyntaxKind.VariableDeclaration ?
1239112458
(<PropertyAccessExpression | VariableDeclaration>node).name :
1239212459
(<QualifiedName>node).right;
12460+
12461+
if (getCheckFlags(prop) & CheckFlags.ContainsPrivate) {
12462+
// Synthetic property with private constituent property
12463+
error(errorNode, Diagnostics.Property_0_has_conflicting_declarations_and_is_inaccessible_in_type_1, symbolToString(prop), typeToString(type));
12464+
return false;
12465+
}
12466+
1239312467
if (left.kind === SyntaxKind.SuperKeyword) {
1239412468
// TS 1.0 spec (April 2014): 4.8.2
1239512469
// - In a constructor, instance member function, instance member accessor, or
@@ -12408,14 +12482,12 @@ namespace ts {
1240812482
return false;
1240912483
}
1241012484
}
12411-
1241212485
if (flags & ModifierFlags.Abstract) {
1241312486
// A method cannot be accessed in a super property access if the method is abstract.
1241412487
// This error could mask a private property access error. But, a member
1241512488
// cannot simultaneously be private and abstract, so this will trigger an
1241612489
// additional error elsewhere.
12417-
12418-
error(errorNode, Diagnostics.Abstract_method_0_in_class_1_cannot_be_accessed_via_super_expression, symbolToString(prop), typeToString(declaringClass));
12490+
error(errorNode, Diagnostics.Abstract_method_0_in_class_1_cannot_be_accessed_via_super_expression, symbolToString(prop), typeToString(getDeclaringClass(prop)));
1241912491
return false;
1242012492
}
1242112493
}
@@ -12431,7 +12503,7 @@ namespace ts {
1243112503
if (flags & ModifierFlags.Private) {
1243212504
const declaringClassDeclaration = <ClassLikeDeclaration>getClassLikeDeclarationOfSymbol(getParentOfSymbol(prop));
1243312505
if (!isNodeWithinClass(node, declaringClassDeclaration)) {
12434-
error(errorNode, Diagnostics.Property_0_is_private_and_only_accessible_within_class_1, symbolToString(prop), typeToString(declaringClass));
12506+
error(errorNode, Diagnostics.Property_0_is_private_and_only_accessible_within_class_1, symbolToString(prop), typeToString(getDeclaringClass(prop)));
1243512507
return false;
1243612508
}
1243712509
return true;
@@ -12444,15 +12516,15 @@ namespace ts {
1244412516
return true;
1244512517
}
1244612518

12447-
// Get the enclosing class that has the declaring class as its base type
12519+
// Find the first enclosing class that has the declaring classes of the protected constituents
12520+
// of the property as base classes
1244812521
const enclosingClass = forEachEnclosingClass(node, enclosingDeclaration => {
1244912522
const enclosingClass = <InterfaceType>getDeclaredTypeOfSymbol(getSymbolOfNode(enclosingDeclaration));
12450-
return hasBaseType(enclosingClass, declaringClass) ? enclosingClass : undefined;
12523+
return isClassDerivedFromDeclaringClasses(enclosingClass, prop) ? enclosingClass : undefined;
1245112524
});
12452-
1245312525
// A protected property is accessible if the property is within the declaring class or classes derived from it
1245412526
if (!enclosingClass) {
12455-
error(errorNode, Diagnostics.Property_0_is_protected_and_only_accessible_within_class_1_and_its_subclasses, symbolToString(prop), typeToString(declaringClass));
12527+
error(errorNode, Diagnostics.Property_0_is_protected_and_only_accessible_within_class_1_and_its_subclasses, symbolToString(prop), typeToString(getDeclaringClass(prop) || type));
1245612528
return false;
1245712529
}
1245812530
// No further restrictions for static properties
@@ -12464,9 +12536,7 @@ namespace ts {
1246412536
// get the original type -- represented as the type constraint of the 'this' type
1246512537
type = getConstraintOfTypeParameter(<TypeParameter>type);
1246612538
}
12467-
12468-
// TODO: why is the first part of this check here?
12469-
if (!(getObjectFlags(getTargetType(type)) & ObjectFlags.ClassOrInterface && hasBaseType(<InterfaceType>type, enclosingClass))) {
12539+
if (!(getObjectFlags(getTargetType(type)) & ObjectFlags.ClassOrInterface && hasBaseType(type, enclosingClass))) {
1247012540
error(errorNode, Diagnostics.Property_0_is_protected_and_only_accessible_through_an_instance_of_class_1, symbolToString(prop), typeToString(enclosingClass));
1247112541
return false;
1247212542
}
@@ -12553,9 +12623,7 @@ namespace ts {
1255312623

1255412624
getNodeLinks(node).resolvedSymbol = prop;
1255512625

12556-
if (prop.parent && prop.parent.flags & SymbolFlags.Class) {
12557-
checkClassPropertyAccess(node, left, apparentType, prop);
12558-
}
12626+
checkPropertyAccessibility(node, left, apparentType, prop);
1255912627

1256012628
const propType = getTypeOfSymbol(prop);
1256112629
const assignmentKind = getAssignmentTargetKind(node);
@@ -12587,8 +12655,8 @@ namespace ts {
1258712655
const type = checkExpression(left);
1258812656
if (type !== unknownType && !isTypeAny(type)) {
1258912657
const prop = getPropertyOfType(getWidenedType(type), propertyName);
12590-
if (prop && prop.parent && prop.parent.flags & SymbolFlags.Class) {
12591-
return checkClassPropertyAccess(node, left, type, prop);
12658+
if (prop) {
12659+
return checkPropertyAccessibility(node, left, type, prop);
1259212660
}
1259312661
}
1259412662
return true;
@@ -17527,8 +17595,8 @@ namespace ts {
1752717595
const name = node.propertyName || <Identifier>node.name;
1752817596
const property = getPropertyOfType(parentType, getTextOfPropertyName(name));
1752917597
markPropertyAsReferenced(property);
17530-
if (parent.initializer && property && getParentOfSymbol(property)) {
17531-
checkClassPropertyAccess(parent, parent.initializer, parentType, property);
17598+
if (parent.initializer && property) {
17599+
checkPropertyAccessibility(parent, parent.initializer, parentType, property);
1753217600
}
1753317601
}
1753417602

‎src/compiler/diagnosticMessages.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1787,6 +1787,10 @@
17871787
"category": "Error",
17881788
"code": 2545
17891789
},
1790+
"Property '{0}' has conflicting declarations and is inaccessible in type '{1}'.": {
1791+
"category": "Error",
1792+
"code": 2546
1793+
},
17901794
"JSX element attributes type '{0}' may not be a union type.": {
17911795
"category": "Error",
17921796
"code": 2600

‎src/compiler/types.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2727,8 +2727,12 @@
27272727
Instantiated = 1 << 0, // Instantiated symbol
27282728
SyntheticProperty = 1 << 1, // Property in union or intersection type
27292729
Readonly = 1 << 2, // Readonly transient symbol
2730-
Partial = 1 << 3, // Property present in some but not all constituents
2731-
HasNonUniformType = 1 << 4, // Constituents have non-uniform types
2730+
Partial = 1 << 3, // Synthetic property present in some but not all constituents
2731+
HasNonUniformType = 1 << 4, // Synthetic property with non-uniform type in constituents
2732+
ContainsPublic = 1 << 5, // Synthetic property with public constituent(s)
2733+
ContainsProtected = 1 << 6, // Synthetic property with protected constituent(s)
2734+
ContainsPrivate = 1 << 7, // Synthetic property with private constituent(s)
2735+
ContainsStatic = 1 << 8, // Synthetic property with static constituent(s)
27322736
}
27332737

27342738
/* @internal */

0 commit comments

Comments
 (0)