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

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

Open
matAtWork opened this issue Sep 26, 2024 · 12 comments
Assignees
Labels
invalid This doesn't seem right question Further information is requested wontfix This will not be worked on

Comments

@matAtWork
Copy link

matAtWork commented Sep 26, 2024

📝 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
Copy link
Author

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
Copy link
Author

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 matAtWork changed the title Equal computed type and literal type don't validate with the same results Types mapped from objects containing named functions do not generate validation for those members. Sep 27, 2024
@matAtWork
Copy link
Author

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
Copy link
Author

matAtWork commented Sep 27, 2024

@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 pushed a commit to MatAtBread/typia that referenced this issue Sep 27, 2024
@matAtWork
Copy link
Author

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
Copy link
Author

matAtWork commented Sep 27, 2024

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 samchon self-assigned this Sep 28, 2024
@samchon samchon added invalid This doesn't seem right question Further information is requested wontfix This will not be worked on labels Sep 28, 2024
@samchon
Copy link
Owner

samchon commented Sep 28, 2024

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
Copy link
Owner

samchon commented Sep 28, 2024

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
Copy link
Author

matAtWork commented Sep 30, 2024

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
Copy link
Author

matAtWork commented Sep 30, 2024

Similarly, if you look at the generated code in the playground link in #1302 (comment), you will see there is no generated code for foo, qux or quux, when there should be

@matAtWork
Copy link
Author

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
Copy link
Author

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right question Further information is requested wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants