refactor(bazel): Fix warning about overridden tsconfig options (#28674)

Under Bazel, some compilerOptions in tsconfig.json are controlled by
downstream rules. The default tsconfig.json causes Bazel to print out
warnings about overriden settings.

This commit makes a backup of the original tsconfig.json and removes
tsconfig settings that are controlled by Bazel.

As part of this fix, JsonAst utils are refactored into separate package
and unit tests are added.

PR closes https://github.com/angular/angular/issues/28034

PR Close #28674

packages/bazel/src/schematics/BUILD.bazel

@@ -17,6 +17,7 @@ jasmine_node_test(
         "//packages/bazel/src/schematics/bazel-workspace:test",
         "//packages/bazel/src/schematics/ng-add:test",
         "//packages/bazel/src/schematics/ng-new:test",
+        "//packages/bazel/src/schematics/utility:test",
         "//tools/testing:node",
     ],
 )

packages/bazel/src/schematics/ng-add/BUILD.bazel

@@ -13,6 +13,7 @@ ts_library(
     ],
     deps = [
         "//packages/bazel/src/schematics/bazel-workspace",
+        "//packages/bazel/src/schematics/utility",
         "@ngdeps//@angular-devkit/core",
         "@ngdeps//@angular-devkit/schematics",
         "@ngdeps//@schematics/angular",

packages/bazel/src/schematics/ng-add/index.ts

@@ -8,11 +8,12 @@
  * @fileoverview Schematics for ng-new project that builds with Bazel.
  */
 
-import {SchematicContext, apply, applyTemplates, chain, mergeWith, move, Rule, schematic, Tree, url, SchematicsException, UpdateRecorder,} from '@angular-devkit/schematics';
-import {parseJsonAst, JsonAstObject, strings, JsonValue} from '@angular-devkit/core';
+import {JsonAstObject, parseJsonAst, strings} from '@angular-devkit/core';
+import {Rule, SchematicContext, SchematicsException, Tree, apply, applyTemplates, chain, mergeWith, move, schematic, url} from '@angular-devkit/schematics';
+import {getWorkspacePath} from '@schematics/angular/utility/config';
 import {findPropertyInAstObject, insertPropertyInAstObjectInOrder} from '@schematics/angular/utility/json-utils';
 import {validateProjectName} from '@schematics/angular/utility/validation';
-import {getWorkspacePath} from '@schematics/angular/utility/config';
+import {isJsonAstObject, removeKeyValueInAstObject as removeKeyValueInAstObject, replacePropertyInAstObject} from '../utility/json-utils';
 import {Schema} from './schema';
 
 /**
@@ -102,21 +103,6 @@ function updateGitignore() {
   };
 }
 
-function replacePropertyInAstObject(
-    recorder: UpdateRecorder, node: JsonAstObject, propertyName: string, value: JsonValue,
-    indent: number) {
-  const property = findPropertyInAstObject(node, propertyName);
-  if (property === null) {
-    throw new Error(`Property ${propertyName} does not exist in JSON object`);
-  }
-  const {start, text} = property;
-  recorder.remove(start.offset, text.length);
-  const indentStr = '\n' +
-      ' '.repeat(indent);
-  const content = JSON.stringify(value, null, '  ').replace(/\n/g, indentStr);
-  recorder.insertLeft(start.offset, content);
-}
-
 function updateAngularJsonToUseBazelBuilder(options: Schema): Rule {
   return (host: Tree, context: SchematicContext) => {
     const {name} = options;
@@ -216,6 +202,61 @@ function backupAngularJson(): Rule {
   };
 }
 
+/**
+ * Create a backup for the original tsconfig.json file in case user wants to
+ * eject Bazel and revert to the original workflow.
+ */
+function backupTsconfigJson(): Rule {
+  return (host: Tree, context: SchematicContext) => {
+    const tsconfigPath = 'tsconfig.json';
+    if (!host.exists(tsconfigPath)) {
+      return;
+    }
+    host.create(
+        `${tsconfigPath}.bak`, '// This is a backup file of the original tsconfig.json. ' +
+            'This file is needed in case you want to revert to the workflow without Bazel.\n\n' +
+            host.read(tsconfigPath));
+  };
+}
+
+/**
+ * Bazel controls the compilation options of tsc, so many options in
+ * tsconfig.json generated by the default CLI schematics are not applicable.
+ * This function updates the tsconfig.json to remove Bazel-controlled
+ * parameters. This prevents Bazel from printing out warnings about overriden
+ * settings.
+ */
+function updateTsconfigJson(): Rule {
+  return (host: Tree, context: SchematicContext) => {
+    const tsconfigPath = 'tsconfig.json';
+    if (!host.exists(tsconfigPath)) {
+      return host;
+    }
+    const content = host.read(tsconfigPath).toString();
+    const ast = parseJsonAst(content);
+    if (!isJsonAstObject(ast)) {
+      return host;
+    }
+    const compilerOptions = findPropertyInAstObject(ast, 'compilerOptions');
+    if (!isJsonAstObject(compilerOptions)) {
+      return host;
+    }
+    const recorder = host.beginUpdate(tsconfigPath);
+    // target and module are controlled by downstream dependencies, such as
+    // ts_devserver
+    removeKeyValueInAstObject(recorder, content, compilerOptions, 'target');
+    removeKeyValueInAstObject(recorder, content, compilerOptions, 'module');
+    // typeRoots is always set to the @types subdirectory of the node_modules
+    // attribute
+    removeKeyValueInAstObject(recorder, content, compilerOptions, 'typeRoots');
+    // rootDir and baseUrl are always the workspace root directory
+    removeKeyValueInAstObject(recorder, content, compilerOptions, 'rootDir');
+    removeKeyValueInAstObject(recorder, content, compilerOptions, 'baseUrl');
+    host.commitUpdate(recorder);
+    return host;
+  };
+}
+
 export default function(options: Schema): Rule {
   return (host: Tree) => {
     validateProjectName(options.name);
@@ -225,8 +266,10 @@ export default function(options: Schema): Rule {
       addDevAndProdMainForAot(options),
       addDevDependenciesToPackageJson(options),
       backupAngularJson(),
+      backupTsconfigJson(),
       updateAngularJsonToUseBazelBuilder(options),
       updateGitignore(),
+      updateTsconfigJson(),
     ]);
   };
 }

