Use StringBuilder.Append(char) for single character strings

[terrajobst]

It's common to see calls to StringBuilder.Append(string) with a const string containing a single character, e.g. ",". These would be slightly cheaper as calls using a const char instead.

Category: Performance

[jeffhandley]

Estimates:

• Analyzer: Small
• Fixer: Small

[pgovind]

Quick question about a corner case here:

Consider the following code:

            const string aa = "a", bb = "b";
            sb.Append(aa); //sb is a StringBuilder

The analyzer would detect const string aa = "a" as the element to warn about, but Roslyn considers the whole line const string aa = "a", bb = "b"; as a single declaration node. We have 2 options here:

1. If we want to offer a code fix, we should end up with the following code:

            const char aa = "a";
            const string bb = "b";
            sb.Append(aa); //sb is a StringBuilder

OR
2) We bail out when we detect multiple declarations and don't offer a code fix (but the analyzer could still report a "Prefer char over string" message)

Which do we want?

P.S: There is also the more straightforward case when we have:

            const string aa = "a", bb = "b";
            sb.Append(aa).Append(bb); //sb is a StringBuilder

where the code-fix is more obvious and just results in:

            const char aa = 'a', bb = 'b';

[jeffhandley]

I recommend that we just bail on multiple declarations instead of trying to get clever.

[pgovind]

There are only 2 real corner cases to consider in this Analyzer:

1. Multiple declarations in a single line. This has already been addressed above.
2. Class fields declared as const strings. I decided to not tag this in the analyzer and only tag(and fix) local const string variable declarations.

1 issue to consider in the code-fix is that it can lead to build breaks in some cases. For ex:

// Assume we have the following method declaration
public string AMethod(string foo)
{

}

// Somewhere later we have
const string str = "a";
stringBuilder.Append(str); // This will tag the analyzer and suggest a code-fix
AMethod(str); // If the code-fix was invoked, this will now be a compiler error since str will now be a char.

@jeffhandley