Skip to content

Commit

Permalink
check-types: remove closure (#37210)
Browse files Browse the repository at this point in the history 
* check-types: remove closure

* Update build-system/tasks/check-types.js

Co-authored-by: Ryan Cebulko <ryan@cebulko.com>

Co-authored-by: Ryan Cebulko <ryan@cebulko.com>
  • Loading branch information
@samouri @rcebulko
samouri and rcebulko committed Dec 14, 2021
1 parent 1f332cf commit cf62ae1
Showing 1 changed file with 5 additions and 258 deletions.
263 changes: 5 additions & 258 deletions build-system/tasks/check-types.js
View file
Original file line number Diff line number Diff line change
@@ -1,43 +1,19 @@
const argv = require('minimist')(process.argv.slice(2));
const fastGlob = require('fast-glob');
const {
createCtrlcHandler,
exitCtrlcHandler,
} = require('../common/ctrlcHandler');
const {
displayLifecycleDebugging,
} = require('../compile/debug-compilation-lifecycle');
const {cleanupBuildDir, closureCompile} = require('../compile/compile');
const {cleanupBuildDir} = require('../compile/compile');
const {compileCss} = require('./css');
const {compileJison} = require('./compile-jison');
const {cyan, green, red, yellow} = require('kleur/colors');
const {cyan, green} = require('kleur/colors');
const {execOrThrow} = require('../common/exec');
const {extensions, maybeInitializeExtensions} = require('./extension-helpers');
const {logClosureCompilerError} = require('../compile/closure-compile');
const {log} = require('../common/logging');
const {typecheckNewServer} = require('../server/typescript-compile');

// We provide glob lists for core src/externs since any other targets are
// allowed to depend on core.
const CORE_SRCS_GLOBS = [
'src/core/**/*.js',

// Needed for CSS escape polyfill
'third_party/css-escape/css-escape.js',
];

/**
* Generates a list of source file paths for extensions to type-check
* Must be run after `maybeInitializeExtensions`
* @function
* @return {!Array<string>}
*/
const getExtensionSrcPaths = () =>
Object.values(extensions)
.filter((ext) => !ext.noTypeCheck)
.map(({name, version}) => `extensions/${name}/${version}/${name}.js`)
.sort();

/**
* Object of targets to check with TypeScript.
*
Expand All @@ -49,148 +25,12 @@ const TSC_TYPECHECK_TARGETS = {
'core': 'src/core',
};

/**
* The main configuration location to add/edit targets for type checking.
* Properties besides `entryPoints` are passed on to `closureCompile` as
* options. * Values may be objects or functions, as some require initialization
* or filesystem access and shouldn't be run until needed.
*
* When updating type-check targets, `srcGlobs` is the primary value you care
* about. This is a list of source files to include in type-checking. For any
* glob pattern ending in *.js, externs are picked up following the same pattern
* but ending in *.extern.js. Note this only applies to *.js globs, and not
* specific filenames. If just an array of strings is provided instead of an
* object, it is treated as srcGlobs.
*
* @type {Object<string, Array<string>|Object|function():Object>}
*/
const CLOSURE_TYPE_CHECK_TARGETS = {
// Below are targets containing individual directories which are fully passing
// type-checking. Do not remove or disable anything on this list.
// Goal: Remove 'QUIET' from all of them.
// To test a target locally:
// `amp check-types --target=src-foo-bar --warning_level=verbose`
'src-amp-story-player': {
srcGlobs: ['src/amp-story-player/**/*.js'],
warningLevel: 'QUIET',
},
'src-inabox': {
srcGlobs: ['src/inabox/**/*.js'],
warningLevel: 'QUIET',
},
'src-preact': {
srcGlobs: ['src/preact/**/*.js', ...CORE_SRCS_GLOBS],
warningLevel: 'QUIET',
},
'src-purifier': {
srcGlobs: ['src/purifier/**/*.js'],
warningLevel: 'QUIET',
},
'src-service': {
srcGlobs: ['src/service/**/*.js'],
warningLevel: 'QUIET',
},
'src-utils': {
srcGlobs: ['src/utils/**/*.js'],
warningLevel: 'QUIET',
},
'src-web-worker': {
srcGlobs: ['src/web-worker/**/*.js'],
warningLevel: 'QUIET',
},

// Ensures that all files in src and extensions pass the specified set of
// errors.
'low-bar': {
entryPoints: ['src/amp.js'],
extraGlobs: ['{src,extensions}/**/*.js', ...getLowBarExclusions()],
onError(msg) {
const lowBarErrors = [
'JSC_BAD_JSDOC_ANNOTATION',
'JSC_INVALID_PARAM',
'JSC_TYPE_PARSE_ERROR',
];
const lowBarRegex = new RegExp(lowBarErrors.join('|'));

const targetErrors = msg
.split('\n')
.filter((s) => lowBarRegex.test(s))
.join('\n')
.trim();

if (targetErrors.length) {
logClosureCompilerError(targetErrors);
throw new Error(`Type-checking failed for target ${cyan('low-bar')}`);
}
},
},

// TODO(#33631): Targets below this point are not expected to pass.
// They can possibly be removed?
'src': {
entryPoints: [
'src/amp.js',
'src/amp-shadow.js',
'src/inabox/amp-inabox.js',
'ads/alp/install-alp.js',
'ads/inabox/inabox-host.js',
'src/web-worker/web-worker.js',
],
extraGlobs: ['src/inabox/*.js', '!node_modules/preact/**'],
warningLevel: 'QUIET',
},
'extensions': () => ({
entryPoints: getExtensionSrcPaths(),
extraGlobs: ['src/inabox/*.js', '!node_modules/preact/**'],
warningLevel: 'QUIET',
}),
'integration': {
entryPoints: '3p/integration.js',
externs: ['ads/ads.extern.js'],
warningLevel: 'QUIET',
},
'ampcontext': {
entryPoints: '3p/ampcontext-lib.js',
externs: ['ads/ads.extern.js'],
warningLevel: 'QUIET',
},
'iframe-transport-client': {
entryPoints: '3p/iframe-transport-client-lib.js',
externs: ['ads/ads.extern.js'],
warningLevel: 'QUIET',
},
};

/**
* Produces a list of extern glob patterns from a list of source glob patterns.
* ex. ['src/core/** /*.js'] => ['src/core/** /*.extern.js']
* @param {!Array<string>} srcGlobs
* @return {!Array<string>}
*/
function externGlobsFromSrcGlobs(srcGlobs) {
return srcGlobs
.filter((glob) => glob.endsWith('*.js'))
.map((glob) => glob.replace(/\*\.js$/, '*.extern.js'));
}

/**
* Typecheck the given target using either tsc or closure.
*
* @param {string} targetName
* @return {Promise<void>}
*/
async function typeCheck(targetName) {
return TSC_TYPECHECK_TARGETS[targetName]
? tscTypeCheck(targetName)
: closureTypeCheck(targetName);
}

/**
* Performs tsc type-checking on the target provided.
* @param {string} targetName key in TSC_TYPECHECK_TARGETS
* @return {!Promise<void>}
*/
async function tscTypeCheck(targetName) {
async function typeCheck(targetName) {
execOrThrow(
`npx -p typescript tsc --project ${TSC_TYPECHECK_TARGETS[targetName]}/tsconfig.json`,
`Type checking ${targetName} failed`
Expand All @@ -199,96 +39,7 @@ async function tscTypeCheck(targetName) {
}

/**
* Returns the exclusion glob for telling closure to ignore all paths
* being checked via TS.
*
* @return {string[]}
*/
function getLowBarExclusions() {
return Object.values(TSC_TYPECHECK_TARGETS).map((dir) => `!${dir}`);
}

/**
* Performs closure type-checking on the target provided.
* @param {string} targetName key in CLOSURE_TYPE_CHECK_TARGETS
* @return {!Promise<void>}
*/
async function closureTypeCheck(targetName) {
let target = CLOSURE_TYPE_CHECK_TARGETS[targetName];
// Allow targets to be dynamically evaluated
if (typeof target == 'function') {
target = target();
}
// Allow targets to be specified as just an array of source globs
if (Array.isArray(target)) {
target = {srcGlobs: target};
}

if (!target) {
log(
red('ERROR:'),
'No type-check configuration defined for target',
cyan(targetName)
);
throw new Error(
`No type-check configuration defined for target ${targetName}`
);
}

const {entryPoints = [], srcGlobs = [], externGlobs = [], ...opts} = target;
externGlobs.push(...externGlobsFromSrcGlobs(srcGlobs));

// If srcGlobs and externGlobs are defined, determine the externs/extraGlobs
if (srcGlobs.length || externGlobs.length) {
opts.externs = externGlobs.flatMap(fastGlob.sync);

// Included globs should explicitly exclude any externs
const excludedExterns = externGlobs.map((glob) => `!${glob}`);
opts.extraGlobs = srcGlobs.concat(excludedExterns);
}

// If no entry point is defined, we want to scan the globs provided without
// injecting extra dependencies.
const noAddDeps = !entryPoints.length;
// If the --warning_level flag is passed explicitly, it takes precedence.
opts.warningLevel = argv.warning_level || opts.warningLevel || 'VERBOSE';

// For type-checking, QUIET suppresses all warnings and can't affect the
// resulting status, so there's no point in doing it.
if (opts.warningLevel == 'QUIET') {
log(
yellow('WARNING:'),
'Warning level for target',
cyan(targetName),
`is set to ${cyan('QUIET')}; skipping`
);
return;
}

let errorMsg;
if (target.onError) {
// If an onError handler is defined, steal the output and let onError handle
// logging
opts.logger = (m) => (errorMsg = m);
}

await closureCompile(entryPoints, './dist', `${targetName}-check-types.js`, {
noAddDeps,
include3pDirectories: !noAddDeps,
includePolyfills: !noAddDeps,
typeCheckOnly: true,
...opts,
}).catch((error) => {
if (!target.onError) {
throw error;
}
target.onError(errorMsg);
});
log(green('SUCCESS:'), 'Type-checking passed for target', cyan(targetName));
}

/**
* Runs closure compiler's type checker against all AMP code.
* Runs TypeScript Compiler's type checker.
* @return {!Promise<void>}
*/
async function checkTypes() {
Expand All @@ -297,14 +48,13 @@ async function checkTypes() {
// Prepare build environment
process.env.NODE_ENV = 'production';
cleanupBuildDir();
maybeInitializeExtensions();
typecheckNewServer();
await Promise.all([compileCss(), compileJison()]);

// Use the list of targets if provided, otherwise check all targets
const targets = argv.targets
? argv.targets.split(/,/)
: Object.keys({...TSC_TYPECHECK_TARGETS, ...CLOSURE_TYPE_CHECK_TARGETS});
: Object.keys(TSC_TYPECHECK_TARGETS);

log(`Checking types for targets: ${targets.map(cyan).join(', ')}`);
displayLifecycleDebugging();
Expand All @@ -320,9 +70,6 @@ module.exports = {
/* eslint "local/camelcase": 0 */
checkTypes.description = 'Check source code for JS type errors';
checkTypes.flags = {
closure_concurrency: 'Set the number of concurrent invocations of closure',
debug: 'Output the file contents during compilation lifecycles',
targets: 'Comma-delimited list of targets to type-check',
warning_level:
"Optionally set closure's warning level to one of [quiet, default, verbose]",
};

0 comments on commit cf62ae1

Please sign in to comment.