🏗 ♻️ Fix type errors in /build-system/tasks (#33074)

* fix around 300 type errors in /build-system/tasks * add error messages as per suggestion * change file read format in print-report * remove empty comments. I will add better JSDoc prior to enabling enforcement Co-authored-by: Raghu Simha <rsimha@amp.dev>
ampproject · Mar 17, 2021 · f6c1dde · f6c1dde
1 parent 948ddf5
commit f6c1dde
Show file tree

Hide file tree

Showing 70 changed files with 160 additions and 285 deletions.
diff --git a/build-system/babel-plugins/babel-plugin-transform-prune-namespace/index.js b/build-system/babel-plugins/babel-plugin-transform-prune-namespace/index.js
@@ -40,7 +40,7 @@ module.exports = function () {
 
   /**
    * @param {BabelPath} path
-   * @return {void}
+   * @return {BabelPath|void}
    */
   function deepestMember(path) {
     while (true) {

diff --git a/build-system/babel-plugins/babel-plugin-transform-stringish-literals/index.js b/build-system/babel-plugins/babel-plugin-transform-stringish-literals/index.js
@@ -37,7 +37,6 @@ module.exports = function ({types: t}) {
   /**
    * @param {CompilerNode} leftPath
    * @param {CompilerNode} rightPath
-   * @return {void}
    */
   function joinTemplateLiterals(leftPath, rightPath) {
     const {node: leftNode} = leftPath;
@@ -59,7 +58,6 @@ module.exports = function ({types: t}) {
 
   /**
    * @param {BabelPath} path
-   * @return {void}
    */
   function joinMaybeTemplateLiteral(path) {
     const left = path.get('left');
@@ -111,7 +109,6 @@ module.exports = function ({types: t}) {
 
       /**
        * @param {BabelPath} path
-       * @return {void}
        */
       TemplateLiteral(path) {
         // Convert any items inside a template literal that are static literals.

diff --git a/build-system/compile/compile.js b/build-system/compile/compile.js
@@ -58,26 +58,23 @@ let OptionsDef;
  * production use. During development we intend to continue using
  * babel, as it has much faster incremental compilation.
  *
- * @param {string} entryModuleFilename
+ * @param {string|string[]} entryModuleFilename
  * @param {string} outputDir
  * @param {string} outputFilename
  * @param {!OptionsDef} options
- * @param {{startTime?: number}} timeInfo
+ * @param {{startTime?: number}=} timeInfo
  * @return {Promise<void>}
  */
 async function closureCompile(
   entryModuleFilename,
   outputDir,
   outputFilename,
   options,
-  timeInfo
+  timeInfo = {}
 ) {
   // Rate limit closure compilation to MAX_PARALLEL_CLOSURE_INVOCATIONS
   // concurrent processes.
   return new Promise(function (resolve, reject) {
-    /**
-     * @return {void}
-     */
     function start() {
       inProgress++;
       compile(
@@ -95,9 +92,7 @@ async function closureCompile(
         (reason) => reject(reason)
       );
     }
-    /**
-     * @return {void}
-     */
+
     function next() {
       if (!queue.length) {
         return;
@@ -111,9 +106,6 @@ async function closureCompile(
   });
 }
 
-/**
- * @return {void}
- */
 function cleanupBuildDir() {
   del.sync('build/fake-module');
   del.sync('build/patched-module');
@@ -455,9 +447,6 @@ async function compile(
   }
 }
 
-/**
- * @return {void}
- */
 function printClosureConcurrency() {
   log(
     green('Using up to'),

diff --git a/build-system/compile/debug-compilation-lifecycle.js b/build-system/compile/debug-compilation-lifecycle.js
@@ -41,8 +41,8 @@ const LIFECYCLES = {
  *
  * @param {string} lifecycle
  * @param {string} fullpath
- * @param {Buffer} content
- * @param {Object} sourcemap
+ * @param {string=} content
+ * @param {Object=} sourcemap
  */
 function debug(lifecycle, fullpath, content, sourcemap) {
   if (argv.debug && Object.keys(LIFECYCLES).includes(lifecycle)) {
@@ -72,7 +72,6 @@ function debug(lifecycle, fullpath, content, sourcemap) {
 
 /**
  * Logs debug information.
- * @return {void}
  */
 function displayLifecycleDebugging() {
   if (argv.debug) {

diff --git a/build-system/eslint-rules/dict-string-keys.js b/build-system/eslint-rules/dict-string-keys.js
@@ -40,7 +40,6 @@ module.exports = function (context) {
 /**
  * @param {*} node
  * @param {*} context
- * @return {void}
  */
 function checkNode(node, context) {
   if (node.type === 'ObjectExpression') {

diff --git a/build-system/eslint-rules/html-template.js b/build-system/eslint-rules/html-template.js
@@ -31,7 +31,6 @@ const {
 module.exports = function (context) {
   /**
    * @param {CompilerNode} node
-   * @return {void}
    */
   function tagCannotBeCalled(node) {
     const {name} = node.callee;
@@ -47,7 +46,6 @@ module.exports = function (context) {
 
   /**
    * @param {CompilerNode} node
-   * @return {void}
    */
   function factoryUsage(node) {
     const {parent} = node;
@@ -56,7 +54,8 @@ module.exports = function (context) {
     const expectedTagName = staticTemplateFactories[name];
 
     if (parent.type === 'TaggedTemplateExpression' && parent.tag === node) {
-      return tagUsage(parent, `${name}()`);
+      tagUsage(parent, `${name}()`);
+      return;
     }
 
     if (
@@ -89,7 +88,6 @@ module.exports = function (context) {
   /**
    * @param {CompilerNode} node
    * @param {string} opt_name
-   * @return {void}
    */
   function tagUsage(node, opt_name) {
     const {quasi, tag} = node;

diff --git a/build-system/eslint-rules/no-import-rename.js b/build-system/eslint-rules/no-import-rename.js
@@ -55,7 +55,6 @@ module.exports = function (context) {
    * @param {*} node
    * @param {string} modulePath
    * @param {*} mods
-   * @return {void}
    */
   function ImportSpecifier(node, modulePath, mods) {
     const {imported, local} = node;

diff --git a/build-system/eslint-rules/preact.js b/build-system/eslint-rules/preact.js
@@ -37,7 +37,6 @@ module.exports = {
   create(context) {
     /**
      * @param {*} node
-     * @return {void}
      */
     function requirePreact(node) {
       if (imported) {

diff --git a/build-system/eslint-rules/prefer-destructuring.js b/build-system/eslint-rules/prefer-destructuring.js
@@ -98,7 +98,6 @@ module.exports = {
 
     /**
      * @param {Map[]} maps
-     * @return {void}
      */
     function processMaps(maps) {
       for (let i = 0; i < maps.length; i++) {
@@ -111,7 +110,6 @@ module.exports = {
     /**
      * @param {*} struct
      * @param {*} base
-     * @return {void}
      */
     function processVariables(struct, base) {
       const {names, nodes, declarations, node} = struct;
@@ -153,7 +151,6 @@ module.exports = {
     return {
       /**
        * @param {CompilerNode} node
-       * @return {void}
        */
       VariableDeclarator(node) {
         if (!shouldBeDestructure(node)) {
@@ -180,7 +177,6 @@ module.exports = {
 
       /**
        * @param {CompilerNode} node
-       * @return {void}
        */
       'BlockStatement, Program': function (node) {
         const {body} = node;

diff --git a/build-system/eslint-rules/query-selector.js b/build-system/eslint-rules/query-selector.js
@@ -20,7 +20,6 @@ const cssWhat = require('css-what');
 module.exports = function (context) {
   /**
    * @param {CompilerNode} node
-   * @return {void}
    */
   function callQuerySelector(node) {
     const {callee} = node;
@@ -45,10 +44,11 @@ module.exports = function (context) {
     const selector = getSelector(node, 0);
 
     if (!isValidSelector(selector)) {
-      return context.report({
+      context.report({
         node,
         message: 'Failed to parse CSS Selector `' + selector + '`',
       });
+      return;
     }
 
     // What are we calling querySelector on?
@@ -84,7 +84,6 @@ module.exports = function (context) {
 
   /**
    * @param {CompilerNode} node
-   * @return {void}
    */
   function callScopedQuerySelector(node) {
     const {callee} = node;
@@ -103,10 +102,11 @@ module.exports = function (context) {
     const selector = getSelector(node, 1);
 
     if (!isValidSelector(selector)) {
-      return context.report({
+      context.report({
         node,
         message: 'Failed to parse CSS Selector `' + selector + '`',
       });
+      return;
     }
 
     if (selectorNeedsScope(selector)) {

diff --git a/build-system/eslint-rules/unused-private-field.js b/build-system/eslint-rules/unused-private-field.js
@@ -76,7 +76,6 @@ module.exports = {
      * @param {CompilerNode} node
      * @param {string} name
      * @param {boolean} used
-     * @return {void}
      */
     function restrictedUse(node, name, used) {
       if (used) {
@@ -135,7 +134,6 @@ module.exports = {
      * @param {CompilerNode} node
      * @param {string} name
      * @param {boolean} used
-     * @return {void}
      */
     function visibleForTestingUse(node, name, used) {
       if (!used) {
@@ -163,7 +161,6 @@ module.exports = {
      * @param {CompilerNode} node
      * @param {string} name
      * @param {boolean} used
-     * @return {void}
      */
     function unannotatedUse(node, name, used) {
       if (used) {

diff --git a/build-system/pr-check/bundle-size.js b/build-system/pr-check/bundle-size.js
@@ -30,18 +30,12 @@ const {runCiJob} = require('./ci-job');
 
 const jobName = 'bundle-size.js';
 
-/**
- * @return {void}
- */
 function pushBuildWorkflow() {
   downloadNomoduleOutput();
   downloadModuleOutput();
   timedExecOrDie('gulp bundle-size --on_push_build');
 }
 
-/**
- * @return {void}
- */
 function prBuildWorkflow() {
   if (buildTargetsInclude(Targets.RUNTIME)) {
     downloadNomoduleOutput();

diff --git a/build-system/pr-check/checks.js b/build-system/pr-check/checks.js
@@ -26,9 +26,6 @@ const {timedExecOrDie} = require('./utils');
 
 const jobName = 'checks.js';
 
-/**
- * @return {void}
- */
 function pushBuildWorkflow() {
   timedExecOrDie('gulp presubmit');
   timedExecOrDie('gulp lint');

diff --git a/build-system/pr-check/cross-browser-tests.js b/build-system/pr-check/cross-browser-tests.js
@@ -99,9 +99,6 @@ function runUnitTestsForPlatform() {
   }
 }
 
-/**
- * @return {void}
- */
 function pushBuildWorkflow() {
   runUnitTestsForPlatform();
   timedExecOrDie('gulp dist --fortesting');

diff --git a/build-system/pr-check/e2e-tests.js b/build-system/pr-check/e2e-tests.js
@@ -30,9 +30,6 @@ const {runCiJob} = require('./ci-job');
 
 const jobName = 'e2e-tests.js';
 
-/**
- * @return {void}
- */
 function pushBuildWorkflow() {
   downloadNomoduleOutput();
   try {
@@ -49,9 +46,6 @@ function pushBuildWorkflow() {
   }
 }
 
-/**
- * @return {void}
- */
 function prBuildWorkflow() {
   if (buildTargetsInclude(Targets.RUNTIME, Targets.E2E_TEST)) {
     downloadNomoduleOutput();

diff --git a/build-system/pr-check/experiment-build.js b/build-system/pr-check/experiment-build.js
@@ -31,9 +31,6 @@ const {runCiJob} = require('./ci-job');
 
 const jobName = `${experiment}-build.js`;
 
-/**
- * @return {void}
- */
 function pushBuildWorkflow() {
   const config = getExperimentConfig(experiment);
   if (config) {
@@ -48,9 +45,6 @@ function pushBuildWorkflow() {
   }
 }
 
-/**
- * @return {void}
- */
 function prBuildWorkflow() {
   if (
     buildTargetsInclude(

diff --git a/build-system/pr-check/experiment-tests.js b/build-system/pr-check/experiment-tests.js
@@ -31,9 +31,6 @@ const {runCiJob} = require('./ci-job');
 
 const jobName = `${experiment}-tests.js`;
 
-/**
- * @return {void}
- */
 function pushBuildWorkflow() {
   const config = getExperimentConfig(experiment);
   if (config) {
@@ -54,9 +51,6 @@ function pushBuildWorkflow() {
   }
 }
 
-/**
- * @return {void}
- */
 function prBuildWorkflow() {
   if (
     buildTargetsInclude(

diff --git a/build-system/pr-check/module-build.js b/build-system/pr-check/module-build.js
@@ -29,17 +29,11 @@ const {runCiJob} = require('./ci-job');
 
 const jobName = 'module-build.js';
 
-/**
- * @return {void}
- */
 function pushBuildWorkflow() {
   timedExecOrDie('gulp dist --esm --fortesting');
   uploadModuleOutput();
 }
 
-/**
- * @return {void}
- */
 function prBuildWorkflow() {
   // TODO(#31102): This list must eventually match the same buildTargets check
   // found in pr-check/nomodule-build.js as we turn on the systems that