diff --git a/tools/ts-api-guardian/lib/cli.ts b/tools/ts-api-guardian/lib/cli.ts index b16a15903a..72f6fbcf07 100644 --- a/tools/ts-api-guardian/lib/cli.ts +++ b/tools/ts-api-guardian/lib/cli.ts @@ -24,6 +24,9 @@ export function startCli() { const options: SerializationOptions = { stripExportPattern: [].concat(argv['stripExportPattern']), allowModuleIdentifiers: [].concat(argv['allowModuleIdentifiers']), + exportTags: {required: [], banned: [], toCopy: ['deprecated', 'experimental']}, + memberTags: {required: [], banned: [], toCopy: ['deprecated', 'experimental']}, + paramTags: {required: [], banned: [], toCopy: ['deprecated', 'experimental']} }; for (const error of errors) { diff --git a/tools/ts-api-guardian/lib/serializer.ts b/tools/ts-api-guardian/lib/serializer.ts index 194cfd7745..b3c37518eb 100644 --- a/tools/ts-api-guardian/lib/serializer.ts +++ b/tools/ts-api-guardian/lib/serializer.ts @@ -15,6 +15,23 @@ const baseTsOptions: ts.CompilerOptions = { moduleResolution: ts.ModuleResolutionKind.Classic }; +export interface JsDocTagOptions { + /** + * An array of names of jsdoc tags that must exist. + */ + required?: string[]; + + /** + * An array of names of jsdoc tags that must not exist. + */ + banned?: string[]; + + /** + * An array of names of jsdoc tags that will be copied to the serialized code. + */ + toCopy?: string[]; +} + export interface SerializationOptions { /** * Removes all exports matching the regular expression. @@ -31,6 +48,15 @@ export interface SerializationOptions { * whitelisting angular. */ allowModuleIdentifiers?: string[]; + + /** The jsdoc tag options for top level exports */ + exportTags?: JsDocTagOptions; + + /** The jsdoc tag options for properties/methods/etc of exports */ + memberTags?: JsDocTagOptions; + + /** The jsdoc tag options for parameters of members/functions */ + paramTags?: JsDocTagOptions; } export type DiagnosticSeverity = 'warn' | 'error' | 'none'; @@ -46,6 +72,14 @@ export function publicApiInternal( // the path needs to be normalized with forward slashes in order to work within Windows. const entrypoint = path.normalize(fileName).replace(/\\/g, '/'); + // Setup default tag options + options = { + ...options, + exportTags: applyDefaultTagOptions(options.exportTags), + memberTags: applyDefaultTagOptions(options.memberTags), + paramTags: applyDefaultTagOptions(options.paramTags) + }; + if (!entrypoint.match(/\.d\.ts$/)) { throw new Error(`Source file "${fileName}" is not a declaration file`); } @@ -115,12 +149,9 @@ class ResolvedDeclarationEmitter { output += '\n'; } - // Print stability annotation - const sourceText = decl.getSourceFile().text; - const trivia = sourceText.substr(decl.pos, decl.getLeadingTriviaWidth()); - const match = stabilityAnnotationPattern.exec(trivia); - if (match) { - output += `/** @${match[1]} */\n`; + const jsdocComment = this.processJsDocTags(decl, this.options.exportTags); + if (jsdocComment) { + output += jsdocComment + '\n'; } output += stripEmptyLines(this.emitNode(decl)) + '\n'; @@ -254,13 +285,13 @@ class ResolvedDeclarationEmitter { .map(n => this.emitNode(n)) .join(''); - // Print stability annotation for fields + // Print stability annotation for fields and parmeters if (ts.isParameter(node) || node.kind in memberDeclarationOrder) { - const trivia = sourceText.substr(node.pos, node.getLeadingTriviaWidth()); - const match = stabilityAnnotationPattern.exec(trivia); - if (match) { + const tagOptions = ts.isParameter(node) ? this.options.paramTags : this.options.memberTags; + const jsdocComment = this.processJsDocTags(node, tagOptions); + if (jsdocComment) { // Add the annotation after the leading whitespace - output = output.replace(/^(\n\s*)/, `$1/** @${match[1]} */ `); + output = output.replace(/^(\n\s*)/, `$1${jsdocComment} `); } } @@ -276,6 +307,56 @@ class ResolvedDeclarationEmitter { return sourceText.substring(tail, node.end); } } + + private processJsDocTags(node: ts.Node, tagOptions: JsDocTagOptions) { + const jsDocTags = getJsDocTags(node); + const missingRequiredTags = + tagOptions.required.filter(requiredTag => jsDocTags.every(tag => tag !== requiredTag)); + if (missingRequiredTags.length) { + this.diagnostics.push({ + type: 'error', + message: createErrorMessage( + node, 'Required jsdoc tags - ' + + missingRequiredTags.map(tag => `"@${tag}"`).join(', ') + + ` - are missing on ${getName(node)}.`) + }); + } + const bannedTagsFound = + tagOptions.banned.filter(bannedTag => jsDocTags.some(tag => tag === bannedTag)); + if (bannedTagsFound.length) { + this.diagnostics.push({ + type: 'error', + message: createErrorMessage( + node, 'Banned jsdoc tags - ' + bannedTagsFound.map(tag => `"@${tag}"`).join(', ') + + ` - were found on ${getName(node)}.`) + }); + } + const tagsToCopy = + jsDocTags.filter(tag => tagOptions.toCopy.some(tagToCopy => tag === tagToCopy)); + + if (tagsToCopy.length === 1) { + return `/** @${tagsToCopy[0]} */`; + } else if (tagsToCopy.length > 1) { + return '/**\n' + tagsToCopy.map(tag => ` * @${tag}`).join('\n') + ' */\n'; + } else { + return ''; + } + } +} + +const tagRegex = /@(\w+)/g; + +function getJsDocTags(node: ts.Node): string[] { + const sourceText = node.getSourceFile().text; + const trivia = sourceText.substr(node.pos, node.getLeadingTriviaWidth()); + // We use a hash so that we don't collect duplicate jsdoc tags + // (e.g. if a property has a getter and setter with the same tag). + const jsdocTags: {[key: string]: boolean} = {}; + let match: RegExpExecArray; + while (match = tagRegex.exec(trivia)) { + jsdocTags[match[1]] = true; + } + return Object.keys(jsdocTags); } function symbolCompareFunction(a: ts.Symbol, b: ts.Symbol) { @@ -299,8 +380,6 @@ const memberDeclarationOrder: {[key: number]: number} = { [ts.SyntaxKind.MethodDeclaration]: 4 }; -const stabilityAnnotationPattern = /@(experimental|stable|deprecated)\b/; - function stripEmptyLines(text: string): string { return text.split('\n').filter(x => !!x.length).join('\n'); } @@ -349,3 +428,11 @@ function createErrorMessage(node: ts.Node, message: string): string { function hasModifier(node: ts.Node, modifierKind: ts.SyntaxKind): boolean { return !!node.modifiers && node.modifiers.some(x => x.kind === modifierKind); } + +function applyDefaultTagOptions(tagOptions: JsDocTagOptions | undefined): JsDocTagOptions { + return {required: [], banned: [], toCopy: [], ...tagOptions}; +} + +function getName(node: any) { + return '`' + (node.name && node.name.text ? node.name.text : node.getText()) + '`'; +} \ No newline at end of file diff --git a/tools/ts-api-guardian/test/unit_test.ts b/tools/ts-api-guardian/test/unit_test.ts index faa1ab903a..7b26aa31ba 100644 --- a/tools/ts-api-guardian/test/unit_test.ts +++ b/tools/ts-api-guardian/test/unit_test.ts @@ -404,7 +404,7 @@ describe('unit test', () => { check({'file.d.ts': input}, expected); }); - it('should keep stability annotations of exports in docstrings', () => { + it('should copy specified jsdoc tags of exports in docstrings', () => { const input = ` /** * @deprecated This is useless now @@ -428,14 +428,14 @@ describe('unit test', () => { /** @experimental */ export declare const b: string; - /** @stable */ export declare var c: number; `; - check({'file.d.ts': input}, expected); + check({'file.d.ts': input}, expected, {exportTags: {toCopy: ['deprecated', 'experimental']}}); }); - it('should keep stability annotations of fields in docstrings', () => { + it('should copy specified jsdoc tags of fields in docstrings', () => { const input = ` + /** @otherTag */ export declare class A { /** * @stable @@ -443,6 +443,7 @@ describe('unit test', () => { value: number; /** * @experimental + * @otherTag */ constructor(); /** @@ -453,12 +454,107 @@ describe('unit test', () => { `; const expected = ` export declare class A { - /** @stable */ value: number; + value: number; /** @experimental */ constructor(); /** @deprecated */ foo(): void; } `; - check({'file.d.ts': input}, expected); + check({'file.d.ts': input}, expected, {memberTags: {toCopy: ['deprecated', 'experimental']}}); + }); + + it('should copy specified jsdoc tags of parameters in docstrings', () => { + const input = ` + export declare class A { + foo(str: string, /** @deprecated */ value: number): void; + } + `; + const expected = ` + export declare class A { + foo(str: string, /** @deprecated */ value: number): void; + } + `; + check({'file.d.ts': input}, expected, {paramTags: {toCopy: ['deprecated', 'experimental']}}); + }); + + it('should throw on using banned jsdoc tags on exports', () => { + const input = ` + /** + * @stable + */ + export declare class A { + value: number; + } + `; + checkThrows( + {'file.d.ts': input}, + 'file.d.ts(4,1): error: Banned jsdoc tags - "@stable" - were found on `A`.', + {exportTags: {banned: ['stable']}}); + }); + + it('should throw on using banned jsdoc tags on fields', () => { + const input = ` + export declare class A { + /** + * @stable + */ + value: number; + } + `; + checkThrows( + {'file.d.ts': input}, + 'file.d.ts(5,3): error: Banned jsdoc tags - "@stable" - were found on `value`.', + {memberTags: {banned: ['stable']}}); + }); + + it('should throw on using banned jsdoc tags on parameters', () => { + const input = ` + export declare class A { + foo(/** @stable */ param: number): void; + } + `; + checkThrows( + {'file.d.ts': input}, + 'file.d.ts(2,22): error: Banned jsdoc tags - "@stable" - were found on `param`.', + {paramTags: {banned: ['stable']}}); + }); + + it('should throw on missing required jsdoc tags on exports', () => { + const input = ` + /** @experimental */ + export declare class A { + value: number; + } + `; + checkThrows( + {'file.d.ts': input}, + 'file.d.ts(2,1): error: Required jsdoc tags - "@stable" - are missing on `A`.', + {exportTags: {required: ['stable']}}); + }); + + it('should throw on missing required jsdoc tags on fields', () => { + const input = ` + /** @experimental */ + export declare class A { + value: number; + } + `; + checkThrows( + {'file.d.ts': input}, + 'file.d.ts(3,3): error: Required jsdoc tags - "@stable" - are missing on `value`.', + {memberTags: {required: ['stable']}}); + }); + + it('should throw on missing required jsdoc tags on parameters', () => { + const input = ` + /** @experimental */ + export declare class A { + foo(param: number): void; + } + `; + checkThrows( + {'file.d.ts': input}, + 'file.d.ts(3,7): error: Required jsdoc tags - "@stable" - are missing on `param`.', + {paramTags: {required: ['stable']}}); }); }); @@ -487,8 +583,10 @@ function check( chai.assert.equal(actual.trim(), stripExtraIndentation(expected).trim()); } -function checkThrows(files: {[name: string]: string}, error: string) { - chai.assert.throws(() => { publicApiInternal(getMockHost(files), 'file.d.ts', {}); }, error); +function checkThrows( + files: {[name: string]: string}, error: string, options: SerializationOptions = {}) { + chai.assert.throws( + () => { publicApiInternal(getMockHost(files), 'file.d.ts', {}, options); }, error); } function stripExtraIndentation(text: string) {