CA2241 - Possibility to support custom string formatting methods?

[mayerj]

Analyzer package

Microsoft.NetCore.Analyzers

Package Version

v2.9.4

Diagnostic ID

CA2241

Repro steps

Have a custom method that accepts the typical format string + params args.

IE:
public static string Log(string format, params object[] args)
{
...
}

Expected behavior

Log("{1}", 2) triggers CA2241

Actual behavior

CA2241 is hardcoded to look at Console.Write(Line) and string.Format, so no error.

I assume adding this would be a breaking change WRT backwards compatibility, but could this be made configurable?

[mavasani]

Yes, certainly. I am planning to address bunch of the configurable functionality requests today, and this seems a reasonable request.

[paulomorgado]

This would be better served with an attribute introduced by the BCL that would mark format template and parameters.

[manfred-brands]

Trying to get standard attributes into the BCL is hard (we are still waiting on IDisposable ones)

The pattern using the parameters: 'string format, params object[] arguments' is so common, should it not detect this instead of forcing everybody to even add standard methods like StringBuilder.AppendFormat to every .editorconfig file?

In our codebase we would have to add dozens of methods
The rule already has hard-coded the name of the "format" specification parameter. It could check if it is followed by a 'params object[]'.

I might give adding this a try myself.

[paulomorgado]

@manfred-brands.

add standard methods like StringBuilder.AppendFormat to every .editorconfig file

What do you mean by that?

[manfred-brands]

StringBuilder.AppendFormat is a 'string formatting' method that is not hard-coded in the source code of the analyzer. By default therefore the rule will not check parameters.
Try the following:

            var stringBuilder = new StringBuilder();
            stringBuilder.AppendFormat(CultureInfo.InvariantCulture, "The Value is {0}");
            stringBuilder.Append(string.Format(CultureInfo.InvariantCulture, "The Value is {0}"));

The analyzer only complains about the last line missing a parameter.
Note that the configuration doesn't seem to work for me. I cannot get an error if I add:
dotnet_code_quality.CA2241.additional_string_formatting_methods = AppendFormat
nor if I use the fully qualified name: System.Text.StringBuilder.AppendFormat.

[mavasani]

@manfred-brands Does your editorconfig lie in the project folder or repo folder? If it’s the latter, you might need to move to a 3.3.x prerelease package. Can you check https://github.com/dotnet/roslyn-analyzers/blob/master/docs/Analyzer%20Configuration.md#enabling-editorconfig-based-configuration?

[manfred-brands]

I added a pull request for this: #3653.
It would make configuration or attributes superfluous.

[manfred-brands]

@mavasani The .editorconfig is above the projects as it is common to all projects in the solution. The config works for enabling/disabling rules, so it is read. I will try the workaround.

[manfred-brands]

@mavasani For the workaround to work, it looks I need to have two copies of .editorconfig. One in the solution directory that is used to find what rules are configured and the other one in the specified location to find the additional configuration for the rules.