From fb97e67fcb4a546a9f8f734ed4f19a4e71ef40e5 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Mon, 18 Jan 2021 15:32:35 +0000 Subject: [PATCH] build(docs-infra): fail if there are unused example regions (#40479) We can define regions in our examples that can be referenced and rendered in guides as code snippets. It is quite hard to ensure that these regions are maintained correctly. One reason for this is it is hard to know whether a region is being used or not. This commit adds a new processor that checks for unused named regions in examples and fails if any are found. Fixes #19761 PR Close #40479 --- .../transforms/angular-base-package/index.js | 5 +- .../transforms/angular.io-package/index.js | 6 ++ .../transforms/examples-package/index.js | 1 + .../check-for-unused-example-regions.js | 38 +++++++++ .../check-for-unused-example-regions.spec.js | 77 +++++++++++++++++++ .../services/getExampleRegion.js | 6 +- .../services/getExampleRegion.spec.js | 11 +++ 7 files changed, 140 insertions(+), 4 deletions(-) create mode 100644 aio/tools/transforms/examples-package/processors/check-for-unused-example-regions.js create mode 100644 aio/tools/transforms/examples-package/processors/check-for-unused-example-regions.spec.js diff --git a/aio/tools/transforms/angular-base-package/index.js b/aio/tools/transforms/angular-base-package/index.js index ab7020c6d3..890b480668 100644 --- a/aio/tools/transforms/angular-base-package/index.js +++ b/aio/tools/transforms/angular-base-package/index.js @@ -53,10 +53,11 @@ module.exports = new Package('angular-base', [ inlineTagProcessor.inlineTagDefinitions.push(require('./inline-tag-defs/custom-search-defs/')); }) - .config(function(checkAnchorLinksProcessor) { - // This is disabled here to prevent false negatives for the `docs-watch` task. + .config(function(checkAnchorLinksProcessor, checkForUnusedExampleRegions) { + // These are disabled here to prevent false negatives for the `docs-watch` task. // It is re-enabled in the main `angular.io-package` checkAnchorLinksProcessor.$enabled = false; + checkForUnusedExampleRegions.$enabled = false; }) // Where do we get the source files? diff --git a/aio/tools/transforms/angular.io-package/index.js b/aio/tools/transforms/angular.io-package/index.js index 139937e839..dd53e65985 100644 --- a/aio/tools/transforms/angular.io-package/index.js +++ b/aio/tools/transforms/angular.io-package/index.js @@ -29,6 +29,12 @@ module.exports = new Package('angular.io', [gitPackage, apiPackage, contentPacka renderDocsProcessor.extraData.versionInfo = versionInfo; }) + .config(function(checkForUnusedExampleRegions) { + // Re-enable this processor that was disabled in the `angular-base` package to + // avoid running it during `serve-and-sync` runs. + checkForUnusedExampleRegions.$enabled = true; + }) + .config(function(checkAnchorLinksProcessor, linkInlineTagDef, renderExamples) { // Fail the processing if there is an invalid link diff --git a/aio/tools/transforms/examples-package/index.js b/aio/tools/transforms/examples-package/index.js index 436cde974f..39da1cb346 100644 --- a/aio/tools/transforms/examples-package/index.js +++ b/aio/tools/transforms/examples-package/index.js @@ -13,6 +13,7 @@ module.exports = .processor(require('./processors/collect-examples')) .processor(require('./processors/render-examples')) + .processor(require('./processors/check-for-unused-example-regions')) .config(function(readFilesProcessor, exampleFileReader) { readFilesProcessor.fileReaders.push(exampleFileReader); diff --git a/aio/tools/transforms/examples-package/processors/check-for-unused-example-regions.js b/aio/tools/transforms/examples-package/processors/check-for-unused-example-regions.js new file mode 100644 index 0000000000..f94b4d78c7 --- /dev/null +++ b/aio/tools/transforms/examples-package/processors/check-for-unused-example-regions.js @@ -0,0 +1,38 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +/** + * A processor that will fail if there are named example regions that are not used in any docs. + * + * @param {*} exampleMap - contains all the regions extracted from example files. + */ +module.exports = function checkForUnusedExampleRegions(exampleMap) { + return { + $runAfter: ['renderExamples'], + $runBefore: ['writing-files'], + $process() { + const unusedExampleRegions = []; + for (const exampleFolder of Object.values(exampleMap)) { + for (const exampleFile of Object.values(exampleFolder)) { + for (const [regionName, region] of Object.entries(exampleFile.regions)) { + if (regionName === '' || region.usedInDoc) continue; + unusedExampleRegions.push(region); + } + } + } + + if (unusedExampleRegions.length > 0) { + const message = (unusedExampleRegions.length === 1 ? + 'There is 1 unused example region:\n' : + `There are ${unusedExampleRegions.length} unused example regions:\n`) + + unusedExampleRegions.map(region => ` - ${region.id}`).join('\n'); + throw new Error(message); + } + }, + }; +}; diff --git a/aio/tools/transforms/examples-package/processors/check-for-unused-example-regions.spec.js b/aio/tools/transforms/examples-package/processors/check-for-unused-example-regions.spec.js new file mode 100644 index 0000000000..56021e38b2 --- /dev/null +++ b/aio/tools/transforms/examples-package/processors/check-for-unused-example-regions.spec.js @@ -0,0 +1,77 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ +const testPackage = require('../../helpers/test-package'); +const Dgeni = require('dgeni'); + +describe('checkForUnusedExampleRegions', () => { + var processor, exampleMap; + + beforeEach(function() { + const dgeni = new Dgeni([testPackage('examples-package', true)]); + const injector = dgeni.configureInjector(); + exampleMap = injector.get('exampleMap'); + processor = injector.get('checkForUnusedExampleRegions'); + }); + + describe('$process()', () => { + it('should error if there is a named example region which has not been used', () => { + exampleMap['examples'] = { + 'some/file': { + regions: { + '': {id: 'some/file#'}, + 'used': {id: 'some/file#used', usedInDoc: {}}, + 'not-used': {id: 'some/file#not-used'}, + } + } + }; + expect(() => processor.$process()) + .toThrowError('There is 1 unused example region:\n - some/file#not-used'); + }); + + it('should not error if there are no example folders', () => { + expect(() => processor.$process()).not.toThrowError(); + }); + + it('should not error if there are no example files', () => { + exampleMap['examples'] = {}; + expect(() => processor.$process()).not.toThrowError(); + }); + + it('should not error if there are no example regions', () => { + exampleMap['examples'] = { + 'some/file': { + regions: {}, + }, + }; + expect(() => processor.$process()).not.toThrowError(); + }); + + it('should not error if there are no named example regions', () => { + exampleMap['examples'] = { + 'some/file': { + regions: { + '': {id: 'some/file#'}, + }, + }, + }; + expect(() => processor.$process()).not.toThrowError(); + }); + + it('should not error if there are no unused named example regions', () => { + exampleMap['examples'] = { + 'some/file': { + regions: { + '': {id: 'some/file#'}, + 'used': {id: 'some/file#used', usedInDoc: {}}, + } + } + }; + expect(() => processor.$process()).not.toThrowError(); + }); + }); +}); diff --git a/aio/tools/transforms/examples-package/services/getExampleRegion.js b/aio/tools/transforms/examples-package/services/getExampleRegion.js index 62defda9ba..94fd9274f9 100644 --- a/aio/tools/transforms/examples-package/services/getExampleRegion.js +++ b/aio/tools/transforms/examples-package/services/getExampleRegion.js @@ -1,4 +1,4 @@ -module.exports = function getExampleRegion(exampleMap, createDocMessage, collectExamples) { +module.exports = function getExampleRegion(exampleMap, createDocMessage, collectExamples, log) { return function getExampleRegionImpl(doc, relativePath, regionName) { const EXAMPLES_FOLDERS = collectExamples.exampleFolders; @@ -29,10 +29,12 @@ module.exports = function getExampleRegion(exampleMap, createDocMessage, collect var sourceCodeDoc = exampleFile.regions[regionName || '']; if (!sourceCodeDoc) { const message = createDocMessage('Missing example region... relativePath: "' + relativePath + '", region: "' + regionName + '".', doc) + '\n' + - 'Regions available are: ' + Object.keys(exampleFile.regions).map(function(region) { return '"' + region + '"'; }).join(', '); + 'Regions available are: ' + Object.keys(exampleFile.regions).map(function(region) { return '"' + region + '"'; }).join(', '); throw new Error(message); } + sourceCodeDoc.usedInDoc = doc; + log.debug(`Example region ${sourceCodeDoc.id} used in ${doc.id}`); return sourceCodeDoc.renderedContent; }; }; diff --git a/aio/tools/transforms/examples-package/services/getExampleRegion.spec.js b/aio/tools/transforms/examples-package/services/getExampleRegion.spec.js index ba42d11805..362ff0a2cc 100644 --- a/aio/tools/transforms/examples-package/services/getExampleRegion.spec.js +++ b/aio/tools/transforms/examples-package/services/getExampleRegion.spec.js @@ -43,4 +43,15 @@ describe('getExampleRegion', () => { }).toThrowError('Ignored example file... relativePath: "filtered/path" - doc\n' + 'This example file exists but has been ignored by a rule, in "some/gitignore".'); }); + + it('should mark the example as having been "used"', () => { + const doc1 = {}; + const doc2 = {}; + expect(exampleMap['examples']['test/url'].regions['region-1'].usedInDoc).toBeUndefined(); + getExampleRegion(doc1, 'test/url', 'region-1'); + expect(exampleMap['examples']['test/url'].regions['region-1'].usedInDoc).toBe(doc1); + expect(exampleMap['examples']['test/url'].regions[''].usedInDoc).toBeUndefined(); + getExampleRegion(doc2, 'test/url'); + expect(exampleMap['examples']['test/url'].regions[''].usedInDoc).toBe(doc2); + }); });