From 998b31e410c1a454da0374d1a5dce03aaff55a1a Mon Sep 17 00:00:00 2001 From: Dimitri B Date: Mon, 24 May 2021 00:57:15 +0200 Subject: [PATCH 1/2] Handle multiple diagnostic errors in a single `expectError` assertion Fixes #87 --- source/lib/compiler.ts | 35 ++++++++++++++----- source/lib/parser.ts | 8 +++-- .../expect-error/functions/index.d.ts | 7 ++++ .../fixtures/expect-error/functions/index.js | 6 ++-- .../expect-error/functions/index.test-d.ts | 5 ++- 5 files changed, 46 insertions(+), 15 deletions(-) diff --git a/source/lib/compiler.ts b/source/lib/compiler.ts index e456e9ef..cd786bb9 100644 --- a/source/lib/compiler.ts +++ b/source/lib/compiler.ts @@ -4,7 +4,7 @@ import { Diagnostic as TSDiagnostic, SourceFile } from '@tsd/typescript'; -import {extractAssertions, parseErrorAssertionToLocation} from './parser'; +import {ExpectedError, extractAssertions, parseErrorAssertionToLocation} from './parser'; import {Diagnostic, DiagnosticCode, Context, Location} from './interfaces'; import {handle} from './assertions'; @@ -28,6 +28,8 @@ const expectErrordiagnosticCodesToIgnore = new Set([ DiagnosticCode.PropertyMissingInType1ButRequiredInType2 ]); +type IgnoreDiagnosticResult = 'preserve' | 'ignore' | Location; + /** * Check if the provided diagnostic should be ignored. * @@ -35,14 +37,17 @@ const expectErrordiagnosticCodesToIgnore = new Set([ * @param expectedErrors - Map of the expected errors. * @returns Boolean indicating if the diagnostic should be ignored or not. */ -const ignoreDiagnostic = (diagnostic: TSDiagnostic, expectedErrors: Map): boolean => { +const ignoreDiagnostic = ( + diagnostic: TSDiagnostic, + expectedErrors: Map +): IgnoreDiagnosticResult => { if (ignoredDiagnostics.has(diagnostic.code)) { // Filter out diagnostics which are present in the `ignoredDiagnostics` set - return true; + return 'ignore'; } if (!expectErrordiagnosticCodesToIgnore.has(diagnostic.code)) { - return false; + return 'preserve'; } const diagnosticFileName = (diagnostic.file as SourceFile).fileName; @@ -51,13 +56,11 @@ const ignoreDiagnostic = (diagnostic: TSDiagnostic, expectedErrors: Map location.start && start < location.end) { - // Remove the expected error from the Map so it's not being reported as failure - expectedErrors.delete(location); - return true; + return location; } } - return false; + return 'preserve'; }; /** @@ -80,9 +83,19 @@ export const getDiagnostics = (context: Context): Diagnostic[] => { diagnostics.push(...handle(program.getTypeChecker(), assertions)); const expectedErrors = parseErrorAssertionToLocation(assertions); + const expectedErrorsLocationsWithFoundDiagnostics: Location[] = []; for (const diagnostic of tsDiagnostics) { - if (!diagnostic.file || ignoreDiagnostic(diagnostic, expectedErrors)) { + if (!diagnostic.file) { + continue; + } + + const ignoreDiagnosticResult = ignoreDiagnostic(diagnostic, expectedErrors); + if (ignoreDiagnosticResult !== 'preserve') { + if (ignoreDiagnosticResult !== 'ignore') { + expectedErrorsLocationsWithFoundDiagnostics.push(ignoreDiagnosticResult); + } + continue; } @@ -97,6 +110,10 @@ export const getDiagnostics = (context: Context): Diagnostic[] => { }); } + for (const errorLocationToRemove of expectedErrorsLocationsWithFoundDiagnostics) { + expectedErrors.delete(errorLocationToRemove); + } + for (const [, diagnostic] of expectedErrors) { diagnostics.push({ ...diagnostic, diff --git a/source/lib/parser.ts b/source/lib/parser.ts index cba29f77..24e4819c 100644 --- a/source/lib/parser.ts +++ b/source/lib/parser.ts @@ -42,15 +42,19 @@ export const extractAssertions = (program: Program): Map; + /** * Loop over all the error assertion nodes and convert them to a location map. * * @param assertions - Assertion map. */ -export const parseErrorAssertionToLocation = (assertions: Map>) => { +export const parseErrorAssertionToLocation = ( + assertions: Map> +): Map => { const nodes = assertions.get(Assertion.EXPECT_ERROR); - const expectedErrors = new Map>(); + const expectedErrors = new Map(); if (!nodes) { // Bail out if we don't have any error nodes diff --git a/source/test/fixtures/expect-error/functions/index.d.ts b/source/test/fixtures/expect-error/functions/index.d.ts index 89c68502..acd85899 100644 --- a/source/test/fixtures/expect-error/functions/index.d.ts +++ b/source/test/fixtures/expect-error/functions/index.d.ts @@ -5,3 +5,10 @@ declare const one: { }; export default one; + +export const three: { + (foo: '*'): string; + (foo: 'a' | 'b'): string; + (foo: ReadonlyArray<'a' | 'b'>): string; + (foo: never): string; +}; diff --git a/source/test/fixtures/expect-error/functions/index.js b/source/test/fixtures/expect-error/functions/index.js index f17717f5..d858d09d 100644 --- a/source/test/fixtures/expect-error/functions/index.js +++ b/source/test/fixtures/expect-error/functions/index.js @@ -1,3 +1,3 @@ -module.exports.default = (foo, bar) => { - return foo + bar; -}; +module.exports.default = (foo, bar) => foo + bar; + +exports.three = (foo) => 'a'; diff --git a/source/test/fixtures/expect-error/functions/index.test-d.ts b/source/test/fixtures/expect-error/functions/index.test-d.ts index 6d0ef4b3..3d4da8a3 100644 --- a/source/test/fixtures/expect-error/functions/index.test-d.ts +++ b/source/test/fixtures/expect-error/functions/index.test-d.ts @@ -1,5 +1,8 @@ import {expectError} from '../../../..'; -import one from '.'; +import one, {three} from '.'; expectError(one(true, true)); expectError(one('foo', 'bar')); + +// Produces multiple type checker errors in a single `expectError` assertion +expectError(three(['a', 'bad'])); From ba266e5779bf03ae1fd8d0aee6c9433f7755a63d Mon Sep 17 00:00:00 2001 From: Dimitri B Date: Wed, 26 May 2021 20:53:32 +0200 Subject: [PATCH 2/2] Address review comments --- source/lib/compiler.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/source/lib/compiler.ts b/source/lib/compiler.ts index cd786bb9..8f688821 100644 --- a/source/lib/compiler.ts +++ b/source/lib/compiler.ts @@ -35,7 +35,9 @@ type IgnoreDiagnosticResult = 'preserve' | 'ignore' | Location; * * @param diagnostic - The diagnostic to validate. * @param expectedErrors - Map of the expected errors. - * @returns Boolean indicating if the diagnostic should be ignored or not. + * @returns Whether the diagnostic should be `'preserve'`d, `'ignore'`d or, in case that + * the diagnostic is reported from inside of an `expectError` assertion, the `Location` + * of the assertion. */ const ignoreDiagnostic = ( diagnostic: TSDiagnostic, @@ -91,6 +93,7 @@ export const getDiagnostics = (context: Context): Diagnostic[] => { } const ignoreDiagnosticResult = ignoreDiagnostic(diagnostic, expectedErrors); + if (ignoreDiagnosticResult !== 'preserve') { if (ignoreDiagnosticResult !== 'ignore') { expectedErrorsLocationsWithFoundDiagnostics.push(ignoreDiagnosticResult);