From 70b7b9022db74857d579baca91b720fbe8f8b555 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Tue, 14 Nov 2023 06:11:15 +0100 Subject: [PATCH 1/2] Allow functions to redeclare vars and functions in function scopes (#5248) --- src/ast/scopes/CatchBodyScope.ts | 7 --- src/ast/scopes/FunctionBodyScope.ts | 46 +++++++++++++++++++ src/ast/scopes/ParameterScope.ts | 3 +- src/ast/scopes/Scope.ts | 7 +-- .../_config.js | 24 ++++++++++ .../redeclare-block-function-function/main.js | 4 ++ .../_config.js | 22 +++++++++ .../main.js | 2 + .../_config.js | 22 +++++++++ .../redeclare-top-level-var-function/main.js | 2 + .../redeclare-var-function/_config.js | 3 ++ .../redeclare-var-function/main.js | 10 ++++ 12 files changed, 139 insertions(+), 13 deletions(-) create mode 100644 src/ast/scopes/FunctionBodyScope.ts create mode 100644 test/function/samples/ast-validations/redeclare-block-function-function/_config.js create mode 100644 test/function/samples/ast-validations/redeclare-block-function-function/main.js create mode 100644 test/function/samples/ast-validations/redeclare-top-level-function-function/_config.js create mode 100644 test/function/samples/ast-validations/redeclare-top-level-function-function/main.js create mode 100644 test/function/samples/ast-validations/redeclare-top-level-var-function/_config.js create mode 100644 test/function/samples/ast-validations/redeclare-top-level-var-function/main.js create mode 100644 test/function/samples/ast-validations/redeclare-var-function/_config.js create mode 100644 test/function/samples/ast-validations/redeclare-var-function/main.js diff --git a/src/ast/scopes/CatchBodyScope.ts b/src/ast/scopes/CatchBodyScope.ts index 3b8289dd3..172ff3c06 100644 --- a/src/ast/scopes/CatchBodyScope.ts +++ b/src/ast/scopes/CatchBodyScope.ts @@ -70,13 +70,6 @@ export default class CatchBodyScope extends ChildScope { this.addHoistedVariable(name, declaredVariable); return declaredVariable; } - // Functions can never re-declare catch parameters - if (kind === VariableKind.function) { - const name = identifier.name; - if (this.hoistedVariables?.get(name)) { - context.error(logRedeclarationError(name), identifier.start); - } - } return super.addDeclaration(identifier, context, init, kind); } } diff --git a/src/ast/scopes/FunctionBodyScope.ts b/src/ast/scopes/FunctionBodyScope.ts new file mode 100644 index 000000000..060460e42 --- /dev/null +++ b/src/ast/scopes/FunctionBodyScope.ts @@ -0,0 +1,46 @@ +import type { AstContext } from '../../Module'; +import { logRedeclarationError } from '../../utils/logs'; +import type Identifier from '../nodes/Identifier'; +import type { ExpressionEntity } from '../nodes/shared/Expression'; +import { VariableKind } from '../nodes/shared/VariableKinds'; +import LocalVariable from '../variables/LocalVariable'; +import ChildScope from './ChildScope'; +import type ParameterScope from './ParameterScope'; + +export default class FunctionBodyScope extends ChildScope { + constructor( + readonly parent: ParameterScope, + readonly context: AstContext + ) { + super(parent, context); + } + + // There is stuff that is only allowed in function scopes, i.e. functions can + // be redeclared, functions and var can redeclare each other + addDeclaration( + identifier: Identifier, + context: AstContext, + init: ExpressionEntity, + kind: VariableKind + ): LocalVariable { + const name = identifier.name; + const existingVariable = + this.hoistedVariables?.get(name) || (this.variables.get(name) as LocalVariable); + if (existingVariable) { + const existingKind = existingVariable.kind; + if ( + (kind === VariableKind.var || kind === VariableKind.function) && + (existingKind === VariableKind.var || + existingKind === VariableKind.function || + existingKind === VariableKind.parameter) + ) { + existingVariable.addDeclaration(identifier, init); + return existingVariable; + } + context.error(logRedeclarationError(name), identifier.start); + } + const newVariable = new LocalVariable(identifier.name, identifier, init, context, kind); + this.variables.set(name, newVariable); + return newVariable; + } +} diff --git a/src/ast/scopes/ParameterScope.ts b/src/ast/scopes/ParameterScope.ts index d9722f881..ea3cc5e72 100644 --- a/src/ast/scopes/ParameterScope.ts +++ b/src/ast/scopes/ParameterScope.ts @@ -7,6 +7,7 @@ import type { ExpressionEntity } from '../nodes/shared/Expression'; import ParameterVariable from '../variables/ParameterVariable'; import CatchBodyScope from './CatchBodyScope'; import ChildScope from './ChildScope'; +import FunctionBodyScope from './FunctionBodyScope'; import type Scope from './Scope'; export default class ParameterScope extends ChildScope { @@ -19,7 +20,7 @@ export default class ParameterScope extends ChildScope { super(parent, context); this.bodyScope = isCatchScope ? new CatchBodyScope(this, context) - : new ChildScope(this, context); + : new FunctionBodyScope(this, context); } /** diff --git a/src/ast/scopes/Scope.ts b/src/ast/scopes/Scope.ts index a61f1c8ca..aeeb9e8ff 100644 --- a/src/ast/scopes/Scope.ts +++ b/src/ast/scopes/Scope.ts @@ -15,6 +15,7 @@ export default class Scope { /* Redeclaration rules: - var can redeclare var + - in function scopes, function and var can redeclare function and var - var is hoisted across scopes, function remains in the scope it is declared - var and function can redeclare function parameters, but parameters cannot redeclare parameters - function cannot redeclare catch scope parameters @@ -34,11 +35,7 @@ export default class Scope { this.hoistedVariables?.get(name) || (this.variables.get(name) as LocalVariable); if (existingVariable) { const existingKind = existingVariable.kind; - if ( - (kind === VariableKind.var && - (existingKind === VariableKind.var || existingKind === VariableKind.parameter)) || - (kind === VariableKind.function && existingKind === VariableKind.parameter) - ) { + if (kind === VariableKind.var && existingKind === VariableKind.var) { existingVariable.addDeclaration(identifier, init); return existingVariable; } diff --git a/test/function/samples/ast-validations/redeclare-block-function-function/_config.js b/test/function/samples/ast-validations/redeclare-block-function-function/_config.js new file mode 100644 index 000000000..e09240c80 --- /dev/null +++ b/test/function/samples/ast-validations/redeclare-block-function-function/_config.js @@ -0,0 +1,24 @@ +const path = require('node:path'); +const ID_MAIN = path.join(__dirname, 'main.js'); + +module.exports = defineTest({ + description: 'throws when redeclaring a function binding as a function in a block scope', + error: { + code: 'REDECLARATION_ERROR', + frame: ` + 1: { + 2: function foo() {} + 3: function foo() {} + ^ + 4: }`, + id: ID_MAIN, + loc: { + column: 10, + file: ID_MAIN, + line: 3 + }, + message: 'Identifier "foo" has already been declared', + pos: 31, + watchFiles: [ID_MAIN] + } +}); diff --git a/test/function/samples/ast-validations/redeclare-block-function-function/main.js b/test/function/samples/ast-validations/redeclare-block-function-function/main.js new file mode 100644 index 000000000..0a3369317 --- /dev/null +++ b/test/function/samples/ast-validations/redeclare-block-function-function/main.js @@ -0,0 +1,4 @@ +{ + function foo() {} + function foo() {} +} diff --git a/test/function/samples/ast-validations/redeclare-top-level-function-function/_config.js b/test/function/samples/ast-validations/redeclare-top-level-function-function/_config.js new file mode 100644 index 000000000..e486d0f1b --- /dev/null +++ b/test/function/samples/ast-validations/redeclare-top-level-function-function/_config.js @@ -0,0 +1,22 @@ +const path = require('node:path'); +const ID_MAIN = path.join(__dirname, 'main.js'); + +module.exports = defineTest({ + description: 'throws when redeclaring a top-level function binding as a function', + error: { + code: 'REDECLARATION_ERROR', + frame: ` + 1: function foo() {} + 2: function foo() {} + ^`, + id: ID_MAIN, + loc: { + column: 9, + file: ID_MAIN, + line: 2 + }, + message: 'Identifier "foo" has already been declared', + pos: 27, + watchFiles: [ID_MAIN] + } +}); diff --git a/test/function/samples/ast-validations/redeclare-top-level-function-function/main.js b/test/function/samples/ast-validations/redeclare-top-level-function-function/main.js new file mode 100644 index 000000000..11a6da2a7 --- /dev/null +++ b/test/function/samples/ast-validations/redeclare-top-level-function-function/main.js @@ -0,0 +1,2 @@ +function foo() {} +function foo() {} diff --git a/test/function/samples/ast-validations/redeclare-top-level-var-function/_config.js b/test/function/samples/ast-validations/redeclare-top-level-var-function/_config.js new file mode 100644 index 000000000..887a39e6c --- /dev/null +++ b/test/function/samples/ast-validations/redeclare-top-level-var-function/_config.js @@ -0,0 +1,22 @@ +const path = require('node:path'); +const ID_MAIN = path.join(__dirname, 'main.js'); + +module.exports = defineTest({ + description: 'throws when redeclaring a top-level var binding as a function', + error: { + code: 'REDECLARATION_ERROR', + frame: ` + 1: var foo; + 2: function foo() {} + ^`, + id: ID_MAIN, + loc: { + column: 9, + file: ID_MAIN, + line: 2 + }, + message: 'Identifier "foo" has already been declared', + pos: 18, + watchFiles: [ID_MAIN] + } +}); diff --git a/test/function/samples/ast-validations/redeclare-top-level-var-function/main.js b/test/function/samples/ast-validations/redeclare-top-level-var-function/main.js new file mode 100644 index 000000000..1403e08b8 --- /dev/null +++ b/test/function/samples/ast-validations/redeclare-top-level-var-function/main.js @@ -0,0 +1,2 @@ +var foo; +function foo() {} diff --git a/test/function/samples/ast-validations/redeclare-var-function/_config.js b/test/function/samples/ast-validations/redeclare-var-function/_config.js new file mode 100644 index 000000000..9547a3c58 --- /dev/null +++ b/test/function/samples/ast-validations/redeclare-var-function/_config.js @@ -0,0 +1,3 @@ +module.exports = defineTest({ + description: 'allows to redeclare vars and functions as vars and functions in function scopes' +}); diff --git a/test/function/samples/ast-validations/redeclare-var-function/main.js b/test/function/samples/ast-validations/redeclare-var-function/main.js new file mode 100644 index 000000000..ac8d372d4 --- /dev/null +++ b/test/function/samples/ast-validations/redeclare-var-function/main.js @@ -0,0 +1,10 @@ +function foo() { + var fn = 1; + function fn() {} + function fn() {} + var fn = 2; + + assert.equal(fn, 2); +} + +foo(); From 01d8c9d1b68919c2c429427ae7e60f503a8bb5f4 Mon Sep 17 00:00:00 2001 From: Lukas Taegert-Atkinson Date: Tue, 14 Nov 2023 06:13:32 +0100 Subject: [PATCH 2/2] 4.4.1 --- CHANGELOG.md | 12 ++++++++++++ browser/package.json | 2 +- package-lock.json | 4 ++-- package.json | 2 +- 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8326f6af5..d36543f63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,17 @@ # rollup changelog +## 4.4.1 + +_2023-11-14_ + +### Bug Fixes + +- Do not flag duplicate function declarations in function scopes as syntax errors (#5248) + +### Pull Requests + +- [#5248](https://github.com/rollup/rollup/pull/5248): Allow functions to redeclare vars and functions in function scopes (@lukastaegert) + ## 4.4.0 _2023-11-12_ diff --git a/browser/package.json b/browser/package.json index 50b02de8c..50359112d 100644 --- a/browser/package.json +++ b/browser/package.json @@ -1,6 +1,6 @@ { "name": "@rollup/browser", - "version": "4.4.0", + "version": "4.4.1", "description": "Next-generation ES module bundler browser build", "main": "dist/rollup.browser.js", "module": "dist/es/rollup.browser.js", diff --git a/package-lock.json b/package-lock.json index 08ecda2fd..591425f98 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "rollup", - "version": "4.4.0", + "version": "4.4.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "rollup", - "version": "4.4.0", + "version": "4.4.1", "license": "MIT", "bin": { "rollup": "dist/bin/rollup" diff --git a/package.json b/package.json index 00f29e01a..9ed0afc9c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "rollup", - "version": "4.4.0", + "version": "4.4.1", "description": "Next-generation ES module bundler", "main": "dist/rollup.js", "module": "dist/es/rollup.js",