Skip to content

Plain JS binder errors #46816

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Nov 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 35 additions & 7 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -818,6 +818,25 @@ namespace ts {
}
}

/** @internal */
export const plainJSErrors: Set<number> = new Set([
Diagnostics.Cannot_redeclare_block_scoped_variable_0.code,
Diagnostics.A_module_cannot_have_multiple_default_exports.code,
Diagnostics.Another_export_default_is_here.code,
Diagnostics.The_first_export_default_is_here.code,
Diagnostics.Identifier_expected_0_is_a_reserved_word_at_the_top_level_of_a_module.code,
Diagnostics.Identifier_expected_0_is_a_reserved_word_in_strict_mode_Modules_are_automatically_in_strict_mode.code,
Diagnostics.Identifier_expected_0_is_a_reserved_word_that_cannot_be_used_here.code,
Diagnostics.constructor_is_a_reserved_word.code,
Diagnostics.delete_cannot_be_called_on_an_identifier_in_strict_mode.code,
Diagnostics.Code_contained_in_a_class_is_evaluated_in_JavaScript_s_strict_mode_which_does_not_allow_this_use_of_0_For_more_information_see_https_Colon_Slash_Slashdeveloper_mozilla_org_Slashen_US_Slashdocs_SlashWeb_SlashJavaScript_SlashReference_SlashStrict_mode.code,
Diagnostics.Invalid_use_of_0_Modules_are_automatically_in_strict_mode.code,
Diagnostics.Invalid_use_of_0_in_strict_mode.code,
Diagnostics.A_label_is_not_allowed_here.code,
Diagnostics.Octal_literals_are_not_allowed_in_strict_mode.code,
Diagnostics.with_statements_are_not_allowed_in_strict_mode.code,
]);

/**
* Determine if source file needs to be re-created even if its text hasn't changed
*/
Expand Down Expand Up @@ -2004,15 +2023,24 @@ namespace ts {

Debug.assert(!!sourceFile.bindDiagnostics);

const isCheckJs = isCheckJsEnabledForFile(sourceFile, options);
const isJs = sourceFile.scriptKind === ScriptKind.JS || sourceFile.scriptKind === ScriptKind.JSX;
const isCheckJs = isJs && isCheckJsEnabledForFile(sourceFile, options);
const isPlainJs = isJs && !sourceFile.checkJsDirective && options.checkJs === undefined;
Comment on lines +2027 to +2028
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What’s the difference between isCheckJsEnabledForFile the property checks you’re doing for isPlainJs?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I almost inlined that function to make this easier to read, except that it's used everywhere. isCheckJsEnabledForFile is true whenever checkJs is explicitly true, whether locally or globally. To put it in the most parallel form:

sourceFile.checkJsDirective?.enabled || options.checkJs === true vs
sourceFile.checkJsDirective === undefined && options.checkJs === undefined

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, so the third kind of JS is JS where checkJs is explicitly disabled?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right. // @ts-nocheck is the local form of that.

const isTsNoCheck = !!sourceFile.checkJsDirective && sourceFile.checkJsDirective.enabled === false;
// By default, only type-check .ts, .tsx, 'Deferred' and 'External' files (external files are added by plugins)
const includeBindAndCheckDiagnostics = !isTsNoCheck && (sourceFile.scriptKind === ScriptKind.TS || sourceFile.scriptKind === ScriptKind.TSX
|| sourceFile.scriptKind === ScriptKind.External || isCheckJs || sourceFile.scriptKind === ScriptKind.Deferred);
const bindDiagnostics: readonly Diagnostic[] = includeBindAndCheckDiagnostics ? sourceFile.bindDiagnostics : emptyArray;
const checkDiagnostics = includeBindAndCheckDiagnostics ? typeChecker.getDiagnostics(sourceFile, cancellationToken) : emptyArray;

return getMergedBindAndCheckDiagnostics(sourceFile, includeBindAndCheckDiagnostics, bindDiagnostics, checkDiagnostics, isCheckJs ? sourceFile.jsDocDiagnostics : undefined);
// By default, only type-check .ts, .tsx, Deferred, plain JS, checked JS and External
// - plain JS: .js files with no // ts-check and checkJs: undefined
// - check JS: .js files with either // ts-check or checkJs: true
// - external: files that are added by plugins
const includeBindAndCheckDiagnostics = !isTsNoCheck && (sourceFile.scriptKind === ScriptKind.TS || sourceFile.scriptKind === ScriptKind.TSX
|| sourceFile.scriptKind === ScriptKind.External || isPlainJs || isCheckJs || sourceFile.scriptKind === ScriptKind.Deferred);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isPlainJs || isCheckJs || ...

Related to my other question, but what other kind of JS is there?

It took me a second to realize, it’s easier to think about this condition in terms of what it excludes, although it may or may not be easier to write it that way:

  • // @ts-nocheck files
  • JSON files
  • ???

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • plain JS (no check options anywhere)
  • checked JS (check turned on locally or globally)
  • no-checked JS (check turned off locally)
  • TS
  • no-checked TS (check turned off locally)
  • external (from plugins?)
  • deferred (??)
  • ???

I have no idea what's in ???, that's the problem with inverting the predicate.

let bindDiagnostics: readonly Diagnostic[] = includeBindAndCheckDiagnostics ? sourceFile.bindDiagnostics : emptyArray;
const checkDiagnostics = includeBindAndCheckDiagnostics && !isPlainJs ? typeChecker.getDiagnostics(sourceFile, cancellationToken) : emptyArray;
if (isPlainJs) {
bindDiagnostics = filter(bindDiagnostics, d => plainJSErrors.has(d.code));
}
// skip ts-expect-error errors in plain JS files, and skip JSDoc errors except in checked JS
return getMergedBindAndCheckDiagnostics(sourceFile, includeBindAndCheckDiagnostics && !isPlainJs, bindDiagnostics, checkDiagnostics, isCheckJs ? sourceFile.jsDocDiagnostics : undefined);
});
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/a.js(18,9): error TS1210: Code contained in a class is evaluated in JavaScript's strict mode which does not allow this use of 'arguments'. For more information, see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode.


