Follow up to #3420

src/commands/logout.ts

@@ -9,7 +9,8 @@ import { promptOnce } from "../prompt";
 module.exports = new Command("logout [email]")
   .description("log the CLI out of Firebase")
   .action(async (email: string | undefined, options: any) => {
-    const globalToken = utils.getInheritedOption(options, "token") as string | undefined;
+    const globalToken = utils.getInheritedOption(options, "token");
+    utils.assertIsStringOrUndefined(globalToken);
 
     const allAccounts = auth.getAllAccounts();
     if (allAccounts.length === 0 && !globalToken) {

src/commands/remoteconfig-get.ts

@@ -6,6 +6,8 @@ import { RemoteConfigTemplate } from "../remoteconfig/interfaces";
 import getProjectId = require("../getProjectId");
 import { requirePermissions } from "../requirePermissions";
 import { parseTemplateForTable } from "../remoteconfig/get";
+import { Options } from "../options";
+import * as utils from "../utils";
 
 import Table = require("cli-table");
 import * as fs from "fs";
@@ -32,7 +34,8 @@ module.exports = new Command("remoteconfig:get")
   )
   .before(requireAuth)
   .before(requirePermissions, ["cloudconfig.configs.get"])
-  .action(async (options) => {
+  .action(async (options: Options) => {
+    utils.assertIsString(options.versionNumber);
     const template: RemoteConfigTemplate = await rcGet.getTemplate(
       getProjectId(options),
       checkValidNumber(options.versionNumber)
@@ -59,9 +62,15 @@ module.exports = new Command("remoteconfig:get")
     const fileOut = !!options.output;
     if (fileOut) {
       const shouldUseDefaultFilename = options.output === true || options.output === "";
-      const filename = shouldUseDefaultFilename
-        ? options.config.get("remoteconfig.template")
-        : options.output;
+
+      let filename = undefined;
+      if (shouldUseDefaultFilename) {
+        filename = options.config.src.remoteconfig!.template;
+      } else {
+        utils.assertIsString(options.output);
+        filename = options.output;
+      }
+
       const outTemplate = { ...template };
       delete outTemplate.version;
       fs.writeFileSync(filename, JSON.stringify(outTemplate, null, 2));

src/config.ts

@@ -2,26 +2,26 @@
 
 import { FirebaseConfig } from "./firebaseConfig";
 
-const _ = require("lodash");
-const clc = require("cli-color");
+import * as _ from "lodash";
+import * as clc from "cli-color";
+import * as fs from "fs-extra";
+import * as path from "path";
 const cjson = require("cjson");
-const fs = require("fs-extra");
-const path = require("path");
 
-const detectProjectRoot = require("./detectProjectRoot").detectProjectRoot;
-const { FirebaseError } = require("./error");
-const fsutils = require("./fsutils");
+import { detectProjectRoot } from "./detectProjectRoot";
+import { FirebaseError } from "./error";
+import * as fsutils from "./fsutils";
+import { promptOnce } from "./prompt";
+import { resolveProjectPath } from "./projectPath";
+import * as utils from "./utils";
 const loadCJSON = require("./loadCJSON");
 const parseBoltRules = require("./parseBoltRules");
-const { promptOnce } = require("./prompt");
-const { resolveProjectPath } = require("./projectPath");
-const utils = require("./utils");
-
-type PlainObject = Record<string, unknown>;
 
 export class Config {
+  static DEFAULT_FUNCTIONS_SOURCE = "functions";
+
   static FILENAME = "firebase.json";
-  static MATERIALIZE_TARGETS = [
+  static MATERIALIZE_TARGETS: Array<keyof FirebaseConfig> = [
     "database",
     "emulators",
     "firestore",
@@ -39,6 +39,10 @@ export class Config {
 
   private _src: any;
 
+  /**
+   * @param src incoming firebase.json source, parsed by not validated.
+   * @param options command-line options.
+   */
   constructor(src: any, options: any) {
     this.options = options || {};
     this.projectDir = options.projectDir || detectProjectRoot(options);
@@ -54,52 +58,43 @@ export class Config {
       );
     }
 
+    // Move the deprecated top-level "rules" ket into the "database" object
     if (_.has(this._src, "rules")) {
       _.set(this._src, "database.rules", this._src.rules);
     }
 
+    // If a top-level key contains a string path pointing to a suported file
+    // type (JSON or Bolt), we read the file.
+    //
+    // TODO: This is janky and confusing behavior, we should remove it ASAP.
     Config.MATERIALIZE_TARGETS.forEach((target) => {
       if (_.get(this._src, target)) {
-        _.set(this.data, target, this._materialize(target));
+        _.set(this.data, target, this.materialize(target));
       }
     });
 
-    // auto-detect functions from package.json in directory
+    // Auto-detect functions from package.json in directory
     if (
       this.projectDir &&
       !this.get("functions.source") &&
       fsutils.fileExistsSync(this.path("functions/package.json"))
     ) {
-      this.set("functions.source", "functions");
-    }
-  }
-
-  _hasDeepKey(obj: PlainObject, key: string) {
-    if (_.has(obj, key)) {
-      return true;
+      this.set("functions.source", Config.DEFAULT_FUNCTIONS_SOURCE);
     }
-
-    for (const k in obj) {
-      if (obj.hasOwnProperty(k)) {
-        if (_.isPlainObject(obj[k]) && this._hasDeepKey(obj[k] as PlainObject, key)) {
-          return true;
-        }
-      }
-    }
-    return false;
   }
 
-  _materialize(target: string) {
+  materialize(target: string) {
     const val = _.get(this._src, target);
-    if (_.isString(val)) {
-      let out = this._parseFile(target, val);
+    if (typeof val === "string") {
+      let out = this.parseFile(target, val);
       // if e.g. rules.json has {"rules": {}} use that
-      const lastSegment = _.last(target.split("."));
-      if (_.size(out) === 1 && _.has(out, lastSegment)) {
+      const segments = target.split(".");
+      const lastSegment = segments[segments.length - 1];
+      if (Object.keys(out).length === 1 && _.has(out, lastSegment)) {
         out = out[lastSegment];
       }
       return out;
-    } else if (_.isPlainObject(val) || _.isArray(val)) {
+    } else if (val !== null && typeof val === "object") {
       return val;
     }
 
@@ -108,7 +103,7 @@ export class Config {
     });
   }
 
-  _parseFile(target: string, filePath: string) {
+  parseFile(target: string, filePath: string) {
     const fullPath = resolveProjectPath(this.options, filePath);
     const ext = path.extname(filePath);
     if (!fsutils.fileExistsSync(fullPath)) {
@@ -158,6 +153,10 @@ export class Config {
   }
 
   set(key: string, value: any) {
+    // TODO: We should really remove all instances of config.set() around the
+    //       codebase but until we do we need this to prevent src from going stale.
+    _.set(this._src, key, value);
+
     return _.set(this.data, key, value);
   }
 
@@ -167,7 +166,7 @@ export class Config {
 
   path(pathName: string) {
     const outPath = path.normalize(path.join(this.projectDir, pathName));
-    if (_.includes(path.relative(this.projectDir, outPath), "..")) {
+    if (path.relative(this.projectDir, outPath).includes("..")) {
       throw new FirebaseError(clc.bold(pathName) + " is outside of project directory", { exit: 1 });
     }
     return outPath;

src/database/rulesConfig.ts

@@ -1,6 +1,8 @@
 import { FirebaseError } from "../error";
 import { Config } from "../config";
 import { logger } from "../logger";
+import { Options } from "../options";
+import * as utils from "../utils";
 
 export interface RulesInstanceConfig {
   instance: string;
@@ -18,9 +20,9 @@ interface DatabaseConfig {
  */
 export function normalizeRulesConfig(
   rulesConfig: RulesInstanceConfig[],
-  options: any
+  options: Options
 ): RulesInstanceConfig[] {
-  const config = options.config as Config;
+  const config = options.config;
   return rulesConfig.map((rc) => {
     return {
       instance: rc.instance,
@@ -29,18 +31,15 @@ export function normalizeRulesConfig(
   });
 }
 
-export function getRulesConfig(projectId: string, options: any): RulesInstanceConfig[] {
-  // TODO(samstern): Config should be typed
-  const config = options.config as any;
-
-  const dbConfig: { rules?: string } | DatabaseConfig[] | undefined = config.get("database");
-
+export function getRulesConfig(projectId: string, options: Options): RulesInstanceConfig[] {
+  const dbConfig = options.config.src.database;
   if (dbConfig === undefined) {
     return [];
   }
 
   if (!Array.isArray(dbConfig)) {
     if (dbConfig && dbConfig.rules) {
+      utils.assertIsStringOrUndefined(options.instance);
       const instance = options.instance || `${options.project}-default-rtdb`;
       return [{ rules: dbConfig.rules, instance }];
     } else {
@@ -50,13 +49,14 @@ export function getRulesConfig(projectId: string, options: any): RulesInstanceCo
   }
 
   const results: RulesInstanceConfig[] = [];
+  const rc = options.rc as any;
   for (const c of dbConfig) {
     if (c.target) {
       // Make sure the target exists (this will throw otherwise)
-      options.rc.requireTarget(projectId, "database", c.target);
+      rc.requireTarget(projectId, "database", c.target);
 
       // Get a list of db instances the target maps to
-      const instances: string[] = options.rc.target(projectId, "database", c.target);
+      const instances: string[] = rc.target(projectId, "database", c.target);
       for (const i of instances) {
         results.push({ instance: i, rules: c.rules });
       }

src/deploy/firestore/prepare.ts

@@ -4,14 +4,15 @@ import * as clc from "cli-color";
 import loadCJSON = require("../../loadCJSON");
 import { RulesDeploy, RulesetServiceType } from "../../rulesDeploy";
 import utils = require("../../utils");
+import { Options } from "../../options";
 
 /**
  * Prepares Firestore Rules deploys.
  * @param context The deploy context.
  * @param options The CLI options object.
  */
-async function prepareRules(context: any, options: any): Promise<void> {
-  const rulesFile = options.config.get("firestore.rules");
+async function prepareRules(context: any, options: Options): Promise<void> {
+  const rulesFile = options.config.src.firestore?.rules;
 
   if (context.firestoreRules && rulesFile) {
     const rulesDeploy = new RulesDeploy(options, RulesetServiceType.CLOUD_FIRESTORE);
@@ -25,12 +26,12 @@ async function prepareRules(context: any, options: any): Promise<void> {
  * @param context The deploy context.
  * @param options The CLI options object.
  */
-function prepareIndexes(context: any, options: any): void {
-  if (!context.firestoreIndexes || !options.config.get("firestore.indexes")) {
+function prepareIndexes(context: any, options: Options): void {
+  if (!context.firestoreIndexes || !options.config.src.firestore?.indexes) {
     return;
   }
 
-  const indexesFileName = options.config.get("firestore.indexes");
+  const indexesFileName = options.config.src.firestore.indexes;
   const indexesPath = options.config.path(indexesFileName);
   const parsedSrc = loadCJSON(indexesPath);
 

src/deploy/functions/deploy.ts

@@ -10,6 +10,8 @@ import * as fs from "fs";
 import * as gcs from "../../gcp/storage";
 import * as gcf from "../../gcp/cloudfunctions";
 import { Options } from "../../options";
+import { Config } from "../../config";
+import * as utils from "../../utils";
 
 const GCP_REGION = functionsUploadRegion;
 
@@ -45,7 +47,7 @@ export async function deploy(
   options: Options,
   payload: args.Payload
 ): Promise<void> {
-  if (!options.config.get("functions")) {
+  if (!options.config.src.functions) {
     return;
   }
 
@@ -66,10 +68,14 @@ export async function deploy(
     }
     await Promise.all(uploads);
 
+    utils.assertDefined(
+      options.config.src.functions.source,
+      "Error: 'functions.source' is not defined"
+    );
     logSuccess(
       clc.green.bold("functions:") +
         " " +
-        clc.bold(options.config.get("functions.source")) +
+        clc.bold(options.config.src.functions.source) +
         " folder uploaded successfully"
     );
   } catch (err) {

src/deploy/functions/prepare.ts

@@ -1,6 +1,5 @@
 import * as clc from "cli-color";
 
-import { FirebaseError } from "../../error";
 import { Options } from "../../options";
 import { ensureCloudBuildEnabled } from "./ensureCloudBuildEnabled";
 import { functionMatchesAnyGroup, getFilterGroups } from "./functionsDeployHelper";
@@ -14,14 +13,15 @@ import * as functionsConfig from "../../functionsConfig";
 import * as getProjectId from "../../getProjectId";
 import * as runtimes from "./runtimes";
 import * as validate from "./validate";
+import * as utils from "../../utils";
 import { logger } from "../../logger";
 
 export async function prepare(
   context: args.Context,
   options: Options,
   payload: args.Payload
 ): Promise<void> {
-  if (!options.config.has("functions")) {
+  if (!options.config.src.functions) {
     return;
   }
 
@@ -60,10 +60,14 @@ export async function prepare(
   }
 
   // Prepare the functions directory for upload, and set context.triggers.
+  utils.assertDefined(
+    options.config.src.functions.source,
+    "Error: 'functions.source' is not defined"
+  );
   logBullet(
     clc.cyan.bold("functions:") +
       " preparing " +
-      clc.bold(options.config.get("functions.source")) +
+      clc.bold(options.config.src.functions.source) +
       " directory for uploading..."
   );
   context.functionsSource = await prepareFunctionsUpload(runtimeConfig, options);