Fix S3900 FN: Conversion operations #7002
Conversation
github-actions
bot
assigned Tim-Pohlmann and antonioaversa and unassigned zsolt-kolbay-sonarsource
| private static bool IsParameterDereferenced(IOperationWrapperSonar operation) => | ||
| operation.Parent != null | ||
| && operation.Parent.IsAnyKind( | ||
| && (operation.Parent.IsAnyKind( | ||
| OperationKindEx.Invocation, | ||
| OperationKindEx.FieldReference, | ||
| OperationKindEx.PropertyReference, | ||
| OperationKindEx.EventReference, | ||
| OperationKindEx.Await, | ||
| OperationKindEx.ArrayElementReference); | ||
| OperationKindEx.ArrayElementReference) | ||
| || (operation.Parent.Kind == OperationKindEx.Conversion && IsParameterDereferenced(operation.Parent.ToSonar()))); |
There was a problem hiding this comment.
The formatting makes this hard to understand. Try indenting the last line.
github-actions
bot
assigned zsolt-kolbay-sonarsource and unassigned antonioaversa and Tim-Pohlmann
| OperationKindEx.Invocation, | ||
| OperationKindEx.FieldReference, | ||
| OperationKindEx.PropertyReference, | ||
| OperationKindEx.EventReference, | ||
| OperationKindEx.Await, | ||
| OperationKindEx.ArrayElementReference); | ||
| OperationKindEx.ArrayElementReference) | ||
| || (operation.Parent.Kind == OperationKindEx.Conversion && IsParameterDereferenced(operation.Parent.ToSonar()))); |
There was a problem hiding this comment.
The || operator should be indented one level further w.r.t. the && operator above (to which is not aligned either, by one char).
I think however, that it would be better to simplify the structure of this logical condition, since you have different logical operators at different levels of nesting.
| @@ -93,13 +93,14 @@ protected override ProgramState PreProcessSimple(SymbolicContext context) | |||
|
|
|||
| private static bool IsParameterDereferenced(IOperationWrapperSonar operation) => | |||
| operation.Parent != null | |||
| && operation.Parent.IsAnyKind( | |||
| && (operation.Parent.IsAnyKind( | |||
There was a problem hiding this comment.
This condition is getting a bit difficult to read.
Maybe it would make sense to build an array of OperationKindEx, to be used in IsAnyKind?
| { | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
I would also add the following unit test, which was not working before the change from Parent to Ancestors(), and works now:
public void Parentheses(object[] arr)
{
foreach (object o in ((arr))) { } // Noncompliant
}If you want to push it one level further:
public void Parentheses(object[] arr)
{
foreach (object o in ((object[])(object)((arr)))) { } // Noncompliant
}and similar.
github-actions
bot
assigned zsolt-kolbay-sonarsource and unassigned zsolt-kolbay-sonarsource
github-actions
bot
assigned pavel-mikula-sonarsource and unassigned zsolt-kolbay-sonarsource
| public void ForEachLoop(object[] array) | ||
| { | ||
| foreach (object o in array) { } // Noncompliant | ||
| } |
There was a problem hiding this comment.
Please add also IEnumerable<object>. Can be 2nd parameter of the same method.
| public void CastWithPropertyAccess(object o) | ||
| { | ||
| _ = ((Exception)o).Message; // Noncompliant | ||
| } |
There was a problem hiding this comment.
Please add also field access
| public void CastWithPropertyAccess(object o) | ||
| { | ||
| _ = ((Exception)o).Message; // Noncompliant | ||
| } |
There was a problem hiding this comment.
I'd also add delegate access, should be FN for now - as seen in the original issue
_ = ((Exception)o).ToString;
|
Kudos, SonarCloud Quality Gate passed!
|
|
Kudos, SonarCloud Quality Gate passed! |
Part of #6997
Task 1 and 9