Closed mary-georgiou-sonarsource closed 4 months ago
Exclude inherited methods (because there is no option for the user than to implement them)
This may reduce the reach a lot, as many of the big projects have hierarchies of controllers. If there is an abstract
base class and there is at least an method which is abstract
, there is indeed no option for the user and we may want to exclude the controller altogether.
Regarding the issue on this controller: it's both an API and an MVC controller at the same time. This case seems to be a false positive because the overridden methods are abstract two levels above in the hierarchy and those abstract methods are used by public actions at that level.
An issue with static methods in file https://peach.aws-prd.sonarsource.com/issues?projects=umbraco&resolved=false&rules=csharpsquid%3AS6960&open=AY8vNwi3r4ooVA6A4A55
Should we exclude static methods?
In my opinion:
Exclude inherited methods (because there is no option for the user than to implement them)
This may reduce the reach a lot, as many of the big projects have hierarchies of controllers. If there is an
abstract
base class and there is at least an method which isabstract
, there is indeed no option for the user and we may want to exclude the controller altogether.Regarding the issue on this controller: it's both an API and an MVC controller at the same time. This case seems to be a false positive because the overridden methods are abstract two levels above in the hierarchy and those abstract methods are used by public actions at that level.
For the second case, we should ignore inherited controllers. In that particular case we should not raise because the class is indeed focused. We have no way to know that in the general case (what if we inherit from an external DLL?) and the easy way out is to ignore it.
IMO we want to help people who are starting to use the framework and push them towards good habits. Someone using inheritance either
RSPEC PR: https://github.com/SonarSource/rspec/pull/3845
Parts:
Func<service>
andLazy<service>
: Service should not be well-knownIHttpClientFactory
(consider as well-known dependency)S6960IncludeAttribute
andS6960ExcludeAttribute
~abstract
controllers (because of scenarios like this one)