♻️ 🏗 🚀 Begin adding type checking to the build-system directory. (#32531

)

Begin adding type checking to the build-system directory. 
* create tsconfig and fix lots of type errors

build-system/common/ctrlcHandler.js

@@ -61,7 +61,7 @@ exports.createCtrlcHandler = function (command) {
 /**
  * Exits the Ctrl C handler process.
  *
- * @param {string} handlerProcess
+ * @param {string|number} handlerProcess
  */
 exports.exitCtrlcHandler = function (handlerProcess) {
   const exitCmd = killCmd + ' ' + handlerProcess + ' ' + killSuffix;

build-system/common/exec.js

@@ -31,11 +31,10 @@ const shellCmd = process.platform == 'win32' ? 'cmd' : '/bin/bash';
  * object.
  *
  * @param {string} cmd
- * @param {?Object} options
+ * @param {?Object=} options
  * @return {!Object}
  */
-function exec(cmd, options) {
-  options = options || {'stdio': 'inherit'};
+function exec(cmd, options = {'stdio': 'inherit'}) {
   return spawnProcess(cmd, options);
 }
 
@@ -44,7 +43,7 @@ function exec(cmd, options) {
  *
  * @param {string} script
  * @param {?Object} options
- * @return {!ChildProcess}
+ * @return {!childProcess.ChildProcessWithoutNullStreams}
  */
 function execScriptAsync(script, options) {
   return childProcess.spawn(script, {shell: shellCmd, ...options});
@@ -54,7 +53,7 @@ function execScriptAsync(script, options) {
  * Executes the provided command, and terminates the program in case of failure.
  *
  * @param {string} cmd
- * @param {?Object} options
+ * @param {?Object=} options
  */
 function execOrDie(cmd, options) {
   const p = exec(cmd, options);

build-system/common/logging.js

@@ -63,7 +63,7 @@ function logOnSameLine(...messages) {
   if (!isCiBuild() && process.stdout.isTTY) {
     process.stdout.moveCursor(0, -1);
     process.stdout.cursorTo(0);
-    process.stdout.clearLine();
+    process.stdout.clearLine(0);
   }
   log(...messages);
 }

build-system/common/process.js

@@ -38,7 +38,7 @@ function spawnProcess(cmd, options) {
 /**
  * Executes the provided command, returning the process object.
  * @param {string} cmd
- * @param {?Object} options
+ * @param {?Object=} options
  * @return {!Object}
  */
 function getOutput(cmd, options = {}) {
@@ -54,7 +54,7 @@ function getOutput(cmd, options = {}) {
 /**
  * Executes the provided command, returning its stdout.
  * @param {string} cmd
- * @param {?Object} options
+ * @param {?Object=} options
  * @return {string}
  */
 function getStdout(cmd, options) {
@@ -64,7 +64,7 @@ function getStdout(cmd, options) {
 /**
  * Executes the provided command, returning its stderr.
  * @param {string} cmd
- * @param {?Object} options
+ * @param {?Object=} options
  * @return {string}
  */
 function getStderr(cmd, options) {

build-system/compile/check-for-unknown-deps.js

@@ -29,7 +29,7 @@ const {red, cyan, yellow} = require('kleur/colors');
 exports.checkForUnknownDeps = function () {
   const regex = /[\w$]*module\$[\w$]+/;
 
-  return through.obj(function (file, encoding, cb) {
+  return through.obj(function (file, _encoding, cb) {
     const contents = file.contents.toString();
     if (!contents.includes('module$')) {
       // Fast check, since regexes can backtrack like crazy.

build-system/compile/compile.js

@@ -46,9 +46,29 @@ let inProgress = 0;
 const MAX_PARALLEL_CLOSURE_INVOCATIONS =
   parseInt(argv.closure_concurrency, 10) || cpus().length;
 
-// Compiles AMP with the closure compiler. This is intended only for
-// production use. During development we intend to continue using
-// babel, as it has much faster incremental compilation.
+/**
+ * Compiles AMP with the closure compiler. This is intended only for
+ * production use. During development we intend to continue using
+ * babel, as it has much faster incremental compilation.
+ *
+ * @param {string} entryModuleFilename
+ * @param {string} outputDir
+ * @param {string} outputFilename
+ * @param {{
+ *  esmPassCompilation?: string,
+ *  wrapper?: string,
+ *  extraGlobs?: string,
+ *  include3pDirectories?: boolean,
+ *  includePolyfills?: boolean,
+ *  externs?: string[],
+ *  compilationLevel?: string,
+ *  verboseLogging?: boolean,
+ *  typeCheckOnly?: boolean,
+ *  skipUnknownDepsCheck?: boolean,
+ * }} options
+ * @param {{startTime?: number}} timeInfo
+ * @return {Promise<void>}
+ */
 async function closureCompile(
   entryModuleFilename,
   outputDir,
@@ -98,6 +118,25 @@ function cleanupBuildDir() {
   fs.mkdirsSync('build/fake-polyfills/src/polyfills');
 }
 
+/**
+ * @param {string[]|string} entryModuleFilenames
+ * @param {string} outputDir
+ * @param {string} outputFilename
+ * @param {{
+ *  esmPassCompilation?: string,
+ *  wrapper?: string,
+ *  extraGlobs?: string,
+ *  include3pDirectories?: boolean,
+ *  includePolyfills?: boolean,
+ *  externs?: string[],
+ *  compilationLevel?: string,
+ *  verboseLogging?: boolean,
+ *  typeCheckOnly?: boolean,
+ *  skipUnknownDepsCheck?: boolean,
+ * }} options
+ * @param {{startTime?: number}} timeInfo
+ * @return {Promise<void>}
+ */
 function compile(
   entryModuleFilenames,
   outputDir,
@@ -211,6 +250,10 @@ function compile(
     }
     externs.push('build-system/externs/amp.multipass.extern.js');
 
+    /**
+     * TODO(#28387) write a type for this.
+     * @type {Object}
+     */
     /* eslint "google-camelcase/google-camelcase": 0*/
     const compilerOptions = {
       compilation_level: options.compilationLevel || 'SIMPLE_OPTIMIZATIONS',
@@ -377,7 +420,7 @@ function compile(
 function printClosureConcurrency() {
   log(
     green('Using up to'),
-    cyan(MAX_PARALLEL_CLOSURE_INVOCATIONS),
+    cyan(MAX_PARALLEL_CLOSURE_INVOCATIONS.toString()),
     green('concurrent invocations of closure compiler.')
   );
   if (!argv.closure_concurrency) {

build-system/compile/internal-version.js

@@ -124,8 +124,8 @@ function getVersion() {
     `HEAD~${numberOfCherryPicks}`
   ).slice(0, -2);
 
-  numberOfCherryPicks = String(numberOfCherryPicks).padStart(3, '0');
-  return `${lastCommitFormattedTime}${numberOfCherryPicks}`;
+  const numberOfCherryPicksStr = String(numberOfCherryPicks).padStart(3, '0');
+  return `${lastCommitFormattedTime}${numberOfCherryPicksStr}`;
 }
 
 // Used to e.g. references the ads binary from the runtime to get version lock.

build-system/compile/post-closure-babel.js

@@ -75,7 +75,7 @@ async function terserMinify(code, filename) {
  * @return {!Promise}
  */
 exports.postClosureBabel = function () {
-  return through.obj(async function (file, enc, next) {
+  return through.obj(async function (file, _enc, next) {
     if ((!argv.esm && !argv.sxg) || path.extname(file.path) === '.map') {
       debug(
         CompilationLifecycles['complete'],

build-system/compile/pre-closure-babel.js

@@ -116,8 +116,8 @@ function preClosureBabel() {
  *
  * @param {Error} err
  * @param {string} outputFilename
- * @param {?Object} options
- * @param {?Function} resolve
+ * @param {?Object=} options
+ * @param {?Function=} resolve
  */
 function handlePreClosureError(err, outputFilename, options, resolve) {
   log(red('ERROR:'), err.message, '\n');

build-system/compile/sanitize.js

@@ -20,7 +20,7 @@ const through = require('through2');
 const argv = require('minimist')(process.argv.slice(2));
 
 exports.sanitize = function () {
-  return through.obj((file, enc, next) => {
+  return through.obj((file, _enc, next) => {
     if (!argv.sanitize_vars_for_diff) {
       return next(null, file);
     }

build-system/compile/typescript.js

@@ -58,6 +58,8 @@ exports.transpileTs = async function (srcDir, srcFilename) {
     }
     return fileName;
   };
+  // TODO(#28387) fix this typing.
+  /** @type {Object} */
   const transformerHost = {
     host: compilerHost,
     options: tsOptions,

build-system/global.d.ts

@@ -0,0 +1,14 @@
+// TODO(#28387) delete this once all uses of these properties have been removed.
+declare global {
+  interface Error {
+    /**
+     * In the build-system, Error objects contain useful info like underlying
+     * stack traces in the message field, so we print that and hide the less
+     * useful nodejs stack.
+     */
+    showStack?: boolean;
+    status?: string;
+  }
+}
+
+export { }

build-system/server/amp4test.js

@@ -233,7 +233,7 @@ app.get('/a4a/:bid', (req, res) => {
 });
 
 /**
- * @param {{body: string, css: string|undefined, extensions: Array<string>|undefined, head: string|undefined, spec: string|undefined}} config
+ * @param {{body: string, css?: string|undefined, extensions: Array<string>|undefined, head?: string|undefined, spec?: string|undefined, mode?: string|undefined}} config
  * @return {string}
  */
 function composeDocument(config) {

build-system/server/app-index/amphtml-helpers.js

@@ -110,11 +110,11 @@ const addRequiredExtensionsToHead = (
     addExtension(name, {isTemplate: true, ...defaultConf});
 
   Array.from(matchIterator(componentTagNameRegex, docStr))
-    .map(([unusedFullMatch, tagName]) => componentExtensionName(tagName))
+    .map(([, tagName]) => componentExtensionName(tagName))
     .forEach(addExtension);
 
   Array.from(matchIterator(templateTagTypeRegex, docStr))
-    .map(([unusedFullMatch, type]) => type)
+    .map(([, type]) => type)
     .forEach(addTemplate);
 
   // TODO(alanorozco): Too greedy. Parse "on" attributes instead.

build-system/server/app-index/file-list.js

@@ -79,6 +79,10 @@ const ExamplesSelectModeOptional = ({basepath, selectModePrefix}) =>
         selectModePrefix,
       });
 
+/**
+ * @param {{ name: string, href: string, boundHref?: string|undefined }} config
+ * @return {string}
+ */
 const FileListItem = ({name, href, boundHref}) =>
   html`
     <div class="file-link-container" role="listitem">

build-system/server/app-index/html.js

@@ -28,11 +28,11 @@ const joinFragments = (fragments, renderer = identity) =>
 
 /**
  * pass-through for syntax highlighting
- * @param {!Array<string>} strings
+ * @param {!Array<string>|TemplateStringsArray} strings
  * @param {...*} values
  * @return {string}
  */
 const html = (strings, ...values) =>
-  joinFragments(strings, (string, i) => string + (values[i] || ''));
+  joinFragments(Array.from(strings), (string, i) => string + (values[i] || ''));
 
 module.exports = {html, joinFragments};

build-system/server/app-index/template.js

@@ -54,15 +54,15 @@ const HeaderBackToMainLink = () => html` <a href="/">← Back to main</a> `;
 
 const ProxyFormOptional = ({isMainPage}) => (isMainPage ? ProxyForm() : '');
 
-function renderTemplate(opt_params) {
-  const {basepath, css, isMainPage, fileSet, serveMode, selectModePrefix} = {
-    basepath: '/',
-    isMainPage: false,
-    fileSet: [],
-    serveMode: 'default',
-    selectModePrefix: '/',
-    ...(opt_params || {}),
-  };
+function renderTemplate(opt_params = {}) {
+  const {
+    basepath = '/',
+    css,
+    isMainPage = false,
+    fileSet = [],
+    serveMode = 'default',
+    selectModePrefix = '/',
+  } = opt_params;
 
   const body = joinFragments([
     html`

build-system/server/app-index/test/helpers.js

@@ -36,7 +36,7 @@ const getBoundAttr = ({outerHTML}, attr) => {
   if (!match) {
     return;
   }
-  const [_, valuePart] = match;
+  const [, valuePart] = match;
   if (valuePart.charAt(0) == '"' ||
       valuePart.charAt(0) == '\'') {
     return valuePart.substring(1, valuePart.length - 1);

build-system/server/app-index/test/test-amphtml-helpers.js

@@ -42,6 +42,7 @@ describe('devdash', () => {
       });
 
       it('fails without min required fields', () => {
+        // @ts-ignore
         expect(() => AmpDoc({})).to.throw;
       });
 

build-system/server/app-index/test/test-html.js

@@ -25,8 +25,8 @@ describe('devdash', () => {
       it('joins simple fragments', () => {
         expect(joinFragments(['a', 'b', 'c'])).to.equal('abc');
       });
-
       it('joins mapped fragments', () => {
+        // @ts-ignore
         expect(joinFragments([1, 2, 3], a => a + 1)).to.equal('234');
       });