samchon / typia

Super-fast/easy runtime validators and serializers via transformation
https://typia.io/
MIT License
4.52k stars 157 forks source link

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

Open matAtWork opened 1 week ago

matAtWork commented 1 week ago

šŸ“ 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.

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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

@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 commented 1 week ago

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 commented 1 week ago

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,

samchon commented 1 week ago
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 commented 1 week ago

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
matAtWork commented 6 days ago

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 commented 6 days ago

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 commented 5 days ago

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 commented 5 days ago

@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