Skip to content

Commit 4ffa810

Browse files
gabrittogabritto
committed
Fix comments
1 parent 2546be8 commit 4ffa810

File tree

2 files changed

+114
-99
lines changed

2 files changed

+114
-99
lines changed

‎src/compiler/checker.ts

Lines changed: 113 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -263,30 +263,6 @@ namespace ts {
263263
Uncapitalize: IntrinsicTypeKind.Uncapitalize
264264
}));
265265

266-
enum TypeChecks {
267-
Ok,
268-
HasDiagnostic
269-
}
270-
271-
interface CheckerResultOk {
272-
result: TypeChecks.Ok
273-
}
274-
275-
interface CheckerResultHasDiagnostic {
276-
result: TypeChecks.HasDiagnostic,
277-
message: DiagnosticMessage;
278-
args:
279-
| never[]
280-
| [string | number]
281-
| [string | number, string | number]
282-
| [string | number, string | number, string | number]
283-
| [string | number, string | number, string | number, string | number];
284-
}
285-
286-
type CheckerResult =
287-
| CheckerResultOk
288-
| CheckerResultHasDiagnostic;
289-
290266
function SymbolLinks(this: SymbolLinks) {
291267
}
292268

@@ -736,9 +712,7 @@ namespace ts {
736712

737713
getLocalTypeParametersOfClassOrInterfaceOrTypeAlias,
738714
isDeclarationVisible,
739-
isPropertyAccessible: (node, isSuper, writing, type, prop) => {
740-
return checkPropertyAccessibilityAtNode(node, isSuper, writing, type, prop).result === TypeChecks.Ok;
741-
}
715+
isPropertyAccessible,
742716
};
743717

