build(aio): ensure the correct decorator properties are merged (angular#24289)

Previously only the `description` and `usageNotes` were being copied over
from the call-member of the decorator interface. Important properties such
as `shortDescription` were missed.

These are now added and the code has been refactored to make it simpler and
clearer to update which properties get copied as the requirements change.

PR Close angular#24289

Author: petebacondarwin

aio/tools/transforms/angular-api-package/index.js

@@ -119,6 +119,17 @@ module.exports = new Package('angular-api', [basePackage, typeScriptPackage])
     addNotYetDocumentedProperty.docTypes = API_DOC_TYPES;
   })
 
+  .config(function(mergeDecoratorDocs) {
+    mergeDecoratorDocs.propertiesToMerge = [
+      'shortDescription',
+      'description',
+      'security',
+      'deprecated',
+      'see',
+      'usageNotes',
+    ];
+  })
+
   .config(function(checkContentRules, EXPORT_DOC_TYPES) {
     // Min length rules
     const createMinLengthRule = require('./content-rules/minLength');

aio/tools/transforms/angular-api-package/processors/mergeDecoratorDocs.js

@@ -1,3 +1,5 @@
+const {mergeProperties} = require('../../helpers/utils');
+
 /**
  * Decorators in the Angular code base are made up from three code items:
  *
@@ -47,6 +49,7 @@ module.exports = function mergeDecoratorDocs(log) {
   return {
     $runAfter: ['processing-docs'],
     $runBefore: ['docs-processed'],
+    propertiesToMerge: [],
     makeDecoratorCalls: [
       {type: '', description: 'toplevel', functionName: 'makeDecorator'},
       {type: 'Prop', description: 'property', functionName: 'makePropDecorator'},
@@ -55,6 +58,7 @@ module.exports = function mergeDecoratorDocs(log) {
     $process: function(docs) {
 
       var makeDecoratorCalls = this.makeDecoratorCalls;
+      var propertiesToMerge = this.propertiesToMerge;
       var docsToMerge = Object.create(null);
 
       docs.forEach(function(doc) {
@@ -88,9 +92,9 @@ module.exports = function mergeDecoratorDocs(log) {
           log.debug(
               'mergeDecoratorDocs: merging', doc.name, 'into', decoratorDoc.name,
               callMember.description.substring(0, 50));
+
           // Merge the documentation found in this call signature into the original decorator
-          decoratorDoc.description = callMember.description;
-          decoratorDoc.usageNotes = callMember.usageNotes;
+          mergeProperties(decoratorDoc, callMember, propertiesToMerge);
 
           // remove doc from its module doc's exports
           doc.moduleDoc.exports =

aio/tools/transforms/angular-api-package/processors/mergeDecoratorDocs.spec.js

@@ -9,33 +9,37 @@ describe('mergeDecoratorDocs processor', () => {
     const injector = dgeni.configureInjector();
     processor = injector.get('mergeDecoratorDocs');
 
+    // Note that we do not include usageNotes in the tests.
+    processor.propertiesToMerge = ['description', 'shortDescription'];
+
     moduleDoc = {};
 
     decoratorDoc = {
       name: 'Component',
       docType: 'const',
-      description: 'A description of the metadata for the Component decorator',
+      shortDescription: 'decorator - short description',
+      description: 'decorator - description',
       symbol: {
         valueDeclaration: { initializer: { expression: { text: 'makeDecorator' }, arguments: [{ text: 'X' }] } }
       },
       members: [
-        { name: 'templateUrl', description: 'A description of the templateUrl property' }
+        { name: 'templateUrl', description: 'templateUrl - description' }
       ],
       moduleDoc
     };
 
     metadataDoc = {
       name: 'ComponentDecorator',
       docType: 'interface',
-      description: 'A description of the interface for the call signature for the Component decorator',
+      description: 'call interface - description',
       members: [
         {
           isCallMember: true,
-          description: 'The actual description of the call signature',
-          usageNotes: 'Use it like this...'
+          description: 'call interface - call member - description',
+          usageNotes: 'call interface - call member - usageNotes',
         },
         {
-          description: 'Some other member'
+          description: 'call interface - non call member - description'
         }
       ],
       moduleDoc
@@ -65,10 +69,13 @@ describe('mergeDecoratorDocs processor', () => {
     expect(decoratorDoc.decoratorType).toEqual('X');
   });
 
-  it('should copy across properties from the call signature doc', () => {
+  it('should copy across specified properties from the call signature doc', () => {
     processor.$process([decoratorDoc, metadataDoc, otherDoc]);
-    expect(decoratorDoc.description).toEqual('The actual description of the call signature');
-    expect(decoratorDoc.usageNotes).toEqual('Use it like this...');
+    expect(decoratorDoc.description).toEqual('call interface - call member - description');
+    // Since usageNotes is not in `propertiesToMerge` it will not get copied over in these tests.
+    expect(decoratorDoc.usageNotes).toBeUndefined();
+    // Since `shortDescription` does not exist on the call-member this will not get overridden.
+    expect(decoratorDoc.shortDescription).toEqual('decorator - short description');
   });
 
   it('should remove the metadataDoc from the module exports', () => {

aio/tools/transforms/helpers/utils.js

@@ -96,5 +96,19 @@ module.exports = {
       attrMap[key] === false ? '' :
       attrMap[key] === true ? ` ${key}` :
       ` ${key}="${attrMap[key].replace(/"/g, '"')}"`).join('');
-  }
+  },
+
+  /**
+   * Merge the specified properties from the source to the target document
+   * @param {Document} target The document to receive the properties from the source
+   * @param {Document} source The document from which to get the properties to merge
+   * @param {string[]} properties A collection of the names of the properties to merge
+   */
+  mergeProperties(target, source, properties) {
+    properties.forEach(property => {
+      if (source.hasOwnProperty(property)) {
+        target[property] = source[property];
+      }
+    });
+  },
 };

aio/tools/transforms/helpers/utils.spec.js

@@ -1,4 +1,4 @@
-const { mapObject, parseAttributes, renderAttributes } = require('./utils');
+const { mergeProperties, mapObject, parseAttributes, renderAttributes } = require('./utils');
 
 describe('utils', () => {
   describe('mapObject', () => {
@@ -96,4 +96,34 @@ describe('utils', () => {
       expect(renderAttributes({ })).toEqual('');
     });
   });
+
+  describe('mergeProperties', () => {
+    it('should write specified properties from the source to the target', () => {
+      const source = { a: 1, b: 2, c: 3 };
+      const target = { };
+      mergeProperties(target, source, ['a', 'b']);
+      expect(target).toEqual({ a: 1, b: 2 });
+    });
+
+    it('should not overwrite target properties that are not specified', () => {
+      const source = { a: 1, b: 2, c: 3 };
+      const target = { b: 10 };
+      mergeProperties(target, source, ['a']);
+      expect(target).toEqual({ a: 1, b: 10 });
+    });
+
+    it('should not overwrite target properties that are specified but do not exist in the source', () => {
+      const source = { a: 1 };
+      const target = { b: 10 };
+      mergeProperties(target, source, ['a', 'b']);
+      expect(target).toEqual({ a: 1, b: 10 });
+    });
+
+    it('should overwrite target properties even if they are `undefined` in the source', () => {
+      const source = { a: undefined };
+      const target = { a: 10 };
+      mergeProperties(target, source, ['a']);
+      expect(target).toEqual({ a: undefined });
+    });
+  });
 });