Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assertion functions don't work when defined as methods #36931

Closed
mlhaufe opened this issue Feb 21, 2020 · 36 comments
Closed

Assertion functions don't work when defined as methods #36931

mlhaufe opened this issue Feb 21, 2020 · 36 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@mlhaufe
Copy link

mlhaufe commented Feb 21, 2020

TypeScript Version:

3.9.0-dev.20200220

Search Terms:

assertion signature

Code

type Constructor<T> = new (...args: any[]) => T

let str: any = 'foo'

str.toUpperCase() // str is any

function assert(condition: unknown, message: string = 'Assertion failure', ErrorConstructor: Constructor<Error> = AssertionError): asserts condition {
    if(!condition) {
        throw new ErrorConstructor(message);
    }
}

assert(typeof str == 'string')

str.toUpperCase() // str is string

class AssertionError extends Error { }

class Assertion {
    assert(condition: unknown, message: string = 'Assertion failure', ErrorConstructor: Constructor<Error> = AssertionError): asserts condition {
        if(condition) {
            throw new ErrorConstructor(message);
        }
    }
}

let azzert = new Assertion().assert

let str2: any = 'bar'

azzert(typeof str2 == 'string')  //error 2775

str2.toUpperCase() // str2 is any

Expected behavior:

Assertion functions are usable as methods of a class

Actual behavior:

Assertions require every name in the call target to be declared with an explicit type annotation.(2775)
'azzert' needs an explicit type annotation.

Playground Link:

https://www.typescriptlang.org/play/?ts=3.9.0-dev.20200220&ssl=1&ssc=1&pln=33&pc=34#code/C4TwDgpgBAwg9gOwM7AE4FcDGw6oDwAqAfFALxQIQDuUAFAHSMCGqA5kgFxRMIgDaAXQCUZEgQCwAKCkAbCMCgpUXHiDJQA5ADM4cDVKlL6OAKphIqGEyQRaIgPT3FaKAEsk3Xgclb0CbK6I3Eg2qMC0mIgAJq7AgQhcfgDWCHBUCAA0UAC2ECFMrBBcSq4IrOoaAIIhEGHxUFpMrjLoqBAaWQCiqKi48MhoWDjKsIhKQ7h43b2oJOTVoXGI07hCKjVhHpEIMUsIUADeUlAnblq0AITbu-EiR5Knj1DAABa9NJQ0K5Zjg9i4tFy+UKQgA3MdTgBfKTQ6SSayLWigSBwLTOVBkcgaEplDRCbxGUzmWpWGx2KCOdFuDw41jeTAyBFQBa1PbfKAQAAewAgOw87IOUFhUgZTJZdSC90eCNZEWisXiiQQKTSmRyeSQBSK6NK5Sx4r2DSaLTaHSg33643+I0tf2GUx6uDmzI2bMdqDWwUWW3lhqlTxOrnO1wViDuEIDj1e7wo1HN7ttGGtgI1WrBEaesMesOFkjkCiYAC9C6z1J8XYt4nZ6DKwt58+iAEwqXgVABGLH0cKLJbCSPAEFRTcxmlpeIp9lqMygjYA7LOAKwEtCN4xwMwWUm2BxOJSN6meEBAA

Related Issues:

Perhaps this a design limitation per the following PR?

#33622
#33622 (comment)

@RyanCavanaugh
Copy link
Member

Assertion functions always need explicit annotations. You can write this:

let azzert: Assertion['assert'] = new Assertion().assert;

@RyanCavanaugh RyanCavanaugh added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Feb 21, 2020
@justinmchase
Copy link

@RyanCavanaugh Can you explain in a little more detail? I don't understand how to resolve the issue.

This also is failing with the same error:

import * as assert from "assert";
export function exists(value: unknown): asserts value {
  assert(value);
}

export default {
  exists
};

Using:

import assert from "./assert";

assert.exists(true); // error ts(2775)

How does the exists function not have an explicit annotation? Or what can I do to resolve it in this case.

If I just export the function as non-default and do the import as import { exists } from "./assert" then it works but I'd like to have them grouped on the object assert.xyz.

@ghost
Copy link

ghost commented May 6, 2020

I am also seeing error ts(2775) when using require instead of import

Following has error:

const assert = require('assert')

Following has no error

import assert from 'assert'

Troubleshoot Info

> tsc --version
Version 3.8.3

@mlhaufe
Copy link
Author

mlhaufe commented May 6, 2020

@justinmchase , @amber-pli

Create a let/const/var and assign it the assert function you imported. Give that variable an explicit type like in the example provided by @RyanCavanaugh .

@justinmchase
Copy link

