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

get the value from literal #4903

Closed
wants to merge 1 commit into from
Closed

Conversation

chunyu3
Copy link
Member

@chunyu3 chunyu3 commented Jul 9, 2024

Description

Fix #4898

Checklist

To ensure a quick review and merge, please ensure:

  • The PR has a understandable title and description explaining the why and what.
  • The PR is opened in draft if not ready for review yet.
    • If opened in draft, please allocate sufficient time (24 hours) after moving out of draft for review
  • The branch is recent enough to not have merge conflicts upon creation.

Ready to Land?

  • Build is completely green
    • Submissions with test failures require tracking issue and approval of a CODEOWNER
  • At least one +1 review by a CODEOWNER
  • All -1 reviews are confirmed resolved by the reviewer
    • Override/Marking reviews stale must be discussed with CODEOWNERS first

Comment on lines +182 to +183
if (contentTypeParameter.Type.Value as string)
mediaTypes.push(contentTypeParameter.Type.Value as string);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the as in typescript does not work in this way.
If this value is not undefined, even it is a number, this check is true.
We should do:

if (typeof contentTypeParameter.Type.Value === "string")

to check if the value is string.
Or if you just want to put in whatever value it has, we could just do mediaTypes.push without the if.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the context in #4898
the abuse of as is the root cause of we are having null inside arrays, because as just skips the compiler type static check, and in runtime, it will just give anything into the list.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#4898 is just a trigger of the issue. It passed Union as array with null value, but also when we handle InputLiteralType, we use the DefaultValue to get the value of an literal which is not correct, we should use InputLiteralType.Value.

I will close this PR, and file another PR in MGC emitter to resolve this.

@chunyu3 chunyu3 closed this Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[defect] codegen gen crash : null value in list is not allowed error
2 participants