Skip to content

Commit

Permalink
feat: support ||s and type normalization for comparisons (#634)
Browse files Browse the repository at this point in the history
## PR Checklist

- [x] Addresses an existing open issue: fixes #108; fixes #109
- [x] That issue was marked as [`status: accepting
prs`](https://github.com/JoshuaKGoldberg/eslint-plugin-expect-type/issues?q=is%3Aopen+is%3Aissue+label%3A%22status%3A+accepting+prs%22)
- [x] Steps in
[CONTRIBUTING.md](https://github.com/JoshuaKGoldberg/eslint-plugin-expect-type/blob/main/.github/CONTRIBUTING.md)
were taken

## Overview

#108: Adopts the strategy used in DefinitelyTyped-tools of printing
types into a full in-memory TypeScript source file. Union types like
`string | number` and `number | string` that mean the same thing but are
in a different order will finally be considered compatible! 🥳

#109: Also adds "candidate" checking to support `||` types, while I'm in
the area. `$ExpectType 0 || number` will now match `0` and `number`
nicely.

_Also_ adds more test coverage around ordering, parenthesis, and
readonly arrays.

💖
  • Loading branch information
JoshuaKGoldberg authored Nov 28, 2024
1 parent f750672 commit 8724da8
Show file tree
Hide file tree
Showing 7 changed files with 238 additions and 70 deletions.
35 changes: 28 additions & 7 deletions src/failures/getExpectTypeFailures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import {
getLanguageServiceHost,
getNodeForExpectType,
matchModuloWhitespace,
matchReadonlyArray,
} from "../utils/typescript.js";
import { normalizedTypeToString } from "./normalizedTypeToString.js";
import { ExpectTypeFailures, UnmetExpectation } from "./types.js";

export function getExpectTypeFailures(
Expand Down Expand Up @@ -45,19 +45,40 @@ export function getExpectTypeFailures(
ts.TypeFormatFlags.NoTruncation,
);

if (
!expected ||
(actual !== expected && !matchReadonlyArray(actual, expected))
) {
typeAssertions.delete(line);

const candidates = expected
?.split(/\s*\|\|\s*/)
.map((s) => s.trim())
.filter(Boolean);

if (!candidates || !candidateTypeMatches(actual, candidates)) {
unmetExpectations.push({ actual, assertion, node });
}

typeAssertions.delete(line);
}

ts.forEachChild(node, iterate);
});

function candidateTypeMatches(actual: string, candidates: string[]) {
let actualNormalized: string | undefined;

for (const candidate of candidates) {
if (candidate === actual) {
return true;
}

actualNormalized ??= normalizedTypeToString(actual);
const candidateNormalized = normalizedTypeToString(candidate);

if (actualNormalized === candidateNormalized) {
return true;
}
}

return false;
}

const twoSlashFailureLines: number[] = [];
if (twoSlashAssertions.length) {
for (const assertion of twoSlashAssertions) {
Expand Down
63 changes: 63 additions & 0 deletions src/failures/normalizedTypeToString.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Code based on DefinitelyTyped-tools implementation:
// https://github.com/microsoft/DefinitelyTyped-tools/blob/42484ff245f6f18018de729f12c9a28436daa08a/packages/eslint-plugin/src/rules/expect.ts#L466

import ts from "typescript";

export function normalizedTypeToString(type: string) {
const sourceFile = ts.createSourceFile(
"foo.ts",
`declare var x: ${type};`,
ts.ScriptTarget.Latest,
);
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const typeNode = (sourceFile.statements[0] as ts.VariableStatement)
.declarationList.declarations[0].type!;

const printer = ts.createPrinter();
function print(node: ts.Node) {
return printer.printNode(ts.EmitHint.Unspecified, node, sourceFile);
}

// TODO: pass undefined instead once all supported TS versions support it per:
// https://github.com/microsoft/TypeScript/pull/52941
const context = ts.nullTransformationContext;

function visit(node: ts.Node) {
node = ts.visitEachChild(node, visit, context);

if (ts.isUnionTypeNode(node)) {
const types = node.types
.map((t) => [t, print(t)] as const)
.sort((a, b) => (a[1] < b[1] ? -1 : 1))
.map((t) => t[0]);
return ts.factory.updateUnionTypeNode(
node,
ts.factory.createNodeArray(types),
);
}

if (
ts.isTypeOperatorNode(node) &&
node.operator === ts.SyntaxKind.ReadonlyKeyword &&
ts.isArrayTypeNode(node.type)
) {
// It's possible that this would conflict with a library which defines their own type with this name,
// but that's unlikely (and was not previously handled in a prior revision of type string normalization).
return ts.factory.createTypeReferenceNode("ReadonlyArray", [
skipTypeParentheses(node.type.elementType),
]);
}

return node;
}

const visited = visit(typeNode);
return print(visited);
}

function skipTypeParentheses(node: ts.TypeNode): ts.TypeNode {
while (ts.isParenthesizedTypeNode(node)) {
node = node.type;
}
return node;
}
38 changes: 36 additions & 2 deletions src/rules/expect-type-snapshot.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ ruleTester.run("expect", expect, {
filename,
options: [{ disableExpectTypeSnapshotFix: true }],
},
// Snapshot has different type.
{
code: dedent`
// $ExpectTypeSnapshot TypeSnapshotDoNotMatch
Expand All @@ -64,17 +63,52 @@ ruleTester.run("expect", expect, {
},
],
filename,
name: "Snapshot has different type",
options: [{ disableExpectTypeSnapshotFix: true }],
},
{
code: dedent`
// $ExpectTypeSnapshot TypeSnapshotDoNotMatchOr
const configB = { d: 0 };
`,
errors: [
{
column: 1,
line: 2,
messageId: "TypeSnapshotDoNotMatch",
},
],
filename,
name: "Snapshot has different type than ||",
options: [{ disableExpectTypeSnapshotFix: true }],
},
],
valid: [
// Snapshot matches.
{
code: dedent`
// $ExpectTypeSnapshot SnapshotMatches
const c = { a: 15, b: "b" as const, c: "c" };
`,
filename,
name: "Snapshot matches",
options: [{ disableExpectTypeSnapshotFix: true }],
},
{
code: dedent`
// $ExpectTypeSnapshot SnapshotMatchesOr
const c = { a: 15, b: "b" as const, c: "c" };
`,
filename,
name: "Snapshot matches first || constituent",
options: [{ disableExpectTypeSnapshotFix: true }],
},
{
code: dedent`
// $ExpectTypeSnapshot SnapshotMatchesOr
const c = { d: true };
`,
filename,
name: "Snapshot matches second || constituent",
options: [{ disableExpectTypeSnapshotFix: true }],
},
],
Expand Down
107 changes: 103 additions & 4 deletions src/rules/expect-type.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ ruleTester.run("expect", expect, {
code: dedent`
// $ExpectType number
const t = 'a';
`,
np `,
errors: [
{
column: 1,
Expand Down Expand Up @@ -38,6 +38,20 @@ ruleTester.run("expect", expect, {
code: dedent`
// $ExpectType { a: number; b: "on"; }
const t = { b: 'on' as const, a: 17 };
`,
errors: [
{
column: 1,
line: 2,
messageId: "TypesDoNotMatch",
},
],
filename,
},
{
code: dedent`
// $ExpectType number || string;
const t = false;
`,
errors: [
{
Expand All @@ -50,23 +64,86 @@ ruleTester.run("expect", expect, {
},
],
valid: [
// Primitive type
{
code: dedent`
// $ExpectType number
const t = 6 as number;
`,
filename,
name: "Primitive type",
},
{
code: dedent`
// $ExpectType number
const t = 6 as (number);
`,
filename,
name: "Parenthesized primitive type",
},
{
code: dedent`
// $ExpectType number | string
const t = 6 as number | string;
`,
filename,
name: "Union of primitive types, in order",
},
{
code: dedent`
// $ExpectType string | number
const t = 6 as number | string;
`,
filename,
name: "Union of primitive types, out of order",
},
{
code: dedent`
// $ExpectType string[]
const t = [] as string[]
`,
filename,
name: "readonly array",
},
{
code: dedent`
// $ExpectType readonly string[]
const t = [] as readonly string[]
`,
filename,
name: "readonly array",
},
{
code: dedent`
// $ExpectType readonly string[]
const t = [] as ReadonlyArray<string>
`,
filename,
name: "readonly array and ReadonlyArray",
},
{
code: dedent`
// $ExpectType ReadonlyArray<string>
const t = [] as readonly string[]
`,
filename,
name: "ReadonlyArray and readonly array",
},
{
code: dedent`
// $ExpectType ReadonlyArray<string>
const t = [] as ReadonlyArray<string>
`,
filename,
name: "ReadonlyArray",
},
// Complex type
{
code: dedent`
// $ExpectType { a: number; b: "on"; }
const t = { a: 17, b: 'on' as const };
`,
filename,
name: "Complex type",
},
// Ignored TypeScript compiler complaints
{
code: dedent`
function hasUnusedParam(unusedParam: number, implicitAnyParam) {
Expand All @@ -75,6 +152,28 @@ ruleTester.run("expect", expect, {
}
`,
filename,
name: "Ignored TypeScript compiler complaints",
},
{
code: dedent`
// $ExpectType number || string
const t = 6 as number;
`,
filename,
},
{
code: dedent`
// $ExpectType number || string | number
const t = 6 as number;
`,
filename,
},
{
code: dedent`
// $ExpectType string | number || number
const t = 6 as number;
`,
filename,
},
],
});
4 changes: 3 additions & 1 deletion src/rules/sandbox/__type-snapshots__/file.ts.snap.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
{
"SnapshotMatches": "{ a: number; b: \"b\"; c: string; }",
"TypeSnapshotDoNotMatch": "{ a: number; b: \"b\"; c: boolean; }"
"SnapshotMatchesOr": "{ a: number; b: \"b\"; c: string; } || { d: boolean }",
"TypeSnapshotDoNotMatch": "{ a: number; b: \"b\"; c: boolean; }",
"TypeSnapshotDoNotMatchOr": "{ a: number; b: \"b\"; c: boolean; } { d: boolean }"
}
5 changes: 5 additions & 0 deletions src/types/typescript.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { TransformationContext } from "typescript";

declare module "typescript" {
export const nullTransformationContext: TransformationContext;
}
Loading

0 comments on commit 8724da8

Please sign in to comment.