justinmchase commented May 6, 2020

I guess whats confusing me is that he said:

Assertion functions always need explicit annotations

But I believe in my code the function does have an explicit type annotation, yet its not working. Are we saying there is a bug and that the variable assigned to the function needs the explicit type as well?

@borisyordanov
Copy link

@justinmchase The type needs to be explicit, it's not a bug
#34523
#34596

@justinmchase
Copy link

Ok, one last time, I swear I'm not being intentionally obtuse but I think I'm homing in.

I think my confusion is because, to my understanding, this function has an explicit type:

export function exists(value: unknown): asserts value {
  assert(value);
}

So when you say the type needs to be explicit its boggling my mind because I don't get how it couldn't be explicit.

But now, I'm wondering if you don't mean the function's type but the type of the argument needs to be explicit? As in the value: unknown is the issue? It needs to be non-generic and not something such as any or unknown? Is that what you mean?

@mlhaufe
Copy link
Author

mlhaufe commented May 7, 2020

@justinmchase explicitly give it a type where you imported it.

@jasonk
Copy link

jasonk commented May 25, 2020

@justinmchase I think the part that is boggling your mind is that the message isn't clear about what exactly doesn't have an explicit type. You quoted this code, which clearly does have an explicit type:

export function exists(value: unknown): asserts value {
  assert(value);
}

But when you used it, you imported it like this:

import assert from "./assert";

assert.exists(true); // error ts(2775)

The 2775 error you are getting on that line is not about exists needing an explicit type, it's about assert needing one. You can verify this by importing exists directly:

import { exists } from "./assert";
exists( true ); // works without the error.

So the error isn't telling you about problems with exists, it's telling you that your default export needs to be explicitly typed:

const defaults: {
  exists( value: unknown ): asserts value;
} = {
  exists,
};

export default defaults;

It isn't entirely clear to me why that is the case, and it seems that making the error message clearer about where exactly the problem lies was discussed and rejected.

What I have found can work to avoid having to repeat the typings twice is to move the actual assertion functions into a separate file and then re-export them:

export * from './real-assertions';
import * as assertions from './real-assertions';
export default assertions;

I'm also not entirely clear on why this works, but it seems like it does..

@justinmchase
Copy link

Ok thanks, I'll try it out!

It does seem like there is a bug in here but I'll settle for work arounds for now.

@chriskrycho
Copy link

As a prime example where you would expect this to work, but it doesn't—in a way that seems to me at least to pretty clearly be a bug—is namespaces.

For backwards compatibility, Ember's types have both modern module imports and re-exports from the Ember namespace. So we have:

export function assert(desc: string): never;
export function assert(desc: string, test: unknown): asserts test;
import { assert } from '@ember/debug';
assert('this works', typeof 'hello' === 'string');

And we also re-export that on the Ember namespace, with a re-export like this:

import * as EmberDebugNs from '@ember/debug';

export namespace Ember {
    // ...
    const assert: typeof EmberDebugNs.assert;
    // ...
}
const { assert } = Ember;
assert('this FAILS', typeof 'hello' === 'string');

'assert' needs an explicit type annotation.

I understand the constraints as specified, and I'm afraid I must disagree with the team's conclusion about this error message

We discussed in the team room and agreed that the existing message is fine, so I didn't change it.

—seeing as it took me about half an hour to finally find my way to this thread and understand the problem, and another half an hour to figure out how to rewrite our DefinitelyTyped tests to continue handling this! 😅

For the record, if anyone else is curious, this is how I'm ending up working around it for the types PR I'll be pushing up shortly:

const assertDescOnly: (desc: string) => never = Ember.assert;
const assertFull: (desc: string, test: unknown) => asserts test = Ember.assert;

