refactor: address TODO about id type

src/agent/debuglet.ts

@@ -156,7 +156,7 @@ export interface FindFilesResult {
   jsStats: scanner.ScanStats;
   mapFiles: string[];
   errors: Map<string, Error>;
-  hash?: string;
+  hash: string;
 }
 
 export class Debuglet extends EventEmitter {
@@ -285,9 +285,10 @@ export class Debuglet extends EventEmitter {
     return extend(true, {}, defaultConfig, config, envConfig);
   }
 
-  static async findFiles(shouldHash: boolean, baseDir: string):
+  static async findFiles(baseDir: string, precomputedHash?: string):
       Promise<FindFilesResult> {
-    const fileStats = await scanner.scan(shouldHash, baseDir, /.js$|.js.map$/);
+    const fileStats =
+        await scanner.scan(baseDir, /.js$|.js.map$/, precomputedHash);
     const jsStats = fileStats.selectStats(/.js$/);
     const mapFiles = fileStats.selectFiles(/.js.map$/, process.cwd());
     const errors = fileStats.errors();
@@ -325,15 +326,15 @@ export class Debuglet extends EventEmitter {
       return;
     }
 
-    // TODO: Verify that it is fine for `id` to be undefined.
-    let id: string|undefined;
+    let gaeId: string|undefined;
     if (process.env.GAE_MINOR_VERSION) {
-      id = 'GAE-' + process.env.GAE_MINOR_VERSION;
+      gaeId = 'GAE-' + process.env.GAE_MINOR_VERSION;
     }
 
     let findResults: FindFilesResult;
     try {
-      findResults = await Debuglet.findFiles(!id, that.config.workingDirectory);
+      findResults =
+          await Debuglet.findFiles(that.config.workingDirectory, gaeId);
       findResults.errors.forEach(that.logger.warn);
     } catch (err) {
       that.logger.error('Error scanning the filesystem.', err);
@@ -352,7 +353,7 @@ export class Debuglet extends EventEmitter {
     that.v8debug =
         debugapi.create(that.logger, that.config, findResults.jsStats, mapper);
 
-    id = id || findResults.hash;
+    const id: string = gaeId || findResults.hash;
 
     that.logger.info('Unique ID for this Application: ' + id);
 
@@ -406,11 +407,9 @@ export class Debuglet extends EventEmitter {
     that.logger.debug('Starting debuggee, project', project);
     that.running = true;
 
-    // TODO: Address the case where `project` is `undefined`.
     that.project = project;
     that.debuggee = Debuglet.createDebuggee(
-        // TODO: Address the case when `id` is `undefined`.
-        project, id as string, that.config.serviceContext, sourceContext, onGCP,
+        project, id, that.config.serviceContext, sourceContext, onGCP,
         that.debug.packageInfo, that.config.description, undefined);
     that.scheduleRegistration_(0 /* immediately */);
     that.emit('started');

src/agent/io/scanner.ts

@@ -42,7 +42,7 @@ export interface ScanResults {
   all(): ScanStats;
   selectStats(regex: RegExp): ScanStats;
   selectFiles(regex: RegExp, baseDir: string): string[];
-  hash?: string;
+  hash: string;
 }
 
 class ScanResultsImpl implements ScanResults {
@@ -60,7 +60,7 @@ class ScanResultsImpl implements ScanResults {
    */
   constructor(
       private readonly stats: ScanStats, readonly errorMap: Map<string, Error>,
-      readonly hash?: string) {}
+      readonly hash: string) {}
 
   errors(): Map<string, Error> {
     return this.errorMap;
@@ -112,24 +112,24 @@ class ScanResultsImpl implements ScanResults {
 }
 
 export async function scan(
-    shouldHash: boolean, baseDir: string, regex: RegExp): Promise<ScanResults> {
+    baseDir: string, regex: RegExp,
+    precomputedHash?: string): Promise<ScanResults> {
   const fileList = await findFiles(baseDir, regex);
-  return await computeStats(fileList, shouldHash);
+  return await computeStats(fileList, precomputedHash);
 }
 
 /**
  * This function accept an array of filenames and computes a unique hash-code
  * based on the contents.
  *
- * @param {!Array<string>} fileList array of filenames
- * @param {Boolean} shouldHash whether a hash should be computed
- * @param {!function(?Error, ?string, Object)} callback error-back style callback
- *    returning the hash-code and an object containing file statistics.
+ * @param fileList array of filenames
+ * @param precomputedHash if available, hashing operations will be omitted
+ * during scan
  */
 // TODO: Typescript: Fix the docs associated with this function to match the
 // call signature
 function computeStats(
-    fileList: string[], shouldHash: boolean): Promise<ScanResults> {
+    fileList: string[], precomputedHash?: string): Promise<ScanResults> {
   return new Promise<ScanResults>(async (resolve, reject) => {
     // return a valid, if fake, result when there are no js files to hash.
     if (fileList.length === 0) {
@@ -144,8 +144,8 @@ function computeStats(
 
     for (const filename of fileList) {
       try {
-        const fileStats = await statsForFile(filename, shouldHash);
-        if (shouldHash) {
+        const fileStats = await statsForFile(filename, !precomputedHash);
+        if (!precomputedHash) {
           hashes.push(fileStats.hash);
         }
         statistics[filename] = fileStats;
@@ -154,13 +154,15 @@ function computeStats(
       }
     }
 
-    let hash;
-    if (shouldHash) {
+    let hash: string;
+    if (!precomputedHash) {
       // Sort the hashes to get a deterministic order as the files may
       // not be in the same order each time we scan the disk.
       const buffer = hashes.sort().join();
       const sha1 = crypto.createHash('sha1').update(buffer).digest('hex');
       hash = 'SHA1-' + sha1;
+    } else {
+      hash = precomputedHash!;
     }
     resolve(new ScanResultsImpl(statistics, errors, hash));
   });

test/test-circular.ts

@@ -50,18 +50,17 @@ describe(__filename, () => {
 
   beforeEach((done) => {
     if (!api) {
-      scanner.scan(true, config.workingDirectory, /.js$/)
-          .then(async (fileStats) => {
-            assert.strictEqual(fileStats.errors().size, 0);
-            const jsStats = fileStats.selectStats(/.js$/);
-            const mapFiles = fileStats.selectFiles(/.map$/, process.cwd());
-            const mapper = await SourceMapper.create(mapFiles);
-            api = debugapi.create(
-                      logger, config, jsStats,
-                      mapper as SourceMapper.SourceMapper) as debugapi.DebugApi;
-            assert.ok(api, 'should be able to create the api');
-            done();
-          });
+      scanner.scan(config.workingDirectory, /.js$/).then(async (fileStats) => {
+        assert.strictEqual(fileStats.errors().size, 0);
+        const jsStats = fileStats.selectStats(/.js$/);
+        const mapFiles = fileStats.selectFiles(/.map$/, process.cwd());
+        const mapper = await SourceMapper.create(mapFiles);
+        api = debugapi.create(
+                  logger, config, jsStats,
+                  mapper as SourceMapper.SourceMapper) as debugapi.DebugApi;
+        assert.ok(api, 'should be able to create the api');
+        done();
+      });
     } else {
       assert(stateIsClean(api));
       done();

test/test-debuglet.ts

@@ -92,15 +92,16 @@ describe('Debuglet', () => {
     it('throws an error for an invalid directory', async () => {
       let err: Error|null = null;
       try {
-        await Debuglet.findFiles(false, path.join(SOURCEMAP_DIR, '!INVALID!'));
+        await Debuglet.findFiles(
+            path.join(SOURCEMAP_DIR, '!INVALID!'), 'fake-id');
       } catch (e) {
         err = e;
       }
       assert.ok(err);
     });
 
     it('finds the correct sourcemaps files', async () => {
-      const searchResults = await Debuglet.findFiles(false, SOURCEMAP_DIR);
+      const searchResults = await Debuglet.findFiles(SOURCEMAP_DIR, 'fake-id');
       assert(searchResults.jsStats);
       assert.strictEqual(Object.keys(searchResults.jsStats).length, 1);
       assert(searchResults.jsStats[path.join(SOURCEMAP_DIR, 'js-file.js')]);
@@ -252,7 +253,7 @@ describe('Debuglet', () => {
       }
       const mockedDebuglet = proxyquire('../src/agent/debuglet', {
         './io/scanner': {
-          scan: (shouldHash: boolean, baseDir: string, regex: RegExp) => {
+          scan: (baseDir: string, regex: RegExp, precomputedHash?: string) => {
             assert.strictEqual(baseDir, MOCKED_DIRECTORY);
             const results: ScanResults = {
               errors: () => {
@@ -270,7 +271,8 @@ describe('Debuglet', () => {
               },
               selectFiles: (regex: RegExp, baseDir: string) => {
                 return [];
-              }
+              },
+              hash: precomputedHash || 'fake-hash'
             };
             return results;
           }
@@ -659,13 +661,14 @@ describe('Debuglet', () => {
          // Don't actually scan the entire filesystem.  Act like the filesystem
          // is empty.
          mockedDebuglet.Debuglet.findFiles =
-             (shouldHash: boolean, baseDir: string):
+             (baseDir: string, precomputedHash?: string):
                  Promise<FindFilesResult> => {
                    assert.strictEqual(baseDir, root);
                    return Promise.resolve({
                      jsStats: {},
                      mapFiles: [],
-                     errors: new Map<string, Error>()
+                     errors: new Map<string, Error>(),
+                     hash: 'fake-hash'
                    });
                  };
 

test/test-duplicate-expressions.ts

@@ -50,20 +50,19 @@ describe(__filename, () => {
 
   beforeEach((done) => {
     if (!api) {
-      scanner.scan(true, config.workingDirectory, /.js$/)
-          .then(async (fileStats) => {
-            assert.strictEqual(fileStats.errors().size, 0);
-            const jsStats = fileStats.selectStats(/.js$/);
-            const mapFiles = fileStats.selectFiles(/.map$/, process.cwd());
-            const mapper = await SourceMapper.create(mapFiles);
-            // TODO: Handle the case when mapper is undefined
-            // TODO: Handle the case when v8debugapi.create returns null
-            api = debugapi.create(
-                      logger, config, jsStats,
-                      mapper as SourceMapper.SourceMapper) as debugapi.DebugApi;
-            assert.ok(api, 'should be able to create the api');
-            done();
-          });
+      scanner.scan(config.workingDirectory, /.js$/).then(async (fileStats) => {
+        assert.strictEqual(fileStats.errors().size, 0);
+        const jsStats = fileStats.selectStats(/.js$/);
+        const mapFiles = fileStats.selectFiles(/.map$/, process.cwd());
+        const mapper = await SourceMapper.create(mapFiles);
+        // TODO: Handle the case when mapper is undefined
+        // TODO: Handle the case when v8debugapi.create returns null
+        api = debugapi.create(
+                  logger, config, jsStats,
+                  mapper as SourceMapper.SourceMapper) as debugapi.DebugApi;
+        assert.ok(api, 'should be able to create the api');
+        done();
+      });
     } else {
       assert(stateIsClean1(api));
       done();

test/test-duplicate-nested-expressions.ts

@@ -45,20 +45,19 @@ describe(__filename, () => {
 
   beforeEach((done) => {
     if (!api) {
-      scanner.scan(true, config.workingDirectory, /.js$/)
-          .then(async (fileStats) => {
-            assert.strictEqual(fileStats.errors().size, 0);
-            const jsStats = fileStats.selectStats(/.js$/);
-            const mapFiles = fileStats.selectFiles(/.map$/, process.cwd());
-            const mapper = await SourceMapper.create(mapFiles);
-            // TODO: Handle the case when mapper is undefined
-            // TODO: Handle the case when v8debugapi.create returns null
-            api = debugapi.create(
-                      logger, config, jsStats,
-                      mapper as SourceMapper.SourceMapper) as debugapi.DebugApi;
-            assert.ok(api, 'should be able to create the api');
-            done();
-          });
+      scanner.scan(config.workingDirectory, /.js$/).then(async (fileStats) => {
+        assert.strictEqual(fileStats.errors().size, 0);
+        const jsStats = fileStats.selectStats(/.js$/);
+        const mapFiles = fileStats.selectFiles(/.map$/, process.cwd());
+        const mapper = await SourceMapper.create(mapFiles);
+        // TODO: Handle the case when mapper is undefined
+        // TODO: Handle the case when v8debugapi.create returns null
+        api = debugapi.create(
+                  logger, config, jsStats,
+                  mapper as SourceMapper.SourceMapper) as debugapi.DebugApi;
+        assert.ok(api, 'should be able to create the api');
+        done();
+      });
     } else {
       assert(stateIsClean2(api));
       done();

test/test-evaluated-expressions.ts

@@ -36,15 +36,14 @@ describe('debugger provides useful information', () => {
   before(done => {
     const logger =
         consoleLogLevel({level: Debuglet.logLevelToName(config.logLevel)});
-    scanner.scan(true, config.workingDirectory, /\.js$/)
-        .then(async fileStats => {
-          const jsStats = fileStats.selectStats(/\.js$/);
-          const mapFiles = fileStats.selectFiles(/\.map$/, process.cwd());
-          const mapper = await SourceMapper.create(mapFiles);
-          assert(mapper);
-          api = debugapi.create(logger, config, jsStats, mapper!);
-          done();
-        });
+    scanner.scan(config.workingDirectory, /\.js$/).then(async fileStats => {
+      const jsStats = fileStats.selectStats(/\.js$/);
+      const mapFiles = fileStats.selectFiles(/\.map$/, process.cwd());
+      const mapper = await SourceMapper.create(mapFiles);
+      assert(mapper);
+      api = debugapi.create(logger, config, jsStats, mapper!);
+      done();
+    });
   });
 
   function getValue(

test/test-expression-side-effect.ts

@@ -40,15 +40,14 @@ describe('evaluating expressions', () => {
   before(done => {
     const logger =
         consoleLogLevel({level: Debuglet.logLevelToName(config.logLevel)});
-    scanner.scan(true, config.workingDirectory, /\.js$/)
-        .then(async fileStats => {
-          const jsStats = fileStats.selectStats(/\.js$/);
-          const mapFiles = fileStats.selectFiles(/\.map$/, process.cwd());
-          const mapper = await SourceMapper.create(mapFiles);
-          assert(mapper);
-          api = debugapi.create(logger, config, jsStats, mapper!);
-          done();
-        });
+    scanner.scan(config.workingDirectory, /\.js$/).then(async fileStats => {
+      const jsStats = fileStats.selectStats(/\.js$/);
+      const mapFiles = fileStats.selectFiles(/\.map$/, process.cwd());
+      const mapper = await SourceMapper.create(mapFiles);
+      assert(mapper);
+      api = debugapi.create(logger, config, jsStats, mapper!);
+      done();
+    });
   });
 
   itWithInspector(

test/test-fat-arrow.ts

@@ -47,21 +47,20 @@ describe(__filename, () => {
   });
   beforeEach((done) => {
     if (!api) {
-      scanner.scan(true, config.workingDirectory, /.js$/)
-          .then(async (fileStats) => {
-            assert.strictEqual(fileStats.errors().size, 0);
-            const jsStats = fileStats.selectStats(/.js$/);
-            const mapFiles = fileStats.selectFiles(/.map$/, process.cwd());
-            // TODO: Determine if the err parameter should be used.
-            const mapper = await SourceMapper.create(mapFiles);
-            // TODO: Handle the case when mapper is undefined
-            // TODO: Handle the case when v8debugapi.create returns null
-            api = debugapi.create(
-                      logger, config, jsStats,
-                      mapper as SourceMapper.SourceMapper) as debugapi.DebugApi;
-            assert.ok(api, 'should be able to create the api');
-            done();
-          });
+      scanner.scan(config.workingDirectory, /.js$/).then(async (fileStats) => {
+        assert.strictEqual(fileStats.errors().size, 0);
+        const jsStats = fileStats.selectStats(/.js$/);
+        const mapFiles = fileStats.selectFiles(/.map$/, process.cwd());
+        // TODO: Determine if the err parameter should be used.
+        const mapper = await SourceMapper.create(mapFiles);
+        // TODO: Handle the case when mapper is undefined
+        // TODO: Handle the case when v8debugapi.create returns null
+        api = debugapi.create(
+                  logger, config, jsStats,
+                  mapper as SourceMapper.SourceMapper) as debugapi.DebugApi;
+        assert.ok(api, 'should be able to create the api');
+        done();
+      });
     } else {
       assert(stateIsClean(api));
       done();