Skip to content

C#: Improve cs/invalid-string-formatting and add to the Code Quality suite. #19148

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

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

michaelnebel
Copy link
Contributor

@michaelnebel michaelnebel commented Mar 28, 2025

In this PR we

  • Add the cs/invalid-string-formatting to the code quality suite.
  • Re-write tests to use inline expectations tests; In this process a couple of faulty testcases were discovered and these were corrected (for instance - we only check that format strings are valid in case there are insertions).

Improve the cs/invalid-string-formatting query by

  • Adding invalid string format results for string.Format where no additional arguments are provided (for instance string.Format("{")). Also start taking CompositeFormat versions of Format methods into account and consider System.Text.CompositeFormat.Parse a method that parses format strings and might throw exceptions and cause a runtime crash.
  • Add support for more format methods like generic versions of string.Format and StringBuilder.AppendFormat and also add support for MemoryExtensions.TryWrite.
  • Remove false positives related to Console.WriteLine(string) and friends (methods that itsn't format methods).
  • Remove false positives related to `Console.WriteLine(string, ReadOnlySpan) (methods that use params collection type overloads).

    Review commit by commit is encouraged.

    @hvitved : The data flow configurations are quite similar. Is it a better approach to use flow state to track calls to CompositeFormat.Parse? (instead of merging two similar graphs).

@github-actions github-actions bot added the C# label Mar 28, 2025
@michaelnebel michaelnebel force-pushed the csharp/invalid-string-format branch 5 times, most recently from cfe6cbd to 286ae79 Compare April 8, 2025 09:29
@michaelnebel michaelnebel force-pushed the csharp/invalid-string-format branch 2 times, most recently from 852eab9 to e8f8145 Compare April 9, 2025 12:43
@michaelnebel
Copy link
Contributor Author

DCA

  • There doesn't appear to be any changes to performance or results for our DCA suite.
  • The level produced on our DCA suite for this query looks acceptable (query is not unreasonable noisy).

@michaelnebel michaelnebel force-pushed the csharp/invalid-string-format branch from 51a03d2 to be8d379 Compare April 10, 2025 06:48
@michaelnebel michaelnebel marked this pull request as ready for review April 10, 2025 08:12
@michaelnebel michaelnebel requested a review from a team as a code owner April 10, 2025 08:12
@michaelnebel michaelnebel changed the title C#: Add cs/invalid-string-formatting to the codeql quality suite. C#: Improve cs/invalid-string-formatting and add to the codeql quality suite. Apr 10, 2025
@michaelnebel michaelnebel force-pushed the csharp/invalid-string-format branch from be8d379 to 5c6b1dd Compare April 10, 2025 11:46
@michaelnebel michaelnebel changed the title C#: Improve cs/invalid-string-formatting and add to the codeql quality suite. C#: Improve cs/invalid-string-formatting and add to the Code Quality suite. Apr 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant