CA2100 is being reported for literal strings with no parameters.

[jinujoseph]

Reported by vsfeedback here Code analysis is flagging two types of non-parameter SQL as violations. Ones that are plain string values, and ones that are constructed via StringBuilder but converted to string prior to being sent to the command.

using FirebirdSql.Data.FirebirdClient;

// this is a string builder that is getting flagged
public string BuildRawNumbersPoolCreateTableStatement()
{
    var sql = new StringBuilder();
    sql.AppendLine("CREATE TABLE raw_numbers_pool (                                     ");
    sql.AppendLine("        id  BIGINT  GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY    ");
    sql.AppendLine("        , serial_number     BIGINT NOT NULL                         ");
    sql.AppendLine("        , ordering_token    BIGINT NOT NULL                         ");
    sql.AppendLine("        , is_used           BOOLEAN NOT NULL                        ");
    sql.AppendLine("    );                                                              ");
    return sql.ToString();
}

//this is a plain string that is getting flagged
public string BuildStatementForRemainingCount()
{
     return "SELECT COUNT(*) FROM raw_numbers_pool WHERE is_used IS FALSE";
}

// this is an example usage, the FbCommand is the line that is flagged
public int NumbersRemaining()
{ 
    int result;
    var connString = GetConnectionString();
    using (var conn = new FbConnection(connString))
    {
        var cmd = new FbCommand(_schemaBuilder.BuildStatementForRemainingCount(), conn);
        conn.Open();
        result = Convert.ToInt32(cmd.ExecuteScalar());
    }
    return result;
 }

[StingyJack]

@jinujoseph - You also linked to #628, but this does not require data flow analysis. The strings are all effectively constants, no string formatting or interpolation.

[mavasani]

This is a special case, but in general flow analysis is needed for precise results. Also note that current plan for DFA is only intra-procedural analysis. V2 might have inter procedural analysis which would detect such cases.

[StingyJack]

How long has this rule been crippled like this? I remember it working much better than this all the way back to CAT.NET and FxCop.

[mavasani]

Update: One of the cases mentioned here works now on the latest prerelease package.

1. Install https://www.nuget.org/packages/Microsoft.CodeAnalysis.FxCopAnalyzers/2.9.0-beta1.final
2. Edit the project file as per https://github.com/dotnet/roslyn-analyzers/issues/2003#issuecomment-453661844 to enable flow-analysis feature.
3. Add a file named .editorconfig at the root of the project directory with the following content to enable interprocedural analysis of values: dotnet_code_quality.interprocedural_analysis_kind = ContextSensitive See https://github.com/dotnet/roslyn-analyzers/blob/master/docs/Analyzer%20Configuration.md for more details.

[mavasani]

Just to clarify: The second scenario (BuildStatementForRemainingCount) works now with the above mentioned steps. The first scenario requires https://github.com/dotnet/roslyn-analyzers/issues/1547 (analysis to understand different well known string manipulation APIs).

[StingyJack]

This needs to work correctly (no CA violation) without any kind of .editorconfig hack.

There are no variables being concatenated to build the statements, and no parameters being supplied while executing them.

[mavasani]

This needs to work correctly (no CA violation) without any kind of .editorconfig hack.

This is not a hack. This is a supported configuration mechanism to enable richer analyses. By default this analyzer only analyses single method and assumes values returned from interprocedural calls to be unknown/non-constant. The option configuration mentioned above switches the analysis to more precise, and more expensive interprocedural analysis mode. We might make that the default mode in future, but until then, the solution provided above is the only recommended and supported approach.

[StingyJack]

@mavasani - that seems like a lot of engineering to address the problem of a CA violation being reported when...

There are no variables being concatenated to build the statements, and no parameters being supplied while executing them

I don't recall having to enable anything special in CAT.net or FxCop to avoid blatant false positives. I don't get why I would have to set a special, semisecret value in an file I'm not currently using (and have never had to use before.)

only recommended and supported approach.

That kind of statement is only remotely acceptable when the default option is not broken in a regressed, silly way.

[StingyJack]

This is a big enough "whoops" that this issue should have a big REGRESSION tag. I used to use CAT.net as part of GCI builds to flag improperly crafted SQL (concatenated), and the rule worked correctly (when it flagged something - CAT.net just couldn't evaluate consistently, which is why it was dropped.)

Big talk of algorithms and dataflow analysis being required to correct this is just a weak attempt at bamboozling.

They are data-free-strings. If the rule can't analyze properly it should say that or say nothing, and not fill build logs and error lists with nonsense.