fix #2757: duplicate function definition edge case

CHANGELOG.md

@@ -48,6 +48,10 @@
 
     Note that you can still customize this behavior with the `--legal-comments=` flag. For example, you can use `--legal-comments=none` to turn this off, or you can use `--legal-comments=linked` to put these comments in a separate `.LEGAL.txt` file instead.
 
+* Fix duplicate function declaration edge cases ([#2757](https://github.com/evanw/esbuild/issues/2757))
+
+    The change in the previous release to forbid duplicate function declarations in certain cases accidentally forbid some edge cases that should have been allowed. Specifically duplicate function declarations are forbidden in nested blocks in strict mode and at the top level of modules, but are allowed when they are declared at the top level of function bodies. This release fixes the regression by re-allowing the last case.
+
 ## 0.16.9
 
 * Update to Unicode 15.0.0

internal/js_parser/js_parser.go

@@ -1269,13 +1269,27 @@ func (a scopeMemberArray) Less(i int, j int) bool {
 }
 
 func (p *parser) hoistSymbols(scope *js_ast.Scope) {
-	if scope.StrictMode != js_ast.SloppyMode {
+	// Duplicate function declarations are forbidden in nested blocks in strict
+	// mode. Separately, they are also forbidden at the top-level of modules.
+	// This check needs to be delayed until now instead of being done when the
+	// functions are declared because we potentially need to scan the whole file
+	// to know if the file is considered to be in strict mode (or is considered
+	// to be a module). We might only encounter an "export {}" clause at the end
+	// of the file.
+	if (scope.StrictMode != js_ast.SloppyMode && scope.Kind == js_ast.ScopeBlock) || (scope.Parent == nil && p.isFileConsideredESM) {
 		for _, replaced := range scope.Replaced {
 			symbol := &p.symbols[replaced.Ref.InnerIndex]
 			if symbol.Kind.IsFunction() {
 				if member, ok := scope.Members[symbol.OriginalName]; ok && p.symbols[member.Ref.InnerIndex].Kind.IsFunction() {
-					where, notes := p.whyStrictMode(scope)
-					notes[0].Text = fmt.Sprintf("Duplicate lexically-declared names are not allowed %s. %s", where, notes[0].Text)
+					var notes []logger.MsgData
+					if scope.Parent == nil && p.isFileConsideredESM {
+						_, notes = p.whyESModule()
+						notes[0].Text = fmt.Sprintf("Duplicate top-level function declarations are not allowed in an ECMAScript module. %s", notes[0].Text)
+					} else {
+						var where string
+						where, notes = p.whyStrictMode(scope)
+						notes[0].Text = fmt.Sprintf("Duplicate function declarations are not allowed in nested blocks %s. %s", where, notes[0].Text)
+					}
 
 					p.log.AddErrorWithNotes(&p.tracker,
 						js_lexer.RangeOfIdentifier(p.source, member.Loc),

internal/js_parser/js_parser_test.go

@@ -534,6 +534,12 @@ func TestStrictMode(t *testing.T) {
 	expectParseError(t, "for await (x of y); with (y) z", tlaKeyword)
 	expectParseError(t, "with (y) z; for await (x of y);", tlaKeyword)
 
+	fAlreadyDeclaredError := "<stdin>: ERROR: The symbol \"f\" has already been declared\n" +
+		"<stdin>: NOTE: The symbol \"f\" was originally declared here:\n"
+	nestedNote := "<stdin>: NOTE: Duplicate function declarations are not allowed in nested blocks"
+	moduleNote := "<stdin>: NOTE: Duplicate top-level function declarations are not allowed in an ECMAScript module. " +
+		"This file is considered to be an ECMAScript module because of the \"export\" keyword here:\n"
+
 	cases := []string{
 		"function f() {} function f() {}",
 		"function f() {} function *f() {}",
@@ -542,28 +548,35 @@ func TestStrictMode(t *testing.T) {
 		"async function f() {} function f() {}",
 		"function f() {} async function *f() {}",
 		"async function *f() {} function f() {}",
-		"{ function f() {} function f() {} }",
-		"switch (0) { case 1: function f() {} default: function f() {} }",
 	}
 
-	fAlreadyDeclaredError :=
-		"<stdin>: ERROR: The symbol \"f\" has already been declared\n" +
-			"<stdin>: NOTE: The symbol \"f\" was originally declared here:\n" +
-			"<stdin>: NOTE: Duplicate lexically-declared names are not allowed"
-
 	for _, c := range cases {
 		expectParseError(t, c, "")
-
-		expectParseError(t, "'use strict'; "+c, fAlreadyDeclaredError+" in strict mode. "+
-			"Strict mode is triggered by the \"use strict\" directive here:\n")
-
-		expectParseError(t, "function foo() { 'use strict'; "+c+" }", fAlreadyDeclaredError+" in strict mode. "+
-			"Strict mode is triggered by the \"use strict\" directive here:\n")
-
-		expectParseError(t, c+" export {}", fAlreadyDeclaredError+" in an ECMAScript module. "+
-			"This file is considered to be an ECMAScript module because of the \"export\" keyword here:\n")
+		expectParseError(t, "'use strict'; "+c, "")
+		expectParseError(t, "function foo() { 'use strict'; "+c+" }", "")
 	}
 
+	expectParseError(t, "function f() {} function f() {} export {}", fAlreadyDeclaredError+moduleNote)
+	expectParseError(t, "function f() {} function *f() {} export {}", fAlreadyDeclaredError+moduleNote)
+	expectParseError(t, "function f() {} async function f() {} export {}", fAlreadyDeclaredError+moduleNote)
+	expectParseError(t, "function *f() {} function f() {} export {}", fAlreadyDeclaredError+moduleNote)
+	expectParseError(t, "async function f() {} function f() {} export {}", fAlreadyDeclaredError+moduleNote)
+
+	expectParseError(t, "'use strict'; { function f() {} function f() {} }",
+		fAlreadyDeclaredError+nestedNote+" in strict mode. Strict mode is triggered by the \"use strict\" directive here:\n")
+	expectParseError(t, "'use strict'; switch (0) { case 1: function f() {} default: function f() {} }",
+		fAlreadyDeclaredError+nestedNote+" in strict mode. Strict mode is triggered by the \"use strict\" directive here:\n")
+
+	expectParseError(t, "function foo() { 'use strict'; { function f() {} function f() {} } }",
+		fAlreadyDeclaredError+nestedNote+" in strict mode. Strict mode is triggered by the \"use strict\" directive here:\n")
+	expectParseError(t, "function foo() { 'use strict'; switch (0) { case 1: function f() {} default: function f() {} } }",
+		fAlreadyDeclaredError+nestedNote+" in strict mode. Strict mode is triggered by the \"use strict\" directive here:\n")
+
+	expectParseError(t, "{ function f() {} function f() {} } export {}",
+		fAlreadyDeclaredError+nestedNote+" in an ECMAScript module. This file is considered to be an ECMAScript module because of the \"export\" keyword here:\n")
+	expectParseError(t, "switch (0) { case 1: function f() {} default: function f() {} } export {}",
+		fAlreadyDeclaredError+nestedNote+" in an ECMAScript module. This file is considered to be an ECMAScript module because of the \"export\" keyword here:\n")
+
 	expectParseError(t, "var x; var x", "")
 	expectParseError(t, "'use strict'; var x; var x", "")
 	expectParseError(t, "var x; var x; export {}", "")