C
C#•16mo ago
Ge Xiaohe

✅ How to resolve CS9017 when trying passing captured arguments by primary constructor to base class?

I am using C# 12. The base class: 
public class DbExtension(IOptionsMonitor<AppConfig> appConfig, LogExtension logger, string connectionString)
{ ... }
public class DbExtension(IOptionsMonitor<AppConfig> appConfig, LogExtension logger, string connectionString)
{ ... }
The derived class: 
public class LocalDbExtension : DbExtension
{
    private readonly IOptionsMonitor<AppConfig> _appConfig;

    public LocalDbExtension(IOptionsMonitor<AppConfig> appConfig, LogExtension logger, string connectionString)
        : base(appConfig, logger, connectionString)
    {
        _appConfig = appConfig;
    }

    ...
}
public class LocalDbExtension : DbExtension
{
    private readonly IOptionsMonitor<AppConfig> _appConfig;

    public LocalDbExtension(IOptionsMonitor<AppConfig> appConfig, LogExtension logger, string connectionString)
        : base(appConfig, logger, connectionString)
    {
        _appConfig = appConfig;
    }

    ...
}
Everything works well without any warnings. Rider told me the LocalDbExtension constructor can be converted into primary constructor, and helped me executed the conversion. Now it looks like this: 
public class LocalDbExtension(IOptionsMonitor<AppConfig> appConfig, LogExtension logger, string connectionString)
    : DbExtension(appConfig, logger, connectionString)
{ ... }
public class LocalDbExtension(IOptionsMonitor<AppConfig> appConfig, LogExtension logger, string connectionString)
    : DbExtension(appConfig, logger, connectionString)
{ ... }
Now, when I build the project, compiler tells me: 
warning CS9107: Parameter `IOptionsMonitor<AppConfig> appConfig` is captured into the state of the enclosing type and its value is also passed to the base constructor. The value might be captured by the base class as well.
warning CS9107: Parameter `IOptionsMonitor<AppConfig> appConfig` is captured into the state of the enclosing type and its value is also passed to the base constructor. The value might be captured by the base class as well.
From the doc:
https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/compiler-messages/constructor-errors#primary-constructor-syntax CS9107 - Parameter is captured into the state of the enclosing type and its value is also passed to the base constructor. The value might be captured by the base class as well. This warning indicates that your code may be allocated two copies of a primary constructor parameter. Because the parameter is passed to the base class, the base class likely uses it. Because the derived class accesses it, it may have a second copy of the same parameter. That extra storage may not be intended.
I have no idea how to resolve this warning if keeping using primary constructor on this class.
Resolve errors related to constructor declarations
These compiler errors and warnings indicate violations when declaring constructors in classes or structs, including records. This article provides guidance on resolving those errors.
12 Replies
333fred
333fred•16mo ago
Show your whole type
Ge Xiaohe
Ge XiaoheOP•16mo ago
I just tried to create a minimal reproducible example. I deleted all business-related fields, properties and methods. 
public class DbExtensionTest(IOptionsMonitor<AppConfig> appConfig, LogExtension logger, string connectionString)
{
}

public class LocalDbExtensionTest(IOptionsMonitor<AppConfig> appConfig, LogExtension logger, string connectionString)
    : DbExtensionTest(appConfig, logger, connectionString)
{
    public string Demo()
    {
        AppConfig appConfigCurrentValue = appConfig.CurrentValue;
        return $"Demo{appConfigCurrentValue}";
    }
}
public class DbExtensionTest(IOptionsMonitor<AppConfig> appConfig, LogExtension logger, string connectionString)
{
}

