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.
aws · Jul 16, 2019 · dd66afb · dd66afb
1 parent 028868d
commit dd66afb
Show file tree

Hide file tree

Showing 8 changed files with 140 additions and 24 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 <reason>'
+  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;
+}
diff --git 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(`