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

Extension methods not being handled correctly in Made.Of service returning expression #576

Closed
Metadorius opened this issue May 24, 2023 · 5 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@Metadorius
Copy link
Contributor

Metadorius commented May 24, 2023

Background: I am integrating Serilog in my application. I used your examples with context-based resolution but adapted them to a generic Microsoft.Extensions.Logging + Serilog approach to decouple from Serilog in the business logic code. I got something like this:

var container = new Container()
    .WithMefAttributedModel();

var serilogLogger = new LoggerConfiguration()
    /* ... */
    .CreateLogger();

var loggerFactory = new SerilogLoggerFactory(serilogLogger);

container.RegisterInstance<ILoggerFactory>(loggerFactory);

container.Register(
    Made.Of(_ => ServiceInfo.Of<ILoggerFactory>(),
        f => f.CreateLogger(/*categoryName:*/ null!)),
    setup: Setup.With(condition: r => r.Parent.ImplementationType == null));

container.Register(
    Made.Of(_ => ServiceInfo.Of<ILoggerFactory>(),
        f => f.CreateLogger(Arg.Index<Type>(0)),
        r => r.Parent.ImplementationType),
    setup: Setup.With(condition: r => r.Parent.ImplementationType != null));

It works fine on the first registration because string-accepting method is actually a class method and fails on the second one, which is an extension method, because for some reason extension methods in C# behave differently internally compared to regular instance methods and this is not handled by DryIoc Made.Of handling code.

Non-extension method:
image

Extension method:
image

As a result it fails because it gets an unexpected non-constant parameter (being the factory which is the extension method is for)

DryIoc.ContainerException: code: Error.UnexpectedExpressionInsteadOfConstantInMadeOf;
message: Expected `ConstantExpression` for value of parameter, property, or field, but found `f` 
in Made.Of expression: `f => f.CreateLogger(Index(0))`
   at DryIoc.Throw.For[T](Int32 error, Object arg0, Object arg1, Object arg2, Object arg3) in /_/src/DryIoc/Container.cs:line 14549
   at DryIoc.Made.GetArgExpressionValueOrThrow(Expression wholeServiceExpr, Expression argExpr) in /_/src/DryIoc/Container.cs:line 7450
   at DryIoc.Made.ComposeParameterSelectorFromArgs(Boolean& hasCustomValue, Expression wholeServiceExpr, ParameterInfo[] paramInfos, IList`1 argExprs, Func`2[] argValues) in /_/src/DryIoc/Container.cs:line 7305
   at DryIoc.Made.FromExpression[TService](Func`2 getFactoryMethodSelector, LambdaExpression serviceReturningExpr, Func`2[] argValues) in /_/src/DryIoc/Container.cs:line 7220
   at DryIoc.Made.Of[TFactory,TService](Func`2 getFactoryInfo, Expression`1 serviceReturningExpr, Func`2[] argValues) in /_/src/DryIoc/Container.cs:line 7149
...
@Metadorius
Copy link
Contributor Author

How "native" to class method call expression looks:
image

Extension methods:
image

@dadhi
Copy link
Owner

dadhi commented May 25, 2023

@Metadorius Thanks for the finding, will fix
Btw, I am accepting the PRs if you're willing to.

@dadhi dadhi added this to the v5.4.1 milestone May 25, 2023
@dadhi dadhi added the bug Something isn't working label May 25, 2023
@Metadorius
Copy link
Contributor Author

I would love to help, but I am afraid my expertise on this is not enough 😅
#577 I might have an idea on possible ways of solving, though I am not sure about the correct way to solve it there either, should write my thoughts there. Regardless, I might help but not sure how much would be possible for me, so you shouldn't rely on me much.

@dadhi
Copy link
Owner

dadhi commented May 25, 2023

That's fine. You are already helping.

dadhi added a commit that referenced this issue May 25, 2023
@dadhi dadhi closed this as completed in a5757c3 May 25, 2023
@dadhi dadhi self-assigned this May 25, 2023
@dadhi
Copy link
Owner

dadhi commented May 25, 2023

fixed now.

dadhi added a commit that referenced this issue May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants