Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add analyzer: Warn on illegal format specifiers #45960

Open
MaStr11 opened this issue Dec 11, 2020 · 2 comments
Open

Add analyzer: Warn on illegal format specifiers #45960

MaStr11 opened this issue Dec 11, 2020 · 2 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Runtime code-analyzer Marks an issue that suggests a Roslyn analyzer
Milestone

Comments

@MaStr11
Copy link

MaStr11 commented Dec 11, 2020

Based on feedback from issue dotnet/roslyn#49471

Format specifiers are often compile-time constants that can be checked by an analyzer. This could be done in roslyn itself or as an SDK analyzer. For some specifiers a fix can be provided: dt.ToString("h") -> dt.ToString("%h").

var dt = new DateTime(2020, 12, 24, 6, 30, 45);
dt.ToString("h") // Fails at runtime
dt.ToString("h ")
dt.ToString("%h")
$"{dt:h}" // Fails
string.Format("{0:h}", dt) // Fails at runtime
string.Format("{0:/}", dt) // Fails at runtime
string.Format("{0::}", dt) // Fails at runtime
string.Format("{0:\\}", dt) // Fails at runtime
string.Format("{0:,}", dt) // Fails at runtime
string.Format("{0,d}", 2) // Fails at runtime
string.Format("{0", 3) // Fails at runtime

Sample on sharplab.io

The analyzer should detect format specifiers and check whether the specifier is valid. One possibility could be to simulate the behavior at runtime by passing default values:
Parsed code:

var dt = new DateTime(2020, 12, 24, 6, 30, 45);
string.Format("Hour of day {0:h}", dt);

In the analyzer the logic could be something like this:

...
InvocationExpressionSyntax stringFormatInvocation;
var firstArgument = stringFormatInvocation.ArgumentList[0];
var remainingArguments = stringFormatInvocation.ArgumentList[1..];
var formatString = semanticModel.GetConstantValue(firstArgument);
var args = remainingArguments.Select(a => 
    GetDefaultValueOfTypeIfCLRTypeOtherwiseNull(semanticModel.GetTypeInfo(a).Type)).ToArray();
try
{
  string.Format(formatString, args);
}
catch (Exception ex)
{
// Create a diagnostic.
}

If an custom format provider is used, the analyzer should do nothing.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Dec 11, 2020
@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@danmoseley danmoseley added area-System.Runtime code-analyzer Marks an issue that suggests a Roslyn analyzer labels Dec 11, 2020
@tannergooding tannergooding removed the untriaged New issue has not been triaged by the area owner label Jul 12, 2021
@tannergooding tannergooding added this to the Future milestone Jul 12, 2021
@buyaa-n
Copy link
Member

buyaa-n commented Nov 9, 2022

Sounds like could be covered by #64009

@buyaa-n buyaa-n added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Runtime code-analyzer Marks an issue that suggests a Roslyn analyzer
Projects
None yet
Development

No branches or pull requests

5 participants