build: refactor ts-api-guardian jsdoc tag handling (#26595)
Allow the jsdoc tag processing to be configured by type (export, member, param) and by action (required, banned, toCopy). This is a pre-requisite to moving over to using `@publicApi` tags rather than `@stable` and `@experimental`. PR Close #26595
This commit is contained in:
		
							parent
							
								
									31022cbecf
								
							
						
					
					
						commit
						2ea57cdcc3
					
				| @ -24,6 +24,9 @@ export function startCli() { | |||||||
|   const options: SerializationOptions = { |   const options: SerializationOptions = { | ||||||
|     stripExportPattern: [].concat(argv['stripExportPattern']), |     stripExportPattern: [].concat(argv['stripExportPattern']), | ||||||
|     allowModuleIdentifiers: [].concat(argv['allowModuleIdentifiers']), |     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) { |   for (const error of errors) { | ||||||
|  | |||||||
| @ -15,6 +15,23 @@ const baseTsOptions: ts.CompilerOptions = { | |||||||
|   moduleResolution: ts.ModuleResolutionKind.Classic |   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 { | export interface SerializationOptions { | ||||||
|   /** |   /** | ||||||
|    * Removes all exports matching the regular expression. |    * Removes all exports matching the regular expression. | ||||||
| @ -31,6 +48,15 @@ export interface SerializationOptions { | |||||||
|    * whitelisting angular. |    * whitelisting angular. | ||||||
|    */ |    */ | ||||||
|   allowModuleIdentifiers?: string[]; |   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'; | 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.
 |   // the path needs to be normalized with forward slashes in order to work within Windows.
 | ||||||
|   const entrypoint = path.normalize(fileName).replace(/\\/g, '/'); |   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$/)) { |   if (!entrypoint.match(/\.d\.ts$/)) { | ||||||
|     throw new Error(`Source file "${fileName}" is not a declaration file`); |     throw new Error(`Source file "${fileName}" is not a declaration file`); | ||||||
|   } |   } | ||||||
| @ -115,12 +149,9 @@ class ResolvedDeclarationEmitter { | |||||||
|           output += '\n'; |           output += '\n'; | ||||||
|         } |         } | ||||||
| 
 | 
 | ||||||
|         // Print stability annotation
 |         const jsdocComment = this.processJsDocTags(decl, this.options.exportTags); | ||||||
|         const sourceText = decl.getSourceFile().text; |         if (jsdocComment) { | ||||||
|         const trivia = sourceText.substr(decl.pos, decl.getLeadingTriviaWidth()); |           output += jsdocComment + '\n'; | ||||||
|         const match = stabilityAnnotationPattern.exec(trivia); |  | ||||||
|         if (match) { |  | ||||||
|           output += `/** @${match[1]} */\n`; |  | ||||||
|         } |         } | ||||||
| 
 | 
 | ||||||
|         output += stripEmptyLines(this.emitNode(decl)) + '\n'; |         output += stripEmptyLines(this.emitNode(decl)) + '\n'; | ||||||
| @ -254,13 +285,13 @@ class ResolvedDeclarationEmitter { | |||||||
|                                .map(n => this.emitNode(n)) |                                .map(n => this.emitNode(n)) | ||||||
|                                .join(''); |                                .join(''); | ||||||
| 
 | 
 | ||||||
|       // Print stability annotation for fields
 |       // Print stability annotation for fields and parmeters
 | ||||||
|       if (ts.isParameter(node) || node.kind in memberDeclarationOrder) { |       if (ts.isParameter(node) || node.kind in memberDeclarationOrder) { | ||||||
|         const trivia = sourceText.substr(node.pos, node.getLeadingTriviaWidth()); |         const tagOptions = ts.isParameter(node) ? this.options.paramTags : this.options.memberTags; | ||||||
|         const match = stabilityAnnotationPattern.exec(trivia); |         const jsdocComment = this.processJsDocTags(node, tagOptions); | ||||||
|         if (match) { |         if (jsdocComment) { | ||||||
|           // Add the annotation after the leading whitespace
 |           // 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); |       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) { | function symbolCompareFunction(a: ts.Symbol, b: ts.Symbol) { | ||||||
| @ -299,8 +380,6 @@ const memberDeclarationOrder: {[key: number]: number} = { | |||||||
|   [ts.SyntaxKind.MethodDeclaration]: 4 |   [ts.SyntaxKind.MethodDeclaration]: 4 | ||||||
| }; | }; | ||||||
| 
 | 
 | ||||||
| const stabilityAnnotationPattern = /@(experimental|stable|deprecated)\b/; |  | ||||||
| 
 |  | ||||||
| function stripEmptyLines(text: string): string { | function stripEmptyLines(text: string): string { | ||||||
|   return text.split('\n').filter(x => !!x.length).join('\n'); |   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 { | function hasModifier(node: ts.Node, modifierKind: ts.SyntaxKind): boolean { | ||||||
|   return !!node.modifiers && node.modifiers.some(x => x.kind === modifierKind); |   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()) + '`'; | ||||||
|  | } | ||||||
| @ -404,7 +404,7 @@ describe('unit test', () => { | |||||||
|     check({'file.d.ts': input}, expected); |     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 = ` |     const input = ` | ||||||
|       /** |       /** | ||||||
|        * @deprecated This is useless now |        * @deprecated This is useless now | ||||||
| @ -428,14 +428,14 @@ describe('unit test', () => { | |||||||
|       /** @experimental */ |       /** @experimental */ | ||||||
|       export declare const b: string; |       export declare const b: string; | ||||||
| 
 | 
 | ||||||
|       /** @stable */ |  | ||||||
|       export declare var c: number; |       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 = ` |     const input = ` | ||||||
|  |       /** @otherTag */ | ||||||
|       export declare class A { |       export declare class A { | ||||||
|         /** |         /** | ||||||
|          * @stable |          * @stable | ||||||
| @ -443,6 +443,7 @@ describe('unit test', () => { | |||||||
|         value: number; |         value: number; | ||||||
|         /** |         /** | ||||||
|          * @experimental |          * @experimental | ||||||
|  |          * @otherTag | ||||||
|          */ |          */ | ||||||
|         constructor(); |         constructor(); | ||||||
|         /** |         /** | ||||||
| @ -453,12 +454,107 @@ describe('unit test', () => { | |||||||
|     `;
 |     `;
 | ||||||
|     const expected = ` |     const expected = ` | ||||||
|       export declare class A { |       export declare class A { | ||||||
|         /** @stable */ value: number; |         value: number; | ||||||
|         /** @experimental */ constructor(); |         /** @experimental */ constructor(); | ||||||
|         /** @deprecated */ foo(): void; |         /** @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()); |   chai.assert.equal(actual.trim(), stripExtraIndentation(expected).trim()); | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| function checkThrows(files: {[name: string]: string}, error: string) { | function checkThrows( | ||||||
|   chai.assert.throws(() => { publicApiInternal(getMockHost(files), 'file.d.ts', {}); }, error); |     files: {[name: string]: string}, error: string, options: SerializationOptions = {}) { | ||||||
|  |   chai.assert.throws( | ||||||
|  |       () => { publicApiInternal(getMockHost(files), 'file.d.ts', {}, options); }, error); | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| function stripExtraIndentation(text: string) { | function stripExtraIndentation(text: string) { | ||||||
|  | |||||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user