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 FP: Don't raise if parameter is captured #7060

Closed
martin-strecker-sonarsource opened this issue Apr 12, 2023 · 0 comments · Fixed by #7065
Closed

Fix S3900 FP: Don't raise if parameter is captured #7060

martin-strecker-sonarsource opened this issue Apr 12, 2023 · 0 comments · Fixed by #7065
Assignees
Labels
Area: C# C# rules related issues. Sprint: SE Short-lived* label for epic MMF-3077 *troll Type: CFG/SE FPs Rule IS triggered when it shouldn't be for CFG and SE rules.
Projects
Milestone

Comments

@martin-strecker-sonarsource
Copy link
Contributor

Description

S3900 is falsely raised when the parameter is captured:

    public string InsideLambda(string s)
    {
        if (s == null)
        {
            return null;
        }

        Func<string> someFunc = () => s.ToString();

        return s.ToString(); // Noncompliant FP: S3900 "Refactor this method to add a validation of parameter 's'"
    }

Root cause

When we enter the block after the return, s has the NotNull constraint as expected. s is also in the captured list of LiveVariableAnalysisBase. Therefore it is not included in the LiveOut list:

        /// <summary>
        /// LiveOut variables are alive when exiting block. They are read in any of it's successors.
        /// </summary>
        public IReadOnlyCollection<ISymbol> LiveOut(TBlock block) =>
            blockLiveOut[block].Except(captured).ToImmutableArray();

And that is why it gets removed in ProcessBranchState:

var liveVariables = lva.LiveOut(branch.Source).ToHashSet();
return state.RemoveSymbols(x => lva.IsLocal(x) && (x is ILocalSymbol or IParameterSymbol { RefKind: RefKind.None }) && !liveVariables.Contains(x))
.ResetOperations();

@martin-strecker-sonarsource martin-strecker-sonarsource added Area: C# C# rules related issues. Type: CFG/SE FPs Rule IS triggered when it shouldn't be for CFG and SE rules. labels Apr 12, 2023
@pavel-mikula-sonarsource pavel-mikula-sonarsource added the Sprint: SE Short-lived* label for epic MMF-3077 *troll label Apr 13, 2023
@pavel-mikula-sonarsource pavel-mikula-sonarsource added this to the 8.56 milestone Apr 13, 2023
@pavel-mikula-sonarsource pavel-mikula-sonarsource changed the title Fix S3900 FP: S3900 is raised for Parameters captured in a lambda Fix S3900 FP: Don't raise if parameter is captured Apr 14, 2023
Best Kanban automation moved this from In progress to Validate Peach Apr 14, 2023
@martin-strecker-sonarsource martin-strecker-sonarsource moved this from Validate Peach to Done in Best Kanban Apr 18, 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: CFG/SE FPs Rule IS triggered when it shouldn't be for CFG and SE rules.
Projects
Best Kanban
  
Done
Development

Successfully merging a pull request may close this issue.

2 participants