From cccf207a480e679fc82065d27378368de845b74d Mon Sep 17 00:00:00 2001 From: Chris Manson Date: Thu, 25 Apr 2024 14:09:35 +0100 Subject: [PATCH] expose the concept of different module phases from module-visitor --- packages/compat/src/audit.ts | 23 +++-- packages/compat/src/module-visitor.ts | 104 ++++++++++++++-------- packages/compat/tests/audit.test.ts | 16 +++- test-packages/support/audit-assertions.ts | 27 ++++++ 4 files changed, 122 insertions(+), 48 deletions(-) diff --git a/packages/compat/src/audit.ts b/packages/compat/src/audit.ts index 703ee5349..b20df2f00 100644 --- a/packages/compat/src/audit.ts +++ b/packages/compat/src/audit.ts @@ -9,7 +9,14 @@ import type { NamespaceMarker } from './audit/babel-visitor'; import { CodeFrameStorage, isNamespaceMarker } from './audit/babel-visitor'; import { AuditBuildOptions, AuditOptions } from './audit/options'; import { buildApp, BuildError, isBuildError } from './audit/build'; -import { type ContentType, type Module, visitModules, isLinked, type RootMarker, isRootMarker } from './module-visitor'; +import { + type ContentType, + type Module, + visitModules, + type RootMarker, + isRootMarker, + type CompleteModule, +} from './module-visitor'; export interface AuditMessage { message: string; @@ -41,7 +48,11 @@ export class AuditResults { let results = new this(); results.modules = modules; for (let finding of findings) { - let relFinding = Object.assign({}, finding, { filename: explicitRelative(baseDir, finding.filename) }); + const filename = finding.filename.startsWith('./') + ? finding.filename + : explicitRelative(baseDir, finding.filename); + + let relFinding = Object.assign({}, finding, { filename }); results.findings.push(relFinding); } return results; @@ -245,13 +256,13 @@ export class Audit { private inspectModules(modules: Record) { for (let [filename, module] of Object.entries(modules)) { - if (isLinked(module)) { + if (module.type === 'complete') { this.inspectImports(filename, module, modules); } } } - private inspectImports(filename: string, module: Module, modules: Record) { + private inspectImports(filename: string, module: CompleteModule, modules: Record) { for (let imp of module.imports) { let resolved = module.resolutions[imp.source]; if (!resolved) { @@ -264,7 +275,7 @@ export class Audit { } else if (resolved) { let target = modules[resolved]!; for (let specifier of imp.specifiers) { - if (isLinked(target) && !this.moduleProvidesName(target, specifier.name)) { + if (target.type === 'complete' && !this.moduleProvidesName(target, specifier.name)) { if (specifier.name === 'default') { let backtick = '`'; this.findings.push({ @@ -287,7 +298,7 @@ export class Audit { } } - private moduleProvidesName(target: Module, name: string | NamespaceMarker) { + private moduleProvidesName(target: CompleteModule, name: string | NamespaceMarker) { // any module can provide a namespace. // CJS and AMD are too dynamic to be sure exactly what names are available, // so they always get a pass diff --git a/packages/compat/src/module-visitor.ts b/packages/compat/src/module-visitor.ts index 505cde974..919a7e61e 100644 --- a/packages/compat/src/module-visitor.ts +++ b/packages/compat/src/module-visitor.ts @@ -17,17 +17,28 @@ import { JSDOM } from 'jsdom'; import type { Finding } from './audit'; import type { TransformOptions } from '@babel/core'; -export interface Module { +export type Module = CompleteModule | ParsedModule | UnparseableModule; + +export interface UnparseableModule { + type: 'unparseable'; appRelativePath: string; consumedFrom: (string | RootMarker)[]; +} + +export interface ParsedModule extends Omit { + type: 'parsed'; imports: Import[]; - exports: string[]; resolutions: { [source: string]: string | null }; content: string; isCJS: boolean; isAMD: boolean; } +export interface CompleteModule extends Omit { + type: 'complete'; + exports: string[]; +} + interface InternalModule { consumedFrom: (string | RootMarker)[]; @@ -63,7 +74,7 @@ type LinkedInternalModule = Omit & { linked: NonNullable; }; -export function isLinked(module: InternalModule | undefined): module is LinkedInternalModule { +function isLinked(module: InternalModule | undefined): module is LinkedInternalModule { return Boolean(module?.parsed && module.resolved && module.linked); } @@ -321,44 +332,61 @@ class ModuleVisitor { } } + private toPublicModule(filename: string, module: InternalModule): Module { + let result: UnparseableModule = { + type: 'unparseable', + appRelativePath: explicitRelative(this.base, filename), + consumedFrom: module.consumedFrom.map(entry => { + if (isRootMarker(entry)) { + return entry; + } else { + return explicitRelative(this.base, entry); + } + }), + }; + + if (!module.parsed || !module.resolved) { + return result; + } + + let parsedResult: ParsedModule = { + ...result, + type: 'parsed', + resolutions: fromPairs( + [...module.resolved].map(([source, target]) => [ + source, + isResolutionFailure(target) ? null : explicitRelative(this.base, target), + ]) + ), + imports: module.parsed.imports.map(i => ({ + source: i.source, + specifiers: i.specifiers.map(s => ({ + name: s.name, + local: s.local, + codeFrameIndex: s.codeFrameIndex, + })), + codeFrameIndex: i.codeFrameIndex, + })), + content: module.parsed.transpiledContent.toString(), + isAMD: Boolean(module.parsed.isAMD), + isCJS: Boolean(module.parsed.isCJS), + }; + + if (!module.linked) { + return parsedResult; + } + + return { + ...parsedResult, + type: 'complete', + exports: [...module.linked.exports], + }; + } + private buildResults() { let publicModules: Record = {}; for (let [filename, module] of this.modules) { - let publicModule: Module = { - appRelativePath: explicitRelative(this.base, filename), - consumedFrom: module.consumedFrom.map(entry => { - if (isRootMarker(entry)) { - return entry; - } else { - return explicitRelative(this.base, entry); - } - }), - resolutions: module.resolved - ? fromPairs( - [...module.resolved].map(([source, target]) => [ - source, - isResolutionFailure(target) ? null : explicitRelative(this.base, target), - ]) - ) - : {}, - imports: module.parsed?.imports - ? module.parsed.imports.map(i => ({ - source: i.source, - specifiers: i.specifiers.map(s => ({ - name: s.name, - local: s.local, - codeFrameIndex: s.codeFrameIndex, - })), - codeFrameIndex: i.codeFrameIndex, - })) - : [], - exports: module.linked?.exports ? [...module.linked.exports] : [], - content: module.parsed?.transpiledContent - ? module.parsed?.transpiledContent.toString() - : 'module failed to transpile', - isAMD: Boolean(module.parsed?.isAMD), - isCJS: Boolean(module.parsed?.isCJS), - }; + let publicModule = this.toPublicModule(filename, module); publicModules[explicitRelative(this.base, filename)] = publicModule; } return publicModules; diff --git a/packages/compat/tests/audit.test.ts b/packages/compat/tests/audit.test.ts index a76316517..e96d1ebb4 100644 --- a/packages/compat/tests/audit.test.ts +++ b/packages/compat/tests/audit.test.ts @@ -222,7 +222,11 @@ describe('audit', function () { }); let result = await audit(); expect(result.findings).toEqual([]); - let exports = result.modules['./app.js'].exports; + let appModule = result.modules['./app.js']; + if (appModule.type !== 'complete') { + throw new Error('./app.js module did not parse or resolve correctly'); + } + let exports = appModule.exports; expect(exports).toContain('a'); expect(exports).toContain('b'); expect(exports).toContain('c'); @@ -269,7 +273,11 @@ describe('audit', function () { let result = await audit(); expect(result.findings).toEqual([]); - let exports = result.modules['./app.js'].exports; + let appModule = result.modules['./app.js']; + if (appModule.type !== 'complete') { + throw new Error('./app.js module did not parse or resolve correctly'); + } + let exports = appModule.exports; expect(exports).toContain('a'); expect(exports).toContain('b'); expect(exports).not.toContain('thing'); @@ -277,8 +285,8 @@ describe('audit', function () { expect(exports).toContain('alpha'); expect(exports).toContain('beta'); expect(exports).toContain('libC'); - expect(result.modules['./app.js'].imports.length).toBe(3); - let imports = fromPairs(result.modules['./app.js'].imports.map(imp => [imp.source, imp.specifiers])); + expect(appModule.imports.length).toBe(3); + let imports = fromPairs(appModule.imports.map(imp => [imp.source, imp.specifiers])); expect(withoutCodeFrames(imports)).toEqual({ './lib-a': [ { name: 'default', local: null }, diff --git a/test-packages/support/audit-assertions.ts b/test-packages/support/audit-assertions.ts index 0c1a66415..af421e07f 100644 --- a/test-packages/support/audit-assertions.ts +++ b/test-packages/support/audit-assertions.ts @@ -114,6 +114,10 @@ export class ExpectModule { this.emitMissingModule(); return; } + if (this.module.type === 'unparseable') { + this.emitUnparsableModule(message); + return; + } const result = fn(this.module.content); this.expectAudit.assert.pushResult({ result, @@ -123,11 +127,24 @@ export class ExpectModule { }); } + private emitUnparsableModule(message?: string) { + this.expectAudit.assert.pushResult({ + result: false, + actual: `${this.inputName} failed to parse`, + expected: true, + message: `${this.inputName} failed to parse${message ? `: (${message})` : ''}`, + }); + } + codeEquals(expectedSource: string) { if (!this.module) { this.emitMissingModule(); return; } + if (this.module.type === 'unparseable') { + this.emitUnparsableModule(); + return; + } this.expectAudit.assert.codeEqual(this.module.content, expectedSource); } @@ -136,6 +153,10 @@ export class ExpectModule { this.emitMissingModule(); return; } + if (this.module.type === 'unparseable') { + this.emitUnparsableModule(); + return; + } this.expectAudit.assert.codeContains(this.module.content, expectedSource); } @@ -144,6 +165,12 @@ export class ExpectModule { this.emitMissingModule(); return new EmptyExpectResolution(); } + + if (this.module.type === 'unparseable') { + this.emitUnparsableModule(); + return new EmptyExpectResolution(); + } + if (!(specifier in this.module.resolutions)) { this.expectAudit.assert.pushResult({ result: false,