Skip to content

Commit 15f77b5

Browse files
authored
feat(jsii-diff): also check stability transitions (#592)
Once an API element is marked @stable, it is not allowed to retract that stability guarantee anymore by changing it back to @experimental. This is now verified by `jsii-diff`. This was always intended to be checked, but was missed as an oversight in the initial implementation.
1 parent 17338fd commit 15f77b5

4 files changed

Lines changed: 79 additions & 0 deletions

File tree

packages/jsii-diff/lib/classes-ifaces.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import reflect = require('jsii-reflect');
22
import log4js = require('log4js');
3+
import { compareStabilities } from './stability';
34
import { Analysis, FailedAnalysis, isSuperType } from './type-analysis';
45
import { ComparisonContext } from './types';
56

@@ -12,6 +13,8 @@ const LOG = log4js.getLogger('jsii-diff');
1213
* present on the new type, and that they match in turn.
1314
*/
1415
export function compareReferenceType<T extends reflect.ReferenceType>(original: T, updated: T, context: ComparisonContext) {
16+
compareStabilities(original, updated, context);
17+
1518
if (original.isClassType() && updated.isClassType()) {
1619
if (updated.abstract && !original.abstract) {
1720
context.mismatches.report({
@@ -57,6 +60,8 @@ export function compareReferenceType<T extends reflect.ReferenceType>(original:
5760
}
5861

5962
export function compareStruct(original: reflect.InterfaceType, updated: reflect.InterfaceType, context: ComparisonContext) {
63+
compareStabilities(original, updated, context);
64+
6065
// We don't compare structs here; they will be evaluated for compatibility
6166
// based on input and output positions.
6267
//
@@ -98,6 +103,8 @@ function compareMethod<T extends (reflect.Method | reflect.Initializer)>(
98103
original: T,
99104
updated: T,
100105
context: ComparisonContext) {
106+
compareStabilities(original, updated, context);
107+
101108
// Type guards on original are duplicated on updated to help tsc... They are required to be the same type by the declaration.
102109
if (reflect.isMethod(original) && reflect.isMethod(updated)) {
103110
if (original.static !== updated.static) {
@@ -201,6 +208,8 @@ function findParam(parameters: reflect.Parameter[], i: number): reflect.Paramete
201208
}
202209

203210
function compareProperty(original: reflect.Property, updated: reflect.Property, context: ComparisonContext) {
211+
compareStabilities(original, updated, context);
212+
204213
if (original.static !== updated.static) {
205214
// tslint:disable-next-line:max-line-length
206215
context.mismatches.report({

packages/jsii-diff/lib/enums.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
import reflect = require('jsii-reflect');
2+
import { compareStabilities } from './stability';
23
import { ComparisonContext } from './types';
34

45
export function compareEnum(original: reflect.EnumType, updated: reflect.EnumType, context: ComparisonContext) {
6+
compareStabilities(original, updated, context);
7+
58
for (const origMember of original.members) {
69
const updatedMember = updated.members.find(m => m.name === origMember.name);
710
if (!updatedMember) {
@@ -12,5 +15,7 @@ export function compareEnum(original: reflect.EnumType, updated: reflect.EnumTyp
1215
});
1316
continue;
1417
}
18+
19+
compareStabilities(origMember, updatedMember, context);
1520
}
1621
}
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import reflect = require('jsii-reflect');
2+
import spec = require('jsii-spec');
3+
import { ApiElement, ComparisonContext } from './types';
4+
5+
export function compareStabilities(original: reflect.Documentable & ApiElement, updated: reflect.Documentable, context: ComparisonContext) {
6+
// Nothing to do in these cases
7+
if (original.docs.stability === undefined || original.docs.stability === updated.docs.stability) { return; }
8+
9+
// Not allowed to disavow stability
10+
if (updated.docs.stability === undefined) {
11+
context.mismatches.report({
12+
ruleKey: 'removed-stability',
13+
message: `stability was '${original.docs.stability}', has been removed`,
14+
violator: original,
15+
});
16+
return;
17+
}
18+
19+
const allowed = allowedTransitions(original.docs.stability);
20+
if (!allowed.includes(updated.docs.stability)) {
21+
context.mismatches.report({
22+
ruleKey: 'changed-stability',
23+
message: `stability not allowed to go from '${original.docs.stability}' to '${updated.docs.stability}'`,
24+
violator: original,
25+
});
26+
}
27+
}
28+
29+
function allowedTransitions(start: spec.Stability): spec.Stability[] {
30+
switch (start) {
31+
// Experimental can go to stable or be deprecated
32+
case spec.Stability.Experimental:
33+
return [spec.Stability.Stable, spec.Stability.Deprecated];
34+
35+
// Stable can be deprecated
36+
case spec.Stability.Stable:
37+
return [spec.Stability.Deprecated];
38+
39+
// Deprecated can be reinstated
40+
case spec.Stability.Deprecated:
41+
return [spec.Stability.Stable];
42+
}
43+
44+
throw new Error(`Unrecognized stability: ${start}`);
45+
}

packages/jsii-diff/test/test.diagnostics.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,4 +56,24 @@ export = {
5656
test.done();
5757
},
5858

59+
// ----------------------------------------------------------------------
60+
async 'changing stable to experimental is breaking'(test: Test) {
61+
const mms = await compare(`
62+
/** @stable */
63+
export class Foo1 { }
64+
`, `
65+
/** @experimental */
66+
export class Foo1 { }
67+
`);
68+
69+
const experimentalErrors = false;
70+
const diags = classifyDiagnostics(mms, experimentalErrors, new Set());
71+
72+
test.ok(diags.length > 0);
73+
test.ok(diags.some(d => d.message.match(/stability not allowed to go from 'stable' to 'experimental'/)));
74+
test.equals(true, hasErrors(diags));
75+
76+
test.done();
77+
},
78+
5979
};

0 commit comments

Comments
 (0)