Sonar-dotnet: S4018 FP: some cases when the return type is the same with the type param

Created on 19 Feb 2020  路  6Comments  路  Source: SonarSource/sonar-dotnet

Description

S4018 claims that every generic method should provide type parameters. I thing that there are some exceptions to that rule:

Repro steps

``` C#
public static class S4018
{
public static T MyCast(object obj) // compliant
{
return (T)obj;
}

public static T ObjectIs<T>(object obj) // compliant
{
    if(obj is T casted)
    {
        return casted;
    }
    throw new InvalidOperationException();
}

public static T MyDefault<T>() // compliant
{
    return default;
}

}
```

Related information

  • SonarC# Version 8.3.0.14607
Won't fix

All 6 comments

I agree, the rule is too noisy (this is why it's not part of SonarWay).

I am not sure if cases as below

    public static T Foo<T>(SomeClassX first, SomeClassY sec)
    {
        // ...
    }

should raise or not. I would say they should not, although the goal of the rule won't be met anymore - inferring the type of the method.

cc @nicolas-harraudeau-sonarsource as this rule needs to be re-evaluated, I believe.

@andrei-epure-sonarsource I agree. I thought is was to strong to state it like that. I understand the reasoning, but I asume in most cases, the violations of the basic thought are at least defendable.

I have an update on this, @Corniel . Due to the fact this rule is not in the default profile makes it less prioritary; and we don't want to spend energy on deciding exactly what are the "obvious FPs" to avoid, because from our discussions, these FPs depend on the context quite a lot (they wouldn't be totally obvious).

So we are going to close it as won't fix.

However, if you want, you can make a PR with the "the violations of the basic thought" and we can move the discussion there.

I think it should be dropped.

I hit the same problem.
I do not understand, why e.g. public static T MyCast<T>(object obj) should complain.

I would agree, that the generic type parameter should be used by the method signature: parameters and/or return value.

@lg2de the rule description says

The best way to determine the type of a generic method is by inference based on the type of argument that is passed to the method. This is not possible when a parameter type is missing from the argument list.

we'd need to change the initial idea of the rule if we are to allow return types

the idea was actually taken from FxCop CA1004 - see description https://docs.microsoft.com/en-us/visualstudio/code-quality/ca1004?view=vs-2019#rule-description

it's a noisy rule, and that's why it is not enabled by default

Was this page helpful?
0 / 5 - 0 ratings