Roslyn: Rationalize CA2241 and IDE0043 (detect incorrect arguments to string formatting methods)

Created on 15 Jul 2020  路  4Comments  路  Source: dotnet/roslyn

Both these analyzers attempt to detect incorrect arguments to formatting methods, but have differences in functionality and/or options.

Differences:

  1. CA2241 is configurable and has an editorconfig option such it operates on well known string format method + any user specified methods that matches the formatting method signature. See https://docs.microsoft.com/visualstudio/code-quality/ca2241?view=vs-2019#additional-string-formatting-methods for details.
  2. CA2241 does not fire on skipped placeholder sequence, while IDE0043 does: See https://github.com/dotnet/roslyn-analyzers/issues/1254

We should unify towards a single implementation, instead of supporting duplicate analyzers, especially given both of these will ship in the box (one with .NET SDK and one with VS IDE) soon.

I propose that we keep CA2241 and retire IDE0043 based on the following:

  1. CA2241 has been around for longer (from FxCop days) and has been properly documented.
  2. CA2241 has richer options for analyzing customized user specified formatting methods
  3. This analysis is more of a code quality rule, rather then a code style rule, and we put the former as CA rules in analyzers repo.
Area-IDE Concept-Continuous Improvement help wanted

Most helpful comment

Am I invited to the design meeting? ;)

All 4 comments

Tag @sharwell @Evangelink. Lets bring this up for next Monday's design meeting.

Am I invited to the design meeting? ;)

Design meeting notes
We'll consolidate this to report as CA2241 which should be a superset of the features present in both CA2241 and IDE0043. IDE0043 will be retired. @mavasani to comment on if we should remove it entirely or keep the definition but mark as not configurable to enable unused suppression rules to work.

This is already being tracked with a separate issue in roslyn-analyzers repo and @Evangelink and @jmarolf are working towards getting the corresponding PR in. Closing this issue.

Was this page helpful?
0 / 5 - 0 ratings