Skip to content

Commit bd3a0e0

Browse files
mrgrainmrgrain
committed
feat: allow covariant-overrides
1 parent 7167a6f commit bd3a0e0

19 files changed

+550
-87
lines changed

‎package.json‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/transforms/deprecated-remover.ts‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,21 @@ import {
55
EnumMember,
66
Initializer,
77
InterfaceType,
8+
IntersectionTypeReference,
89
isClassOrInterfaceType,
910
isClassType,
1011
isCollectionTypeReference,
1112
isEnumType,
1213
isMethod,
1314
isNamedTypeReference,
1415
isPrimitiveTypeReference,
16+
isUnionTypeReference,
1517
Method,
1618
Parameter,
1719
Property,
1820
Stability,
1921
TypeReference,
22+
UnionTypeReference,
2023
} from '@jsii/spec';
2124
import * as ts from 'typescript';
2225

@@ -305,7 +308,10 @@ export class DeprecatedRemover {
305308
if (isCollectionTypeReference(ref)) {
306309
return this.tryFindReference(ref.collection.elementtype, fqns);
307310
}
308-
return ref.union.types.map((type) => this.tryFindReference(type, fqns)).find((typeRef) => typeRef != null);
311+
312+
return setTypeMembers(ref)
313+
.map((type) => this.tryFindReference(type, fqns))
314+
.find((typeRef) => typeRef != null);
309315
}
310316

311317
private shouldFqnBeStripped(fqn: string) {
@@ -770,3 +776,11 @@ class DeprecationRemovalTransformer {
770776
);
771777
}
772778
}
779+
780+
function setTypeMembers(x: UnionTypeReference | IntersectionTypeReference) {
781+
if (isUnionTypeReference(x)) {
782+
return x.union.types;
783+
} else {
784+
return x.intersection.types;
785+
}
786+
}

‎src/validator.ts‎

