Skip to content

Enable tests and update features to support new language changes around patterns.#45762

Merged
5 commits merged intodotnet:masterfrom
CyrusNajmabadi:enableTests2
Jul 14, 2020
Merged

Enable tests and update features to support new language changes around patterns.#45762
5 commits merged intodotnet:masterfrom
CyrusNajmabadi:enableTests2

Conversation

@CyrusNajmabadi
Copy link
Contributor

No description provided.

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner July 7, 2020 21:05
[Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
public async Task TestForSwitchCase_SemanticCheck_NotAfterPredefinedType() =>
await VerifyAbsenceAsync(AddInsideMethod(@"switch (new object()) { case int $$ }"));
await VerifyKeywordAsync(AddInsideMethod(@"switch (new object()) { case int $$ }"));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the newly allowed behavior. i.e. you can say case int when ... instead of case int i when ...

[Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
public async Task TestForSwitchCase_SemanticCheck_AfterLocalConstantVar() =>
await VerifyKeywordAsync(AddInsideMethod(@"const object var = null; switch (new object()) { case var $$ }"));
await VerifyAbsenceAsync(AddInsideMethod(@"const object var = null; switch (new object()) { case var $$ }"));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i specifically regressed this behavior because:

  1. teh logic to support it was such a pain to keep
  2. supporting users explicitly namign things like var is explicitly carved out as something we do not cater for in the IDE.

{
return context.IsCatchFilterContext ||
(IsAfterCompleteExpressionOrPatternInCaseLabel(context, out var expressionOrPattern) &&
!IsTypeName(expressionOrPattern, context.SemanticModel, cancellationToken));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

all the logic removed was just jumping through hoops to try to prevent when from showing up too much and to support case var when. i.e. having var not be a variable-pattern, but rather some sort of constant pattern. It was intefering with supporting real cases simply and just added too much complexity for a scenario we do not consider important. So it's all been removed and replaced with something much simpler and easier to maintain.

Copy link
Contributor

@allisonchou allisonchou left a comment

Choose a reason for hiding this comment

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

lgtm!

}

// Never show `when` after `var` in a pattern. It's virtually always going to be unhelpful as the user is
// far more likely to be writing `case var someName...` rather than typing `cae var when...` (in the case
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: cae -> case

}
";
await VerifyNoItemsExistAsync(markup);
await VerifyItemExistsAsync(markup, "P1");
Copy link
Contributor

Choose a reason for hiding this comment

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

Just trying to better understand, why was this file changed? Does it have to do with the when keyword or is it something separate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this used to be illegal in C# but became legal in C# 9.0. i.e. you can continually nest recursive patterns within each other :)


[Fact(Skip = "https://github.com/dotnet/roslyn/issues/40015"), Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
[Fact, Trait(Traits.Feature, Traits.Features.KeywordRecommending)]
public async Task TestForSwitchCase_SemanticCheck_NotAfterPredefinedType_BeforeBreak() =>
Copy link
Contributor

Choose a reason for hiding this comment

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

Update the names of the tests to no longer say "Not" when we expect it

public async Task TestForSwitchCase_SemanticCheck_AfterTypeAliasAndFieldConstantVar()
{
await VerifyKeywordAsync(@"
await VerifyAbsenceAsync(@"
Copy link
Contributor

Choose a reason for hiding this comment

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

Update test names to have "Not"

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Auto-approval

@ghost ghost merged commit 0196341 into dotnet:master Jul 14, 2020
@CyrusNajmabadi CyrusNajmabadi deleted the enableTests2 branch April 11, 2021 18:24
This pull request was closed.
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.

4 participants