Skip to content

Commit

Permalink
Don't elide imports when transforming JS files (#50404)
Browse files Browse the repository at this point in the history
* don't elide imports in JS files

* WIP: get rid of caching of resolved symbol, add transform tests

* get rid of caching only for resolver functions

* use getReferencedSymbol instead of getReferencedValueSymbol in module transform

* WIP: add reportErrors flag to resolveName

* Import transformations now work correctly

* don't emit diagnostics when looking up referenced symbol

* small fixes and get rid of unnecessary comments

* update tests

* clean up

* CR: use nameNotFoundMessage to decide whether to report errors in resolveName
  • Loading branch information
gabritto authored Sep 19, 2022

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent 57c7aa7 commit 3014dec
Showing 23 changed files with 783 additions and 12 deletions.
42 changes: 36 additions & 6 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
@@ -1831,6 +1831,7 @@ namespace ts {
* the nameNotFoundMessage argument is not undefined. Returns the resolved symbol, or undefined if no symbol with
* the given name can be found.
*
* @param nameNotFoundMessage If defined, we will report errors found during resolve.
* @param isUse If true, this will count towards --noUnusedLocals / --noUnusedParameters.
*/
function resolveName(
@@ -1841,8 +1842,8 @@ namespace ts {
nameArg: __String | Identifier | undefined,
isUse: boolean,
excludeGlobals = false,
getSpellingSuggstions = true): Symbol | undefined {
return resolveNameHelper(location, name, meaning, nameNotFoundMessage, nameArg, isUse, excludeGlobals, getSpellingSuggstions, getSymbol);
getSpellingSuggestions = true): Symbol | undefined {
return resolveNameHelper(location, name, meaning, nameNotFoundMessage, nameArg, isUse, excludeGlobals, getSpellingSuggestions, getSymbol);
}

function resolveNameHelper(
@@ -2015,7 +2016,9 @@ namespace ts {
// TypeScript 1.0 spec (April 2014): 3.4.1
// The scope of a type parameter extends over the entire declaration with which the type
// parameter list is associated, with the exception of static member declarations in classes.
error(errorLocation, Diagnostics.Static_members_cannot_reference_class_type_parameters);
if (nameNotFoundMessage) {
error(errorLocation, Diagnostics.Static_members_cannot_reference_class_type_parameters);
}
return undefined;
}
break loop;
@@ -2053,7 +2056,9 @@ namespace ts {
if (isClassLike(grandparent) || grandparent.kind === SyntaxKind.InterfaceDeclaration) {
// A reference to this grandparent's type parameters would be an error
if (result = lookup(getSymbolOfNode(grandparent as ClassLikeDeclaration | InterfaceDeclaration).members!, name, meaning & SymbolFlags.Type)) {
error(errorLocation, Diagnostics.A_computed_property_name_cannot_reference_a_type_parameter_from_its_containing_type);
if (nameNotFoundMessage) {
error(errorLocation, Diagnostics.A_computed_property_name_cannot_reference_a_type_parameter_from_its_containing_type);
}
return undefined;
}
}
@@ -2263,7 +2268,7 @@ namespace ts {
}
return undefined;
}
else if (checkAndReportErrorForInvalidInitializer()) {
else if (nameNotFoundMessage && checkAndReportErrorForInvalidInitializer()) {
return undefined;
}

@@ -43283,7 +43288,8 @@ namespace ts {
}
const node = getParseTreeNode(nodeIn, isIdentifier);
if (node) {
const symbol = getReferencedValueSymbol(node);
const symbol = getReferencedValueOrAliasSymbol(node);

// We should only get the declaration of an alias if there isn't a local value
// declaration for the symbol
if (isNonLocalAlias(symbol, /*excludes*/ SymbolFlags.Value) && !getTypeOnlyAliasDeclaration(symbol)) {
@@ -43687,6 +43693,30 @@ namespace ts {
return resolveName(location, reference.escapedText, SymbolFlags.Value | SymbolFlags.ExportValue | SymbolFlags.Alias, /*nodeNotFoundMessage*/ undefined, /*nameArg*/ undefined, /*isUse*/ true);
}

/**
* Get either a value-meaning symbol or an alias symbol.
* Unlike `getReferencedValueSymbol`, if the cached resolved symbol is the unknown symbol,
* we call `resolveName` to find a symbol.
* This is because when caching the resolved symbol, we only consider value symbols, but here
* we want to also get an alias symbol if one exists.
*/
function getReferencedValueOrAliasSymbol(reference: Identifier): Symbol | undefined {
const resolvedSymbol = getNodeLinks(reference).resolvedSymbol;
if (resolvedSymbol && resolvedSymbol !== unknownSymbol) {
return resolvedSymbol;
}

return resolveName(
reference,
reference.escapedText,
SymbolFlags.Value | SymbolFlags.ExportValue | SymbolFlags.Alias,
/*nodeNotFoundMessage*/ undefined,
/*nameArg*/ undefined,
/*isUse*/ true,
/*excludeGlobals*/ undefined,
/*getSpellingSuggestions*/ undefined);
}

function getReferencedValueDeclaration(referenceIn: Identifier): Declaration | undefined {
if (!isGeneratedIdentifier(referenceIn)) {
const reference = getParseTreeNode(referenceIn, isIdentifier);
8 changes: 5 additions & 3 deletions src/compiler/transformers/ts.ts
Original file line number Diff line number Diff line change
@@ -2463,9 +2463,11 @@ namespace ts {
}

function shouldEmitAliasDeclaration(node: Node): boolean {
return compilerOptions.preserveValueImports
? resolver.isValueAliasDeclaration(node)
: resolver.isReferencedAliasDeclaration(node);
return isInJSFile(node)
? true
: compilerOptions.preserveValueImports
? resolver.isValueAliasDeclaration(node)
: resolver.isReferencedAliasDeclaration(node);
}
}
}
Original file line number Diff line number Diff line change
@@ -45,7 +45,7 @@ b_1["default"];
"use strict";
exports.__esModule = true;
var x = { x: "" };
zzz;
a_1["default"];
var b_1 = require("./b");
b_1["default"];
var y = x;
31 changes: 31 additions & 0 deletions tests/baselines/reference/elidedJSImport1.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
tests/cases/compiler/caller.js(1,21): error TS2307: Cannot find module 'fs' or its corresponding type declarations.
tests/cases/compiler/caller.js(2,8): error TS18042: 'TruffleContract' is a type and cannot be imported in JavaScript files. Use 'import("@truffle/contract").TruffleContract' in a JSDoc type annotation.
tests/cases/compiler/caller.js(4,43): error TS2708: Cannot use namespace 'TruffleContract' as a value.
tests/cases/compiler/caller.js(4,60): error TS2708: Cannot use namespace 'TruffleContract' as a value.


==== tests/cases/compiler/caller.js (4 errors) ====
import * as fs from 'fs';
~~~~
!!! error TS2307: Cannot find module 'fs' or its corresponding type declarations.
import TruffleContract from '@truffle/contract'; // Runtime err: this import is elided in transform
~~~~~~~~~~~~~~~
!!! error TS18042: 'TruffleContract' is a type and cannot be imported in JavaScript files. Use 'import("@truffle/contract").TruffleContract' in a JSDoc type annotation.
console.log(fs);
console.log('TruffleContract is ', typeof TruffleContract, TruffleContract); // `TruffleContract` is considered 'unused'
~~~~~~~~~~~~~~~
!!! error TS2708: Cannot use namespace 'TruffleContract' as a value.
~~~~~~~~~~~~~~~
!!! error TS2708: Cannot use namespace 'TruffleContract' as a value.


==== tests/cases/compiler/node_modules/@truffle/contract/index.d.ts (0 errors) ====
declare module "@truffle/contract" {
interface ContractObject {
foo: number;
}
namespace TruffleContract {
export type Contract = ContractObject;
}
export default TruffleContract;
}
25 changes: 25 additions & 0 deletions tests/baselines/reference/elidedJSImport1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
//// [tests/cases/compiler/elidedJSImport1.ts] ////

//// [caller.js]
import * as fs from 'fs';
import TruffleContract from '@truffle/contract'; // Runtime err: this import is elided in transform
console.log(fs);
console.log('TruffleContract is ', typeof TruffleContract, TruffleContract); // `TruffleContract` is considered 'unused'


//// [index.d.ts]
declare module "@truffle/contract" {
interface ContractObject {
foo: number;
}
namespace TruffleContract {
export type Contract = ContractObject;
}
export default TruffleContract;
}

//// [caller.js]
import * as fs from 'fs';
import TruffleContract from '@truffle/contract'; // Runtime err: this import is elided in transform
console.log(fs);
console.log('TruffleContract is ', typeof TruffleContract, TruffleContract); // `TruffleContract` is considered 'unused'
39 changes: 39 additions & 0 deletions tests/baselines/reference/elidedJSImport1.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
=== tests/cases/compiler/caller.js ===
import * as fs from 'fs';
>fs : Symbol(fs, Decl(caller.js, 0, 6))

import TruffleContract from '@truffle/contract'; // Runtime err: this import is elided in transform
>TruffleContract : Symbol(TruffleContract, Decl(caller.js, 1, 6))

console.log(fs);
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>console : Symbol(console, Decl(lib.dom.d.ts, --, --))
>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>fs : Symbol(fs, Decl(caller.js, 0, 6))

console.log('TruffleContract is ', typeof TruffleContract, TruffleContract); // `TruffleContract` is considered 'unused'
>console.log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))
>console : Symbol(console, Decl(lib.dom.d.ts, --, --))
>log : Symbol(Console.log, Decl(lib.dom.d.ts, --, --))


=== tests/cases/compiler/node_modules/@truffle/contract/index.d.ts ===
declare module "@truffle/contract" {
>"@truffle/contract" : Symbol("@truffle/contract", Decl(index.d.ts, 0, 0))

interface ContractObject {
>ContractObject : Symbol(ContractObject, Decl(index.d.ts, 0, 36))

foo: number;
>foo : Symbol(ContractObject.foo, Decl(index.d.ts, 1, 30))
}
namespace TruffleContract {
>TruffleContract : Symbol(TruffleContract, Decl(index.d.ts, 3, 5))

export type Contract = ContractObject;
>Contract : Symbol(Contract, Decl(index.d.ts, 4, 31))
>ContractObject : Symbol(ContractObject, Decl(index.d.ts, 0, 36))
}
export default TruffleContract;
>TruffleContract : Symbol(TruffleContract, Decl(index.d.ts, 3, 5))
}
40 changes: 40 additions & 0 deletions tests/baselines/reference/elidedJSImport1.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
=== tests/cases/compiler/caller.js ===
import * as fs from 'fs';
>fs : any

import TruffleContract from '@truffle/contract'; // Runtime err: this import is elided in transform
>TruffleContract : any

console.log(fs);
>console.log(fs) : void
>console.log : (...data: any[]) => void
>console : Console
>log : (...data: any[]) => void
>fs : any

console.log('TruffleContract is ', typeof TruffleContract, TruffleContract); // `TruffleContract` is considered 'unused'
>console.log('TruffleContract is ', typeof TruffleContract, TruffleContract) : void
>console.log : (...data: any[]) => void
>console : Console
>log : (...data: any[]) => void
>'TruffleContract is ' : "TruffleContract is "
>typeof TruffleContract : "string" | "number" | "bigint" | "boolean" | "symbol" | "undefined" | "object" | "function"
>TruffleContract : any
>TruffleContract : any


=== tests/cases/compiler/node_modules/@truffle/contract/index.d.ts ===
declare module "@truffle/contract" {
>"@truffle/contract" : typeof import("@truffle/contract")

interface ContractObject {
foo: number;
>foo : number
}
namespace TruffleContract {
export type Contract = ContractObject;
>Contract : ContractObject
}
export default TruffleContract;
>TruffleContract : any
}
83 changes: 83 additions & 0 deletions tests/baselines/reference/elidedJSImport2(module=commonjs).js
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
//// [tests/cases/compiler/elidedJSImport2.ts] ////

//// [index.js]
import { Foo } from "./other.js";
import * as other from "./other.js";
import defaultFoo from "./other.js";

const x = new Foo();
const y = other.Foo();
const z = new defaultFoo();

//// [other.d.ts]
export interface Foo {
bar: number;
}

export default interface Bar {
foo: number;
}

//// [other.js]
export class Foo {
bar = 2.4;
}

export default class Bar {
foo = 1.2;
}


//// [index.js]
"use strict";
var __createBinding = (this && this.__createBinding) || (Object.create ? (function(o, m, k, k2) {
if (k2 === undefined) k2 = k;
var desc = Object.getOwnPropertyDescriptor(m, k);
if (!desc || ("get" in desc ? !m.__esModule : desc.writable || desc.configurable)) {
desc = { enumerable: true, get: function() { return m[k]; } };
}
Object.defineProperty(o, k2, desc);
}) : (function(o, m, k, k2) {
if (k2 === undefined) k2 = k;
o[k2] = m[k];
}));
var __setModuleDefault = (this && this.__setModuleDefault) || (Object.create ? (function(o, v) {
Object.defineProperty(o, "default", { enumerable: true, value: v });
}) : function(o, v) {
o["default"] = v;
});
var __importStar = (this && this.__importStar) || function (mod) {
if (mod && mod.__esModule) return mod;
var result = {};
if (mod != null) for (var k in mod) if (k !== "default" && Object.prototype.hasOwnProperty.call(mod, k)) __createBinding(result, mod, k);
__setModuleDefault(result, mod);
return result;
};
var __importDefault = (this && this.__importDefault) || function (mod) {
return (mod && mod.__esModule) ? mod : { "default": mod };
};
exports.__esModule = true;
var other_js_1 = require("./other.js");
var other = __importStar(require("./other.js"));
var other_js_2 = __importDefault(require("./other.js"));
var x = new other_js_1.Foo();
var y = other.Foo();
var z = new other_js_2["default"]();
//// [other.js]
"use strict";
exports.__esModule = true;
exports.Foo = void 0;
var Foo = /** @class */ (function () {
function Foo() {
this.bar = 2.4;
}
return Foo;
}());
exports.Foo = Foo;
var Bar = /** @class */ (function () {
function Bar() {
this.foo = 1.2;
}
return Bar;
}());
exports["default"] = Bar;
50 changes: 50 additions & 0 deletions tests/baselines/reference/elidedJSImport2(module=commonjs).symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
=== tests/cases/compiler/index.js ===
import { Foo } from "./other.js";
>Foo : Symbol(Foo, Decl(index.js, 0, 8))

import * as other from "./other.js";
>other : Symbol(other, Decl(index.js, 1, 6))

import defaultFoo from "./other.js";
>defaultFoo : Symbol(defaultFoo, Decl(index.js, 2, 6))

const x = new Foo();
>x : Symbol(x, Decl(index.js, 4, 5))

const y = other.Foo();
>y : Symbol(y, Decl(index.js, 5, 5))
>other : Symbol(other, Decl(index.js, 1, 6))

const z = new defaultFoo();
>z : Symbol(z, Decl(index.js, 6, 5))

=== tests/cases/compiler/other.d.ts ===
export interface Foo {
>Foo : Symbol(Foo, Decl(other.d.ts, 0, 0))

bar: number;
>bar : Symbol(Foo.bar, Decl(other.d.ts, 0, 22))
}

export default interface Bar {
>Bar : Symbol(Bar, Decl(other.d.ts, 2, 1))

foo: number;
>foo : Symbol(Bar.foo, Decl(other.d.ts, 4, 30))
}

=== tests/cases/compiler/other.js ===
export class Foo {
>Foo : Symbol(Foo, Decl(other.js, 0, 0))

bar = 2.4;
>bar : Symbol(Foo.bar, Decl(other.js, 0, 18))
}

export default class Bar {
>Bar : Symbol(Bar, Decl(other.js, 2, 1))

foo = 1.2;
>foo : Symbol(Bar.foo, Decl(other.js, 4, 26))
}

Loading

0 comments on commit 3014dec

Please sign in to comment.