public class LocalDbExtensionTest(IOptionsMonitor<AppConfig> appConfig, LogExtension logger, string connectionString)
    : DbExtensionTest(appConfig, logger, connectionString)
{
    public string Demo()
    {
        AppConfig appConfigCurrentValue = appConfig.CurrentValue;
        return $"Demo{appConfigCurrentValue}";
    }
}
Now, ignoring Warning CS9113, the warning CS9107 mentioned above happens to appConfig.
333fred
333fred•16mo ago
Well, that simple example quite nicely demonstrates the issue 🙂 You are passing appConfig to the DbExtensionTest, and also referencing it (ie, capturing it) into LocalDbExtensionTest If you want appConfig to be visible in derived types, make a protected field in DbExtensionTest, and reference the field in LocalDbExtensionTest Or, don't pass the value to the base type
Ge Xiaohe
Ge XiaoheOP•16mo ago
Oh thanks! I will try it.
333fred
333fred•16mo ago
The point of this issue is to avoid double storage. Imagine that you had this slight modification of that example: 
public class DbExtensionTest(IOptionsMonitor<AppConfig> appConfig, LogExtension logger, string connectionString)
{

    public string Demo()
    {
        AppConfig appConfigCurrentValue = appConfig.CurrentValue;
        return $"Demo{appConfigCurrentValue}";
    }
}

public class LocalDbExtensionTest(IOptionsMonitor<AppConfig> appConfig, LogExtension logger, string connectionString)
    : DbExtensionTest(appConfig, logger, connectionString)
{
    public string Demo()
    {
        AppConfig appConfigCurrentValue = appConfig.CurrentValue;
        return $"Demo{appConfigCurrentValue}";
    }
}
public class DbExtensionTest(IOptionsMonitor<AppConfig> appConfig, LogExtension logger, string connectionString)
{

    public string Demo()
    {
        AppConfig appConfigCurrentValue = appConfig.CurrentValue;
        return $"Demo{appConfigCurrentValue}";
    }
}

public class LocalDbExtensionTest(IOptionsMonitor<AppConfig> appConfig, LogExtension logger, string connectionString)
    : DbExtensionTest(appConfig, logger, connectionString)
{
    public string Demo()
    {
        AppConfig appConfigCurrentValue = appConfig.CurrentValue;
        return $"Demo{appConfigCurrentValue}";
    }
}
Note how both DbExtensionTest and LocalDbExtensionTest capture appConfig. The captures aren't mutually visible, so both types need to capture appConfig, unnecessarily storing the field twice You honestly probably had this problem initially (since you had a private field in the derived type and passed the value to the base type), but no one was warning you about it
Ge Xiaohe
Ge XiaoheOP•16mo ago
Thanks, I understand that passing to base class and referencing a parameter will result in double storage. I need to change one of them. 1. Not referencing appConfig in derived class: 
// in base class
// as property:
protected IOptionsMonitor<AppConfig> AppConfig => appConfig;
// as field (SonarLint reports: Remove the member initializer, all constructors set an initial value for the member. It seems to be a misreport.):
protected readonly IOptionsMonitor<AppConfig> AppConfig = appConfig;
// which one is recommended?
// in base class
// as property:
protected IOptionsMonitor<AppConfig> AppConfig => appConfig;
// as field (SonarLint reports: Remove the member initializer, all constructors set an initial value for the member. It seems to be a misreport.):
protected readonly IOptionsMonitor<AppConfig> AppConfig = appConfig;
// which one is recommended?
2. Or not passing appConfig to base type. I am not sure if I do not pass appConfig to base type, how I can access appConfig in base type.
333fred
333fred•16mo ago
Is this a library someone else may depend on and derive from DbExtensionTest? If you need it in the base type, then you need to pass it, and will need to do one of the things you mentioned in 1
Ge Xiaohe
Ge XiaoheOP•16mo ago
No, all codes pasted here are coded by me.
333fred
333fred•16mo ago
Then I'd just do a field, and let SonarLint know that they have a false positive
Ge Xiaohe
Ge XiaoheOP•16mo ago
Got that. Thanks!
Ge Xiaohe
Ge XiaoheOP•16mo ago
Just tag the related issue here. This report exists.
https://github.com/SonarSource/sonar-dotnet/issues/7624
GitHub
Fix S3604 FP: Primary constructors · Issue #7624 · SonarSource/sona...
Description S3604 is reported for classes with primary constructors. Repro steps class SampleClass(object options) { private readonly object _options = options; // Noncompliant - FP: S3604 shouldn&...
Accord
Accord•16mo ago
Was this issue resolved? If so, run /close - otherwise I will mark this as stale and this post will be archived until there is new activity.
Want results from more Discord servers?
Add your server