Lines changed: 164 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,9 @@ function _defaultValidations(): ValidationFunction[] {
220220
if (!overridden) {
221221
return _validateMethodOverride(method, baseType);
222222
}
223-
_assertSignaturesMatch(overridden, method, `${type.fqn}#${method.name}`, `overriding ${baseType.fqn}`);
223+
_assertSignaturesMatch(overridden, method, `${type.fqn}#${method.name}`, `overriding ${baseType.fqn}`, {
224+
allowCovariance: true,
225+
});
224226
method.overrides = baseType.fqn;
225227
return true;
226228
}
@@ -237,7 +239,9 @@ function _defaultValidations(): ValidationFunction[] {
237239
if (!overridden) {
238240
return _validatePropertyOverride(property, baseType);
239241
}
240-
_assertPropertiesMatch(overridden, property, `${type.fqn}#${property.name}`, `overriding ${baseType.fqn}`);
242+
_assertPropertiesMatch(overridden, property, `${type.fqn}#${property.name}`, `overriding ${baseType.fqn}`, {
243+
allowCovariance: true,
244+
});
241245
property.overrides = baseType.fqn;
242246
return true;
243247
}
@@ -254,7 +258,9 @@ function _defaultValidations(): ValidationFunction[] {
254258
const ifaceType = _dereference(iface, assembly, validator) as spec.InterfaceType;
255259
const implemented = (ifaceType.methods ?? []).find((m) => m.name === method.name);
256260
if (implemented) {
257-
_assertSignaturesMatch(implemented, method, `${type.fqn}#${method.name}`, `implementing ${ifaceType.fqn}`);
261+
_assertSignaturesMatch(implemented, method, `${type.fqn}#${method.name}`, `implementing ${ifaceType.fqn}`, {
262+
allowCovariance: false,
263+
});
258264
// We won't replace a previous overrides declaration from a method override, as those have
259265
// higher precedence than an initial implementation.
260266
method.overrides = method.overrides ?? iface;
@@ -290,6 +296,7 @@ function _defaultValidations(): ValidationFunction[] {
290296
property,
291297
`${type.fqn}#${property.name}`,
292298
`implementing ${ifaceType.fqn}`,
299+
{ allowCovariance: false },
293300
);
294301
// We won't replace a previous overrides declaration from a property override, as those
295302
// have higher precedence than an initial implementation.
@@ -303,7 +310,15 @@ function _defaultValidations(): ValidationFunction[] {
303310
return false;
304311
}
305312

306-
function _assertSignaturesMatch(expected: spec.Method, actual: spec.Method, label: string, action: string) {
313+
function _assertSignaturesMatch(
314+
expected: spec.Method,
315+
actual: spec.Method,
316+
label: string,
317+
action: string,
318+
opts: {
319+
allowCovariance: boolean;
320+
},
321+
) {
307322
if (!!expected.protected !== !!actual.protected) {
308323
const expVisibility = expected.protected ? 'protected' : 'public';
309324
const actVisibility = actual.protected ? 'protected' : 'public';
@@ -316,12 +331,24 @@ function _defaultValidations(): ValidationFunction[] {
316331
),
317332
);
318333
}
334+
335+
// Types must generally be the same, but can be covariant sometimes
319336
if (!deepEqual(actual.returns, expected.returns)) {
320-
const expType = spec.describeTypeReference(expected.returns?.type);
321-
const actType = spec.describeTypeReference(actual.returns?.type);
322-
diagnostic(
323-
JsiiDiagnostic.JSII_5003_OVERRIDE_CHANGES_RETURN_TYPE.createDetached(label, action, actType, expType),
324-
);
337+
const actualReturnType = actual.returns?.type;
338+
const expectedReturnType = expected.returns?.type;
339+
340+
if (
341+
// static members can never change
342+
actual.static ||
343+
// Check if this is a valid covariant return type (actual is more specific than expected)
344+
!(opts.allowCovariance && _isCovariantOf(actualReturnType, expectedReturnType))
345+
) {
346+
const expType = spec.describeTypeReference(expectedReturnType);
347+
const actType = spec.describeTypeReference(actualReturnType);
348+
diagnostic(
349+
JsiiDiagnostic.JSII_5003_OVERRIDE_CHANGES_RETURN_TYPE.createDetached(label, action, actType, expType),
350+
);
351+
}
325352
}
326353
const expectedParams = expected.parameters ?? [];
327354
const actualParams = actual.parameters ?? [];
@@ -363,7 +390,113 @@ function _defaultValidations(): ValidationFunction[] {
363390
}
364391
}
365392

366-
function _assertPropertiesMatch(expected: spec.Property, actual: spec.Property, label: string, action: string) {
393+
function _isCovariantOf(candidateType?: spec.TypeReference, expectedType?: spec.TypeReference): boolean {
394+
// one void, while other isn't => not covariant
395+
if ((candidateType === undefined) !== (expectedType === undefined)) {
396+
return false;
397+
}
398+
399+
// Same type is always covariant
400+
if (deepEqual(candidateType, expectedType)) {
401+
return true;
402+
}
403+
404+
if (!spec.isNamedTypeReference(candidateType) || !spec.isNamedTypeReference(expectedType)) {
405+
return false;
406+
}
407+
408+
const candidateTypeSpec = _dereference(candidateType.fqn, assembly, validator);
409+
const expectedTypeSpec = _dereference(expectedType.fqn, assembly, validator);
410+
411+
if (!candidateTypeSpec || !expectedTypeSpec) {
412+
return false;
413+
}
414+
415+
// Handle class-to-class inheritance
416+
if (spec.isClassType(candidateTypeSpec) && spec.isClassType(expectedTypeSpec)) {
417+
// Check if candidateType extends expectedType (directly or indirectly)
418+
return _classExtendsClass(candidateTypeSpec, expectedType.fqn);
419+
}
420+
421+
// Handle class implementing interface
422+
if (spec.isClassType(candidateTypeSpec) && spec.isInterfaceType(expectedTypeSpec)) {
423+
return _classImplementsInterface(candidateTypeSpec, expectedType.fqn);
424+
}
425+
426+
return false;
427+
}
428+
429+
function _classExtendsClass(classType: spec.ClassType, targetFqn: string): boolean {
430+
let current = classType;
431+
while (current.base) {
432+
if (current.base === targetFqn) {
433+
return true;
434+
}
435+
const baseType = _dereference(current.base, assembly, validator);
436+
if (!spec.isClassType(baseType)) {
437+
break;
438+
}
439+
current = baseType;
440+
}
441+
return false;
442+
}
443+
444+
function _classImplementsInterface(classType: spec.ClassType, interfaceFqn: string): boolean {
445+
// Check direct interfaces
446+
if (classType.interfaces?.includes(interfaceFqn)) {
447+
return true;
448+
}
449+
450+
// Check inherited interfaces
451+
if (classType.interfaces) {
452+
for (const iface of classType.interfaces) {
453+
const ifaceType = _dereference(iface, assembly, validator);
454+
if (spec.isInterfaceType(ifaceType) && _interfaceExtendsInterface(ifaceType, interfaceFqn)) {
455+
return true;
456+
}
457+
}
458+
}
459+
460+
// Check base class interfaces
461+
if (classType.base) {
462+
const baseType = _dereference(classType.base, assembly, validator);
463+
if (spec.isClassType(baseType)) {
464+
return _classImplementsInterface(baseType, interfaceFqn);
465+
}
466+
}
467+
468+
return false;
469+
}
470+
471+
function _interfaceExtendsInterface(interfaceType: spec.InterfaceType, targetFqn: string): boolean {
472+
if (interfaceType.fqn === targetFqn) {
473+
return true;
474+
}
475+
476+
if (interfaceType.interfaces) {
477+
for (const iface of interfaceType.interfaces) {
478+
if (iface === targetFqn) {
479+
return true;
480+
}
481+
const ifaceType = _dereference(iface, assembly, validator);
482+
if (spec.isInterfaceType(ifaceType) && _interfaceExtendsInterface(ifaceType, targetFqn)) {
483+
return true;
484+
}
485+
}
486+
}
487+
488+
return false;
489+
}
490+
491+
function _assertPropertiesMatch(
492+
expected: spec.Property,
493+
actual: spec.Property,
494+
label: string,
495+
action: string,
496+
opts: {
497+
allowCovariance: boolean;
498+
},
499+
) {
367500
const actualNode = bindings.getPropertyRelatedNode(actual);
368501
const expectedNode = bindings.getPropertyRelatedNode(expected);
369502
if (!!expected.protected !== !!actual.protected) {
@@ -386,19 +519,27 @@ function _defaultValidations(): ValidationFunction[] {
386519
),
387520
);
388521
}
389-
if (!deepEqual(expected.type, actual.type)) {
390-
diagnostic(
391-
JsiiDiagnostic.JSII_5004_OVERRIDE_CHANGES_PROP_TYPE.create(
392-
actualNode?.type ?? declarationName(actualNode),
393-
label,
394-
action,
395-
actual.type,
396-
expected.type,
397-
).maybeAddRelatedInformation(
398-
expectedNode?.type ?? declarationName(expectedNode),
399-
'The implemented declaration is here.',
400-
),
401-
);
522+
523+
// Types must generally be the same, but can be covariant sometimes
524+
if (!deepEqual(actual.type, expected.type)) {
525+
if (
526+
// static members can never change
527+
actual.static ||
528+
!(opts.allowCovariance && _isCovariantOf(actual.type, expected.type))
529+
) {
530+
diagnostic(
531+
JsiiDiagnostic.JSII_5004_OVERRIDE_CHANGES_PROP_TYPE.create(
532+
actualNode?.type ?? declarationName(actualNode),
533+
label,
534+
action,
535+
actual.type,
536+
expected.type,
537+
).maybeAddRelatedInformation(
538+
expectedNode?.type ?? declarationName(expectedNode),
539+
'The implemented declaration is here.',
540+
),
541+
);
542+
}
402543
}
403544
if (expected.immutable !== actual.immutable) {
404545
diagnostic(

0 commit comments

Comments
 (0)