Skip to content
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

Add full diagnostics to tserror #1706

Merged
18 changes: 15 additions & 3 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions package.json
Expand Up @@ -61,7 +61,7 @@
],
"scripts": {
"lint": "prettier --check .",
"lint-fix": "prettier --write .",
"lint-fix": "prettier --loglevel warn --write .",
"clean": "rimraf dist tsconfig.schema.json tsconfig.schemastore-schema.json tsconfig.tsbuildinfo tests/ts-node-packed.tgz",
"rebuild": "npm run clean && npm run build",
"build": "npm run build-nopack && npm run build-pack",
Expand Down Expand Up @@ -139,7 +139,7 @@
"semver": "^7.1.3",
"throat": "^6.0.1",
"typedoc": "^0.22.10",
"typescript": "4.5.5",
"typescript": "4.6.3",
"typescript-json-schema": "^0.53.0",
"util.promisify": "^1.0.1"
},
Expand Down
14 changes: 12 additions & 2 deletions src/index.ts
Expand Up @@ -470,14 +470,24 @@ export const DEFAULTS: RegisterOptions = {
export class TSError extends BaseError {
name = 'TSError';
diagnosticText!: string;
diagnostics!: ReadonlyArray<_ts.Diagnostic>;

constructor(diagnosticText: string, public diagnosticCodes: number[]) {
constructor(
diagnosticText: string,
public diagnosticCodes: number[],
diagnostics: ReadonlyArray<_ts.Diagnostic> = []
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this argument optional to avoid a breaking change to our API surface, since TSError is part of our API?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this argument optional to avoid a breaking change to our API surface, since TSError is part of our API?

Yes, that's exactly why it's optional — I didn't want to make this a breaking change.

) {
super(`⨯ Unable to compile TypeScript:\n${diagnosticText}`);
Object.defineProperty(this, 'diagnosticText', {
configurable: true,
writable: true,
value: diagnosticText,
});
Object.defineProperty(this, 'diagnostics', {
configurable: true,
writable: true,
value: diagnostics,
});
}

/**
Expand Down Expand Up @@ -829,7 +839,7 @@ export function createFromPreloadedConfig(
function createTSError(diagnostics: ReadonlyArray<_ts.Diagnostic>) {
const diagnosticText = formatDiagnostics(diagnostics, diagnosticHost);
const diagnosticCodes = diagnostics.map((x) => x.code);
return new TSError(diagnosticText, diagnosticCodes);
return new TSError(diagnosticText, diagnosticCodes, diagnostics);
}

function reportTSError(configDiagnosticList: _ts.Diagnostic[]) {
Expand Down
67 changes: 67 additions & 0 deletions src/test/diagnostics.spec.ts
@@ -0,0 +1,67 @@
import type { TSError } from '..';
import { contextTsNodeUnderTest, ts } from './helpers';
import { context, expect } from './testlib';
import * as semver from 'semver';
import { once } from 'lodash';
const test = context(contextTsNodeUnderTest);

test.suite('TSError diagnostics', ({ context }) => {
const test = context(
once(async (t) => {
const service = t.context.tsNodeUnderTest.create({
compilerOptions: { target: 'es5' },
skipProject: true,
});
try {
service.compile('new Error(123)', 'test.ts');
} catch (err) {
return { service, err };
}
return { service, err: undefined };
})
);

const diagnosticCode = 2345;
const diagnosticMessage = semver.satisfies(ts.version, '2.7')
? "Argument of type '123' " +
"is not assignable to parameter of type 'string | undefined'."
: "Argument of type 'number' " +
"is not assignable to parameter of type 'string'.";
const diagnosticErrorMessage = `TS${diagnosticCode}: ${diagnosticMessage}`;

const cwdBefore = process.cwd();
test('should throw errors', ({ log, context: { err, service } }) => {
log({
version: ts.version,
serviceVersion: service.ts.version,
cwdBefore,
cwd: process.cwd(),
configFilePath: service.configFilePath,
config: service.config.options,
});
expect(err).toBeDefined();
expect((err as Error).message).toMatch(diagnosticErrorMessage);
});

test('should throw errors with diagnostic text', ({ context: { err } }) => {
expect((err as TSError).diagnosticText).toMatch(diagnosticErrorMessage);
});

test('should throw errors with diagnostic codes', ({ context: { err } }) => {
expect((err as TSError).diagnosticCodes).toEqual([2345]);
});

test('should throw errors with complete diagnostic information', ({
context: { err },
}) => {
const diagnostics = (err as TSError).diagnostics;

expect(diagnostics).toHaveLength(1);
expect(diagnostics[0]).toMatchObject({
code: 2345,
start: 10,
length: 3,
messageText: expect.stringMatching(diagnosticMessage),
});
});
});
2 changes: 2 additions & 0 deletions src/test/helpers.ts
Expand Up @@ -30,6 +30,8 @@ export const BIN_SCRIPT_PATH = join(
);
export const BIN_CWD_PATH = join(TEST_DIR, 'node_modules/.bin/ts-node-cwd');
export const BIN_ESM_PATH = join(TEST_DIR, 'node_modules/.bin/ts-node-esm');

process.chdir(TEST_DIR);
//#endregion

//#region command lines
Expand Down
3 changes: 2 additions & 1 deletion src/test/pluggable-dep-resolution.spec.ts
Expand Up @@ -2,6 +2,7 @@ import { context } from './testlib';
import {
contextTsNodeUnderTest,
resetNodeEnvironment,
TEST_DIR,
tsSupportsTsconfigInheritanceViaNodePackages,
} from './helpers';
import * as expect from 'expect';
Expand Down Expand Up @@ -87,7 +88,7 @@ test.suite(

const output = t.context.tsNodeUnderTest
.create({
project: resolve('tests/pluggable-dep-resolution', config),
project: resolve(TEST_DIR, 'pluggable-dep-resolution', config),
})
.compile('', 'index.ts');

Expand Down
7 changes: 3 additions & 4 deletions src/test/repl/repl-environment.spec.ts
Expand Up @@ -137,7 +137,6 @@ test.suite(

/** Every possible ./node_modules directory ascending upwards starting with ./tests/node_modules */
const modulePaths = createModulePaths(TEST_DIR);
const rootModulePaths = createModulePaths(ROOT_DIR);
function createModulePaths(dir: string) {
const modulePaths: string[] = [];
for (let path = dir; ; path = dirname(path)) {
Expand Down Expand Up @@ -430,7 +429,7 @@ test.suite(

// Note: vanilla node uses different name. See #1360
stackTest: expect.stringContaining(
` at ${join(ROOT_DIR, '<repl>.ts')}:1:`
` at ${join(TEST_DIR, '<repl>.ts')}:1:`
),
},
});
Expand All @@ -455,13 +454,13 @@ test.suite(
modulePath: '.',
moduleFilename: null,
modulePaths: expect.objectContaining({
...[join(ROOT_DIR, `repl/node_modules`), ...rootModulePaths],
...[join(TEST_DIR, `repl/node_modules`), ...modulePaths],
}),
// Note: vanilla node REPL does not set exports
exportsTest: true,
// Note: vanilla node uses different name. See #1360
stackTest: expect.stringContaining(
` at ${join(ROOT_DIR, '<repl>.ts')}:1:`
` at ${join(TEST_DIR, '<repl>.ts')}:1:`
),
moduleAccessorsTest: true,
},
Expand Down
4 changes: 2 additions & 2 deletions src/test/repl/repl.spec.ts
Expand Up @@ -246,13 +246,13 @@ test.suite('top level await', (_test) => {
'should error with typing information when importing a file with type errors',
async (t) => {
const { stdout, stderr } = await t.context.executeInTlaRepl(
`const {foo} = await import('./tests/repl/tla-import');`,
`const {foo} = await import('./repl/tla-import');`,
'error'
);

expect(stdout).toBe('> > ');
expect(stderr.replace(/\r\n/g, '\n')).toBe(
'tests/repl/tla-import.ts(1,14): error TS2322: ' +
'repl/tla-import.ts(1,14): error TS2322: ' +
(semver.gte(ts.version, '4.0.0')
? `Type 'number' is not assignable to type 'string'.\n`
: `Type '1' is not assignable to type 'string'.\n`) +
Expand Down
3 changes: 3 additions & 0 deletions src/test/testlib.ts
Expand Up @@ -15,6 +15,9 @@ import * as expect from 'expect';

export { ExecutionContext, expect };

// HACK ensure ts-node-specific bootstrapping is executed
import './helpers';

// NOTE: this limits concurrency within a single process, but AVA launches
// each .spec file in its own process, so actual concurrency is higher.
const concurrencyLimiter = throat(16);
Expand Down