From dd66afbbafea2ad13d1d06ddc2e8a2db4c8a6132 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Tue, 16 Jul 2019 09:34:24 -0700 Subject: [PATCH] feat: add support for "external" stability (#596) Add support for a new class of stability called "external". This stability class is treated like "stable" for documentation purposes, but always lead to warnings (not errors) for stability comparison purposes. --- packages/jsii-diff/lib/stability.ts | 14 +++-- packages/jsii-diff/test/test.diagnostics.ts | 36 ++++++++++++ packages/jsii-pacmak/lib/targets/java.ts | 2 + packages/jsii-reflect/lib/docs.ts | 3 +- packages/jsii-reflect/test/typesystem.test.ts | 24 ++++++++ packages/jsii-spec/lib/spec.ts | 6 ++ packages/jsii/lib/docs.ts | 57 +++++++++++++------ packages/jsii/test/test.docs.ts | 22 +++++++ 8 files changed, 140 insertions(+), 24 deletions(-) diff --git a/packages/jsii-diff/lib/stability.ts b/packages/jsii-diff/lib/stability.ts index 65e089bd07..f3894246a5 100644 --- a/packages/jsii-diff/lib/stability.ts +++ b/packages/jsii-diff/lib/stability.ts @@ -28,17 +28,21 @@ export function compareStabilities(original: reflect.Documentable & ApiElement, function allowedTransitions(start: spec.Stability): spec.Stability[] { switch (start) { - // Experimental can go to stable or be deprecated + // Experimental can go to stable, external, or be deprecated case spec.Stability.Experimental: - return [spec.Stability.Stable, spec.Stability.Deprecated]; + return [spec.Stability.Stable, spec.Stability.Deprecated, spec.Stability.External]; - // Stable can be deprecated + // Stable can be deprecated, or switched to external case spec.Stability.Stable: - return [spec.Stability.Deprecated]; + return [spec.Stability.Deprecated, spec.Stability.External]; // Deprecated can be reinstated case spec.Stability.Deprecated: - return [spec.Stability.Stable]; + return [spec.Stability.Stable, spec.Stability.External]; + + // external can be stableified, or deprecated + case spec.Stability.External: + return [spec.Stability.Stable, spec.Stability.Deprecated]; } throw new Error(`Unrecognized stability: ${start}`); diff --git a/packages/jsii-diff/test/test.diagnostics.ts b/packages/jsii-diff/test/test.diagnostics.ts index 7e4c213039..b4de37665f 100644 --- a/packages/jsii-diff/test/test.diagnostics.ts +++ b/packages/jsii-diff/test/test.diagnostics.ts @@ -21,6 +21,24 @@ export = { test.done(); }, + // ---------------------------------------------------------------------- + async 'external stability violations are reported as warnings'(test: Test) { + const mms = await compare(` + /** @stability external */ + export class Foo1 { } + `, ` + export class Foo2 { } + `); + + const experimentalErrors = false; + const diags = classifyDiagnostics(mms, experimentalErrors, new Set()); + + test.equals(1, diags.length); + test.equals(false, hasErrors(diags)); + + test.done(); + }, + // ---------------------------------------------------------------------- async 'warnings can be turned into errors'(test: Test) { const mms = await compare(` @@ -39,6 +57,24 @@ export = { test.done(); }, + // ---------------------------------------------------------------------- + async 'external stability violations are never turned into errors'(test: Test) { + const mms = await compare(` + /** @stability external */ + export class Foo1 { } + `, ` + export class Foo2 { } + `); + + const experimentalErrors = true; + const diags = classifyDiagnostics(mms, experimentalErrors, new Set()); + + test.equals(1, diags.length); + test.equals(false, hasErrors(diags)); + + test.done(); + }, + // ---------------------------------------------------------------------- async 'errors can be skipped'(test: Test) { const mms = await compare(` diff --git a/packages/jsii-pacmak/lib/targets/java.ts b/packages/jsii-pacmak/lib/targets/java.ts index 2fc4fb99fc..458e972603 100644 --- a/packages/jsii-pacmak/lib/targets/java.ts +++ b/packages/jsii-pacmak/lib/targets/java.ts @@ -771,6 +771,8 @@ class JavaGenerator extends Generator { return 'Deprecated'; case spec.Stability.Experimental: return 'Experimental'; + case spec.Stability.External: + return 'External'; case spec.Stability.Stable: return 'Stable'; } diff --git a/packages/jsii-reflect/lib/docs.ts b/packages/jsii-reflect/lib/docs.ts index b5775c5a16..e38ca720b5 100644 --- a/packages/jsii-reflect/lib/docs.ts +++ b/packages/jsii-reflect/lib/docs.ts @@ -70,7 +70,8 @@ export class Docs { const stabilityPrecedence = { [Stability.Deprecated]: 0, [Stability.Experimental]: 1, - [Stability.Stable]: 2, + [Stability.External]: 2, + [Stability.Stable]: 3, }; function lowestStability(a?: Stability, b?: Stability): Stability | undefined { diff --git a/packages/jsii-reflect/test/typesystem.test.ts b/packages/jsii-reflect/test/typesystem.test.ts index e0b660e538..5344af62c8 100644 --- a/packages/jsii-reflect/test/typesystem.test.ts +++ b/packages/jsii-reflect/test/typesystem.test.ts @@ -322,6 +322,30 @@ describe('Stability', () => { expect(initializer.docs.stability).toEqual(Stability.Experimental); expect(method.docs.stability).toEqual(Stability.Experimental); }); + + test('external stability', async () => { + const ts = await typeSystemFromSource(` + /** + * @stability external + */ + export class Foo { + public foo() { + Array.isArray(3); + } + } + + /** + * @stable + */ + export class SubFoo extends Foo { + } + `); + + const classType = ts.findClass('testpkg.SubFoo'); + const method = classType.allMethods.find(m => m.name === 'foo')!; + + expect(method.docs.stability).toEqual(Stability.External); + }); }); }); diff --git a/packages/jsii-spec/lib/spec.ts b/packages/jsii-spec/lib/spec.ts index 8d1e3dde67..ef1a792c34 100644 --- a/packages/jsii-spec/lib/spec.ts +++ b/packages/jsii-spec/lib/spec.ts @@ -360,6 +360,12 @@ export enum Stability { * in breaking ways in a subsequent minor or patch version. */ Stable = 'stable', + + /** + * This API is an representation of an API managed elsewhere and follows + * the other API's versioning model. + */ + External = 'external', } /** diff --git a/packages/jsii/lib/docs.ts b/packages/jsii/lib/docs.ts index ed4f977884..0dd5f340dc 100644 --- a/packages/jsii/lib/docs.ts +++ b/packages/jsii/lib/docs.ts @@ -85,8 +85,24 @@ function parseDocParts(comments: string | undefined, tags: ts.JSDocTagInfo[]): D docs.see = eatTag('see'); docs.subclassable = eatTag('subclassable') !== undefined ? true : undefined; + docs.stability = parseStability(eatTag('stability'), diagnostics); + // @experimental is a shorthand for '@stability experimental', same for '@stable' const experimental = eatTag('experimental') !== undefined; const stable = eatTag('stable') !== undefined; + // Can't combine them + if (countBools(docs.stability !== undefined, experimental, stable) > 1) { + diagnostics.push(`Use only one of @stability, @experimental or @stable`); + } + if (experimental) { docs.stability = spec.Stability.Experimental; } + if (stable) { docs.stability = spec.Stability.Stable; } + + // Can combine '@stability deprecated' with '@deprecated ' + if (docs.deprecated !== undefined) { + if (docs.stability !== undefined && docs.stability !== spec.Stability.Deprecated) { + diagnostics.push(`@deprecated tag requires '@stability deprecated' or no @stability at all.`); + } + docs.stability = spec.Stability.Deprecated; + } if (docs.example && docs.example.indexOf('```') >= 0) { // This is currently what the JSDoc standard expects, and VSCode highlights it in @@ -97,26 +113,10 @@ function parseDocParts(comments: string | undefined, tags: ts.JSDocTagInfo[]): D diagnostics.push('@example must be code only, no code block fences allowed.'); } - if (experimental && stable) { - diagnostics.push('Element is marked both @experimental and @stable.'); - } - - if (docs.deprecated !== undefined) { - if (docs.deprecated.trim() === '') { - diagnostics.push('@deprecated tag needs a reason and/or suggested alternatives.'); - } - if (stable) { - diagnostics.push('Element is marked both @deprecated and @stable.'); - } - if (experimental) { - diagnostics.push('Element is marked both @deprecated and @experimental.'); - } + if (docs.deprecated !== undefined && docs.deprecated.trim() === '') { + diagnostics.push('@deprecated tag needs a reason and/or suggested alternatives.'); } - if (experimental) { docs.stability = spec.Stability.Experimental; } - if (stable) { docs.stability = spec.Stability.Stable; } - if (docs.deprecated) { docs.stability = spec.Stability.Deprecated; } - if (tagNames.size > 0) { docs.custom = {}; for (const [key, value] of tagNames.entries()) { @@ -184,3 +184,24 @@ function summaryLine(str: string) { const PUNCTUATION = ['!', '?', '.', ';'].map(s => '\\' + s).join(''); const ENDS_WITH_PUNCTUATION_REGEX = new RegExp(`[${PUNCTUATION}]$`); const FIRST_SENTENCE_REGEX = new RegExp(`^([^${PUNCTUATION}]+[${PUNCTUATION}] )`); // literal space at the end + +function intBool(x: boolean): number { + return x ? 1 : 0; +} + +function countBools(...x: boolean[]) { + return x.map(intBool).reduce((a, b) => a + b, 0); +} + +function parseStability(s: string | undefined, diagnostics: string[]): spec.Stability | undefined { + if (s === undefined) { return undefined; } + + switch (s) { + case 'stable': return spec.Stability.Stable; + case 'experimental': return spec.Stability.Experimental; + case 'external': return spec.Stability.External; + case 'deprecated': return spec.Stability.Deprecated; + } + diagnostics.push(`Unrecognized @stability: '${s}'`); + return undefined; +} \ No newline at end of file diff --git a/packages/jsii/test/test.docs.ts b/packages/jsii/test/test.docs.ts index 88b02f2c72..42ec609c7f 100644 --- a/packages/jsii/test/test.docs.ts +++ b/packages/jsii/test/test.docs.ts @@ -269,6 +269,28 @@ export = { test.done(); }, + // ---------------------------------------------------------------------- + + async 'can mark external'(test: Test) { + const assembly = await compile(` + /** + * @stability external + */ + export class Foo { + public floop() { + Array.isArray(3); + } + } + `); + + const classType = assembly.types!['testpkg.Foo'] as spec.ClassType; + const method = classType.methods!.find(m => m.name === 'floop'); + + test.deepEqual(classType.docs!.stability, spec.Stability.External); + test.deepEqual(method!.docs!.stability, spec.Stability.External); + test.done(); + }, + // ---------------------------------------------------------------------- async 'can mark subclassable'(test: Test) { const assembly = await compile(`