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

Fix S3900: FN and FP #6997

Closed
12 tasks done
zsolt-kolbay-sonarsource opened this issue Mar 28, 2023 · 1 comment
Closed
12 tasks done

Fix S3900: FN and FP #6997

zsolt-kolbay-sonarsource opened this issue Mar 28, 2023 · 1 comment
Assignees
Labels
Area: C# C# rules related issues. Sprint: SE Short-lived* label for epic MMF-3077 *troll Type: False Positive Rule IS triggered when it shouldn't be.
Projects
Milestone

Comments

@zsolt-kolbay-sonarsource
Copy link
Contributor

zsolt-kolbay-sonarsource commented Mar 28, 2023

  • FN: foreach loops are not supported
public void Method(List<int> list)
{
    foreach (var element in list) {  } // FN - list is not checked for null before the GetEnumerator() method is called on it
}
  • FN: referencing delegates
public void ConvertUsing(ITypeConverter<TSource, TDestination> converter)
{
    ConvertUsing(converter.Convert);
}
  • FN: most likely related to the conditional operator

public void Configure(TypeMap typeMap)
{
    var destMember = DestinationMember;

    if(destMember.DeclaringType.ContainsGenericParameters)
    {
        destMember = typeMap.DestinationSetters.Single(m => m.Name == destMember.Name);
    }

    var propertyMap = typeMap.FindOrCreatePropertyMapFor(destMember, typeof(TMember) == typeof(object) ? destMember.GetMemberType() : typeof(TMember));

    Apply(propertyMap);
}

public static PropertyMap GetPropertyMap(this IGlobalConfiguration config, MemberInfo sourceMemberInfo, Type destinationMemberType)
{
    var typeMap = config.CheckIfMapExists(sourceMemberInfo.DeclaringType, destinationMemberType);

    var propertyMap = typeMap.PropertyMaps
        .FirstOrDefault(pm => pm.CanResolveValue &&
                              pm.SourceMember != null && pm.SourceMember.Name == sourceMemberInfo.Name);

    if (propertyMap == null)
        throw PropertyConfigurationException(typeMap, sourceMemberInfo.Name);   // Noncompliant FP

    return propertyMap;
}
  • FP: SE engine doesn't support Object.ReferenceEquals
public bool Equals(EmberPlugin other)
{
    if (Object.ReferenceEquals(this, other))
        return true;

    if (Object.ReferenceEquals(other, null))
        return false;

    return this.Plugin.GetType() == other.Plugin.GetType();
}
  • FP: ?? operator
public static MidFunc UseNancy(NancyOptions options = null)
{
    options = options ?? new NancyOptions();
    options.Bootstrapper.Initialise(); // Noncompliant - FP
}
public static void ShouldHaveRedirectedTo(this BrowserResponse response, string location, StringComparison stringComparer = StringComparison.Ordinal)
{
    var validRedirectStatuses = new[]
    {
        HttpStatusCode.MovedPermanently,
        HttpStatusCode.SeeOther,
        HttpStatusCode.TemporaryRedirect
    };

    if (!validRedirectStatuses.Any(x => x == response.StatusCode))   // FN here due to lambda
    {
        throw new AssertException(
            string.Format("Status code should be one of 'MovedPermanently, SeeOther, TemporaryRedirect', but was {0}.", response.StatusCode));
    }

    if (!response.Headers["Location"].Equals(location, stringComparer))  // TP considering the lambda not being supported
    {
        throw new AssertException(string.Format("Location should have been: {0}, but was {1}", location, response.Headers["Location"]));   // FP here, the `if` should have learned NotNull
    }
}
  • FP: parameter is assigned a new value or passed to a method as reference before being dereferenced
public void Assignment(object o)
{
   o = Create();
   o.ToString(); // should not raise
}

public void Deconstruction(object o)
{
   (o, _) = (Create(), _);
   o.ToString(); // FP
}

public void RefParam(object o)
{
   MethodWithRefParam(ref o);
   o.ToString(); // FP
}

public void OutParam(object o)
{
   MethodWithOutParam(out o);
   o.ToString(); // FP
}

private static object Create() => new object();
private static void MethodWithRefParam(ref object obj) { }
private static void MethodWithOutParam(out object obj) { obj = new object(); }
  • FN: member access after cast operation
public static ActorSelection ActorSelection(string path, ActorSystem system, IActorRef lookupRoot)
{
    var provider = ((ActorSystemImpl)system).Provider;  // FN
}
  • FP: inside lambda
    Lambda can see ParameterReferenceOperation from outer CFG. And we should not raise when this happens.
public override object RegisterExtension(IExtensionId extension)
{
    if (extension == null) return null;

    _extensions.GetOrAdd(extension.ExtensionType, t => new Lazy<object>(() => extension.CreateExtension(this), LazyThreadSafetyMode.ExecutionAndPublication)); // FP

    return extension.Get(this);
}
  • FP: is operator
public static bool CanBeSet(this MemberInfo member) => member is PropertyInfo property ? property.CanWrite : !((FieldInfo)member).IsInitOnly;
  • Issue message: the rule should raise an issue with the message Refactor this constructor to avoid using members of parameter 'paramName' because it could be null. if the parameter is dereferenced inside the call to the base class constructor. If it's dereferenced inside the body of the constructor then the message should be the regular Refactor this method to add validation of parameter 'paramName' before using it.
public class DerivedClass: BaseClass
{
	public DerivedClass(object o1, object o2): base(o1.ToString()) // Noncompliant {{Refactor this constructor to avoid using members of parameter 'o1' because it could be null.}}
	{
		o2.ToString();				               // Noncompliant {{Refactor this method to add validation of parameter 'o2' before using it.}}
	}  
}
@pavel-mikula-sonarsource
Copy link
Contributor

I'm closing this as fixed. All tasks were done, moved to another issue (#7060) or will be dropped (flow capture)

Best Kanban automation moved this from In progress to Validate Peach Apr 13, 2023
@pavel-mikula-sonarsource pavel-mikula-sonarsource moved this from Validate Peach to Done in Best Kanban Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: C# C# rules related issues. Sprint: SE Short-lived* label for epic MMF-3077 *troll Type: False Positive Rule IS triggered when it shouldn't be.
Projects
Best Kanban
  
Done
Development

No branches or pull requests

2 participants