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

[experiment][dpg] do not convert enum client parameters into string #4457

Draft
wants to merge 2 commits into
base: feature/v3
Choose a base branch
from

Conversation

archerzz
Copy link
Member

Description

In DPG, we use InputModelTypeUsage.None as a tricky way to convert enum client parameters into string.

if (!_isTspInput || enumType.Usage == InputModelTypeUsage.None)
{
return TypeFactory.CreateType(enumType.EnumValueType);
}

This won't work when adopting TCGC because TCGC will give a InputModelTypeUsage.Input for client parameters. Then enum client parameters will still be enum, which will cause exception when trying to get Field from parameter (which is enum value type):

public FieldDeclaration? GetFieldByParameter(Parameter parameter)

This PR investigate one solution to resolve this problem. It will change the logic of finding matching field. For enum field type, we'll try to match its value type as well. If that matches, then we'll return that field.

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

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.

None yet

1 participant