Skip to content

Fix crash when importsNotUsedAsValues is set alongside verbatimModuleSyntax #53386

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 2 commits into from
Mar 20, 2023
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
24 changes: 15 additions & 9 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1472,6 +1472,9 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
var argumentsSymbol = createSymbol(SymbolFlags.Property, "arguments" as __String);
var requireSymbol = createSymbol(SymbolFlags.Property, "require" as __String);
var isolatedModulesLikeFlagName = compilerOptions.verbatimModuleSyntax ? "verbatimModuleSyntax" : "isolatedModules";
// It is an error to use `importsNotUsedAsValues` alongside `verbatimModuleSyntax`, but we still need to not crash.
// Given that, in such a scenario, `verbatimModuleSyntax` is basically disabled, as least as far as alias visibility tracking goes.
var canCollectSymbolAliasAccessabilityData = !compilerOptions.verbatimModuleSyntax || !!compilerOptions.importsNotUsedAsValues;

/** This will be set during calls to `getResolvedSignature` where services determines an apparent number of arguments greater than what is actually provided. */
var apparentArgumentCount: number | undefined;
Expand Down Expand Up @@ -4540,7 +4543,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}

function markExportAsReferenced(node: ImportEqualsDeclaration | ExportSpecifier) {
if (compilerOptions.verbatimModuleSyntax) {
if (!canCollectSymbolAliasAccessabilityData) {
return;
}
const symbol = getSymbolOfDeclaration(node);
Expand All @@ -4559,7 +4562,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
// we reach a non-alias or an exported entity (which is always considered referenced). We do this by checking the target of
// the alias as an expression (which recursively takes us back here if the target references another alias).
function markAliasSymbolAsReferenced(symbol: Symbol) {
Debug.assert(!compilerOptions.verbatimModuleSyntax);
Debug.assert(canCollectSymbolAliasAccessabilityData);
const links = getSymbolLinks(symbol);
if (!links.referenced) {
links.referenced = true;
Expand Down Expand Up @@ -27631,7 +27634,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}

function markAliasReferenced(symbol: Symbol, location: Node) {
if (compilerOptions.verbatimModuleSyntax) {
if (!canCollectSymbolAliasAccessabilityData) {
return;
}
if (isNonLocalAlias(symbol, /*excludes*/ SymbolFlags.Value) && !isInTypeQuery(location) && !getTypeOnlyAliasDeclaration(symbol, SymbolFlags.Value)) {
Expand Down Expand Up @@ -30773,7 +30776,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
jsxFactorySym.isReferenced = SymbolFlags.All;

// If react/jsxFactory symbol is alias, mark it as refereced
if (!compilerOptions.verbatimModuleSyntax && jsxFactorySym.flags & SymbolFlags.Alias && !getTypeOnlyAliasDeclaration(jsxFactorySym)) {
if (canCollectSymbolAliasAccessabilityData && jsxFactorySym.flags & SymbolFlags.Alias && !getTypeOnlyAliasDeclaration(jsxFactorySym)) {
markAliasSymbolAsReferenced(jsxFactorySym);
}
}
Expand Down Expand Up @@ -39710,7 +39713,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const meaning = (typeName.kind === SyntaxKind.Identifier ? SymbolFlags.Type : SymbolFlags.Namespace) | SymbolFlags.Alias;
const rootSymbol = resolveName(rootName, rootName.escapedText, meaning, /*nameNotFoundMessage*/ undefined, /*nameArg*/ undefined, /*isReference*/ true);
if (rootSymbol && rootSymbol.flags & SymbolFlags.Alias) {
if (!compilerOptions.verbatimModuleSyntax
if (canCollectSymbolAliasAccessabilityData
&& symbolIsValue(rootSymbol)
&& !isConstEnumOrConstEnumOnlyModule(resolveAlias(rootSymbol))
&& !getTypeOnlyAliasDeclaration(rootSymbol)) {
Expand Down Expand Up @@ -44227,6 +44230,9 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}

function checkImportsForTypeOnlyConversion(sourceFile: SourceFile) {
if (!canCollectSymbolAliasAccessabilityData) {
return;
}
for (const statement of sourceFile.statements) {
if (canConvertImportDeclarationToTypeOnly(statement) || canConvertImportEqualsDeclarationToTypeOnly(statement)) {
error(
Expand Down Expand Up @@ -45949,7 +45955,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}

function isValueAliasDeclaration(node: Node): boolean {
Debug.assert(!compilerOptions.verbatimModuleSyntax);
Debug.assert(canCollectSymbolAliasAccessabilityData);
switch (node.kind) {
case SyntaxKind.ImportEqualsDeclaration:
return isAliasResolvedToValue(getSymbolOfDeclaration(node as ImportEqualsDeclaration));
Expand Down Expand Up @@ -46003,7 +46009,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}

function isReferencedAliasDeclaration(node: Node, checkChildren?: boolean): boolean {
Debug.assert(!compilerOptions.verbatimModuleSyntax);
Debug.assert(canCollectSymbolAliasAccessabilityData);
if (isAliasSymbolDeclaration(node)) {
const symbol = getSymbolOfDeclaration(node as Declaration);
const links = symbol && getSymbolLinks(symbol);
Expand Down Expand Up @@ -46377,13 +46383,13 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
isValueAliasDeclaration: nodeIn => {
const node = getParseTreeNode(nodeIn);
// Synthesized nodes are always treated like values.
return node ? isValueAliasDeclaration(node) : true;
return node && canCollectSymbolAliasAccessabilityData ? isValueAliasDeclaration(node) : true;
},
hasGlobalName,
isReferencedAliasDeclaration: (nodeIn, checkChildren?) => {
const node = getParseTreeNode(nodeIn);
// Synthesized nodes are always treated as referenced.
return node ? isReferencedAliasDeclaration(node, checkChildren) : true;
return node && canCollectSymbolAliasAccessabilityData ? isReferencedAliasDeclaration(node, checkChildren) : true;
},
getNodeCheckFlags: nodeIn => {
const node = getParseTreeNode(nodeIn);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
error TS5104: Option 'importsNotUsedAsValues' is redundant and cannot be specified with option 'verbatimModuleSyntax'.
tests/cases/compiler/file.ts(1,1): error TS1287: A top-level 'export' modifier cannot be used on value declarations in a CommonJS module when 'verbatimModuleSyntax' is enabled.
tests/cases/compiler/index.ts(1,1): error TS1371: This import is never used as a value and must use 'import type' because 'importsNotUsedAsValues' is set to 'error'.
tests/cases/compiler/index.ts(1,9): error TS1286: ESM syntax is not allowed in a CommonJS module when 'verbatimModuleSyntax' is enabled.


!!! error TS5104: Option 'importsNotUsedAsValues' is redundant and cannot be specified with option 'verbatimModuleSyntax'.
==== tests/cases/compiler/file.ts (1 errors) ====
export class A {}
~~~~~~
!!! error TS1287: A top-level 'export' modifier cannot be used on value declarations in a CommonJS module when 'verbatimModuleSyntax' is enabled.
==== tests/cases/compiler/index.ts (2 errors) ====
import {A} from "./file";
~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS1371: This import is never used as a value and must use 'import type' because 'importsNotUsedAsValues' is set to 'error'.
~
!!! error TS1286: ESM syntax is not allowed in a CommonJS module when 'verbatimModuleSyntax' is enabled.

const a: A = null as any;
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
//// [tests/cases/compiler/noCrashWithVerbatimModuleSyntaxAndImportsNotUsedAsValues.ts] ////

//// [file.ts]
export class A {}
//// [index.ts]
import {A} from "./file";

const a: A = null as any;

//// [file.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.A = void 0;
var A = /** @class */ (function () {
function A() {
}
return A;
}());
exports.A = A;
//// [index.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
var file_1 = require("./file");
var a = null;
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
=== tests/cases/compiler/file.ts ===
export class A {}
>A : Symbol(A, Decl(file.ts, 0, 0))

=== tests/cases/compiler/index.ts ===
import {A} from "./file";
>A : Symbol(A, Decl(index.ts, 0, 8))

const a: A = null as any;
>a : Symbol(a, Decl(index.ts, 2, 5))
>A : Symbol(A, Decl(index.ts, 0, 8))

Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
=== tests/cases/compiler/file.ts ===
export class A {}
>A : A

=== tests/cases/compiler/index.ts ===
import {A} from "./file";
>A : typeof A

const a: A = null as any;
>a : A
>null as any : any

Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// @verbatimModuleSyntax: true
// @importsNotUsedAsValues: error
// @ignoreDeprecations: 5.0
// @filename: file.ts
export class A {}
// @filename: index.ts
import {A} from "./file";

const a: A = null as any;