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 S4158: "Empty collections" raises false positive when analyzed method has too many branches #767

Closed
nnpcYvIVl opened this issue Sep 11, 2017 · 2 comments
Assignees
Labels
Type: False Positive Rule IS triggered when it shouldn't be.
Milestone

Comments

@nnpcYvIVl
Copy link

A false positive S4158 is thrown on the line:

datagridview[columnname, datagridviewrow.Index].Style.BackColor = columnnamecolordictionary[columnname];
public static void ColorRow(UserControl usercontrol, DataGridViewRow datagridviewrow, List<string> columnnamelist)
{
    string name = usercontrol.Name.Replace("UserControl", string.Empty);
    DataGridView datagridview = datagridviewrow.DataGridView;

    Dictionary<string, Color> columnnamecolordictionary = new Dictionary<string, Color>();

    for (int i = 0; i < datagridviewrow.DataGridView.Columns.Count; i++)
    {
        foreach (string columnname in columnnamelist)
        {
            if ((columnname == name || columnname == "*") && columnname == datagridviewrow.DataGridView.Columns[i].Name && datagridviewrow.Cells[i].Value != DBNull.Value && !string.IsNullOrWhiteSpace(datagridviewrow.Cells[i].Value.ToString()))
            {
                if (columnname == "=")
                {
                    // do nothing
                }

                columnnamecolordictionary.Add(columnname, Color.AliceBlue);
            }
        }
    }

    foreach (string columnname in columnnamecolordictionary.Keys)
    {
        if (datagridview.Columns.Contains(columnname))
        {
            datagridview[columnname, datagridviewrow.Index].Style.BackColor = columnnamecolordictionary[columnname];
        }
    }
}
@valhristov
Copy link
Contributor

The false positive is caused because of our limited support for loops - the rule cannot detect that you are filling the dictionary with items a few lines above, hence it raises an issue.

@valhristov valhristov added Area: Rules Type: False Positive Rule IS triggered when it shouldn't be. labels Sep 12, 2017
@valhristov valhristov changed the title S4158 False Positive Fix S4158: "Empty collections" raises false positive on collections populated within a loop Sep 12, 2017
@valhristov valhristov self-assigned this Feb 1, 2018
@valhristov
Copy link
Contributor

Apparently I diagnosed the problem wrong, it is caused because the method has rather many branches and our dataflow analysis engine reaches the limit of explorations count (1000). We would like to avoid increasing the limit for now because it could have performance impact. The rule will currently not report if it detects that the exploded graph exploration did not finish correctly (exactly as the condition evaluates to constant rule).

@valhristov valhristov changed the title Fix S4158: "Empty collections" raises false positive on collections populated within a loop Fix S4158: "Empty collections" raises false positive when analyzed method has too many branches Feb 1, 2018
@valhristov valhristov added this to the 6.8 milestone Feb 1, 2018
@ghost ghost removed the Status: Needs Review label Feb 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: False Positive Rule IS triggered when it shouldn't be.
Projects
None yet
Development

No branches or pull requests

2 participants