From 892d2a76d889cd8a26fbc3d808243b84ccfa19a5 Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Thu, 25 Jul 2024 13:07:24 -0400 Subject: [PATCH] fix #3834: cli sometimes panics with `--analyze` --- CHANGELOG.md | 4 ++++ pkg/cli/cli_impl.go | 40 +++++++++++++++++++++++-------------- scripts/end-to-end-tests.js | 21 +++++++++++++++++++ 3 files changed, 50 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f60f323eab..282d72bd1f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,10 @@ fs.open; ``` +* Fix a panic when using the CLI with invalid build flags if `--analyze` is present ([#3834](https://github.com/evanw/esbuild/issues/3834)) + + Previously esbuild's CLI could crash if it was invoked with flags that aren't valid for a "build" API call and the `--analyze` flag is present. This was caused by esbuild's internals attempting to add a Go plugin (which is how `--analyze` is implemented) to a null build object. The panic has been fixed in this release. + ## 0.23.0 **_This release deliberately contains backwards-incompatible changes._** To avoid automatically picking up releases like this, you should either be pinning the exact version of `esbuild` in your `package.json` file (recommended) or be using a version range syntax that only accepts patch upgrades such as `^0.22.0` or `~0.22.0`. See npm's documentation about [semver](https://docs.npmjs.com/cli/v6/using-npm/semver/) for more information. diff --git a/pkg/cli/cli_impl.go b/pkg/cli/cli_impl.go index 16b5859737c..9a617069126 100644 --- a/pkg/cli/cli_impl.go +++ b/pkg/cli/cli_impl.go @@ -1023,11 +1023,15 @@ outer: return } +func isArgForBuild(arg string) bool { + return !strings.HasPrefix(arg, "-") || arg == "--bundle" +} + // This returns either BuildOptions, TransformOptions, or an error func parseOptionsForRun(osArgs []string) (*api.BuildOptions, *api.TransformOptions, parseOptionsExtras, *cli_helpers.ErrorWithNote) { // If there's an entry point or we're bundling, then we're building for _, arg := range osArgs { - if !strings.HasPrefix(arg, "-") || arg == "--bundle" { + if isArgForBuild(arg) { options := newBuildOptions() // Apply defaults appropriate for the CLI @@ -1091,20 +1095,25 @@ const ( ) func filterAnalyzeFlags(osArgs []string) ([]string, analyzeMode) { - analyze := analyzeDisabled - end := 0 for _, arg := range osArgs { - switch arg { - case "--analyze": - analyze = analyzeEnabled - case "--analyze=verbose": - analyze = analyzeVerbose - default: - osArgs[end] = arg - end++ + if isArgForBuild(arg) { + analyze := analyzeDisabled + end := 0 + for _, arg := range osArgs { + switch arg { + case "--analyze": + analyze = analyzeEnabled + case "--analyze=verbose": + analyze = analyzeVerbose + default: + osArgs[end] = arg + end++ + } + } + return osArgs[:end], analyze } } - return osArgs[:end], analyze + return osArgs, analyzeDisabled } // Print metafile analysis after the build if it's enabled @@ -1150,10 +1159,11 @@ func runImpl(osArgs []string, plugins []api.Plugin) int { // Add any plugins from the caller after parsing the build options if buildOptions != nil { buildOptions.Plugins = append(buildOptions.Plugins, plugins...) - } - if analyze != analyzeDisabled { - addAnalyzePlugin(buildOptions, analyze, osArgs) + // The "--analyze" flag is implemented as a plugin + if analyze != analyzeDisabled { + addAnalyzePlugin(buildOptions, analyze, osArgs) + } } switch { diff --git a/scripts/end-to-end-tests.js b/scripts/end-to-end-tests.js index 7d280807040..9ae26740f86 100644 --- a/scripts/end-to-end-tests.js +++ b/scripts/end-to-end-tests.js @@ -8446,6 +8446,27 @@ tests.push( }), ) +// Tests for analyze +tests.push( + test(['in.js', '--analyze', '--outfile=node.js'], { + 'in.js': `let x = 1 + 2`, + }, { + expectedStderr: ` + node.js 15b 100.0% + └ in.js 15b 100.0% + +`, + }), + test(['in.js', '--invalid-flag', '--analyze'], { + 'in.js': `let x = 1 + 2`, + }, { + expectedStderr: `${errorIcon} [ERROR] Invalid build flag: "--invalid-flag"\n\n`, + }), + test(['--analyze'], {}, { + expectedStderr: `${errorIcon} [ERROR] Invalid transform flag: "--analyze"\n\n`, + }), +) + // Test writing to stdout tests.push( // These should succeed