(I'm also going to have to document that or be ready to explain it, though gladly basically none of our TS users are likely to be doing this.)

@kael-shipman
Copy link

I tried to include an assertion function as part of the exports for an inner module in one of my libraries and was heartbroken to find that, while I could use it perfectly within the module, I could not use it when referencing it from outside via the module namespace. @RyanCavanaugh's answer was a godsend!! Thank you so much!

Here's what I was doing:

Project1::src/Http.ts

export function assertThing(thing: any): asserts thing is string {
  // ...
}

Project1::src/index.ts

import * as Http from "./Http";
// ....
export { Http, /* .... and other things .... */ }

Project2::src/Functions.ts

import { Http } from "project1";

export const handler = (thing: any) => {
  Http.assertThing(thing);
  // ....
}

I was getting the error in the original post because of this, and I couldn't get past it. I successfully utilized Ryan's solution by doing this:

Project2::src/Functions.ts

import { Http } from "project1";

const assertThing: (typeof Http)["assertThing"] = Http.assertThing;

export const handler = (thing: any) => {
  assertThing(thing);
  // ....
}

While a little awkward, this is a total life-saver and has allowed me to successfully use this wonderful assertion functionality without limitation. (Note that it was necessary to use the (typeof Http) notation because typescript complained that namespaces can't be used as types.)

Anyway, just wanted to add this for anyone who may be struggling with something similar. Thanks again for the tip, @RyanCavanaugh!

jasonwilliams added a commit to styled-components/vscode-styled-components that referenced this issue Oct 6, 2020
* jest is no longer needed
* remove ts-check for now until microsoft/TypeScript#36931 or we migrate tests to typescript
@dschnare
Copy link

For what it's worth this has made the Assert library usable again when checking JavaScript with Typescript.

const Assert = /** @type {typeof import('assert')} */(require('assert'))

All the type errors reported about non-explicit types are gone (thankfully).

@leandro-manifesto
Copy link

I'm using Typescript 4.1.5 and it baffles me that this error even exists and that the message is so cryptic you have no idea what's going on.

Here is a sample code:

class StringEnum<T extends string> {
	public constructor(
		private readonly values: readonly T[],
	) { }

	public is(value: string): value is T {
		return (this.values as readonly string[]).includes(value);
	}

	public assert(value: string): asserts value is T {
		if (!this.is(value)) {
			throw new Error('invalid string enum variant');
		}
	}
}

const MyEnum = new StringEnum([
	'sample1',
]);

const value = 'sample1';

if (MyEnum.is(value)) { } // works

MyEnum.assert(value); // error

TS Playground

From my understanding, the signature of MyEnum.assert should be (this: StringEnum<'sample1'>, value: string) => asserts value is 'sample1' and that should be enough for the control flow analysis to know that after its call value should be a valid variant.

Also, the fact that .is(value) works and .assert(value) doesn't adds to the confusion.

Since this is a design limitation, it should be forbidden to use asserts in methods in the first place.

@mlhaufe
Copy link
Author

mlhaufe commented Feb 22, 2021

The error goes away if you're explicit:

const MyEnum: StringEnum<string> = new StringEnum([
	'sample1',
]);

@leandro-manifesto
Copy link

Unfortunately that defeats the purpose of the generic argument, which is to keep the valid variant types around.

@leandro-manifesto
Copy link

Now I'm pretty sure #34596 is about this exact problem too.

@georeith
Copy link

georeith commented Mar 15, 2021

type AssertsIsOneOfType = (
  item: Item,
  types: ItemClass[],
  prop: string
) => void;

const assertIsOneOfType: AssertsIsOneOfType = <T extends ItemClass[]>(
  item: Item,
  types: T,
  prop: string
): asserts item is Item & { className: T[number] } => {
  if (!types.some((type) => isOfType(item, type))) {
    throw new Error(`Cannot apply ${prop} to ${item.className}`);
  }
};

Do I really have to define all the types of this function twice just because I want to use a generic in my assertion?

If you don't const assertIsOneOfType: AssertsIsOneOfType like this then I get the:

Assertions require every name in the call target to be declared with an explicit type annotation.ts(2775)
issue.

Edit: Even then its not narrowing the type like it would in a regular boolean style type guard. But this works:

const _assertIsOneOfType = <T extends ItemClass[]>(
  item: Item,
  types: T,
  prop: string
): asserts item is Item & { className: T[number] } => {
  if (!types.some((type) => isOfType(item, type))) {
    throw new Error(`Cannot apply ${prop} to ${item.className}`);
  }
};

const assertIsOneOfType: typeof _assertIsOneOfType = _assertIsOneOfType;

Edit: I see its an issue with the const if I just declare it is a function I can type it inline:

function assertIsOneOfType<T extends ItemClass[]>(
  item: Item,
  types: T,
  prop: string
): asserts item is Item & { className: T[number] } {
  if (!types.some((type) => isOfType(item, type))) {
    throw new Error(`Cannot apply ${prop} to ${item.className}`);
  }
}

@davidgilbertson
Copy link

With the goal of being able to write assert.exists(condition) - after much faffing about - I've found the following combinations of named exports, default exports, consts and functions to work. Maybe useful to future travellers...

// assert1.ts
export const exists1: (condition: any, message?: string) => asserts condition = (condition, message) => {
  if (!condition) throw new Error(message || 'An error occurred');
};

export function exists2(condition: any, message?: string): asserts condition {
  if (!condition) throw new Error(message || 'An error occurred');
}
// elsewhere
import * as assert1 from './assert1';

assert1.exists1(something);
assert1.exists2(something);

Or if you don't want to import *, this affront to humanity:

// assert2.ts
const exists1: (condition: any, message?: string) => asserts condition = (condition, message) => {
  if (!condition) throw new Error(message || 'An error occurred');
};

function exists2(condition: any, message?: string): asserts condition {
  if (!condition) throw new Error(message || 'An error occurred');
}

const myDefault: {exists1: typeof exists1; exists2: typeof exists2} = {exists1, exists2};

export default myDefault;
// elsewhere
import assert2 from './assert2';

assert2.exists1(something);
assert2.exists2(something);

With the default export approach, IntelliJ will do auto-imports properly.

@MatthiasKunnen
Copy link

What about instance functions that assert something about this? Are they also meant to result into errors?

class Device {
    id: string;
    settingsId?: string | null;

    assertHasSettings(): asserts this is {settingsId: string} & this {
        if (this.settingsId == null) {
            throw new Error(`Device with ID ${this.id}`
                + ` does not have individual settings`);
        }
    }
}

const device = new Device();
device.id = 'hello';
device.assertHasSettings();

TypeScript playground: https://www.typescriptlang.org/play?strictPropertyInitialization=false&ssl=15&ssc=28&pln=1&pc=1#code/MYGwhgzhAEAiCmA3Alse0DeAoavrIBMAuaCAFwCdkA7AcwG4c8J4yybaIBJAgfhPJU60AD7RqAVxAhGTXJBYUyACUgBlVuzoQAFAEoSC+EphkAFshiXMLNh27FSlDgF9oAMmjnr2PH-wAZtA63hAAdLZanDzQALyx4lIgephy-n7mFAD2AO7i8HkAohTZFDoABggoaNA5yObQXLDQACQYoWGELuVp6ekA1NDl0ARZ8DDUWWTQZmCI6DQEyCgEEmAgpJr25XqMfdAuaYeHWMBZ1OQjSKjoCdQFcNdo+owET-CdBHHQAORm8NIsj9Xu8wkYlKoIBo7NoXkA

@btmorex
Copy link

btmorex commented May 21, 2021

Without being too repetitive, the following works for me:

type Assert = (condition: unknown, message?: string) => asserts condition;

const assert: Assert = (condition, message) => {
  if (!condition) {
    throw new Error(message);
  }
};

export default assert;

@tassoevan
Copy link

tassoevan commented Oct 8, 2021

I wonder the reasons for it. I think the compiler infers a function/calling signature type rather than an "assertion function type". It explains stuff like this:

function assertThing(thing: any): asserts thing is string {
  // ...
}

const assertStuff = assertThing; // the constant's type is inferred as `(thing: any) => void`

@conartist6
Copy link

Note that if you need method overloading semantics your workaround will need to create an interface:

interface InvariantStatic {
  (condition: false, reason: string): never;
  (condition: any, reason: string): asserts condition;
}

declare let invariant: InvariantStatic;

@pie6k
Copy link

pie6k commented Jan 31, 2022

My use case when it breaks:

scoped 'logging' utility that adds prefix to logs or asserts simply to save me keystrokes and improve dev-experience when debugging or analyzing complex code scenarios

Example:

const scope = createScope("Foo");

scope.assert(false, "Nope"); // <— expected: throws error `[Foo] Nope` (saves me `[Foo] ` keystrokes (potentially x20 times in some examples)
scope.log("hello"); // <— expected: logs "[Foo] hello"
// etc.

Thus my assert is a method 'by-design' and I want it to be this way.

But I cannot use that due to mentioned TS error:

scope.assert(false, "Foo"); // TS: Assertions require every name in the call target to be declared with an explicit type annotation.
// Same for
const { assert } = scope
assert(false, "Foo");
// or
const assert = scope;
assert(false, "Foo");

The only way to make it work is:

const assert: (input: unknown, message: string) => asserts input = scope.assert
assert(false, "Foo"); // TS does not complain and works

// or
type Assert = (input: unknown, message: string) => asserts input;
const assert: Assert = scope.assert;
assert(false, "Foo"); // I can now re-use `Assert` type, but it is still very manual and misses the point of effective logging / asserting / debugging utility

but doing it this way (explicitly having to assign the type to assert totally misses the point which is my case was - make DX of writing debugging/assert messages easier)


Also, error message is not really friendly. It is hard for me tu guess what exactly TS wants me to do, forcing to Google it and doing a bit deep read (like this issue) to understand it.

It also seems like some internal TS-limitation leaking, rather than some reasonable design decision that makes sense forcing me to not doing what I want to do.

Thus something like :

Typescript currently does not support assert functions that are not explicitly typed at every step. Thus, assert function cannot be part of any object

would be way more friendly and save me some time.

Or even more friendly:

interface Logger {
  assert(input: unknown): asserts input; // <— TS Error: Assert functions cannot be part of interfaces
}

this way I would know it earlier and in well-targeted place. This way I know my idea of putting assert inside object is not possible, rather than later thinking I use it in a wrong way.

@atulgpt
Copy link

atulgpt commented Feb 6, 2022

I am also facing this issue any update when this bug will be resolved?

@WORMSS
Copy link

WORMSS commented Mar 17, 2022

Just throwing in my two-cents because I got struck by this annoying and very unhelpful bug wording today..

My 'branding' string code.

export type DodiAttributeCodeType = string & { _type: 'DodiAttributeCode' };

/* Delcare type to get around TS Bug */
type DodiAttributeCode = {
  guard(str: string): str is DodiAttributeCodeType;
  parse(str: string): DodiAttributeCodeType;
  assert(str: string): asserts str is DodiAttributeCodeType;
};

export const DodiAttributeCode: DodiAttributeCode = {
  guard(str: string): str is DodiAttributeCodeType {
    return !!str; /* some complex test */
  },
  assert(str: string): asserts str is DodiAttributeCodeType {
    if (!DodiAttributeCode.guard(str)) {
      throw new Error();
    }
  },
  parse(str: string): DodiAttributeCodeType {
    DodiAttributeCode.assert(str);
    return str;
  },
};
import { DodiAttributeCode, DodiAttributeCodeType } from '@core/types/DodiAttributeCode';

const a = 'hello';
const b: DodiAttributeCode = a; // TS error 2322, string not valid
DodiAttributeCode.assert(a);
const c: DodiAttributeCode = a; // TS loves it.. Time to do real work now.

@jeremy-rifkin
Copy link

Assertion functions always need explicit annotations. You can write this:

let azzert: Assertion['assert'] = new Assertion().assert;

Thanks for the clear explanation, this is a strange behavior from typescript. Got stumped by this error today.

@DeepDoge
Copy link

DeepDoge commented Feb 1, 2023

Really strange behavior

const value: unknown = 0

const foo = (value: unknown): asserts value is number => { }
foo(value) // error
value // unknown

const foo_: typeof foo = foo
foo_(value) // ok
value // number
const value: unknown = 0

const foo = {
    bar(value: unknown): asserts value is number { }
}
foo.bar(value) // error
value // unknown

const foo_: typeof foo = foo
foo_.bar(value) // ok
value // number

@cmdruid
Copy link

cmdruid commented Jul 12, 2023

Also chiming in that I ran into this bug in 4.9.4. I am saddened to hear that the typescript devs believe that this behavior is somehow working as intended.

@nicolo-ribaudo
Copy link

nicolo-ribaudo commented Aug 16, 2023

Fwiw, if t.assertSomething(val) reports that error because t is not explicitly typed, you can solve it by doing

const t2: typeof t = t;

t2.assertSomething(val);

so that t2 is explicitly annotated as having its own type.

@pckilgore
Copy link

While I can at least understand the decision to consider this working as intended, the error message you get only makes sense after you discover the solution to the problem, which is not an ideal trait in an error message.

@incerta
Copy link

incerta commented Feb 16, 2024

I think everybody would prefer that asserts and is behave the same in the context of they definition. The current situation:

declare function makeStruct(): {
   assert(subj: unknown): asserts subj is string
   guard(subj: unknown): subj is string
}

const struct = makeStruct()

declare const subj: unknown

// ts-error:
// "Assertions require every name in the call target to be
//  declared with an explicit type annotation. 10:1:416 typescript2775"
struct.assert(subj)

// No such problem with `guard`
if (struct.guard(subj)) {
  subj // `string` as expected
}

The current restriction is highly unintuitive and reduces ts community ability to create nice assert method for schema dependent parsers/validators.

For example we could have assert method in zod library structs and typeguard unknown data like this:

import z from 'zod`

declare const subject: unknown

z.string().assert(subject)

subject // `string`

@WORMSS
Copy link

WORMSS commented Feb 24, 2024

@RyanCavanaugh is there somewhere to view the 'completed' work?

@RyanCavanaugh RyanCavanaugh closed this as not planned Won't fix, can't repro, duplicate, stale Feb 24, 2024
@RyanCavanaugh
Copy link
Member

The bulk "Close" action doesn't let you pick which "Close" you're doing.

Design Limitations aren't supposed to be left open - it means we're not aware of any plausible way to address them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests