Skip to content

Support Covariant Return Types#334

Open
huoshan12345 wants to merge 69 commits into
dotnetcore:masterfrom
huoshan12345:feature/support-covariant-return-types
Open

Support Covariant Return Types#334
huoshan12345 wants to merge 69 commits into
dotnetcore:masterfrom
huoshan12345:feature/support-covariant-return-types

Conversation

@huoshan12345

@huoshan12345 huoshan12345 commented Jul 20, 2025

Copy link
Copy Markdown
Collaborator

Key Changes:

  • Support for Covariant Return Types
  • Typo Corrections(they are both internal classes, so no breaking changes)
    • ManyEnumerableServiceDefintion -> ManyEnumerableServiceDefinition
    • EnumerableServiceDefintion -> EnumerableServiceDefinition
  • sign the project AspectCore.Tests to test internal methods

What is Covariant Return Types

see https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-9.0/covariant-returns

@huoshan12345 huoshan12345 changed the title Support covariant return types Support Covariant Return Types Jul 20, 2025
@huoshan12345 huoshan12345 marked this pull request as draft July 20, 2025 23:04
@huoshan12345 huoshan12345 marked this pull request as ready for review July 22, 2025 15:14
…mework into feature/support-covariant-return-types

# Conflicts:
#	src/AspectCore.Core/Utils/ProxyGeneratorUtils.cs
@huoshan12345 huoshan12345 marked this pull request as draft May 13, 2026 17:13
@huoshan12345 huoshan12345 marked this pull request as ready for review May 13, 2026 22:15
@huoshan12345

Copy link
Copy Markdown
Collaborator Author

已解决:

  • 问题 1:多层协变返回
  • 问题 2:泛型方法协变返回

var baseDef = method.GetBaseDefinition();
foreach (var property in m.DeclaringType.GetTypeInfo().GetProperties())
{
if (property.CanRead && property.GetMethod == baseDef)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

这里会把普通 override 属性的 getter/setter 误判成非属性方法。对于 Derived : Base 中的 public override string Name { get; set; }Derived.get_Name.GetBaseDefinition() 返回的是 Base.get_Name,但 Derived.Name.GetMethod 仍然是 Derived.get_Name,所以这里比较失败,IsPropertyBinding() 返回 false

结果是 class proxy 构建时会在 BuildClassMethods 里把 get_Name/set_Name 当普通方法生成一次,又在 BuildClassProperties 里作为属性 accessor 再生成一次。我用临时探针看到 proxy 类型里普通方法 token 和 PropertyInfo.GetMethod/SetMethod 指向的 token 已经不同。

建议同时匹配当前 accessor 和 base definition,例如 getter/setter 分别判断 property.GetMethod == m || property.GetMethod.GetBaseDefinition() == baseDef,并补一个普通 override property 的回归测试。

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

这段revert了

attributes |= MethodAttributes.FamORAssem;
var (serviceMethod, implMethod) = covariantReturnMethod is null
? (method, InterfaceImplBuilder.ResolveImplementationMethod(method, _implType))
: (covariantReturnMethod, covariantReturnMethod);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

这里把 serviceMethod 也替换成了 covariant override,本身能解决发射签名问题,但会丢掉调用方传入的原始 service 方法身份,导致配置型拦截器不再按 service type 命中。

可复现:

configuration.Interceptors.AddDelegate(async (ctx, next) =>
{
    await next(ctx);
    ctx.ReturnValue = new LeafResult(((BaseResult)ctx.ReturnValue).Name + ":intercepted");
}, Predicates.ForService(nameof(BaseService)));

var service = generator.CreateClassProxy<BaseService, LeafService>();
service.Method();

class BaseService { public virtual BaseResult Method() => new("base"); }
class LeafService : BaseService { public override LeafResult Method() => new("leaf"); }

当前结果是 leaf,同样场景如果 LeafService.Method() 非协变地返回 BaseResult,结果是 leaf:intercepted。原因是 aspect body 后续用这个替换后的 serviceMethod 去做 InterceptorCollector.CollectPredicates.ForService(nameof(BaseService)) 看到的是 LeafService.Method,非 inherited 的配置拦截器不会被收集。

建议发射签名仍使用 covariant method,但验证/收集/AspectContext 里的 service metadata 保留原来的 method,或者给 MethodNode/body 分开建模 emitted signature method 与 logical service method。属性 getter 的协变分支也有同类风险。

@huoshan12345 huoshan12345 Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

我在context里面加了一个字段叫public MethodInfo PredicateMethod { get; }专门用于InterceptorCollector.Collect, 不过相关改动就有点多了= =

相关测试用例在CovariantReturnTypeWithInterceptorTests.cs‎这个文件

@@ -202,15 +243,56 @@ private void BuildClassProperties(List<PropertyNode> properties, List<MethodNode

properties.Add(new PropertyNode(property.Name, property.PropertyType, property.Attributes, attrs, getMethod, setMethod));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

协变属性这里仍用原始 property.PropertyType 创建 PropertyNode,会让生成类型的属性元数据和 getter 签名不一致。

复现:

class BaseService { public virtual BaseResult Value { get; } = new("base"); }
class LeafService : BaseService { public override LeafResult Value { get; } = new("leaf"); }
var proxy = generator.CreateClassProxy<BaseService, LeafService>();

原始 LeafService 的 declared property 是 LeafResult Value,但当前 proxy 的 declared property 变成 BaseResult Value,同时 PropertyInfo.GetMethod.ReturnTypeLeafResult。我用反射探针看到:

LeafService.Value property=LeafResult getter=LeafResult
Proxy.Value property=BaseResult getter=LeafResult

这会破坏依赖 PropertyInfo.PropertyType 的调用方,也让 proxy metadata 不再等价于实际 override。协变 getter 分支里应当用实际生成 getter 的返回类型(例如 getMethod.ServiceMethod.ReturnType / serviceMethod.ReturnType)作为 PropertyNode 的 property type,而不是始终使用原始 service property type。

@huoshan12345 huoshan12345 Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

换成getter.ReturnType了
相关测试用例: CreateClassProxy_ForCovariantProperty_ShouldKeepPropertyTypeAlignedWithGetterReturnType

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