744718
function getResolvedSignatureWorker(nodeIn: CallLikeExpression, candidatesOutArray: Signature[] | undefined, argumentCount: number | undefined, checkMode: CheckMode): Signature | undefined {
@@ -27293,35 +27267,28 @@ namespace ts {
2729327267
node: PropertyAccessExpression | QualifiedName | PropertyAccessExpression | VariableDeclaration | ParameterDeclaration | ImportTypeNode | PropertyAssignment | ShorthandPropertyAssignment | BindingElement,
2729427268
isSuper: boolean, writing: boolean, type: Type, prop: Symbol, reportError = true): boolean {
2729527269

27296-
const errorNode = node.kind === SyntaxKind.QualifiedName ? node.right :
27270+
const errorNode = !reportError ? undefined : node.kind === SyntaxKind.QualifiedName ? node.right :
2729727271
node.kind === SyntaxKind.ImportType ? node :
2729827272
node.kind === SyntaxKind.BindingElement && node.propertyName ? node.propertyName : node.name;
2729927273

27300-
const typeChecks = checkPropertyAccessibilityAtNode(node, isSuper, writing, type, prop);
27301-
switch (typeChecks.result) {
27302-
case (TypeChecks.Ok):
27303-
return true;
27304-
case (TypeChecks.HasDiagnostic):
27305-
if (reportError) {
27306-
error(errorNode, typeChecks.message, ...typeChecks.args);
27307-
}
27308-
return false;
27309-
}
27274+
return checkPropertyAccessibilityAtLocation(node, isSuper, writing, type, prop, errorNode);
2731027275
}
2731127276

27312-
// Possible concerns:
27313-
// (1) I'm not sure if this function has the right behavior if `node` is allowed to be any node,
27314-
// although we only use `node` for its location in the parse tree.
27315-
// (2) Does it even make sense to check for property accessibility at a certain arbitrary node?
27316-
// Maybe this function should be called "checkPropertyVisibilityAtNode" or something else.
27317-
function checkPropertyAccessibilityAtNode(node: Node,
27277+
/**
27278+
* Check whether the requested property can be accessed at the requested location.
27279+
* Returns true if node is a valid property access, and false otherwise.
27280+
* @param location The location node where we want to check if the property is accessible.
27281+
* @param isSuper True if the access is from `super.`.
27282+
* @param writing True if this is a write property access, false if it is a read property access.
27283+
* @param type The type of the object whose property is being accessed. (Not the type of the property.)
27284+
* @param prop The symbol for the property being accessed.
27285+
* @param errorNode The node where we should report an invalid property access error, or undefined if we should not report errors.
27286+
*/
27287+
function checkPropertyAccessibilityAtLocation(location: Node,
2731827288
isSuper: boolean, writing: boolean,
27319-
type: Type, prop: Symbol): CheckerResult {
27320-
const flags = getDeclarationModifierFlagsFromSymbol(prop, writing);
27289+
type: Type, prop: Symbol, errorNode?: Node): boolean {
2732127290

27322-
const checkerOk: CheckerResultOk = {
27323-
result: TypeChecks.Ok
27324-
};
27291+
const flags = getDeclarationModifierFlagsFromSymbol(prop, writing);
2732527292

2732627293
if (isSuper) {
2732727294
// TS 1.0 spec (April 2014): 4.8.2
@@ -27333,69 +27300,74 @@ namespace ts {
2733327300
// a super property access is permitted and must specify a public static member function of the base class.
2733427301
if (languageVersion < ScriptTarget.ES2015) {
2733527302
if (symbolHasNonMethodDeclaration(prop)) {
27336-
return {
27337-
result: TypeChecks.HasDiagnostic,
27338-
message: Diagnostics.Only_public_and_protected_methods_of_the_base_class_are_accessible_via_the_super_keyword,
27339-
args: emptyArray
27340-
};
27303+
if (errorNode) {
27304+
error(errorNode, Diagnostics.Only_public_and_protected_methods_of_the_base_class_are_accessible_via_the_super_keyword);
27305+
}
27306+
return false;
2734127307
}
2734227308
}
2734327309
if (flags & ModifierFlags.Abstract) {
2734427310
// A method cannot be accessed in a super property access if the method is abstract.
2734527311
// This error could mask a private property access error. But, a member
2734627312
// cannot simultaneously be private and abstract, so this will trigger an
2734727313
// additional error elsewhere.
27348-
return {
27349-
result: TypeChecks.HasDiagnostic,
27350-
message: Diagnostics.Abstract_method_0_in_class_1_cannot_be_accessed_via_super_expression,
27351-
args: [symbolToString(prop), typeToString(getDeclaringClass(prop)!)]
27352-
};
27314+
if (errorNode) {
27315+
error(errorNode,
27316+
Diagnostics.Abstract_method_0_in_class_1_cannot_be_accessed_via_super_expression,
27317+
symbolToString(prop),
27318+
typeToString(getDeclaringClass(prop)!));
27319+
}
27320+
return false;
2735327321
}
2735427322
}
2735527323

2735627324
// Referencing abstract properties within their own constructors is not allowed
2735727325
if ((flags & ModifierFlags.Abstract) && symbolHasNonMethodDeclaration(prop) &&
27358-
(isThisProperty(node) || isThisInitializedObjectBindingExpression(node) || isObjectBindingPattern(node.parent) && isThisInitializedDeclaration(node.parent.parent))) {
27326+
(isThisProperty(location) || isThisInitializedObjectBindingExpression(location) || isObjectBindingPattern(location.parent) && isThisInitializedDeclaration(location.parent.parent))) {
2735927327
const declaringClassDeclaration = getClassLikeDeclarationOfSymbol(getParentOfSymbol(prop)!);
27360-
if (declaringClassDeclaration && isNodeUsedDuringClassInitialization(node)) {
27361-
return {
27362-
result: TypeChecks.HasDiagnostic,
27363-
message: Diagnostics.Abstract_property_0_in_class_1_cannot_be_accessed_in_the_constructor,
27364-
args: [symbolToString(prop), getTextOfIdentifierOrLiteral(declaringClassDeclaration.name!)]
27365-
};
27328+
if (declaringClassDeclaration && isNodeUsedDuringClassInitialization(location)) {
27329+
if (errorNode) {
27330+
error(errorNode,
27331+
Diagnostics.Abstract_property_0_in_class_1_cannot_be_accessed_in_the_constructor,
27332+
symbolToString(prop),
27333+
getTextOfIdentifierOrLiteral(declaringClassDeclaration.name!));
27334+
}
27335+
return false;
2736627336
}
2736727337
}
2736827338

2736927339
// Public properties are otherwise accessible.
2737027340
if (!(flags & ModifierFlags.NonPublicAccessibilityModifier)) {
27371-
return checkerOk;
27341+
return true;
2737227342
}
2737327343

2737427344
// Property is known to be private or protected at this point
2737527345

2737627346
// Private property is accessible if the property is within the declaring class
2737727347
if (flags & ModifierFlags.Private) {
2737827348
const declaringClassDeclaration = getClassLikeDeclarationOfSymbol(getParentOfSymbol(prop)!)!;
27379-
if (!isNodeWithinClass(node, declaringClassDeclaration)) {
27380-
return {
27381-
result: TypeChecks.HasDiagnostic,
27382-
message: Diagnostics.Property_0_is_private_and_only_accessible_within_class_1,
27383-
args: [symbolToString(prop), typeToString(getDeclaringClass(prop)!)]
27384-
};
27349+
if (!isNodeWithinClass(location, declaringClassDeclaration)) {
27350+
if (errorNode) {
27351+
error(errorNode,
27352+
Diagnostics.Property_0_is_private_and_only_accessible_within_class_1,
27353+
symbolToString(prop),
27354+
typeToString(getDeclaringClass(prop)!));
27355+
}
27356+
return false;
2738527357
}
27386-
return checkerOk;
27358+
return true;
2738727359
}
2738827360

2738927361
// Property is known to be protected at this point
2739027362

2739127363
// All protected properties of a supertype are accessible in a super access
2739227364
if (isSuper) {
27393-
return checkerOk;
27365+
return true;
2739427366
}
2739527367

2739627368
// Find the first enclosing class that has the declaring classes of the protected constituents
2739727369
// of the property as base classes
27398-
let enclosingClass = forEachEnclosingClass(node, enclosingDeclaration => {
27370+
let enclosingClass = forEachEnclosingClass(location, enclosingDeclaration => {
2739927371
const enclosingClass = getDeclaredTypeOfSymbol(getSymbolOfNode(enclosingDeclaration)!) as InterfaceType;
2740027372
return isClassDerivedFromDeclaringClasses(enclosingClass, prop, writing) ? enclosingClass : undefined;
2740127373
});
@@ -27404,33 +27376,36 @@ namespace ts {
2740427376
// allow PropertyAccessibility if context is in function with this parameter
2740527377
// static member access is disallow
2740627378
let thisParameter: ParameterDeclaration | undefined;
27407-
if (flags & ModifierFlags.Static || !(thisParameter = getThisParameterFromNodeContext(node)) || !thisParameter.type) {
27408-
return {
27409-
result: TypeChecks.HasDiagnostic,
27410-
message: Diagnostics.Property_0_is_protected_and_only_accessible_within_class_1_and_its_subclasses,
27411-
args: [symbolToString(prop), typeToString(getDeclaringClass(prop) || type)]
27412-
};
27379+
if (flags & ModifierFlags.Static || !(thisParameter = getThisParameterFromNodeContext(location)) || !thisParameter.type) {
27380+
if (errorNode) {
27381+
error(errorNode,
27382+
Diagnostics.Property_0_is_protected_and_only_accessible_within_class_1_and_its_subclasses,
27383+
symbolToString(prop),
27384+
typeToString(getDeclaringClass(prop) || type));
27385+
}
27386+
return false;
2741327387
}
2741427388

2741527389
const thisType = getTypeFromTypeNode(thisParameter.type);
2741627390
enclosingClass = (((thisType.flags & TypeFlags.TypeParameter) ? getConstraintOfTypeParameter(thisType as TypeParameter) : thisType) as TypeReference).target;
2741727391
}
2741827392
// No further restrictions for static properties
2741927393
if (flags & ModifierFlags.Static) {
27420-
return checkerOk;
27394+
return true;
2742127395
}
2742227396
if (type.flags & TypeFlags.TypeParameter) {
2742327397
// get the original type -- represented as the type constraint of the 'this' type
2742427398
type = (type as TypeParameter).isThisType ? getConstraintOfTypeParameter(type as TypeParameter)! : getBaseConstraintOfType(type as TypeParameter)!; // TODO: GH#18217 Use a different variable that's allowed to be undefined
2742527399
}
2742627400
if (!type || !hasBaseType(type, enclosingClass)) {
27427-
return {
27428-
result: TypeChecks.HasDiagnostic,
27429-
message: Diagnostics.Property_0_is_protected_and_only_accessible_through_an_instance_of_class_1_This_is_an_instance_of_class_2,
27430-
args: [symbolToString(prop), typeToString(enclosingClass), typeToString(type)]
27431-
};
27401+
if (errorNode) {
27402+
error(errorNode,
27403+
Diagnostics.Property_0_is_protected_and_only_accessible_through_an_instance_of_class_1_This_is_an_instance_of_class_2,
27404+
symbolToString(prop), typeToString(enclosingClass), typeToString(type));
27405+
}
27406+
return false;
2743227407
}
27433-
return checkerOk;
27408+
return true;
2743427409
}
2743527410

2743627411
function getThisParameterFromNodeContext(node: Node) {
@@ -28150,8 +28125,22 @@ namespace ts {
2815028125
}
2815128126
}
2815228127

28128+
/**
28129+
* Checks if an existing property access is valid for completions purposes.
28130+
* @param node a property access-like node where we want to check if we can access a property.
28131+
* This node does not need to be an access of the property we are checking.
28132+
* e.g. in completions, this node will often be an incomplete property access node, as in `foo.`.
28133+
* Besides providing a location (i.e. scope) used to check property accessibility, we use this node for
28134+
* computing whether this is a `super` property access.
28135+
* @param type the type whose property we are checking
28136+
* @param property the accessed property's symbol
28137+
*/
2815328138
function isValidPropertyAccessForCompletions(node: PropertyAccessExpression | ImportTypeNode | QualifiedName, type: Type, property: Symbol): boolean {
28154-
return isValidPropertyAccessWithType(node, node.kind === SyntaxKind.PropertyAccessExpression && node.expression.kind === SyntaxKind.SuperKeyword, property.escapedName, type);
28139+
return isPropertyAccessible(node,
28140+
node.kind === SyntaxKind.PropertyAccessExpression && node.expression.kind === SyntaxKind.SuperKeyword,
28141+
/* isWrite */ false,
28142+
type,
28143+
property);
2815528144
// Previously we validated the 'this' type of methods but this adversely affected performance. See #31377 for more context.
2815628145
}
2815728146

@@ -28161,19 +28150,45 @@ namespace ts {
2816128150
propertyName: __String,
2816228151
type: Type): boolean {
2816328152

28153+
// Short-circuiting for improved performance.
2816428154
if (type === errorType || isTypeAny(type)) {
2816528155
return true;
2816628156
}
28157+
2816728158
const prop = getPropertyOfType(type, propertyName);
28168-
if (prop) {
28169-
if (prop.valueDeclaration && isPrivateIdentifierClassElementDeclaration(prop.valueDeclaration)) {
28170-
const declClass = getContainingClass(prop.valueDeclaration);
28171-
return !isOptionalChain(node) && !!findAncestor(node, parent => parent === declClass);
28172-
}
28173-
return checkPropertyAccessibility(node, isSuper, /*writing*/ false, type, prop, /* reportError */ false);
28159+
return !!prop && isPropertyAccessible(node, isSuper, /* isWrite */ false, type, prop);
28160+
}
28161+
28162+
/**
28163+
* Checks if a property can be accessed in a location.
28164+
* The location is given by the `node` parameter.
28165+
* The node does not need to be a property access.
28166+
* @param node location where to check property accessibility
28167+
* @param isSuper whether to consider this a `super` property access, e.g. `super.foo`.
28168+
* @param isWrite whether this is a write access, e.g. `++foo.x`.
28169+
* @param type type where the property comes from.
28170+
* @param property property symbol.
28171+
*/
28172+
function isPropertyAccessible(
28173+
node: Node,
28174+
isSuper: boolean,
28175+
isWrite: boolean,
28176+
type: Type,
28177+
property: Symbol): boolean {
28178+
28179+
// Short-circuiting for improved performance.
28180+
if (type === errorType || isTypeAny(type)) {
28181+
return true;
28182+
}
28183+
28184+
// A #private property access in an optional chain is an error dealt with by the parser.
28185+
// The checker does not check for it, so we need to do our own check here.
28186+
if (property.valueDeclaration && isPrivateIdentifierClassElementDeclaration(property.valueDeclaration)) {
28187+
const declClass = getContainingClass(property.valueDeclaration);
28188+
return !isOptionalChain(node) && !!findAncestor(node, parent => parent === declClass);
2817428189
}
28175-
// In js files properties of unions are allowed in completion
28176-
return isInJSFile(node) && (type.flags & TypeFlags.Union) !== 0 && (type as UnionType).types.some(elementType => isValidPropertyAccessWithType(node, isSuper, propertyName, elementType));
28190+
28191+
return checkPropertyAccessibilityAtLocation(node, isSuper, isWrite, type, property);
2817728192
}
2817828193

2817928194
/**

‎src/compiler/types.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4216,7 +4216,7 @@ namespace ts {
42164216

42174217
getConstantValue(node: EnumMember | PropertyAccessExpression | ElementAccessExpression): string | number | undefined;
42184218
isValidPropertyAccess(node: PropertyAccessExpression | QualifiedName | ImportTypeNode, propertyName: string): boolean;
4219-
/** Exclude accesses to private properties or methods with a `this` parameter that `type` doesn't satisfy. */
4219+
/** Exclude accesses to private properties. */
42204220
/* @internal */ isValidPropertyAccessForCompletions(node: PropertyAccessExpression | ImportTypeNode | QualifiedName, type: Type, property: Symbol): boolean;
42214221
/** Follow all aliases to get the original symbol. */
42224222
getAliasedSymbol(symbol: Symbol): Symbol;

0 commit comments

Comments
 (0)