==== /a.js (1 errors) ====
class A {
/**
* Constructor
*
* @param {object} [foo={}]
*/
constructor(foo = {}) {
const key = "bar";

/**
* @type object
*/
this.foo = foo;

/**
* @type object
*/
const arguments = this.arguments;
~~~~~~~~~
!!! error TS1210: Code contained in a class is evaluated in JavaScript's strict mode which does not allow this use of 'arguments'. For more information, see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode.

/**
* @type object
*/
this.bar = arguments.bar;

/**
* @type object
*/
this.baz = arguments[key];

/**
* @type object
*/
this.options = arguments;
}

get arguments() {
return { bar: {} };
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/a.js(16,9): error TS1210: Code contained in a class is evaluated in JavaScript's strict mode which does not allow this use of 'arguments'. For more information, see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode.


==== /a.js (1 errors) ====
class A {
/**
* @param {object} [foo={}]
*/
m(foo = {}) {
const key = "bar";

/**
* @type object
*/
this.foo = foo;

/**
* @type object
*/
const arguments = this.arguments;
~~~~~~~~~
!!! error TS1210: Code contained in a class is evaluated in JavaScript's strict mode which does not allow this use of 'arguments'. For more information, see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode.

/**
* @type object
*/
this.bar = arguments.bar;

/**
* @type object
*/
this.baz = arguments[key];

/**
* @type object
*/
this.options = arguments;
}

get arguments() {
return { bar: {} };
}
}

10 changes: 8 additions & 2 deletions tests/baselines/reference/jsdocTypedefNoCrash2.errors.txt
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
tests/cases/compiler/export.js(1,13): error TS2451: Cannot redeclare block-scoped variable 'foo'.
tests/cases/compiler/export.js(1,13): error TS8008: Type aliases can only be used in TypeScript files.
tests/cases/compiler/export.js(6,14): error TS2451: Cannot redeclare block-scoped variable 'foo'.


==== tests/cases/compiler/export.js (1 errors) ====
==== tests/cases/compiler/export.js (3 errors) ====
export type foo = 5;
~~~
!!! error TS2451: Cannot redeclare block-scoped variable 'foo'.
~~~
!!! error TS8008: Type aliases can only be used in TypeScript files.
/**
* @typedef {{
* }}
*/
export const foo = 5;
export const foo = 5;
~~~
!!! error TS2451: Cannot redeclare block-scoped variable 'foo'.
97 changes: 97 additions & 0 deletions tests/baselines/reference/plainJSBinderErrors.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
tests/cases/conformance/salsa/plainJSBinderErrors.js(1,1): error TS2528: A module cannot have multiple default exports.
tests/cases/conformance/salsa/plainJSBinderErrors.js(2,1): error TS2528: A module cannot have multiple default exports.
tests/cases/conformance/salsa/plainJSBinderErrors.js(3,7): error TS1262: Identifier expected. 'await' is a reserved word at the top-level of a module.
tests/cases/conformance/salsa/plainJSBinderErrors.js(4,7): error TS1214: Identifier expected. 'yield' is a reserved word in strict mode. Modules are automatically in strict mode.
tests/cases/conformance/salsa/plainJSBinderErrors.js(6,11): error TS1359: Identifier expected. 'await' is a reserved word that cannot be used here.
tests/cases/conformance/salsa/plainJSBinderErrors.js(9,11): error TS1214: Identifier expected. 'yield' is a reserved word in strict mode. Modules are automatically in strict mode.
tests/cases/conformance/salsa/plainJSBinderErrors.js(12,5): error TS18012: '#constructor' is a reserved word.
tests/cases/conformance/salsa/plainJSBinderErrors.js(15,20): error TS1102: 'delete' cannot be called on an identifier in strict mode.
tests/cases/conformance/salsa/plainJSBinderErrors.js(18,16): error TS1102: 'delete' cannot be called on an identifier in strict mode.
tests/cases/conformance/salsa/plainJSBinderErrors.js(19,16): error TS1102: 'delete' cannot be called on an identifier in strict mode.
tests/cases/conformance/salsa/plainJSBinderErrors.js(22,15): error TS1210: Code contained in a class is evaluated in JavaScript's strict mode which does not allow this use of 'eval'. For more information, see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode.
tests/cases/conformance/salsa/plainJSBinderErrors.js(23,15): error TS1210: Code contained in a class is evaluated in JavaScript's strict mode which does not allow this use of 'arguments'. For more information, see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode.
tests/cases/conformance/salsa/plainJSBinderErrors.js(26,27): error TS1121: Octal literals are not allowed in strict mode.
tests/cases/conformance/salsa/plainJSBinderErrors.js(27,9): error TS1101: 'with' statements are not allowed in strict mode.
tests/cases/conformance/salsa/plainJSBinderErrors.js(33,13): error TS1344: 'A label is not allowed here.
tests/cases/conformance/salsa/plainJSBinderErrors.js(39,7): error TS1215: Invalid use of 'eval'. Modules are automatically in strict mode.
tests/cases/conformance/salsa/plainJSBinderErrors.js(40,7): error TS1215: Invalid use of 'arguments'. Modules are automatically in strict mode.


==== tests/cases/conformance/salsa/plainJSBinderErrors.js (17 errors) ====
export default 12
~~~~~~~~~~~~~~~~~
!!! error TS2528: A module cannot have multiple default exports.
!!! related TS2753 tests/cases/conformance/salsa/plainJSBinderErrors.js:2:1: Another export default is here.
export default 13
~~~~~~~~~~~~~~~~~
!!! error TS2528: A module cannot have multiple default exports.
!!! related TS2752 tests/cases/conformance/salsa/plainJSBinderErrors.js:1:1: The first export default is here.
const await = 1
~~~~~
!!! error TS1262: Identifier expected. 'await' is a reserved word at the top-level of a module.
const yield = 2
~~~~~
!!! error TS1214: Identifier expected. 'yield' is a reserved word in strict mode. Modules are automatically in strict mode.
async function f() {
const await = 3
~~~~~
!!! error TS1359: Identifier expected. 'await' is a reserved word that cannot be used here.
}
function* g() {
const yield = 4
~~~~~
!!! error TS1214: Identifier expected. 'yield' is a reserved word in strict mode. Modules are automatically in strict mode.
}
class C {
#constructor = 5
~~~~~~~~~~~~
!!! error TS18012: '#constructor' is a reserved word.
deleted() {
function container(f) {
delete f
~
!!! error TS1102: 'delete' cannot be called on an identifier in strict mode.
}
var g = 6
delete g
~
!!! error TS1102: 'delete' cannot be called on an identifier in strict mode.
delete container
~~~~~~~~~
!!! error TS1102: 'delete' cannot be called on an identifier in strict mode.
}
evalArguments() {
const eval = 7
~~~~
!!! error TS1210: Code contained in a class is evaluated in JavaScript's strict mode which does not allow this use of 'eval'. For more information, see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode.
const arguments = 8
~~~~~~~~~
!!! error TS1210: Code contained in a class is evaluated in JavaScript's strict mode which does not allow this use of 'arguments'. For more information, see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode.
}
withOctal() {
const redundant = 010
~~~
!!! error TS1121: Octal literals are not allowed in strict mode.
with (redundant) {
~~~~
!!! error TS1101: 'with' statements are not allowed in strict mode.
return toFixed()
}
}
label() {
for(;;) {
label: var x = 1
~~~~~
!!! error TS1344: 'A label is not allowed here.
break label
}
return x
}
}
const eval = 9
~~~~
!!! error TS1215: Invalid use of 'eval'. Modules are automatically in strict mode.
const arguments = 10
~~~~~~~~~
!!! error TS1215: Invalid use of 'arguments'. Modules are automatically in strict mode.

84 changes: 84 additions & 0 deletions tests/baselines/reference/plainJSBinderErrors.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
//// [plainJSBinderErrors.js]
export default 12
export default 13
const await = 1
const yield = 2
async function f() {
const await = 3
}
function* g() {
const yield = 4
}
class C {
#constructor = 5
deleted() {
function container(f) {
delete f
}
var g = 6
delete g
delete container
}
evalArguments() {
const eval = 7
const arguments = 8
}
withOctal() {
const redundant = 010
with (redundant) {
return toFixed()
}
}
label() {
for(;;) {
label: var x = 1
break label
}
return x
}
}
const eval = 9
const arguments = 10


//// [plainJSBinderErrors.js]
export default 12;
export default 13;
const await = 1;
const yield = 2;
async function f() {
const await = 3;
}
function* g() {
const yield = 4;
}
class C {
#constructor = 5;
deleted() {
function container(f) {
delete f;
}
var g = 6;
delete g;
delete container;
}
evalArguments() {
const eval = 7;
const arguments = 8;
}
withOctal() {
const redundant = 010;
with (redundant) {
return toFixed();
}
}
label() {
for (;;) {
label: var x = 1;
break label;
}
return x;
}
}
const eval = 9;
const arguments = 10;
Loading