Skip to content

Escape curly braces in string literals when converting concatenation to interpolated string#23589

Merged
mavasani merged 2 commits intodotnet:masterfrom
rik-smeets:escape-curly-braces-in-interpolated-string-refactoring
May 9, 2018
Merged

Escape curly braces in string literals when converting concatenation to interpolated string#23589
mavasani merged 2 commits intodotnet:masterfrom
rik-smeets:escape-curly-braces-in-interpolated-string-refactoring

Conversation

@rik-smeets
Copy link
Copy Markdown
Contributor

Customer scenario

Customer has a string concatenation where the string literal contains curly braces. For example:

var firstExample = "Something {X} = " + 9;
var secondExample = "Something {" + 9 + "}";

Using the "Convert to interpolated string" refactoring does not escape the curly braces in the string literal(s). When applying the refactoring, this can lead to compile errors or unmeant escapes:

// results as of now:
var firstExample = $"Something {X} = {9}"; // compile error: {X} does not exist in context
var secondExample = $"Something {{9}}"; // '9' is escaped when it should not be

Bugs this fixes

#23536

Workarounds, if any

Escape the braces manually in the string literal(s) before or after applying the refactoring.

Risk

The scope of the fix is limited to this specific refactoring.

Performance impact

Low, the fix only adds two extra string operations (which are only executed if necessary).

Is this a regression from a previous update?

I don't know.

Root cause analysis

This specific situation was not covered by unit tests yet. I added these tests now.

How was the bug found?

Customer reported, see #23536.

Test documentation updated?

No impact on test documentation.

@rik-smeets rik-smeets requested a review from a team as a code owner December 5, 2017 18:44
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add tests with verbatim literals as well? Thanks!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your review. I added two unit tests for verbatim literals as well.

Copy link
Copy Markdown
Contributor

@jmarolf jmarolf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! I would just add the tests Cyrus suggested.

@jcouv jcouv added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label Mar 31, 2018
@jinujoseph
Copy link
Copy Markdown
Contributor

test windows_debug_vs-integration_prtest

@jinujoseph jinujoseph closed this May 8, 2018
@jinujoseph jinujoseph reopened this May 8, 2018
@mavasani
Copy link
Copy Markdown
Contributor

mavasani commented May 9, 2018

@jinujoseph for approval

@jinujoseph
Copy link
Copy Markdown
Contributor

Approved to merge for 15.8.Preview2

@mavasani mavasani merged commit a6cdbf5 into dotnet:master May 9, 2018
@rik-smeets rik-smeets deleted the escape-curly-braces-in-interpolated-string-refactoring branch May 9, 2018 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved to merge Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants