typia icon indicating copy to clipboard operation
typia copied to clipboard

Types mapped from objects containing named functions do not generate validation for those members.

Open matAtWork opened this issue 1 year ago • 12 comments

📝 Summary

I have a set of types to validate arguments (and returns, tho not necessary to illustrate the issue) in an async API object. The TS types that extract the arguments, when passed to typia.createValidate does not reliably fail invalid arguments. Hovering over the type in the playground and copying the computed type DOES generate a working validator.

  • Typia Version: Playground 6.11.0
  • Expected behavior: Args and Args2 validate with the same results
  • Actual behavior: Args incorrectly passes an invalid value, Args2 correctly fails an invalid value

UPDATE: NB: Please see this comment for the simplified canonical example which demonstrates that mapped objects containing function declarations vs function expressions behave differently

⏯ Playground Link

If you click on the playground link above and hover over Args, you see it evaluates to

type Args = {
    foo: [];
    bar: never;
} | {
    foo: never;
    bar: [];
}

This type represents the possible ways the test API can be called (ie a single API, with it's associated parameters). It has been copied and pasted and assigned to the type Args2.

When an invalid argument is passed to the return of typia.createValidate<Args>(), it is incorrectly validated as correct. When the same invalid argument is passed to the return of typia.createValidate<Args2>(), it is correctly validated as incorrect.

In TS (and the Typia Playground), Args and Args2 are identical, however the execution of the validation function is not.

Included for completeness (same as the playground link):

import typia from "typia";

export type Explode<T> = keyof T extends infer K
  ? K extends unknown
  ? { [I in keyof T]: I extends K ? T[I] : never }
  : never
  : never;

type ApiArguments<API extends {}> = Explode<{
  [api in keyof API]: API[api] extends (this: any, ...a:infer A extends any[])=>Promise<any> ? A : never;
}>

const api = {
  async foo() { return 1 },
  async bar() { return "x" }
}

type Args = ApiArguments<typeof api>;
type Args2 = {
    foo: [];
    bar: never;
} | {
    foo: never; 
    bar: [];
};

const validator = typia.createValidate<Args>();
const validator2 = typia.createValidate<Args2>();

console.log(validator({ foo: [] }))
console.log(validator2({ foo: [] }))
console.log(validator({ foo: [true] })) // PASSES, BUT SHOULD FAIL
console.log(validator2({ foo: [true] }))

matAtWork avatar Sep 26 '24 15:09 matAtWork

Simpler version, that remove the Explode and async from the possible source of errors. It's now just a simple mapped type that extracts the Parameters for each function in the object.

Playground

import typia from "typia";

type ApiArguments<API extends { [k:string]: (...a:any[])=>any}> = {
  [api in keyof API]: Parameters<API[api]>;
}

type Args = ApiArguments<{
    foo(x: number): number;
}>;

type Args2 = {
    foo: [x: number];
};

const validator = typia.createValidate<Args>();
const validator2 = typia.createValidate<Args2>();

console.log(validator({ foo: [10] }))
console.log(validator2({ foo: [10] }))
console.log(validator({ foo: [true] })) // PASSES, BUT SHOULD FAIL
console.log(validator2({ foo: [true] }))

matAtWork avatar Sep 26 '24 15:09 matAtWork

Narrowed it down further: typia doesn't correctly recognise keys that are functions in mapped types. However it DOES recognise property keys that are initialized to functions.

Playground link

Key observation:

type Args = ApiArguments<{
    //foo(x: number): number; // BREAKS: `foo` is not mapped to a validation
    foo:(x: number) => number; // Works: `foo` is mapped to a validation
}>;

Going back to the initial example (with the complexity of Parameters, extends, async), changing

const api = {
  async foo() { return 1 },
  async bar() { return "x" }
}

...to...

const api = {
  foo: async function foo() { return 1 },
  bar: async function bar() { return "x" }
}

...generates the correct validation. This demonstrates that the only issue here is that mapped keys defined by functions are for some reason not generating correct validation, even if they are mapped to data (such as to their parameters)

I'll update the issue title to reflect this observation

matAtWork avatar Sep 27 '24 10:09 matAtWork

Canonical example, that shows that the function declarations vs function expression behave differently.

Playground

import typia from "typia";

type Mapper<X> = {
  [K in keyof X]: string
}

type V1 = {
  foo(): void;
}

console.log(
  typia.validate<Mapper<V1>>({ foo: 'foo' }),
  typia.validate<Mapper<V1>>({ foo: null }) // INCORRECTLY PASSES
)

type V2 = {
  foo: ()=> void;
}

console.log(
  typia.validate<Mapper<V2>>({ foo: 'foo' }),
  typia.validate<Mapper<V2>>({ foo: null })
)

matAtWork avatar Sep 27 '24 10:09 matAtWork

@samchon Perhaps https://github.com/samchon/typia/blob/master/src/factories/internal/metadata/emplace_metadata_object.ts#L179 should be:

   ts.isShorthandPropertyAssignment(node) ||
   ts.isPropertySignature(node) ||
   ts.isMethodSignature(node) ||
   ts.isMethodDeclaration(node) ||

?

I've not yet worked out if this will have any other detrimental effects, as I'm having issues running npm test

matAtWork avatar Sep 27 '24 11:09 matAtWork

Ok, so that "fix" breaks lots of code as it new tries (fails) to generate validators for all function members.

Typia does not (at present) generate validation tests for function elements, as can be shown here (aside: a basic test here could be that typeof obj[field]==='function' would at least test the function member exists and is a function, even though it can't check the parameter types, only arity).

So the issue is, from a type perspective, that

type FunctionParameters<X extends { [k:string]: (...a:any)=>any }> = {
  [K in keyof X]: Parameters<X[K]>
}

...fails because it's input is stripped of all functional members, even tho the resulting type contains only data members.

I think there are only two approaches to fixing this, and I have little insight into which is the best to implement:

  1. Move the "significant" test up the call tree so that callees can optionally ignore any functional members, or
  2. Pass another flag to significant = (functional: boolean, terminal?: boolean) => so that isMethodDeclaration, isMethodSignature and isShorthandPropertyAssignment are conditionally included.

Given isShorthandPropertyAssignment is not call-specific, I don't think (2) will work. The tests to include/exclude these nodes which define function member types need to be plumbed into iterate_metadata and explore_metadata.

The only other way I can think of doing this is to leave the function members in place, and simply generate a null or trivial validation test for them (or the "aside" above), since the mapped type above will correctly generate data tests as the resulting type does not contain functional members.

There's a strong chance I've over-complicated this case, as I'm still learning how typia is implemented.

Any hints on how to progress this to a fix would be much appreciated 🙏

matAtWork avatar Sep 27 '24 15:09 matAtWork

This playground link seems to narrow down the issue to a cover all cases (updated: 30/9/2024 to cover 'quux' case)

Given that it can work as expected, depending on the function declaration style, I think the above explanation is too complex. It simply seems to depend on the syntax (and therefore TS AST node type), rather than some complication inclusion/exclusion mechanism.

The workaround would be to use the property syntax fn: function(){...} in preference to fn(){...}, unfortunately, I have a codebase of 10,000s of lines and many additional modules over which I have no control, so this is not feasible,

matAtWork avatar Sep 27 '24 15:09 matAtWork

import typia, { tags } from "typia";

typia.createIs<{
  foo: [];
  bar: never;
}>();

https://typia.io/playground/?script=JYWwDg9gTgLgBDAnmYBDANHA3g1BzAZzgF84AzKCEOAIiRVRoG4AoF+tAOgGMoBTVDD4BJAgB4sLOOQgQAXHADaAXVbSARqigKAdnwBufKK2IA+ABQBKJkA

At first, the never typed property is considered as only undefined type in the TypeScript type system.

Also, checking only property typed function is not a bug, bug intended spec following the standard way. The member methods are defined in the prototype instance. If typia checks not only property function, but also member methods, it is not a proper validation.

class SomeClass {
  public methodA() {}
  public propertyFunctionB = () => {};
}

samchon avatar Sep 28 '24 18:09 samchon

Simpler version, that remove the Explode and async from the possible source of errors. It's now just a simple mapped type that extracts the Parameters for each function in the object.

Playground

import typia from "typia";

type ApiArguments<API extends { [k:string]: (...a:any[])=>any}> = {
  [api in keyof API]: Parameters<API[api]>;
}

type Args = ApiArguments<{
    foo(x: number): number;
}>;

type Args2 = {
    foo: [x: number];
};

const validator = typia.createValidate<Args>();
const validator2 = typia.createValidate<Args2>();

console.log(validator({ foo: [10] }))
console.log(validator2({ foo: [10] }))
console.log(validator({ foo: [true] })) // PASSES, BUT SHOULD FAIL
console.log(validator2({ foo: [true] }))

Also, the third case console.log is tracing failure. What is the problem?

image

samchon avatar Sep 28 '24 18:09 samchon

The problem is the types:

type V1 = {
  foo(): void;
}
type V2 = {
  foo: ()=>void;
}

....are logically equivalent, but behave differently in typia.

import typia from "typia";

type ApiArguments<API extends { [k:string]: (...a:any[])=>any}> = {
  [api in keyof API]: Parameters<API[api]>;
}

type V1 = ApiArguments<{
  foo(x:number): void;
}>;

type V2 = ApiArguments<{
  foo: (x:number)=>void;
}>

typia.is<V1>({foo:['x']}); // Incorrectly validates
/*
  const $io0 = (input) => true; // NO VALIDATION OF `foo`
  return (input) =>
    "object" === typeof input &&
    null !== input &&
    false === Array.isArray(input) &&
    $io0(input);

*/
typia.is<V2>({foo:['x']});
/*
  const $io0 = (input) =>
    Array.isArray(input.foo) &&
    input.foo.length === 1 &&
    "number" === typeof input.foo[0]; // CORRECTLY VALIDATES `foo`
  return (input) => "object" === typeof input && null !== input && $io0(input);
*/

Playground You have assumed the former will not generate a type validator because it is a function and omit the key in significant(), however if mapped (or subject to a keyof), this is not true, and the entire iteratie_metadata is terminated early.

A similar issue exists when the type is derived from typeof value where value is initialized by shorthand properties.

matAtWork avatar Sep 30 '24 08:09 matAtWork

Similarly, if you look at the generated code in the playground link in https://github.com/samchon/typia/issues/1302#issuecomment-2379562090, you will see there is no generated code for foo, qux or quux, when there should be

matAtWork avatar Sep 30 '24 08:09 matAtWork

RE you comment above:

Also, checking only property typed function is not a bug, bug intended spec following the standard way. The member methods are defined in the prototype instance. If typia checks not only property function, but also member methods, it is not a proper validation.

This is only true in a class declaration, not plain objects or types.

matAtWork avatar Oct 01 '24 09:10 matAtWork

@samchon - thank you for fixing the shorthand property issue 🙇

However, the original issue with an object containing a member function that is mapped to a data type remains in v6.11.1

matAtWork avatar Oct 01 '24 09:10 matAtWork