Skip to content

Commit

Permalink
Add --show-diff CLI option (#165)
Browse files Browse the repository at this point in the history
Co-authored-by: Tommy <[email protected]>
  • Loading branch information
skarab42 and tommy-mitchell authored Mar 13, 2023
1 parent 621aad9 commit 8387d9e
Show file tree
Hide file tree
Showing 15 changed files with 289 additions and 21 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
"@tsd/typescript": "~4.9.5",
"eslint-formatter-pretty": "^4.1.0",
"globby": "^11.0.1",
"jest-diff": "^29.0.3",
"meow": "^9.0.0",
"path-exists": "^4.0.0",
"read-pkg-up": "^7.0.0"
Expand Down
17 changes: 10 additions & 7 deletions source/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@ const cli = meow(`
The given directory must contain a package.json and a typings file.
Info
--help Display help text
--version Display version info
--help Display help text
--version Display version info
Options
--typings -t Type definition file to test [Default: "types" property in package.json]
--files -f Glob of files to test [Default: '/path/test-d/**/*.test-d.ts' or '.tsx']
--typings -t Type definition file to test [Default: "types" property in package.json]
--files -f Glob of files to test [Default: '/path/test-d/**/*.test-d.ts' or '.tsx']
--show-diff Show type error diffs [Default: don't show]
Examples
$ tsd /path/to/project
Expand All @@ -37,21 +38,23 @@ const cli = meow(`
alias: 'f',
isMultiple: true,
},
showDiff: {
type: 'boolean',
},
},
});

(async () => {
try {
const cwd = cli.input.length > 0 ? cli.input[0] : process.cwd();
const typingsFile = cli.flags.typings;
const testFiles = cli.flags.files;
const {typings: typingsFile, files: testFiles, showDiff} = cli.flags;

const options = {cwd, typingsFile, testFiles};

const diagnostics = await tsd(options);

if (diagnostics.length > 0) {
throw new Error(formatter(diagnostics));
throw new Error(formatter(diagnostics, showDiff));
}
} catch (error: unknown) {
const potentialError = error as Error | undefined;
Expand Down
14 changes: 10 additions & 4 deletions source/lib/assertions/handlers/assignability.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {CallExpression, TypeChecker} from '@tsd/typescript';
import {Diagnostic} from '../../interfaces';
import {makeDiagnostic} from '../../utils';
import {makeDiagnosticWithDiff} from '../../utils';

/**
* Asserts that the argument of the assertion is not assignable to the generic type of the assertion.
Expand All @@ -24,13 +24,19 @@ export const isNotAssignable = (checker: TypeChecker, nodes: Set<CallExpression>

// Retrieve the type to be expected. This is the type inside the generic.
const expectedType = checker.getTypeFromTypeNode(node.typeArguments[0]);
const argumentType = checker.getTypeAtLocation(node.arguments[0]);
const receivedType = checker.getTypeAtLocation(node.arguments[0]);

if (checker.isTypeAssignableTo(argumentType, expectedType)) {
if (checker.isTypeAssignableTo(receivedType, expectedType)) {
/**
* The argument type is assignable to the expected type, we don't want this so add a diagnostic.
*/
diagnostics.push(makeDiagnostic(node, `Argument of type \`${checker.typeToString(argumentType)}\` is assignable to parameter of type \`${checker.typeToString(expectedType)}\`.`));
diagnostics.push(makeDiagnosticWithDiff({
message: 'Argument of type `{receivedType}` is assignable to parameter of type `{expectedType}`.',
expectedType,
receivedType,
checker,
node,
}));
}
}

Expand Down
26 changes: 19 additions & 7 deletions source/lib/assertions/handlers/identicality.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {CallExpression, TypeChecker, TypeFlags} from '@tsd/typescript';
import {Diagnostic} from '../../interfaces';
import {makeDiagnostic} from '../../utils';
import {makeDiagnostic, makeDiagnosticWithDiff} from '../../utils';

/**
* Asserts that the argument of the assertion is identical to the generic type of the assertion.
Expand All @@ -26,25 +26,37 @@ export const isIdentical = (checker: TypeChecker, nodes: Set<CallExpression>): D
const expectedType = checker.getTypeFromTypeNode(node.typeArguments[0]);

// Retrieve the argument type. This is the type to be checked.
const argumentType = checker.getTypeAtLocation(node.arguments[0]);
const receivedType = checker.getTypeAtLocation(node.arguments[0]);

if (!checker.isTypeAssignableTo(argumentType, expectedType)) {
if (!checker.isTypeAssignableTo(receivedType, expectedType)) {
// The argument type is not assignable to the expected type. TypeScript will catch this for us.
continue;
}

if (!checker.isTypeAssignableTo(expectedType, argumentType)) {
if (!checker.isTypeAssignableTo(expectedType, receivedType)) {
/**
* The expected type is not assignable to the argument type, but the argument type is
* assignable to the expected type. This means our type is too wide.
*/
diagnostics.push(makeDiagnostic(node, `Parameter type \`${checker.typeToString(expectedType)}\` is declared too wide for argument type \`${checker.typeToString(argumentType)}\`.`));
} else if (!checker.isTypeIdenticalTo(expectedType, argumentType)) {
diagnostics.push(makeDiagnosticWithDiff({
message: 'Parameter type `{expectedType}` is declared too wide for argument type `{receivedType}`.',
expectedType,
receivedType,
checker,
node,
}));
} else if (!checker.isTypeIdenticalTo(expectedType, receivedType)) {
/**
* The expected type and argument type are assignable in both directions. We still have to check
* if the types are identical. See https://github.com/Microsoft/TypeScript/blob/master/doc/spec.md#3.11.2.
*/
diagnostics.push(makeDiagnostic(node, `Parameter type \`${checker.typeToString(expectedType)}\` is not identical to argument type \`${checker.typeToString(argumentType)}\`.`));
diagnostics.push(makeDiagnosticWithDiff({
message: 'Parameter type `{expectedType}` is not identical to argument type `{receivedType}`.',
expectedType,
receivedType,
checker,
node,
}));
}
}

