Skip to content

fix: prevent generation for nullable delegate parameters#397

Open
teneko wants to merge 1 commit intoeiriktsarpalis:mainfrom
tenekon:main
Open

fix: prevent generation for nullable delegate parameters#397
teneko wants to merge 1 commit intoeiriktsarpalis:mainfrom
tenekon:main

Conversation

@teneko
Copy link

@teneko teneko commented Feb 24, 2026

  • Ensure source generation for delegate signatures does not append a second nullable suffix when the type is already nullable (e.g. ).
  • Add regression test to cover nullable bool delegate parameters and prevent recurrence.

Reproducable code:

using PolyType;

_ = typeof(CliShapes);

[GenerateShapeFor(typeof(BrokenCommand))]
public partial class CliShapes;

public delegate Task<int> BrokenCommand(bool? skipDuplicate = null);

- Ensure source generation for delegate signatures does not append a second nullable suffix when the type is already nullable (e.g. ).
- Add regression test  to cover nullable bool delegate parameters and prevent recurrence.
@teneko

This comment was marked as outdated.


string delegateSignature = string.Join(", ", functionShapeModel.Parameters
.Select(parameter => $"{FormatRefPrefix(parameter)}{parameter.ParameterType.FullyQualifiedName}{GetNullableSuffix(parameter)} {parameter.Name}"));
.Select(parameter => $"{FormatRefPrefix(parameter)}{FormatDelegateParameterType(parameter)} {parameter.Name}"));
Copy link
Owner

Choose a reason for hiding this comment

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

Does the issue occur with constructor methods? I feel the underlying issue here is that nullable annotations isn't being stripped in delegate parameters, unlike what is happening with constructor or method shapes.

Copy link
Author

Choose a reason for hiding this comment

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

I do not understand. Is this a clarification question: "does this issue occur with constructor methods [too]"? If not I can only clarify: when using top-level statements, the delegate turns into a global non-Program-class owned delegate.

Ahh I think I understand what you mean. So you say, that nullable annotations are stripped for method parameters, but not for constructor parameters and method parameters.

I am not sure what you mean by "nullable annotation stripping". Am I right in the assumption, that you mean PolyType that does nullable annotation stripping?

I did not researched, how the nullable annotation (stripping) is handled by PolyType in case of constructor parameters or method parameters. If you want I can do that and report back.

Copy link
Owner

Choose a reason for hiding this comment

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

There are many parts of the source generator that deal with method/constructor parameter. I was asking if this is the only location where the bug manifests. What happens if the same pattern occurs in a constructor annotated with [ConstructorShape]?

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.

2 participants