Check for require when binding commonjs

Previously, the binder didn't check for a declared function named
'require' when binding require in JS. It unconditionally treated
'require' as the commonjs function, even when a local function named
'require' existed and even in ES modules. Now, the binder checks for a
local function named require and avoids binding in that case.

Both before and after, actually importing the module causes the checker
to issue an error saying that top-level functions named 'require' are
illegal. But this error doesn't show up until you've imported from the
module, so you won't notice it in the editor, where JS errors are most
useful.

Within-binder checks for declared functions are dependent on binding order, but
function binding simulates hoisting by binding all function declarations
first, so it should be pretty reliable.
This commit is contained in:
Nathan Shively-Sanders
2022-08-01 13:57:53 -07:00
parent 427d43691a
commit 2a22528c81
6 changed files with 276 additions and 0 deletions
+1
View File
@@ -3260,6 +3260,7 @@ namespace ts {
if (isInJSFile(node) &&
isVariableDeclarationInitializedToBareOrAccessedRequire(possibleVariableDecl) &&
!getJSDocTypeTag(node) &&
!(lookupSymbolForName(container, "require" as __String)) &&
!(getCombinedModifierFlags(node) & ModifierFlags.Export)
) {
declareSymbolAndAddToSymbolTable(node as Declaration, SymbolFlags.Alias, SymbolFlags.AliasExcludes);
@@ -0,0 +1,39 @@
tests/cases/conformance/salsa/first.js(5,10): error TS2441: Duplicate identifier 'require'. Compiler reserves name 'require' in top level scope of a module.
tests/cases/conformance/salsa/global.js(5,10): error TS2441: Duplicate identifier 'require'. Compiler reserves name 'require' in top level scope of a module.
tests/cases/conformance/salsa/last.js(6,10): error TS2441: Duplicate identifier 'require'. Compiler reserves name 'require' in top level scope of a module.
==== tests/cases/conformance/salsa/first.js (1 errors) ====
/**
* @param {string} str
* @return {number}
*/
function require(str) { return str.length }
~~~~~~~
!!! error TS2441: Duplicate identifier 'require'. Compiler reserves name 'require' in top level scope of a module.
const someModule = require("google-closure-compiler");
export {someModule};
==== tests/cases/conformance/salsa/last.js (1 errors) ====
const someModule = require("google-closure-compiler");
/**
* @param {string} str
* @return {number}
*/
function require(str) { return str.length }
~~~~~~~
!!! error TS2441: Duplicate identifier 'require'. Compiler reserves name 'require' in top level scope of a module.
export {someModule};
==== tests/cases/conformance/salsa/global.js (1 errors) ====
/**
* @param {string} str
* @return {number}
*/
function require(str) { return str.length }
~~~~~~~
!!! error TS2441: Duplicate identifier 'require'. Compiler reserves name 'require' in top level scope of a module.
const someModule = require("google-closure-compiler");
==== tests/cases/conformance/salsa/main.ts (0 errors) ====
import { someModule as m1 } from './first'
import { someModule as m2 } from './last'
@@ -0,0 +1,73 @@
//// [tests/cases/conformance/salsa/requireAsDeclaredFunction.ts] ////
//// [first.js]
/**
* @param {string} str
* @return {number}
*/
function require(str) { return str.length }
const someModule = require("google-closure-compiler");
export {someModule};
//// [last.js]
const someModule = require("google-closure-compiler");
/**
* @param {string} str
* @return {number}
*/
function require(str) { return str.length }
export {someModule};
//// [global.js]
/**
* @param {string} str
* @return {number}
*/
function require(str) { return str.length }
const someModule = require("google-closure-compiler");
//// [main.ts]
import { someModule as m1 } from './first'
import { someModule as m2 } from './last'
//// [first.js]
"use strict";
exports.__esModule = true;
exports.someModule = void 0;
/**
* @param {string} str
* @return {number}
*/
function require(str) { return str.length; }
var someModule = require("google-closure-compiler");
exports.someModule = someModule;
//// [last.js]
"use strict";
exports.__esModule = true;
exports.someModule = void 0;
var someModule = require("google-closure-compiler");
exports.someModule = someModule;
/**
* @param {string} str
* @return {number}
*/
function require(str) { return str.length; }
//// [global.js]
/**
* @param {string} str
* @return {number}
*/
function require(str) { return str.length; }
var someModule = require("google-closure-compiler");
//// [main.js]
"use strict";
exports.__esModule = true;
//// [first.d.ts]
export const someModule: number;
//// [last.d.ts]
export const someModule: number;
//// [global.d.ts]
export {};
//// [main.d.ts]
export {};
@@ -0,0 +1,63 @@
=== tests/cases/conformance/salsa/first.js ===
/**
* @param {string} str
* @return {number}
*/
function require(str) { return str.length }
>require : Symbol(require, Decl(first.js, 0, 0))
>str : Symbol(str, Decl(first.js, 4, 17))
>str.length : Symbol(String.length, Decl(lib.es5.d.ts, --, --))
>str : Symbol(str, Decl(first.js, 4, 17))
>length : Symbol(String.length, Decl(lib.es5.d.ts, --, --))
const someModule = require("google-closure-compiler");
>someModule : Symbol(someModule, Decl(first.js, 5, 5))
>require : Symbol(require, Decl(first.js, 0, 0))
export {someModule};
>someModule : Symbol(someModule, Decl(first.js, 6, 8))
=== tests/cases/conformance/salsa/last.js ===
const someModule = require("google-closure-compiler");
>someModule : Symbol(someModule, Decl(last.js, 0, 5))
>require : Symbol(require, Decl(last.js, 0, 54))
/**
* @param {string} str
* @return {number}
*/
function require(str) { return str.length }
>require : Symbol(require, Decl(last.js, 0, 54))
>str : Symbol(str, Decl(last.js, 5, 17))
>str.length : Symbol(String.length, Decl(lib.es5.d.ts, --, --))
>str : Symbol(str, Decl(last.js, 5, 17))
>length : Symbol(String.length, Decl(lib.es5.d.ts, --, --))
export {someModule};
>someModule : Symbol(someModule, Decl(last.js, 6, 8))
=== tests/cases/conformance/salsa/global.js ===
/**
* @param {string} str
* @return {number}
*/
function require(str) { return str.length }
>require : Symbol(require, Decl(global.js, 0, 0))
>str : Symbol(str, Decl(global.js, 4, 17))
>str.length : Symbol(String.length, Decl(lib.es5.d.ts, --, --))
>str : Symbol(str, Decl(global.js, 4, 17))
>length : Symbol(String.length, Decl(lib.es5.d.ts, --, --))
const someModule = require("google-closure-compiler");
>someModule : Symbol(someModule, Decl(global.js, 5, 5))
>require : Symbol(require, Decl(global.js, 0, 0))
=== tests/cases/conformance/salsa/main.ts ===
import { someModule as m1 } from './first'
>someModule : Symbol(someModule, Decl(first.js, 6, 8))
>m1 : Symbol(m1, Decl(main.ts, 0, 8))
import { someModule as m2 } from './last'
>someModule : Symbol(someModule, Decl(last.js, 6, 8))
>m2 : Symbol(m2, Decl(main.ts, 1, 8))
@@ -0,0 +1,69 @@
=== tests/cases/conformance/salsa/first.js ===
/**
* @param {string} str
* @return {number}
*/
function require(str) { return str.length }
>require : (str: string) => number
>str : string
>str.length : number
>str : string
>length : number
const someModule = require("google-closure-compiler");
>someModule : number
>require("google-closure-compiler") : number
>require : (str: string) => number
>"google-closure-compiler" : "google-closure-compiler"
export {someModule};
>someModule : number
=== tests/cases/conformance/salsa/last.js ===
const someModule = require("google-closure-compiler");
>someModule : number
>require("google-closure-compiler") : number
>require : (str: string) => number
>"google-closure-compiler" : "google-closure-compiler"
/**
* @param {string} str
* @return {number}
*/
function require(str) { return str.length }
>require : (str: string) => number
>str : string
>str.length : number
>str : string
>length : number
export {someModule};
>someModule : number
=== tests/cases/conformance/salsa/global.js ===
/**
* @param {string} str
* @return {number}
*/
function require(str) { return str.length }
>require : (str: string) => number
>str : string
>str.length : number
>str : string
>length : number
const someModule = require("google-closure-compiler");
>someModule : number
>require("google-closure-compiler") : number
>require : (str: string) => number
>"google-closure-compiler" : "google-closure-compiler"
=== tests/cases/conformance/salsa/main.ts ===
import { someModule as m1 } from './first'
>someModule : number
>m1 : number
import { someModule as m2 } from './last'
>someModule : number
>m2 : number
@@ -0,0 +1,31 @@
// @outDir: out/
// @declaration: true
// @checkJs: true
// @filename: first.js
/**
* @param {string} str
* @return {number}
*/
function require(str) { return str.length }
const someModule = require("google-closure-compiler");
export {someModule};
// @filename: last.js
const someModule = require("google-closure-compiler");
/**
* @param {string} str
* @return {number}
*/
function require(str) { return str.length }
export {someModule};
// @filename: global.js
/**
* @param {string} str
* @return {number}
*/
function require(str) { return str.length }
const someModule = require("google-closure-compiler");
// @filename: main.ts
import { someModule as m1 } from './first'
import { someModule as m2 } from './last'