Skip to content

Commit

Permalink
Make expectType assertion strict
Browse files Browse the repository at this point in the history
  • Loading branch information
SamVerschueren committed Sep 26, 2019
1 parent b13533b commit e4c78c7
Show file tree
Hide file tree
Showing 18 changed files with 326 additions and 145 deletions.
4 changes: 4 additions & 0 deletions libraries/typescript/lib/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1995,6 +1995,10 @@ declare namespace ts {
getAugmentedPropertiesOfType(type: Type): Symbol[];
getRootSymbols(symbol: Symbol): ReadonlyArray<Symbol>;
getContextualType(node: Expression): Type | undefined;
/**
* Checks if type `a` is assignable to type `b`.
*/
isAssignableTo(a: Type, b: Type): boolean;
/**
* returns unknownSignature in the case of an error.
* returns undefined if the node is not valid.
Expand Down
1 change: 1 addition & 0 deletions libraries/typescript/lib/typescript.js
Original file line number Diff line number Diff line change
Expand Up @@ -32344,6 +32344,7 @@ var ts;
var parsed = ts.getParseTreeNode(node, ts.isFunctionLike);
return parsed ? isImplementationOfOverload(parsed) : undefined;
},
isAssignableTo: isTypeAssignableTo,
getImmediateAliasedSymbol: getImmediateAliasedSymbol,
getAliasedSymbol: resolveAlias,
getEmitResolver: getEmitResolver,
Expand Down
Binary file added media/screenshot.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added media/strict-assert.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@
"dist/index.js",
"dist/index.d.ts",
"dist/cli.js",
"dist/lib"
"dist/lib",
"libraries"
],
"keywords": [
"typescript",
Expand All @@ -54,6 +55,7 @@
"cpy-cli": "^2.0.0",
"del-cli": "^1.1.0",
"react": "^16.9.0",
"rxjs": "^6.5.3",
"tslint": "^5.11.0",
"tslint-xo": "^0.9.0",
"typescript": "^3.6.3"
Expand Down
18 changes: 17 additions & 1 deletion readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,23 @@ export default concat;

If we don't change the test file and we run the `tsd` command again, the test will fail.

<img src="screenshot.png" width="1330">
<img src="media/screenshot.png" width="1330">

### Strict type assertions

Type assertions are strict. This means that if you expect the type to be `string | number` but the argument is of type `string`, the tests will fail.

```ts
import {expectType} from 'tsd';
import concat from '.';

expectType<string>(concat('foo', 'bar'));
expectType<string | number>(concat('foo', 'bar'));
```

If we run `tsd`, we will notice that it reports an error because the `concat` method returns the type `string` and not `string | number`.

<img src="media/strict-assert.png" width="1330">

### Top-level `await`

Expand Down
Binary file removed screenshot.png
Binary file not shown.
128 changes: 104 additions & 24 deletions source/lib/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,24 @@ import * as path from 'path';
import {
flattenDiagnosticMessageText,
createProgram,
SyntaxKind,
Diagnostic as TSDiagnostic,
Program,
SourceFile,
Node,
forEachChild
forEachChild,
isCallExpression,
Identifier,
TypeChecker,
CallExpression
} from '../../libraries/typescript';
import {Diagnostic, DiagnosticCode, Context, Location} from './interfaces';

// List of diagnostic codes that should be ignored
// List of diagnostic codes that should be ignored in general
const ignoredDiagnostics = new Set<number>([
DiagnosticCode.AwaitIsOnlyAllowedInAsyncFunction
]);

// List of diagnostic codes which should be ignored inside `expectError` statements
const diagnosticCodesToIgnore = new Set<DiagnosticCode>([
DiagnosticCode.ArgumentTypeIsNotAssignableToParameterType,
DiagnosticCode.PropertyDoesNotExistOnType,
Expand All @@ -27,30 +31,23 @@ const diagnosticCodesToIgnore = new Set<DiagnosticCode>([
]);

/**
* Extract all the `expectError` statements and convert it to a range map.
* Extract all assertions.
*
* @param program - The TypeScript program.
* @param program - TypeScript program.
*/
const extractExpectErrorRanges = (program: Program) => {
const expectedErrors = new Map<Location, Pick<Diagnostic, 'fileName' | 'line' | 'column'>>();
const extractAssertions = (program: Program) => {
const typeAssertions = new Set<CallExpression>();
const errorAssertions = new Set<CallExpression>();

function walkNodes(node: Node) {
if (node.kind === SyntaxKind.ExpressionStatement && node.getText().startsWith('expectError')) {
const location = {
fileName: node.getSourceFile().fileName,
start: node.getStart(),
end: node.getEnd()
};

const pos = node
.getSourceFile()
.getLineAndCharacterOfPosition(node.getStart());

expectedErrors.set(location, {
fileName: location.fileName,
line: pos.line + 1,
column: pos.character
});
if (isCallExpression(node)) {
const text = (node.expression as Identifier).getText();

if (text === 'expectType') {
typeAssertions.add(node);
} else if (text === 'expectError') {
errorAssertions.add(node);
}
}

forEachChild(node, walkNodes);
Expand All @@ -60,9 +57,88 @@ const extractExpectErrorRanges = (program: Program) => {
walkNodes(sourceFile);
}

return {
typeAssertions,
errorAssertions
};
};

/**
* Loop over all the `expectError` nodes and convert them to a range map.
*
* @param nodes - Set of `expectError` nodes.
*/
const extractExpectErrorRanges = (nodes: Set<Node>) => {
const expectedErrors = new Map<Location, Pick<Diagnostic, 'fileName' | 'line' | 'column'>>();

// Iterate over the nodes and add the node range to the map
for (const node of nodes) {
const location = {
fileName: node.getSourceFile().fileName,
start: node.getStart(),
end: node.getEnd()
};

const pos = node
.getSourceFile()
.getLineAndCharacterOfPosition(node.getStart());

expectedErrors.set(location, {
fileName: location.fileName,
line: pos.line + 1,
column: pos.character
});
}

return expectedErrors;
};

/**
* Assert the expected type from `expectType` calls with the provided type in the argument.
* Returns a list of custom diagnostics.
*
* @param checker - The TypeScript type checker.
* @param nodes - The `expectType` AST nodes.
* @return List of custom diagnostics.
*/
const assertTypes = (checker: TypeChecker, nodes: Set<CallExpression>): Diagnostic[] => {
const diagnostics: Diagnostic[] = [];

for (const node of nodes) {
if (!node.typeArguments) {
// Skip if the node does not have generics
continue;
}

// 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]);

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

if (!checker.isAssignableTo(expectedType, argumentType)) { // tslint:disable-line:early-exit
/**
* At this point, 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.
*/
const position = node.getSourceFile().getLineAndCharacterOfPosition(node.getStart());

diagnostics.push({
fileName: node.getSourceFile().fileName,
message: `Parameter type \`${checker.typeToString(expectedType)}\` is declared too wide for argument type \`${checker.typeToString(argumentType)}\`.`,
severity: 'error',
line: position.line + 1,
column: position.character,
});
}
}

return diagnostics;
};

/**
* Check if the provided diagnostic should be ignored.
*
Expand Down Expand Up @@ -112,7 +188,11 @@ export const getDiagnostics = (context: Context): Diagnostic[] => {
.getSemanticDiagnostics()
.concat(program.getSyntacticDiagnostics());

const expectedErrors = extractExpectErrorRanges(program);
const {typeAssertions, errorAssertions} = extractAssertions(program);

const expectedErrors = extractExpectErrorRanges(errorAssertions);

result.push(...assertTypes(program.getTypeChecker(), typeAssertions));

for (const diagnostic of diagnostics) {
if (!diagnostic.file || ignoreDiagnostic(diagnostic, expectedErrors)) {
Expand Down
6 changes: 3 additions & 3 deletions source/test/fixtures/missing-import/index.test-d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@ import {LiteralUnion} from '.';

type Pet = LiteralUnion<'dog' | 'cat', string>;

expectType<Pet>('dog');
expectType<Pet>('cat');
expectType<Pet>('unicorn');
expectType<Pet>('dog' as Pet);
expectType<Pet>('cat' as Pet);
expectType<Pet>('unicorn' as Pet);
7 changes: 7 additions & 0 deletions source/test/fixtures/strict-types/loose/index.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
declare const one: {
(foo: string, bar: string): string;
(foo: number, bar: number): number;
<T>(): T;
};

export default one;
3 changes: 3 additions & 0 deletions source/test/fixtures/strict-types/loose/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports.default = (foo, bar) => {
return foo + bar;
};
30 changes: 30 additions & 0 deletions source/test/fixtures/strict-types/loose/index.test-d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import {Observable} from 'rxjs';
import {expectType} from '../../../..';
import one from '.';

expectType<string>('cat');

expectType<string | number>(one('foo', 'bar'));
expectType<string | number>(one(1, 2));

expectType<Date | string>(new Date('foo'));
expectType<Promise<number | string>>(new Promise<number>(resolve => resolve(1)));
expectType<Promise<number | string> | string>(new Promise<number | string>(resolve => resolve(1)));

expectType<Promise<string | number>>(Promise.resolve(1));

expectType<Observable<string | number>>(
one<Observable<string>>()
);

expectType<Observable<string | number> | Observable<string | number | boolean>>(
one<Observable<string | number> | Observable<string>>()
);

abstract class Foo<T> {
abstract unicorn(): T;
}

expectType<Foo<string | Foo<string | number>> | Foo<Date> | Foo<Symbol>>(
one<Foo<Date> | Foo<Symbol> | Foo<Foo<number> | string>>()
);
6 changes: 6 additions & 0 deletions source/test/fixtures/strict-types/loose/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "foo",
"dependencies": {
"rxjs": "^6.5.3"
}
}
7 changes: 7 additions & 0 deletions source/test/fixtures/strict-types/strict/index.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
declare const one: {
(foo: string, bar: string): string;
(foo: number, bar: number): number;
<T>(): T;
};

export default one;
3 changes: 3 additions & 0 deletions source/test/fixtures/strict-types/strict/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports.default = (foo, bar) => {
return foo + bar;
};
26 changes: 26 additions & 0 deletions source/test/fixtures/strict-types/strict/index.test-d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import {Observable} from 'rxjs';
import {expectType} from '../../../..';
import one from '.';

abstract class Foo<T> {
abstract unicorn(): T;
}

expectType<string>(one('foo', 'bar'));
expectType<number>(one(1, 2));

expectType<Date>(new Date('foo'));
expectType<Promise<number>>(new Promise<number>(resolve => resolve(1)));
expectType<Promise<number | string>>(new Promise<number | string>(resolve => resolve(1)));

expectType<Promise<number>>(Promise.resolve(1));

expectType<Observable<string>>(one<Observable<string>>());

expectType<Observable<string | number> | Observable<Date> | Observable<Symbol>>(
one<Observable<Date> | Observable<Symbol> | Observable<number | string>>()
);

expectType<Foo<string | Foo<string | number>> | Foo<Date> | Foo<Symbol>>(
one<Foo<Date> | Foo<Symbol> | Foo<Foo<number | string> | string>>()
);
6 changes: 6 additions & 0 deletions source/test/fixtures/strict-types/strict/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "foo",
"dependencies": {
"rxjs": "^6.5.3"
}
}
Loading

0 comments on commit e4c78c7

Please sign in to comment.