From 1497ace076febdc265f25440057b1d1912843c50 Mon Sep 17 00:00:00 2001 From: Nick Lynch Date: Thu, 21 Oct 2021 12:49:07 +0100 Subject: [PATCH 1/4] feat(jsii): experimental --strip-deprecated-allowlist-file feature Introduce an optional new parameter for the `--strip-deprecated` feature, which allows constrains the feature to only strip deprecated API elements present in an allowlist file. This can be used to preserve some deprecated API elements while stripping others. Feedback welcome on the nomenclature throughout. :) --- .../user-guides/lib-author/toolchain/jsii.md | 12 + packages/jsii/bin/jsii.ts | 6 + packages/jsii/lib/assembler.ts | 26 +- packages/jsii/lib/compiler.ts | 3 + .../jsii/lib/transforms/deprecated-remover.ts | 78 +++- packages/jsii/test/deprecated-remover.test.ts | 338 ++++++++++++++++-- 6 files changed, 408 insertions(+), 55 deletions(-) diff --git a/gh-pages/content/user-guides/lib-author/toolchain/jsii.md b/gh-pages/content/user-guides/lib-author/toolchain/jsii.md index 7ee1a3bd74..daeb99c58c 100644 --- a/gh-pages/content/user-guides/lib-author/toolchain/jsii.md +++ b/gh-pages/content/user-guides/lib-author/toolchain/jsii.md @@ -78,3 +78,15 @@ However, in order to ensure the underlying code continues to work as designed, the *implementation* of such declarations will remain in the **JavaScript** (`.js`) files produced by the compilation. This is, in fact, similar to marking all `@deprecated` members `@internal`. + +Additionally, the `--strip-deprecated-allowlist-file` option allows the above +behavior to be limited to a specific set of allow-listed fully-qualified names. +Each line in the file should contain a single fully-qualified name of a +declaration that should be stripped. If this option is provided, all +`@deprecated` elements not present in the allow list will be retained. As an +example, the allowlist file might look like: + + testpkg.IDeprecated + testpkg.DeprecatedOne + testpkg.DeprecatedTwo.deprecatedProperty + testpkg.DeprecatedTwo.deprecatedMethod diff --git a/packages/jsii/bin/jsii.ts b/packages/jsii/bin/jsii.ts index 802e99873b..9587c8d1b3 100644 --- a/packages/jsii/bin/jsii.ts +++ b/packages/jsii/bin/jsii.ts @@ -60,6 +60,11 @@ const warningTypes = Object.keys(enabledWarnings); default: false, desc: '[EXPERIMENTAL] Hides all @deprecated members from the API (implementations remain)', }) + .option('strip-deprecated-allowlist-file', { + type: 'string', + default: undefined, + desc: '[EXPERIMENTAL] If provided, only full-qualified names listed in the allow list file (newline-delimited) will be stripped with --strip-deprecated', + }) .option('add-deprecation-warnings', { type: 'boolean', default: false, @@ -112,6 +117,7 @@ const warningTypes = Object.keys(enabledWarnings); projectReferences: argv['project-references'], failOnWarnings: argv['fail-on-warnings'], stripDeprecated: argv['strip-deprecated'], + stripDeprecatedAllowListFile: argv['strip-deprecated-allowlist-file'], addDeprecationWarnings: argv['add-deprecation-warnings'], generateTypeScriptConfig: argv['generate-tsconfig'], }); diff --git a/packages/jsii/lib/assembler.ts b/packages/jsii/lib/assembler.ts index b60fca0817..8a842e31f0 100644 --- a/packages/jsii/lib/assembler.ts +++ b/packages/jsii/lib/assembler.ts @@ -76,7 +76,22 @@ export class Assembler implements Emitter { options: AssemblerOptions = {}, ) { if (options.stripDeprecated) { - this.deprecatedRemover = new DeprecatedRemover(this._typeChecker); + let allowlistedDeprecations: string[] | undefined; + if (options.stripDeprecatedAllowListFile) { + if (!fs.existsSync(options.stripDeprecatedAllowListFile)) { + throw new Error( + `--strip-deprecated file not found: ${options.stripDeprecatedAllowListFile}`, + ); + } + allowlistedDeprecations = fs + .readFileSync(options.stripDeprecatedAllowListFile, 'utf8') + .split('\n'); + } + + this.deprecatedRemover = new DeprecatedRemover( + this._typeChecker, + allowlistedDeprecations, + ); } if (options.addDeprecationWarnings) { @@ -2759,6 +2774,15 @@ export interface AssemblerOptions { */ readonly stripDeprecated?: boolean; + /** + * If `stripDeprecated` is true, and a file is provided here, only the FQNs + * present in the file will actually be removed. This can be useful when + * you wish to deprecate some elements without actually removing them. + * + * @default undefined + */ + readonly stripDeprecatedAllowListFile?: string; + /** * Whether to inject code that warns when a deprecated element is used. * diff --git a/packages/jsii/lib/compiler.ts b/packages/jsii/lib/compiler.ts index bb1098127f..aaae7f1799 100644 --- a/packages/jsii/lib/compiler.ts +++ b/packages/jsii/lib/compiler.ts @@ -52,6 +52,8 @@ export interface CompilerOptions { failOnWarnings?: boolean; /** Whether to strip deprecated members from emitted artifacts */ stripDeprecated?: boolean; + /** The path to an allowlist of FQNs to strip if stripDeprecated is set */ + stripDeprecatedAllowListFile?: string; /** Whether to add warnings for deprecated elements */ addDeprecationWarnings?: boolean; /** @@ -244,6 +246,7 @@ export class Compiler implements Emitter { // to post-process the AST const assembler = new Assembler(this.options.projectInfo, program, stdlib, { stripDeprecated: this.options.stripDeprecated, + stripDeprecatedAllowListFile: this.options.stripDeprecatedAllowListFile, addDeprecationWarnings: this.options.addDeprecationWarnings, }); diff --git a/packages/jsii/lib/transforms/deprecated-remover.ts b/packages/jsii/lib/transforms/deprecated-remover.ts index 7b1e86d221..8875b9eee1 100644 --- a/packages/jsii/lib/transforms/deprecated-remover.ts +++ b/packages/jsii/lib/transforms/deprecated-remover.ts @@ -24,8 +24,12 @@ import * as bindings from '../node-bindings'; export class DeprecatedRemover { private readonly transformations = new Array(); + private readonly nodesToRemove = new Set(); - public constructor(private readonly typeChecker: ts.TypeChecker) {} + public constructor( + private readonly typeChecker: ts.TypeChecker, + private readonly allowlistedDeprecations: string[] | undefined, + ) {} /** * Obtains the configuration for the TypeScript transform(s) that will remove @@ -42,6 +46,7 @@ export class DeprecatedRemover { this.typeChecker, context, this.transformations, + this.nodesToRemove, ); return transformer.transform.bind(transformer); }, @@ -71,13 +76,19 @@ export class DeprecatedRemover { // Find all types that will be stripped out for (const [fqn, typeInfo] of Object.entries(assembly.types)) { if (typeInfo.docs?.stability === Stability.Deprecated) { + if (!this.shouldFqnBeStripped(fqn)) { + continue; + } strippedFqns.add(fqn); + if (isClassType(typeInfo) && typeInfo.base != null) { replaceWithClass.set(fqn, typeInfo.base); } if (isClassOrInterfaceType(typeInfo) && typeInfo.interfaces != null) { replaceWithInterfaces.set(fqn, typeInfo.interfaces); } + + this.nodesToRemove.add(bindings.getRelatedNode(typeInfo)!); } } @@ -181,20 +192,38 @@ export class DeprecatedRemover { } // Drop all `@deprecated` members, and remove "overrides" from stripped types - typeInfo.methods = typeInfo?.methods - ?.filter((meth) => meth.docs?.stability !== Stability.Deprecated) - ?.map((meth) => - meth.overrides != null && strippedFqns.has(meth.overrides) - ? { ...meth, overrides: undefined } - : meth, - ); - typeInfo.properties = typeInfo?.properties - ?.filter((prop) => prop.docs?.stability !== Stability.Deprecated) - ?.map((prop) => - prop.overrides != null && strippedFqns.has(prop.overrides) - ? { ...prop, overrides: undefined } - : prop, - ); + const methods: Method[] = []; + const properties: Property[] = []; + typeInfo.methods?.forEach((meth) => { + if ( + meth.docs?.stability === Stability.Deprecated && + this.shouldFqnBeStripped(`${fqn}.${meth.name}`) + ) { + this.nodesToRemove.add(bindings.getMethodRelatedNode(meth)!); + } else { + methods.push( + meth.overrides != null && strippedFqns.has(meth.overrides) + ? { ...meth, overrides: undefined } + : meth, + ); + } + }); + typeInfo.methods = typeInfo.methods ? methods : undefined; + typeInfo.properties?.forEach((prop) => { + if ( + prop.docs?.stability === Stability.Deprecated && + this.shouldFqnBeStripped(`${fqn}.${prop.name}`) + ) { + this.nodesToRemove.add(bindings.getParameterRelatedNode(prop)!); + } else { + properties.push( + prop.overrides != null && strippedFqns.has(prop.overrides) + ? { ...prop, overrides: undefined } + : prop, + ); + } + }); + typeInfo.properties = typeInfo.properties ? properties : undefined; } const diagnostics = this.findLeftoverUseOfDeprecatedAPIs( @@ -205,7 +234,9 @@ export class DeprecatedRemover { // Remove all `@deprecated` types, after we did everything, so we could // still access the related nodes from the assembly object. for (const fqn of strippedFqns) { - delete assembly.types[fqn]; + if (this.shouldFqnBeStripped(fqn)) { + delete assembly.types[fqn]; + } } return diagnostics; @@ -321,6 +352,13 @@ export class DeprecatedRemover { .find((ref) => ref != null); } + private shouldFqnBeStripped(fqn: string) { + return ( + !this.allowlistedDeprecations || + this.allowlistedDeprecations.includes(fqn) + ); + } + private makeDiagnostic( fqn: string, messagePrefix: 'Method', @@ -722,6 +760,7 @@ class DeprecationRemovalTransformer { private readonly typeChecker: ts.TypeChecker, private readonly context: ts.TransformationContext, private readonly transformations: readonly Transformation[], + private readonly nodesToRemove: Set, ) {} public transform(node: T): T { @@ -863,8 +902,9 @@ class DeprecationRemovalTransformer { private isDeprecated(node: ts.Node): boolean { const original = ts.getOriginalNode(node); - return ts - .getJSDocTags(original) - .some((tag) => tag.tagName.text === 'deprecated'); + return ( + this.nodesToRemove.has(original) && + ts.getJSDocTags(original).some((tag) => tag.tagName.text === 'deprecated') + ); } } diff --git a/packages/jsii/test/deprecated-remover.test.ts b/packages/jsii/test/deprecated-remover.test.ts index c623f0abb3..2e1bb5d9f0 100644 --- a/packages/jsii/test/deprecated-remover.test.ts +++ b/packages/jsii/test/deprecated-remover.test.ts @@ -1,42 +1,14 @@ +import * as fs from 'fs-extra'; +import * as os from 'os'; +import * as path from 'path'; + import { compileJsiiForTest, HelperCompilationResult } from '../lib'; const DEPRECATED = '/** @deprecated stripped */'; test('produces correct output', async () => { const result = await compileJsiiForTest( - { - 'index.ts': ` - export * from './deprecated'; - export * from './retained'; - export { Deprecated, GrandChild, Retained } from './mixed'; - `, - 'deprecated.ts': ` - ${DEPRECATED} - export interface IDeprecatedInterface {} - ${DEPRECATED} - export class DeprecatedClass {} - `, - 'retained.ts': ` - export interface IRetainedInterface {} - export class RetainedClass {} - `, - 'mixed.ts': ` - import { IRetainedInterface } from './retained'; - - export abstract class Retained { - ${DEPRECATED} - readonly deprecated = 1337; - readonly retained = 'YEAH'; - } - ${DEPRECATED} - export class Deprecated extends Retained implements IRetainedInterface {} - export class GrandChild extends Deprecated { - public retainedMethod(): void {} - ${DEPRECATED} - public deprecatedMethod(): void {} - } - `, - }, + MULTI_FILE_EXAMPLE, undefined /* callback */, { stripDeprecated: true }, ); @@ -181,8 +153,8 @@ test('cross-file deprecated heritage', async () => { export interface INotDeprecated { } ////////////////// - - + + /////////////////////// /// deprecated.d.ts /// /////////////////////// @@ -190,6 +162,268 @@ test('cross-file deprecated heritage', async () => { `); }); +describe('stripDeprecatedAllowList', () => { + test('strips all if all FQNs are present in the allowList', async () => { + const tmpDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'jsiideprecatedremover'), + ); + const stripDeprecatedAllowListFile = path.join(tmpDir, 'allowList'); + await fs.writeFile( + stripDeprecatedAllowListFile, + [ + 'testpkg.IDeprecatedInterface', + 'testpkg.DeprecatedClass', + 'testpkg.Retained.deprecated', + 'testpkg.Deprecated', + 'testpkg.GrandChild.deprecatedMethod', + ].join('\n'), + 'utf8', + ); + + const resultWithoutAllowList = await compileJsiiForTest( + MULTI_FILE_EXAMPLE, + undefined /* callback */, + { stripDeprecated: true }, + ); + const resultWithAllowList = await compileJsiiForTest( + MULTI_FILE_EXAMPLE, + undefined /* callback */, + { stripDeprecated: true, stripDeprecatedAllowListFile }, + ); + + expect(resultWithAllowList.assembly.types).toEqual( + resultWithoutAllowList.assembly.types, + ); + expect(declFilesSnapshot(resultWithAllowList)).toEqual( + declFilesSnapshot(resultWithoutAllowList), + ); + }); + + test('strips none if the allowList is empty', async () => { + const tmpDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'jsiideprecatedremover'), + ); + const stripDeprecatedAllowListFile = path.join(tmpDir, 'allowList'); + // Valid but empty file + await fs.writeFile(stripDeprecatedAllowListFile, '', 'utf8'); + + const resultNoStrip = await compileJsiiForTest( + MULTI_FILE_EXAMPLE, + undefined /* callback */, + { stripDeprecated: false }, + ); + const resultStripEmpty = await compileJsiiForTest( + MULTI_FILE_EXAMPLE, + undefined /* callback */, + { stripDeprecated: true, stripDeprecatedAllowListFile }, + ); + + expect(resultStripEmpty.assembly.types).toEqual( + resultNoStrip.assembly.types, + ); + expect(declFilesSnapshot(resultStripEmpty)).toEqual( + declFilesSnapshot(resultNoStrip), + ); + }); + + test('strips only allowlisted items if subset is present', async () => { + const tmpDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'jsiideprecatedremover'), + ); + const stripDeprecatedAllowListFile = path.join(tmpDir, 'allowList'); + await fs.writeFile( + stripDeprecatedAllowListFile, + [ + 'testpkg.IDeprecatedInterface', + 'testpkg.GrandChild.deprecatedMethod', + ].join('\n'), + 'utf8', + ); + + const result = await compileJsiiForTest( + MULTI_FILE_EXAMPLE, + undefined /* callback */, + { stripDeprecated: true, stripDeprecatedAllowListFile }, + ); + + expect(result.assembly.types).toMatchInlineSnapshot(` + Object { + "testpkg.Deprecated": Object { + "assembly": "testpkg", + "base": "testpkg.Retained", + "docs": Object { + "deprecated": "stripped", + "stability": "deprecated", + }, + "fqn": "testpkg.Deprecated", + "initializer": Object {}, + "interfaces": Array [ + "testpkg.IRetainedInterface", + ], + "kind": "class", + "locationInModule": Object { + "filename": "mixed.ts", + "line": 10, + }, + "name": "Deprecated", + "symbolId": "mixed:Deprecated", + }, + "testpkg.DeprecatedClass": Object { + "assembly": "testpkg", + "docs": Object { + "deprecated": "stripped", + "stability": "deprecated", + }, + "fqn": "testpkg.DeprecatedClass", + "initializer": Object {}, + "kind": "class", + "locationInModule": Object { + "filename": "deprecated.ts", + "line": 5, + }, + "name": "DeprecatedClass", + "symbolId": "deprecated:DeprecatedClass", + }, + "testpkg.GrandChild": Object { + "assembly": "testpkg", + "base": "testpkg.Deprecated", + "fqn": "testpkg.GrandChild", + "initializer": Object {}, + "kind": "class", + "locationInModule": Object { + "filename": "mixed.ts", + "line": 11, + }, + "methods": Array [ + Object { + "locationInModule": Object { + "filename": "mixed.ts", + "line": 12, + }, + "name": "retainedMethod", + }, + ], + "name": "GrandChild", + "symbolId": "mixed:GrandChild", + }, + "testpkg.IRetainedInterface": Object { + "assembly": "testpkg", + "fqn": "testpkg.IRetainedInterface", + "kind": "interface", + "locationInModule": Object { + "filename": "retained.ts", + "line": 2, + }, + "name": "IRetainedInterface", + "symbolId": "retained:IRetainedInterface", + }, + "testpkg.Retained": Object { + "abstract": true, + "assembly": "testpkg", + "fqn": "testpkg.Retained", + "initializer": Object {}, + "kind": "class", + "locationInModule": Object { + "filename": "mixed.ts", + "line": 4, + }, + "name": "Retained", + "properties": Array [ + Object { + "docs": Object { + "deprecated": "stripped", + "stability": "deprecated", + }, + "immutable": true, + "locationInModule": Object { + "filename": "mixed.ts", + "line": 6, + }, + "name": "deprecated", + "type": Object { + "primitive": "number", + }, + }, + Object { + "immutable": true, + "locationInModule": Object { + "filename": "mixed.ts", + "line": 7, + }, + "name": "retained", + "type": Object { + "primitive": "string", + }, + }, + ], + "symbolId": "mixed:Retained", + }, + "testpkg.RetainedClass": Object { + "assembly": "testpkg", + "fqn": "testpkg.RetainedClass", + "initializer": Object {}, + "kind": "class", + "locationInModule": Object { + "filename": "retained.ts", + "line": 3, + }, + "name": "RetainedClass", + "symbolId": "retained:RetainedClass", + }, + } + `); + expect(declFilesSnapshot(result)).toMatchInlineSnapshot(` + "////////////////// + /// index.d.ts /// + export * from './deprecated'; + export * from './retained'; + export { Deprecated, GrandChild, Retained } from './mixed'; + ////////////////// + + + /////////////////////// + /// deprecated.d.ts /// + /** + * @deprecated stripped + */ + export declare class DeprecatedClass { + } + /////////////////////// + + + ///////////////////// + /// retained.d.ts /// + export interface IRetainedInterface { + } + export declare class RetainedClass { + } + ///////////////////// + + + ////////////////// + /// mixed.d.ts /// + import { IRetainedInterface } from './retained'; + export declare abstract class Retained { + /** + * @deprecated stripped + */ + readonly deprecated = 1337; + readonly retained = \\"YEAH\\"; + } + /** + * @deprecated stripped + */ + export declare class Deprecated extends Retained implements IRetainedInterface { + } + export declare class GrandChild extends Deprecated { + retainedMethod(): void; + } + ////////////////// + " + `); + }); +}); + function declFilesSnapshot(result: HelperCompilationResult) { return Object.entries(result.files) .filter(([name]) => name.endsWith('.d.ts')) @@ -199,3 +433,37 @@ function declFilesSnapshot(result: HelperCompilationResult) { }) .join('\n\n'); } + +const MULTI_FILE_EXAMPLE = { + 'index.ts': ` + export * from './deprecated'; + export * from './retained'; + export { Deprecated, GrandChild, Retained } from './mixed'; + `, + 'deprecated.ts': ` + ${DEPRECATED} + export interface IDeprecatedInterface {} + ${DEPRECATED} + export class DeprecatedClass {} + `, + 'retained.ts': ` + export interface IRetainedInterface {} + export class RetainedClass {} + `, + 'mixed.ts': ` + import { IRetainedInterface } from './retained'; + + export abstract class Retained { + ${DEPRECATED} + readonly deprecated = 1337; + readonly retained = 'YEAH'; + } + ${DEPRECATED} + export class Deprecated extends Retained implements IRetainedInterface {} + export class GrandChild extends Deprecated { + public retainedMethod(): void {} + ${DEPRECATED} + public deprecatedMethod(): void {} + } + `, +}; From f66252093065125fc61a4414169e8ac649b6d434 Mon Sep 17 00:00:00 2001 From: Nick Lynch Date: Thu, 21 Oct 2021 18:23:19 +0100 Subject: [PATCH 2/4] support for deprecated enum members --- .../jsii/lib/transforms/deprecated-remover.ts | 21 +++- packages/jsii/test/deprecated-remover.test.ts | 105 ++++++++++++++++++ 2 files changed, 125 insertions(+), 1 deletion(-) diff --git a/packages/jsii/lib/transforms/deprecated-remover.ts b/packages/jsii/lib/transforms/deprecated-remover.ts index 8875b9eee1..0fe1dda963 100644 --- a/packages/jsii/lib/transforms/deprecated-remover.ts +++ b/packages/jsii/lib/transforms/deprecated-remover.ts @@ -1,6 +1,7 @@ import { Assembly, ClassType, + EnumMember, Initializer, InterfaceType, isClassOrInterfaceType, @@ -98,8 +99,26 @@ export class DeprecatedRemover { continue; } - // Enums cannot have references to `@deprecated` types + // Enums cannot have references to `@deprecated` types, but can have deprecated members if (isEnumType(typeInfo)) { + const enumNode = bindings.getEnumRelatedNode(typeInfo)!; + const members: EnumMember[] = []; + typeInfo.members.forEach((mem) => { + if ( + mem.docs?.stability === Stability.Deprecated && + this.shouldFqnBeStripped(`${fqn}.${mem.name}`) + ) { + const matchingMemberNode = enumNode.members.find( + (enumMem) => enumMem.name.getText() === mem.name, + ); + if (matchingMemberNode) { + this.nodesToRemove.add(matchingMemberNode); + } + } else { + members.push(mem); + } + }); + typeInfo.members = members; continue; } diff --git a/packages/jsii/test/deprecated-remover.test.ts b/packages/jsii/test/deprecated-remover.test.ts index 2e1bb5d9f0..5603c89df2 100644 --- a/packages/jsii/test/deprecated-remover.test.ts +++ b/packages/jsii/test/deprecated-remover.test.ts @@ -88,6 +88,22 @@ test('produces correct output', async () => { "name": "RetainedClass", "symbolId": "retained:RetainedClass", }, + "testpkg.SomeEnum": Object { + "assembly": "testpkg", + "fqn": "testpkg.SomeEnum", + "kind": "enum", + "locationInModule": Object { + "filename": "enums.ts", + "line": 2, + }, + "members": Array [ + Object { + "name": "VALUE_RETAINED", + }, + ], + "name": "SomeEnum", + "symbolId": "enums:SomeEnum", + }, } `); expect(declFilesSnapshot(result)).toMatchInlineSnapshot(` @@ -95,6 +111,7 @@ test('produces correct output', async () => { /// index.d.ts /// import './deprecated'; export * from './retained'; + export * from './enums'; export { GrandChild, Retained } from './mixed'; ////////////////// @@ -113,6 +130,14 @@ test('produces correct output', async () => { ///////////////////// + ////////////////// + /// enums.d.ts /// + export declare enum SomeEnum { + VALUE_RETAINED = 0 + } + ////////////////// + + ////////////////// /// mixed.d.ts /// import * as retained_1 from \\"./retained\\"; @@ -173,6 +198,8 @@ describe('stripDeprecatedAllowList', () => { [ 'testpkg.IDeprecatedInterface', 'testpkg.DeprecatedClass', + 'testpkg.SomeEnum.VALUE_DEPRECATED', + 'testpkg.DeprecatedEnum', 'testpkg.Retained.deprecated', 'testpkg.Deprecated', 'testpkg.GrandChild.deprecatedMethod', @@ -235,6 +262,7 @@ describe('stripDeprecatedAllowList', () => { stripDeprecatedAllowListFile, [ 'testpkg.IDeprecatedInterface', + 'testpkg.SomeEnum.VALUE_DEPRECATED', 'testpkg.GrandChild.deprecatedMethod', ].join('\n'), 'utf8', @@ -284,6 +312,35 @@ describe('stripDeprecatedAllowList', () => { "name": "DeprecatedClass", "symbolId": "deprecated:DeprecatedClass", }, + "testpkg.DeprecatedEnum": Object { + "assembly": "testpkg", + "docs": Object { + "deprecated": "stripped", + "stability": "deprecated", + }, + "fqn": "testpkg.DeprecatedEnum", + "kind": "enum", + "locationInModule": Object { + "filename": "enums.ts", + "line": 8, + }, + "members": Array [ + Object { + "docs": Object { + "stability": "deprecated", + }, + "name": "VALUE_ONE", + }, + Object { + "docs": Object { + "stability": "deprecated", + }, + "name": "VALUE_TWO", + }, + ], + "name": "DeprecatedEnum", + "symbolId": "enums:DeprecatedEnum", + }, "testpkg.GrandChild": Object { "assembly": "testpkg", "base": "testpkg.Deprecated", @@ -370,6 +427,22 @@ describe('stripDeprecatedAllowList', () => { "name": "RetainedClass", "symbolId": "retained:RetainedClass", }, + "testpkg.SomeEnum": Object { + "assembly": "testpkg", + "fqn": "testpkg.SomeEnum", + "kind": "enum", + "locationInModule": Object { + "filename": "enums.ts", + "line": 2, + }, + "members": Array [ + Object { + "name": "VALUE_RETAINED", + }, + ], + "name": "SomeEnum", + "symbolId": "enums:SomeEnum", + }, } `); expect(declFilesSnapshot(result)).toMatchInlineSnapshot(` @@ -377,6 +450,7 @@ describe('stripDeprecatedAllowList', () => { /// index.d.ts /// export * from './deprecated'; export * from './retained'; + export * from './enums'; export { Deprecated, GrandChild, Retained } from './mixed'; ////////////////// @@ -400,6 +474,27 @@ describe('stripDeprecatedAllowList', () => { ///////////////////// + ////////////////// + /// enums.d.ts /// + export declare enum SomeEnum { + VALUE_RETAINED = 0 + } + /** + * @deprecated stripped + */ + export declare enum DeprecatedEnum { + /** + * @deprecated + */ + VALUE_ONE = 0, + /** + * @deprecated + */ + VALUE_TWO = 1 + } + ////////////////// + + ////////////////// /// mixed.d.ts /// import { IRetainedInterface } from './retained'; @@ -438,6 +533,7 @@ const MULTI_FILE_EXAMPLE = { 'index.ts': ` export * from './deprecated'; export * from './retained'; + export * from './enums'; export { Deprecated, GrandChild, Retained } from './mixed'; `, 'deprecated.ts': ` @@ -450,6 +546,15 @@ const MULTI_FILE_EXAMPLE = { export interface IRetainedInterface {} export class RetainedClass {} `, + 'enums.ts': ` + export enum SomeEnum { + VALUE_RETAINED, + ${DEPRECATED} + VALUE_DEPRECATED, + } + ${DEPRECATED} + export enum DeprecatedEnum { VALUE_ONE, VALUE_TWO } + `, 'mixed.ts': ` import { IRetainedInterface } from './retained'; From d913b0e97081bd142e448d42fa31dc5baab36cf3 Mon Sep 17 00:00:00 2001 From: Nick Lynch Date: Fri, 22 Oct 2021 10:01:46 +0100 Subject: [PATCH 3/4] chore: convert allowlistedDeprecations from array to Set --- packages/jsii/lib/transforms/deprecated-remover.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/jsii/lib/transforms/deprecated-remover.ts b/packages/jsii/lib/transforms/deprecated-remover.ts index 0fe1dda963..0476df30a9 100644 --- a/packages/jsii/lib/transforms/deprecated-remover.ts +++ b/packages/jsii/lib/transforms/deprecated-remover.ts @@ -29,7 +29,7 @@ export class DeprecatedRemover { public constructor( private readonly typeChecker: ts.TypeChecker, - private readonly allowlistedDeprecations: string[] | undefined, + private readonly allowlistedDeprecations: Set | undefined, ) {} /** @@ -372,10 +372,7 @@ export class DeprecatedRemover { } private shouldFqnBeStripped(fqn: string) { - return ( - !this.allowlistedDeprecations || - this.allowlistedDeprecations.includes(fqn) - ); + return this.allowlistedDeprecations?.has(fqn) ?? true; } private makeDiagnostic( From 63bbf3d048efe971b74b69eb347eb60e000bf47d Mon Sep 17 00:00:00 2001 From: Nick Lynch Date: Fri, 22 Oct 2021 11:40:19 +0100 Subject: [PATCH 4/4] simplifying CLI args, switching to # for method/prop separator --- .../user-guides/lib-author/toolchain/jsii.md | 15 +++++++-------- packages/jsii/bin/jsii.ts | 12 +++--------- packages/jsii/lib/assembler.ts | 10 ++++++---- .../jsii/lib/transforms/deprecated-remover.ts | 6 +++--- packages/jsii/test/deprecated-remover.test.ts | 10 +++++----- 5 files changed, 24 insertions(+), 29 deletions(-) diff --git a/gh-pages/content/user-guides/lib-author/toolchain/jsii.md b/gh-pages/content/user-guides/lib-author/toolchain/jsii.md index daeb99c58c..8a2a6f0f77 100644 --- a/gh-pages/content/user-guides/lib-author/toolchain/jsii.md +++ b/gh-pages/content/user-guides/lib-author/toolchain/jsii.md @@ -79,14 +79,13 @@ the *implementation* of such declarations will remain in the **JavaScript** (`.js`) files produced by the compilation. This is, in fact, similar to marking all `@deprecated` members `@internal`. -Additionally, the `--strip-deprecated-allowlist-file` option allows the above -behavior to be limited to a specific set of allow-listed fully-qualified names. -Each line in the file should contain a single fully-qualified name of a -declaration that should be stripped. If this option is provided, all -`@deprecated` elements not present in the allow list will be retained. As an -example, the allowlist file might look like: +Additionally, a file name can be passed to the `--strip-deprecated` option to +limit the above behavior to a specific set of allow-listed fully-qualified +names. Each line in the file should contain a single fully-qualified name of a +declaration that should be stripped. All `@deprecated` elements not present in +the allow list will be retained. An example allowlist file might look like: testpkg.IDeprecated testpkg.DeprecatedOne - testpkg.DeprecatedTwo.deprecatedProperty - testpkg.DeprecatedTwo.deprecatedMethod + testpkg.DeprecatedTwo#deprecatedProperty + testpkg.DeprecatedTwo#deprecatedMethod diff --git a/packages/jsii/bin/jsii.ts b/packages/jsii/bin/jsii.ts index 9587c8d1b3..fa7cfe0c5b 100644 --- a/packages/jsii/bin/jsii.ts +++ b/packages/jsii/bin/jsii.ts @@ -56,14 +56,8 @@ const warningTypes = Object.keys(enabledWarnings); )})`, }) .option('strip-deprecated', { - type: 'boolean', - default: false, - desc: '[EXPERIMENTAL] Hides all @deprecated members from the API (implementations remain)', - }) - .option('strip-deprecated-allowlist-file', { type: 'string', - default: undefined, - desc: '[EXPERIMENTAL] If provided, only full-qualified names listed in the allow list file (newline-delimited) will be stripped with --strip-deprecated', + desc: '[EXPERIMENTAL] Hides all @deprecated members from the API (implementations remain). If an optional file name is given, only FQNs present in the file will be stripped.', }) .option('add-deprecation-warnings', { type: 'boolean', @@ -116,8 +110,8 @@ const warningTypes = Object.keys(enabledWarnings); projectInfo, projectReferences: argv['project-references'], failOnWarnings: argv['fail-on-warnings'], - stripDeprecated: argv['strip-deprecated'], - stripDeprecatedAllowListFile: argv['strip-deprecated-allowlist-file'], + stripDeprecated: !!argv['strip-deprecated'], + stripDeprecatedAllowListFile: argv['strip-deprecated'], addDeprecationWarnings: argv['add-deprecation-warnings'], generateTypeScriptConfig: argv['generate-tsconfig'], }); diff --git a/packages/jsii/lib/assembler.ts b/packages/jsii/lib/assembler.ts index 8a842e31f0..675653ad4e 100644 --- a/packages/jsii/lib/assembler.ts +++ b/packages/jsii/lib/assembler.ts @@ -76,16 +76,18 @@ export class Assembler implements Emitter { options: AssemblerOptions = {}, ) { if (options.stripDeprecated) { - let allowlistedDeprecations: string[] | undefined; + let allowlistedDeprecations: Set | undefined; if (options.stripDeprecatedAllowListFile) { if (!fs.existsSync(options.stripDeprecatedAllowListFile)) { throw new Error( `--strip-deprecated file not found: ${options.stripDeprecatedAllowListFile}`, ); } - allowlistedDeprecations = fs - .readFileSync(options.stripDeprecatedAllowListFile, 'utf8') - .split('\n'); + allowlistedDeprecations = new Set( + fs + .readFileSync(options.stripDeprecatedAllowListFile, 'utf8') + .split('\n'), + ); } this.deprecatedRemover = new DeprecatedRemover( diff --git a/packages/jsii/lib/transforms/deprecated-remover.ts b/packages/jsii/lib/transforms/deprecated-remover.ts index 0476df30a9..9422bad4a2 100644 --- a/packages/jsii/lib/transforms/deprecated-remover.ts +++ b/packages/jsii/lib/transforms/deprecated-remover.ts @@ -106,7 +106,7 @@ export class DeprecatedRemover { typeInfo.members.forEach((mem) => { if ( mem.docs?.stability === Stability.Deprecated && - this.shouldFqnBeStripped(`${fqn}.${mem.name}`) + this.shouldFqnBeStripped(`${fqn}#${mem.name}`) ) { const matchingMemberNode = enumNode.members.find( (enumMem) => enumMem.name.getText() === mem.name, @@ -216,7 +216,7 @@ export class DeprecatedRemover { typeInfo.methods?.forEach((meth) => { if ( meth.docs?.stability === Stability.Deprecated && - this.shouldFqnBeStripped(`${fqn}.${meth.name}`) + this.shouldFqnBeStripped(`${fqn}#${meth.name}`) ) { this.nodesToRemove.add(bindings.getMethodRelatedNode(meth)!); } else { @@ -231,7 +231,7 @@ export class DeprecatedRemover { typeInfo.properties?.forEach((prop) => { if ( prop.docs?.stability === Stability.Deprecated && - this.shouldFqnBeStripped(`${fqn}.${prop.name}`) + this.shouldFqnBeStripped(`${fqn}#${prop.name}`) ) { this.nodesToRemove.add(bindings.getParameterRelatedNode(prop)!); } else { diff --git a/packages/jsii/test/deprecated-remover.test.ts b/packages/jsii/test/deprecated-remover.test.ts index 5603c89df2..302909834d 100644 --- a/packages/jsii/test/deprecated-remover.test.ts +++ b/packages/jsii/test/deprecated-remover.test.ts @@ -198,11 +198,11 @@ describe('stripDeprecatedAllowList', () => { [ 'testpkg.IDeprecatedInterface', 'testpkg.DeprecatedClass', - 'testpkg.SomeEnum.VALUE_DEPRECATED', + 'testpkg.SomeEnum#VALUE_DEPRECATED', 'testpkg.DeprecatedEnum', - 'testpkg.Retained.deprecated', + 'testpkg.Retained#deprecated', 'testpkg.Deprecated', - 'testpkg.GrandChild.deprecatedMethod', + 'testpkg.GrandChild#deprecatedMethod', ].join('\n'), 'utf8', ); @@ -262,8 +262,8 @@ describe('stripDeprecatedAllowList', () => { stripDeprecatedAllowListFile, [ 'testpkg.IDeprecatedInterface', - 'testpkg.SomeEnum.VALUE_DEPRECATED', - 'testpkg.GrandChild.deprecatedMethod', + 'testpkg.SomeEnum#VALUE_DEPRECATED', + 'testpkg.GrandChild#deprecatedMethod', ].join('\n'), 'utf8', );