Skip to content

Commit

Permalink
Add full diagnostics to tserror (#1706)
Browse files Browse the repository at this point in the history
* Add error locations to TSErrors

* Simplify tests / code formatting

* Expose the full TypeScript diagnostics on TSErrors

* Test re-org and deduplication

* lintfix

* fix

* bang head against wall

* lintfix

* fix

* fix

* fix

* fix

* fix

* tweaks

* Revert "tweaks"

This reverts commit 30b78bd.

* fix

* finally

* fix

Co-authored-by: Andrew Bradley <[email protected]>
  • Loading branch information
paulbrimicombe and cspotcode authored Apr 17, 2022
1 parent dc907bc commit deef947
Show file tree
Hide file tree
Showing 9 changed files with 108 additions and 14 deletions.
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
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,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 @@ -140,7 +140,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
Original file line number Diff line number Diff line change
Expand Up @@ -471,14 +471,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> = []
) {
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 @@ -830,7 +840,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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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

0 comments on commit deef947

Please sign in to comment.