Tests for safety of string format calls

[HebaruSan]

Motivation

My first code fix for CKAN was to remove a superfluous String.Format call that was causing exceptions to be thrown because externally-generated strings were being used as part of a format string (see #2111).

This problem generalizes; any call to IUser.RaiseMessage or IUser.RaiseError can throw an exception if the first parameter isn't a valid format string. We can ensure that a string literal is safe by looking at it, and we can ensure that an i18n resource is safe by inspecting our resx files, but anytime a value is externally sourced, it isn't safe to use it as a format string because curly braces could turn up any time and ruin our day.

In principle, the C# compiler could check this and emit warnings similar to what it does for nullable references, but currently it doesn't. The closest thing is the StringSyntax attribute, which can be used to mark a parameter as a format string, but which doesn't have any built-in functionality associated with it.

Changes

• Now a new MonoCecilAnalysisTestsBase base class is factored out of the test from Add test to check GUI thread safety #3914
• Now a new StringFormatSafetyTests class inherits from MonoCecilAnalysisTestsBase and scans all code in all of our assemblies for calls to functions with the StringSyntax attribute. These calls are deemed safe if the argument list is non-empty, if the format string is a string literal, or if the format string is from Properties.Resources; otherwise a test failure is logged.
• Now our format string parameters are tagged with the StringSyntax attribute
• Now the 67 existing places that fail the new test are updated to use a format string of "{0}", or to remove a superfluous call to string.Format, or other such solutions

This guard rail will help us to write safer code.

[techman83]

Safer code and tests! 😍