Expand Down
10 changes: 8 additions & 2 deletions source/lib/assertions/handlers/informational.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {CallExpression, TypeChecker, TypeFormatFlags} from '@tsd/typescript';
import {Diagnostic} from '../../interfaces';
import {makeDiagnostic, tsutils} from '../../utils';
import {makeDiagnostic, makeDiagnosticWithDiff, tsutils} from '../../utils';

/**
* Default formatting flags set by TS plus the {@link TypeFormatFlags.NoTruncation NoTruncation} flag.
Expand Down Expand Up @@ -80,7 +80,13 @@ export const expectDocCommentIncludes = (checker: TypeChecker, nodes: Set<CallEx
continue;
}

diagnostics.push(makeDiagnostic(node, `Documentation comment \`${docComment}\` for expression \`${expression}\` does not include expected \`${expectedDocComment}\`.`));
diagnostics.push(makeDiagnosticWithDiff({
message: `Documentation comment \`{receivedType}\` for expression \`${expression}\` does not include expected \`{expectedType}\`.`,
expectedType: expectedDocComment,
receivedType: docComment,
checker,
node,
}));
}

return diagnostics;
Expand Down
21 changes: 20 additions & 1 deletion source/lib/formatter.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,26 @@
import formatter from 'eslint-formatter-pretty';
import {Diagnostic} from './interfaces';
import {diffStringsUnified} from 'jest-diff';

interface FileWithDiagnostics {
filePath: string;
errorCount: number;
warningCount: number;
messages: Diagnostic[];
diff?: {
expected: string;
received: string;
};
}

/**
* Format the TypeScript diagnostics to a human readable output.
*
* @param diagnostics - List of TypeScript diagnostics.
* @param showDiff - Display difference between expected and received types.
* @returns Beautiful diagnostics output
*/
export default (diagnostics: Diagnostic[]): string => {
export default (diagnostics: Diagnostic[], showDiff = false): string => {
const fileMap = new Map<string, FileWithDiagnostics>();

for (const diagnostic of diagnostics) {
Expand All @@ -31,6 +37,19 @@ export default (diagnostics: Diagnostic[]): string => {
fileMap.set(diagnostic.fileName, entry);
}

if (showDiff && diagnostic.diff) {
let difference = diffStringsUnified(
diagnostic.diff.expected,
diagnostic.diff.received,
{omitAnnotationLines: true}
);

if (difference) {
difference = difference.split('\n').map(line => ` ${line}`).join('\n');
diagnostic.message = `${diagnostic.message}\n\n${difference}`;
}
}

entry.errorCount++;
entry.messages.push(diagnostic);
}
Expand Down
4 changes: 4 additions & 0 deletions source/lib/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ export interface Diagnostic {
severity: 'error' | 'warning';
line?: number;
column?: number;
diff?: {
expected: string;
received: string;
};
}

export interface Location {
Expand Down
2 changes: 2 additions & 0 deletions source/lib/utils/index.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import makeDiagnostic from './make-diagnostic';
import makeDiagnosticWithDiff from './make-diagnostic-with-diff';
import getJSONPropertyPosition from './get-json-property-position';
import * as tsutils from './typescript';

export {
getJSONPropertyPosition,
makeDiagnostic,
makeDiagnosticWithDiff,
tsutils
};
49 changes: 49 additions & 0 deletions source/lib/utils/make-diagnostic-with-diff.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import {Node, Type, TypeChecker, TypeFormatFlags} from '@tsd/typescript';
import {Diagnostic} from '../interfaces';

interface DiagnosticWithDiffOptions {
checker: TypeChecker;
node: Node;
message: string;
expectedType: Type | string;
receivedType: Type | string;
severity?: Diagnostic['severity'];
}

const typeToStringFormatFlags =
TypeFormatFlags.NoTruncation |
TypeFormatFlags.WriteArrayAsGenericType |
TypeFormatFlags.UseStructuralFallback |
TypeFormatFlags.UseAliasDefinedOutsideCurrentScope |
TypeFormatFlags.NoTypeReduction |
TypeFormatFlags.AllowUniqueESSymbolType |
TypeFormatFlags.InArrayType |
TypeFormatFlags.InElementType |
TypeFormatFlags.InFirstTypeArgument |
TypeFormatFlags.InTypeAlias;

/**
* Create a diagnostic with type error diffs from the given `options`, see {@link DiagnosticWithDiffOptions}.
*
* @param options - Options for creating the diagnostic.
* @returns Diagnostic with diffs
*/
export default (options: DiagnosticWithDiffOptions): Diagnostic => {
const {checker, node, expectedType, receivedType} = options;
const position = node.getSourceFile().getLineAndCharacterOfPosition(node.getStart());
const message = options.message
.replace('{expectedType}', typeof expectedType === 'string' ? expectedType : checker.typeToString(expectedType))
.replace('{receivedType}', typeof receivedType === 'string' ? receivedType : checker.typeToString(receivedType));

return {
fileName: node.getSourceFile().fileName,
message,
severity: options.severity ?? 'error',
line: position.line + 1,
column: position.character,
diff: {
expected: typeof expectedType === 'string' ? expectedType : checker.typeToString(expectedType, node, typeToStringFormatFlags),
received: typeof receivedType === 'string' ? receivedType : checker.typeToString(receivedType, node, typeToStringFormatFlags)
}
};
};
81 changes: 81 additions & 0 deletions source/test/diff.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import {verifyWithDiff} from './fixtures/utils';
import execa, {ExecaError} from 'execa';
import path from 'path';
import test from 'ava';
import tsd from '..';

test('diff', async t => {
const diagnostics = await tsd({cwd: path.join(__dirname, 'fixtures/diff')});

verifyWithDiff(t, diagnostics, [
[8, 0, 'error', 'Parameter type `{ life?: number | undefined; }` is declared too wide for argument type `{ life: number; }`.', {
expected: '{ life?: number | undefined; }',
received: '{ life: number; }',
}],
[9, 0, 'error', 'Parameter type `FooFunction` is not identical to argument type `() => number`.', {
expected: '(x?: string | undefined) => number',
received: '() => number',
}],
[10, 0, 'error', 'Parameter type `FooType` is declared too wide for argument type `Required<FooType>`.', {
expected: '{ foo?: "foo" | undefined; }',
received: '{ foo: "foo"; }',
}],
[11, 0, 'error', 'Parameter type `Partial<FooInterface>` is declared too wide for argument type `Required<FooInterface>`.', {
expected: '{ [x: string]: unknown; readonly protected?: boolean | undefined; fooType?: FooType | undefined; id?: "foo-interface" | undefined; }',
received: '{ [x: string]: unknown; readonly protected: boolean; fooType: FooType; id: "foo-interface"; }',
}],
[13, 0, 'error', 'Argument of type `{ life: number; }` is assignable to parameter of type `{ life?: number | undefined; }`.', {
expected: '{ life?: number | undefined; }',
received: '{ life: number; }',
}],
[18, 0, 'error', 'Documentation comment `This is a comment.` for expression `commented` does not include expected `This is not the same comment!`.', {
expected: 'This is not the same comment!',
received: 'This is a comment.',
}]
]);
});

test('diff cli', async t => {
const file = path.join(__dirname, 'fixtures/diff');

const {exitCode, stderr} = await t.throwsAsync<ExecaError>(execa('dist/cli.js', [file, '--show-diff']));

t.is(exitCode, 1);

const expectedLines = [
'✖ 8:0 Parameter type { life?: number | undefined; } is declared too wide for argument type { life: number; }.',
'',
'- { life?: number | undefined; }',
'+ { life: number; }',
'✖ 9:0 Parameter type FooFunction is not identical to argument type () => number.',
'',
'- (x?: string | undefined) => number',
'+ () => number',
'✖ 10:0 Parameter type FooType is declared too wide for argument type Required<FooType>.',
'',
'- { foo?: "foo" | undefined; }',
'+ { foo: "foo"; }',
'✖ 11:0 Parameter type Partial<FooInterface> is declared too wide for argument type Required<FooInterface>.',
'',
'- { [x: string]: unknown; readonly protected?: boolean | undefined; fooType?: FooType | undefined; id?: "foo-interface" | undefined; }',
'+ { [x: string]: unknown; readonly protected: boolean; fooType: FooType; id: "foo-interface"; }',
'✖ 13:0 Argument of type { life: number; } is assignable to parameter of type { life?: number | undefined; }.',
'',
'- { life?: number | undefined; }',
'+ { life: number; }',
'✖ 18:0 Documentation comment This is a comment. for expression commented does not include expected This is not the same comment!.',
'',
'- This is not the same comment!',
'+ This is a comment.',
'',
'6 errors'
];

// NOTE: If lines are added to the output in the future startLine and endLine should be adjusted.
const startLine = 2; // Skip tsd error message and file location.
const endLine = startLine + expectedLines.length; // Grab diff output only and skip stack trace.

const receivedLines = stderr.trim().split('\n').slice(startLine, endLine).map(line => line.trim());

t.deepEqual(receivedLines, expectedLines);
});
14 changes: 14 additions & 0 deletions source/test/fixtures/diff/index.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
export type FooType = {foo?: 'foo'};

export interface FooInterface {
[x: string]: unknown;
readonly protected: boolean;
fooType?: FooType;
id: 'foo-interface';
}

export type FooFunction = (x?: string) => number;

declare const foo: <T>(foo: T) => T;

export default foo;
3 changes: 3 additions & 0 deletions source/test/fixtures/diff/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports.default = (foo) => {
return foo;
};
18 changes: 18 additions & 0 deletions source/test/fixtures/diff/index.test-d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import {expectDocCommentIncludes, expectNotAssignable, expectType} from '../../..';
import foo, { FooFunction, FooInterface, FooType } from '.';

// Should pass
expectType<{life: number}>(foo({life: 42}));

// Should fail
expectType<{life?: number}>(foo({life: 42}));
expectType<FooFunction>(() => 42);
expectType<FooType>({} as Required<FooType>);
expectType<Partial<FooInterface>>({} as Required<FooInterface>);

expectNotAssignable<{life?: number}>(foo({life: 42}));

/** This is a comment. */
const commented = 42;

expectDocCommentIncludes<'This is not the same comment!'>(commented);
Loading

0 comments on commit 8387d9e

Please sign in to comment.