packages/bazel/src/schematics/ng-add/index_spec.ts

@@ -26,6 +26,13 @@ describe('ng-add schematic', () => {
         'typescript': '3.2.2',
       },
     }));
+    host.create('tsconfig.json', JSON.stringify({
+      compileOnSave: false,
+      compilerOptions: {
+        baseUrl: './',
+        outDir: './dist/out-tsc',
+      }
+    }));
     host.create('angular.json', JSON.stringify({
       projects: {
         'demo': {
@@ -168,4 +175,27 @@ describe('ng-add schematic', () => {
         .toBe('@angular-devkit/build-angular:extract-i18n');
     expect(lint.builder).toBe('@angular-devkit/build-angular:tslint');
   });
+
+  it('should create a backup for original tsconfig.json', () => {
+    expect(host.files).toContain('/tsconfig.json');
+    const original = host.readContent('/tsconfig.json');
+    host = schematicRunner.runSchematic('ng-add', defaultOptions, host);
+    expect(host.files).toContain('/tsconfig.json.bak');
+    const content = host.readContent('/tsconfig.json.bak');
+    expect(content.startsWith('// This is a backup file')).toBe(true);
+    expect(content).toMatch(original);
+  });
+
+  it('should remove Bazel-controlled options from tsconfig.json', () => {
+    host = schematicRunner.runSchematic('ng-add', defaultOptions, host);
+    expect(host.files).toContain('/tsconfig.json');
+    const content = host.readContent('/tsconfig.json');
+    expect(() => JSON.parse(content)).not.toThrow();
+    expect(JSON.parse(content)).toEqual({
+      compileOnSave: false,
+      compilerOptions: {
+        outDir: './dist/out-tsc',
+      }
+    });
+  });
 });

packages/bazel/src/schematics/utility/BUILD.bazel

@@ -0,0 +1,30 @@
+package(default_visibility = ["//visibility:public"])
+
+load("//tools:defaults.bzl", "ts_library")
+
+ts_library(
+    name = "utility",
+    srcs = [
+        "json-utils.ts",
+    ],
+    module_name = "@angular/bazel/src/schematics/utility",
+    deps = [
+        "@ngdeps//@angular-devkit/core",
+        "@ngdeps//@angular-devkit/schematics",
+        "@ngdeps//@schematics/angular",
+        "@ngdeps//typescript",
+    ],
+)
+
+ts_library(
+    name = "test",
+    testonly = True,
+    srcs = [
+        "json-utils_spec.ts",
+    ],
+    deps = [
+        ":utility",
+        "@ngdeps//@angular-devkit/core",
+        "@ngdeps//@angular-devkit/schematics",
+    ],
+)

packages/bazel/src/schematics/utility/json-utils.ts

@@ -0,0 +1,65 @@
+/**
+ * @license
+ * Copyright Google Inc. 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
+ */
+
+import {JsonAstNode, JsonAstObject, JsonValue} from '@angular-devkit/core';
+import {UpdateRecorder} from '@angular-devkit/schematics';
+import {findPropertyInAstObject} from '@schematics/angular/utility/json-utils';
+
+/**
+ * Replace the value of the key-value pair in the 'node' object with a different
+ * 'value' and record the update using the specified 'recorder'.
+ */
+export function replacePropertyInAstObject(
+    recorder: UpdateRecorder, node: JsonAstObject, propertyName: string, value: JsonValue,
+    indent: number = 0) {
+  const property = findPropertyInAstObject(node, propertyName);
+  if (property === null) {
+    throw new Error(`Property '${propertyName}' does not exist in JSON object`);
+  }
+  const {start, text} = property;
+  recorder.remove(start.offset, text.length);
+  const indentStr = '\n' +
+      ' '.repeat(indent);
+  const content = JSON.stringify(value, null, '  ').replace(/\n/g, indentStr);
+  recorder.insertLeft(start.offset, content);
+}
+
+/**
+ * Remove the key-value pair with the specified 'key' in the specified 'node'
+ * object and record the update using the specified 'recorder'.
+ */
+export function removeKeyValueInAstObject(
+    recorder: UpdateRecorder, content: string, node: JsonAstObject, key: string) {
+  for (const [i, prop] of node.properties.entries()) {
+    if (prop.key.value === key) {
+      const start = prop.start.offset;
+      const end = prop.end.offset;
+      let length = end - start;
+      const match = content.slice(end).match(/[,\s]+/);
+      if (match) {
+        length += match.pop() !.length;
+      }
+      recorder.remove(start, length);
+      if (i === node.properties.length - 1) {  // last property
+        let offset = 0;
+        while (/(,|\s)/.test(content.charAt(start - offset - 1))) {
+          offset++;
+        }
+        recorder.remove(start - offset, offset);
+      }
+      return;
+    }
+  }
+}
+
+/**
+ * Returns true if the specified 'node' is a JsonAstObject, false otherwise.
+ */
+export function isJsonAstObject(node: JsonAstNode | null): node is JsonAstObject {
+  return !!node && node.kind === 'object';
+}

packages/bazel/src/schematics/utility/json-utils_spec.ts

@@ -0,0 +1,109 @@
+/**
+ * @license
+ * Copyright Google Inc. 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
+ */
+
+import {JsonAstObject, parseJsonAst} from '@angular-devkit/core';
+import {HostTree} from '@angular-devkit/schematics';
+import {UnitTestTree} from '@angular-devkit/schematics/testing';
+import {isJsonAstObject, removeKeyValueInAstObject, replacePropertyInAstObject} from './json-utils';
+
+describe('JsonUtils', () => {
+
+  let tree: UnitTestTree;
+  beforeEach(() => { tree = new UnitTestTree(new HostTree()); });
+
+  describe('replacePropertyInAstObject', () => {
+    it('should replace property', () => {
+      const content = JSON.stringify({foo: {bar: 'baz'}});
+      tree.create('tmp', content);
+      const ast = parseJsonAst(content) as JsonAstObject;
+      const recorder = tree.beginUpdate('tmp');
+      replacePropertyInAstObject(recorder, ast, 'foo', [1, 2, 3]);
+      tree.commitUpdate(recorder);
+      const value = tree.readContent('tmp');
+      expect(JSON.parse(value)).toEqual({
+        foo: [1, 2, 3],
+      });
+      expect(value).toBe(`{"foo":[
+  1,
+  2,
+  3
+]}`);
+    });
+
+    it('should respect the indent parameter', () => {
+      const content = JSON.stringify({hello: 'world'}, null, 2);
+      tree.create('tmp', content);
+      const ast = parseJsonAst(content) as JsonAstObject;
+      const recorder = tree.beginUpdate('tmp');
+      replacePropertyInAstObject(recorder, ast, 'hello', 'world!', 2);
+      tree.commitUpdate(recorder);
+      const value = tree.readContent('tmp');
+      expect(JSON.parse(value)).toEqual({
+        hello: 'world!',
+      });
+      expect(value).toBe(`{
+  "hello": "world!"
+}`);
+    });
+
+    it('should throw error if property is not found', () => {
+      const content = JSON.stringify({});
+      tree.create('tmp', content);
+      const ast = parseJsonAst(content) as JsonAstObject;
+      const recorder = tree.beginUpdate('tmp');
+      expect(() => replacePropertyInAstObject(recorder, ast, 'foo', 'bar'))
+          .toThrowError(`Property 'foo' does not exist in JSON object`);
+    });
+  });
+
+  describe('removeKeyValueInAstObject', () => {
+    it('should remove key-value pair', () => {
+      const content = JSON.stringify({hello: 'world', foo: 'bar'});
+      tree.create('tmp', content);
+      const ast = parseJsonAst(content) as JsonAstObject;
+      let recorder = tree.beginUpdate('tmp');
+      removeKeyValueInAstObject(recorder, content, ast, 'foo');
+      tree.commitUpdate(recorder);
+      const tmp = tree.readContent('tmp');
+      expect(JSON.parse(tmp)).toEqual({
+        hello: 'world',
+      });
+      expect(tmp).toBe('{"hello":"world"}');
+      recorder = tree.beginUpdate('tmp');
+      const newContent = tree.readContent('tmp');
+      removeKeyValueInAstObject(recorder, newContent, ast, 'hello');
+      tree.commitUpdate(recorder);
+      const value = tree.readContent('tmp');
+      expect(JSON.parse(value)).toEqual({});
+      expect(value).toBe('{}');
+    });
+
+    it('should be a noop if key is not found', () => {
+      const content = JSON.stringify({foo: 'bar'});
+      tree.create('tmp', content);
+      const ast = parseJsonAst(content) as JsonAstObject;
+      let recorder = tree.beginUpdate('tmp');
+      expect(() => removeKeyValueInAstObject(recorder, content, ast, 'hello')).not.toThrow();
+      tree.commitUpdate(recorder);
+      const value = tree.readContent('tmp');
+      expect(JSON.parse(value)).toEqual({foo: 'bar'});
+      expect(value).toBe('{"foo":"bar"}');
+    });
+  });
+
+  describe('isJsonAstObject', () => {
+    it('should return true for an object', () => {
+      const ast = parseJsonAst(JSON.stringify({}));
+      expect(isJsonAstObject(ast)).toBe(true);
+    });
+    it('should return false for a non-object', () => {
+      const ast = parseJsonAst(JSON.stringify([]));
+      expect(isJsonAstObject(ast)).toBe(false);
+    });
+  });
+});