Fix S107: "Methods with too many parameters" should not raise on constructors calling base

[mpetito]

Description

This rule comes up frequently for constructors due to dependency injection scenarios, especially with ASP.NET Core / Identity framework related classes.

Example

The Microsoft.AspNetCore.Identity.UserManager<TUser> class has a constructor with several parameters:

public UserManager(IUserStore<User> store, 
            IOptions<IdentityOptions> optionsAccessor,
            IPasswordHasher<User> passwordHasher, 
            IEnumerable<IUserValidator<User>> userValidators,
            IEnumerable<IPasswordValidator<User>> passwordValidators, 
            ILookupNormalizer keyNormalizer, IdentityErrorDescriber errors, 
            IServiceProvider services, 
            ILogger<UserManager<User>> logger) { }

There are other similar library classes which are often subclassed to override specific functionality. Issues are generated by this rule in cases which are unavoidable, because while we might prefer to not have so many arguments, it's necessary to pass these along to base members.

Expected behavior

I'm not sure what's ideal here. Either it should be possible to ignore constructors entirely for this rule, or perhaps examine constructors for base constructor usage. So for instance, if the constructor calls a base constructor, the identical parameters of the base constructor are excluded from the total parameter count. This way you could check that too many additional parameters are not added.

I could see a similar issue with overridden methods, but I think this is far more common with constructors due to DI.

Known workarounds

Not sure. Is there a way to exclude code blocks from SonarC# coverage?

Related information

SonarC# Version is 6.5 (build 3766)

[Shyam268]

"Constructor has 8 parameters, which is greater than the 7 authorized" issue is exist sonarqube version 7.9 LTS for .net core applications

[andrei-epure-sonarsource]

can you please provide a reproducer, @Shyam268 ?

[sarahelsaig]

I still get S107 on SonarAnalyzer.CSharp Version="8.13.1.21947" using .Net Core 3.1 and Microsoft.NET.Sdk.Web. Here is a synthetic example (using stock framework MS services to make repro easier). It trips when analyzed inside our current project.
We have some actual services where both the base dependencies and the number of additional dependencies in the descendants are under threshold, but the two added together goes over threshold. That shouldn't give a warning, right?

using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.Caching.Memory;
using Microsoft.Extensions.Localization;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using System;

namespace Example
{
    public interface IBaseService
    {
        IHttpContextAccessor HttpContextAccessor { get; }
        LinkGenerator LinkGenerator { get; }
        IServiceProvider ServiceProvider { get; }
        IMemoryCache MemoryCache { get; }
        IHttpContextAccessor Hca { get; }
        IOptions<Settings> Settings { get; }
    }

    public class Settings { }

    public class BaseService : IBaseService
    {
        public IHttpContextAccessor HttpContextAccessor { get; }
        public LinkGenerator LinkGenerator { get; }
        public IServiceProvider ServiceProvider { get; }
        public IMemoryCache MemoryCache { get; }
        public IHttpContextAccessor Hca { get; }
        public IOptions<Settings> Settings { get; }

        public BaseService(
            IHttpContextAccessor httpContextAccessor,
            LinkGenerator linkGenerator,
            IServiceProvider serviceProvider,
            IMemoryCache memoryCache,
            IHttpContextAccessor hca,
            IOptions<Settings> settings)
        {
            HttpContextAccessor = httpContextAccessor;
            LinkGenerator = linkGenerator;
            ServiceProvider = serviceProvider;
            MemoryCache = memoryCache;
            Hca = hca;
            Settings = settings;
        }
    }

    public class DescendantService : BaseService
    {
        private readonly IStringLocalizer<BaseService> _stringLocalizer;
        private readonly ILogger<DescendantService> _logger;

        public DescendantService(
            IHttpContextAccessor httpContextAccessor,
            LinkGenerator linkGenerator,
            IServiceProvider serviceProvider,
            IMemoryCache memoryCache,
            IHttpContextAccessor hca,
            IOptions<Settings> settings,
            IStringLocalizer<BaseService> stringLocalizer,
            ILogger<DescendantService> logger) // <-- S107 triggers on this constructor.
            : base(httpContextAccessor, linkGenerator, serviceProvider, memoryCache, hca, settings)
        {
            _stringLocalizer = stringLocalizer;
            _logger = logger;
        }
    }
}

[pavel-mikula-sonarsource]

Hi @DAud-IcI,

I've created #3727 that you can follow to track your scenario.

[azhe403]

I use sonar version 8.33 but constructor still raised.

Any way to override larger value instead of 7 ?

[pavel-mikula-sonarsource]

Hi @azhe403, this rule should raise on constructors like in your case.

This issue was related to a scenario where the constructor calls the base class constructor.

I'm not sure how you're using the analyzer. The easiest way to configure rule parameters is to configure them in SonarCloud Quality Profile and use SonarLint in connected mode to scaffold all configuration needed in your project. If you're not using SonarCloud, you don't need to analyze your project there. You can just benefit from using the UI to do the configuration.

As this issue is related to S107 rule only, you can ask for more help on https://community.sonarsource.com