Fix a formatting exception when {} is used as a dictionary key. #3008
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
3a4911f to
955dafc
Compare
Pull Request Test Coverage Report for Build 13420695412Details
💛 - Coveralls |
Qodana for .NET2 new problems were found
💡 Qodana analysis was run in the pull request mode: only the changed files were checked Contact Qodana teamContact us at qodana-support@jetbrains.com
|
Member
|
Should the latest SAVEPOINT commit be reverted from this branch? |
877c451 to
b2b446d
Compare
b729b9c to
9fe43a7
Compare
Member
Author
Screwed up my branches. Should be fine now. |
bac0db7 to
1479f04
Compare
jnyrup
reviewed
Feb 14, 2025
1479f04 to
3406800
Compare
3406800 to
0bbe8a7
Compare
jnyrup
reviewed
Feb 18, 2025
| // Matches the .NET string format {index[,alignment][:formatString]}, even | ||
| // if the opening and closing curly braces are escaped themselves. | ||
| var matches = Regex.Matches(message, | ||
| @"\{+[0-9]+(,-?[0-9]+)?(:[a-zA-z.,%‰+-\\'"";]+)?\}+"); |
Member
There was a problem hiding this comment.
Where did you find this regex?
(It also has a bug A-z should be A-Z)
E.g. {0:µ} is a valid formatString.
According to the implementation the formatString can be anything except { and }.
Here's another set of tests where the two last still fail with this PR as we mistake them with placeholders.
[Theory]
[InlineData("{")]
[InlineData("}")]
[InlineData("{}")]
[InlineData("{0}")]
[InlineData("{{0}}")]
public void Can_handle_braces_in_dictionary_keys(string key)
{
// Arrange
var subject = new Dictionary<string, string> { [key] = "" };
var expectation = new Dictionary<string, string> { [key] = null };
// Act
var act = () => expectation.Should().BeEquivalentTo(subject);
// Assert
act.Should().Throw<XunitException>()
.WithMessage($"Expected expectation[{key}] to be \"\", but found <null>.*");
}I think the only proper way to solve this would be to avoid early string interpolation.
Anything else will probably be a lost game that only adds complexity and fragility.
0bbe8a7 to
ff19864
Compare
ff19864 to
6d8d1b0
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request includes several changes to improve the handling of placeholders and formatting in various parts of the codebase, making a lot of calls to
EscapePlaceholdersunnecessary. With this, strings like{}in dictionary keys will no longer cause crashes.Resolves #2704