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 S1168 FP: Does not respect nullable annotations #6878

Closed
nesc58 opened this issue Mar 8, 2023 · 1 comment · Fixed by #6952
Closed

Fix S1168 FP: Does not respect nullable annotations #6878

nesc58 opened this issue Mar 8, 2023 · 1 comment · Fixed by #6952
Assignees
Labels
Area: C# C# rules related issues. Type: False Positive Rule IS triggered when it shouldn't be.
Projects
Milestone

Comments

@nesc58
Copy link
Contributor

nesc58 commented Mar 8, 2023

Description

The rule S1168 raises false positive when then nullable annotation is enabled.

Repro steps

public interface IInterface
{
    public byte[]? GetNullableFileContent(string itemId);
    public byte[] GetFileContent(string itemId);
}

public class Implementation : IInterface
{
    public byte[]? GetNullableFileContent(string itemId) => null; // Should not raise because nullable is allowed
    public byte[] GetFileContent(string itemId) => null; // Should raise because null is unexpected
}

Expected behavior

Should not raise when nullable is enabled and the syntax is used.
For me there is a big difference between an empty list and a not-existing list. Those are two completely different statements.
On the one hand, not existing means that there is no information about it. On the other hand, an empty list means that, for example, no information has been entered.

For example:
Returning null means "there is no file". Returning an empty array means "there is a file but this file is empty".

I mean rule is very useful when it is required that the value is notnull. When the annotation allows nullable values it is very annoying because the nullables allows the difference between "there is nothing" and "there is something empty".

I think this rule should not be triggered when nullable is enabled and used because the compiler (on newer versions) warns when not checking a nullable value

Actual behavior

Rule will be raised when nullable is enabled and used

@mary-georgiou-sonarsource
Copy link
Contributor

Thank you for opening this issue, @nesc58. Your feedback helps us improve our products.

I confirm this is an FP.
I added the issue to our backlog to be handled in one of our next sprints.

Best regards
Mary

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. Type: False Positive Rule IS triggered when it shouldn't be.
Projects
Best Kanban
  
Done
Development

Successfully merging a pull request may close this issue.

3 participants