From 00e1dade3ab5344f50156a60f95db7299884ae9b Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Thu, 24 Feb 2022 00:04:32 +0000 Subject: [PATCH] chore(build): Disable visibility warnings (#5958) Because of our mis-use of @package, and the removal of the hack that made it work reasonably (in PR #5918), compilation was generating a large number of JSC_BAD_PACKAGE_PROPERTY_ACCESS warnings, making it hard to see any other warnings/errors that might be generated. To make compiler diagnostics more useful, disable the visibility diagnostic group entirely for the time being. Also improve the documentation for JSCOMP_ERROR and the newly- -introduced JSCOMP_WARNING and JSCOMP_OFF. --- scripts/gulpfiles/build_tasks.js | 79 ++++++++++++++++++++++---------- 1 file changed, 55 insertions(+), 24 deletions(-) diff --git a/scripts/gulpfiles/build_tasks.js b/scripts/gulpfiles/build_tasks.js index 5119ba0af..b95cb7c52 100644 --- a/scripts/gulpfiles/build_tasks.js +++ b/scripts/gulpfiles/build_tasks.js @@ -168,11 +168,20 @@ function stripApacheLicense() { } /** - * Closure compiler warning groups used to treat warnings as errors. - * For a full list of closure compiler groups, consult: - * https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/DiagnosticGroups.java#L113 + * Closure compiler diagnostic groups we want to be treated as errors. + * These are effected when the --debug or --strict flags are passed. + * For a full list of closure compiler groups, consult the output of + * google-closure-compiler --help or look in the source here: + * https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/DiagnosticGroups.java#L117 + * + * The list in JSCOMP_ERROR contains all the diagnostic groups we know + * about, but some are commented out if we don't want them, and may + * appear in JSCOMP_WARNING or JSCOMP_OFF instead. Items not + * appearing on any list will default to setting provided by the + * compiler, which may vary depending on compilation level. */ var JSCOMP_ERROR = [ + // 'accessControls', // Deprecated; means same as visibility. 'checkPrototypalTypes', 'checkRegExp', 'checkTypes', @@ -185,27 +194,29 @@ var JSCOMP_ERROR = [ 'duplicateMessage', 'es5Strict', 'externsValidation', - 'extraRequire', + 'extraRequire', // Undocumented but valid. 'functionParams', 'globalThis', 'invalidCasts', 'misplacedTypeAnnotation', - // 'missingOverride', + // 'missingOverride', // There are many of these, which should be fixed. 'missingPolyfill', 'missingProperties', 'missingProvide', 'missingRequire', 'missingReturn', - // 'missingSourcesWarnings', + // 'missingSourcesWarnings', // Group of several other options. 'moduleLoad', 'msgDescriptions', 'nonStandardJsDocs', - // 'polymer', - // 'reportUnknownTypes', - // 'strictCheckTypes', - // 'strictMissingProperties', + 'partialAlias', + // 'polymer', // Not applicable. + // 'reportUnknownTypes', // VERY verbose. + // 'strictCheckTypes', // Use --strict to enable. + // 'strictMissingProperties', // Part of strictCheckTypes. + 'strictModuleChecks', // Undocumented but valid. 'strictModuleDepCheck', - // 'strictPrimitiveOperators', + // 'strictPrimitiveOperators', // Part of strictCheckTypes. 'suspiciousCode', 'typeInvalidation', 'undefinedVars', @@ -215,19 +226,38 @@ var JSCOMP_ERROR = [ 'unusedPrivateMembers', 'uselessCode', 'untranspilableFeatures', - /* In order to transition to ES modules, modules will need to import one - * another by relative paths. This means that the existing practice of moving - * all source files into the same directory for compilation (see docs for - * flattenCorePaths) would break imports. Not flattening files in this way - * breaks our usage of @package however; files were flattened so that all - * Blockly source files are in the same directory and can use @package to mark - * methods that are only allowed for use by Blockly, while still allowing - * access between e.g. core/events/* and core/utils/*. For now, we're - * downgrading access control violations (including @private) to warnings. - * Once ES module migration is complete, they will be re-enabled and an - * alternative to @package will be established. + // 'visibility', // Disabled; see note in JSCOMP_OFF. +]; + +/** + * Closure compiler diagnostic groups we want to be treated as warnings. + * These are effected when the --debug or --strict flags are passed. + */ +var JSCOMP_WARNING = [ +]; + +/** + * Closure compiler diagnostic groups we want to be ignored. + * These suppressions are always effected by default. + */ +var JSCOMP_OFF = [ + /* In order to transition to ES modules, modules will need to import + * one another by relative paths. This means that the existing + * practice of moving all source files into the same directory for + * compilation (see docs for flattenCorePaths) would break + * imports. Not flattening files in this way breaks our usage + * of @package however; files were flattened so that all Blockly + * source files are in the same directory and can use @package to + * mark methods that are only allowed for use by Blockly, while + * still allowing access between e.g. core/events/* and + * core/utils/*. We were downgrading access control violations + * (including @private) to warnings, but this ends up being so + * spammy that it makes the compiler output nearly useless. + * + * Once ES module migration is complete, they will be re-enabled and + * an alternative to @package will be established. */ - // 'visibility' + 'visibility', ]; /** @@ -514,6 +544,7 @@ function compile(options) { warning_level: argv.verbose ? 'VERBOSE' : 'DEFAULT', language_in: 'ECMASCRIPT_2020', language_out: 'ECMASCRIPT5_STRICT', + jscomp_off: [...JSCOMP_OFF], rewrite_polyfills: true, hide_warnings_for: 'node_modules', define: ['COMPILED=true'], @@ -521,6 +552,7 @@ function compile(options) { }; if (argv.debug || argv.strict) { defaultOptions.jscomp_error = [...JSCOMP_ERROR]; + defaultOptions.jscomp_warning = [...JSCOMP_WARNING]; if (argv.strict) { defaultOptions.jscomp_error.push('strictCheckTypes'); } @@ -586,7 +618,6 @@ function buildAdvancedCompilationTest() { compilation_level: 'ADVANCED_OPTIMIZATIONS', entry_point: './tests/compile/main.js', js_output_file: 'main_compressed.js', - jscomp_warning: ['visibility'], }; return gulp.src(srcs, {base: './'}) .pipe(stripApacheLicense())