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

Text on a InterpolatedVerbatimStringStartToken token #33747

Merged
merged 3 commits into from
Mar 2, 2019

Conversation

jcouv
Copy link
Member

@jcouv jcouv commented Feb 28, 2019

Fixes #33713

@jcouv jcouv self-assigned this Feb 28, 2019
@jcouv jcouv force-pushed the interpolated-text branch from 6d356bb to 1768e77 Compare February 28, 2019 04:05
@jcouv jcouv marked this pull request as ready for review February 28, 2019 06:36
@jcouv jcouv requested a review from a team as a code owner February 28, 2019 06:36
@jcouv jcouv changed the base branch from dev16.0 to master February 28, 2019 06:36
@jcouv jcouv added this to the 16.1.P1 milestone Feb 28, 2019

var token3 = SyntaxFactory.Token(SyntaxKind.InterpolatedVerbatimStringStartToken);
Assert.Equal("$@\"", token3.Text);
Assert.Equal("$@\"", token3.ValueText);
Copy link
Member

Choose a reason for hiding this comment

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

Consider splitting the test into two, where this is the entire test associated with the bug.

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps move to SyntaxFactoryTests.


In reply to: 261397002 [](ancestors = 261397002)

@@ -15,6 +15,14 @@ namespace Microsoft.CodeAnalysis.CSharp.UnitTests
{
public class SyntaxFactoryTests : CSharpTestBase
{
[Fact, WorkItem(33713, "https://github.com/dotnet/roslyn/issues/33713")]
public void AlternateVerbatimString()
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be possible to add a test that loops through all tokens and checks that Text and ValueText are not null or empty?

Copy link
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

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

LGTM (iteration 2)

@jcouv jcouv closed this Mar 2, 2019
@jcouv jcouv reopened this Mar 2, 2019
@jcouv jcouv merged commit 880d35a into dotnet:master Mar 2, 2019
@jcouv jcouv deleted the interpolated-text branch March 2, 2019 23:46
333fred added a commit to 333fred/roslyn that referenced this pull request Mar 4, 2019
* dotnet/master: (903 commits)
  Add UseEnhancedColors to FeatureOnOffOptionsProvider (dotnet#33802)
  Update TypeStyle.cs
  Text on a InterpolatedVerbatimStringStartToken token (dotnet#33747)
  added ability to clear all diagnostics reproted from a IDiagnosticUpd… (dotnet#33805)
  Use a set instead of map to define processed redundant nodes.
  EnC: Remove dependency on solution from AnalyzeDocumentAsync (dotnet#33796)
  Add workitem links to new redundant assignment tests.
  Move leading trivia with node when removing unused values.
  Remove OOP related feature options. (dotnet#31644)
  Merge pull request dotnet#33631 from CyrusNajmabadi/wrapPriority
  Use the correct iteration count in IterationDataAttribute
  Handle interface members in NullableWalker.AsMemberOfType (dotnet#33764)
  Cleanly work around microsoft/nodejstools#2138
  Implement full spec changes for Index/Range (dotnet#33679)
  Further reduce expectations on deep fluent calls
  Lower expectation for deep fluent call
  Take screenshot after writing logs
  Avoid throwing exception in static constructor
  Offer to add parameter to constructor with no existing parameters (dotnet#33624)
  Add regression test for nullable crash (dotnet#33735)
  ...
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.

5 participants