Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

check-types: remove closure #37210

Merged
merged 2 commits into from
Dec 14, 2021
Merged

check-types: remove closure #37210

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4516172
check-types: remove closure
samouri Dec 13, 2021
f01c898
Update build-system/tasks/check-types.js
samouri Dec 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
263 changes: 5 additions & 258 deletions build-system/tasks/check-types.js
View file
Open in desktop
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]",
};