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

Bump code style package to 3.9.0 #4919

Merged
merged 12 commits into from
Apr 14, 2021
Merged

Bump code style package to 3.9.0 #4919

merged 12 commits into from
Apr 14, 2021

Conversation

Youssef1313
Copy link
Member

Fixes #4659

@Youssef1313 Youssef1313 requested a review from a team as a code owner March 3, 2021 16:17
Base automatically changed from master to main March 5, 2021 02:20
@Youssef1313 Youssef1313 force-pushed the patch-14 branch 2 times, most recently from e07d20a to f66766a Compare March 5, 2021 15:28
@Youssef1313 Youssef1313 marked this pull request as draft March 5, 2021 15:57
@Youssef1313
Copy link
Member Author

@mavasani @sharwell Should I disable IDE0078 instead of adding lots of suppressions?

@@ -354,7 +354,7 @@ private static bool IsOnObsoleteMemberChain(ISymbol symbol, WellKnownTypeProvide
while (symbol != null)
{
allAttributes.AddRange(symbol.GetAttributes());
symbol = symbol is IMethodSymbol method && method.AssociatedSymbol != null
symbol = symbol is IMethodSymbol { AssociatedSymbol: not null } method
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a manual change. The codefix produces invalid code.

Copy link
Member

Choose a reason for hiding this comment

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

Has the bug been filed?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, this is dotnet/roslyn#51691.

Comment on lines 124 to 125
}
else if (type is INamedTypeSymbol namedTypeSymbol
&& namedTypeSymbol.EnumUnderlyingType != null)
else if (type is INamedTypeSymbol { EnumUnderlyingType: not null } namedTypeSymbol)
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a manual change. The codefix produces invalid code.

@sharwell
Copy link
Member

sharwell commented Apr 8, 2021

Have we also updated the getting started guide to indicate that VS 16.9+ is required?

@@ -84,7 +84,9 @@ public override void Initialize(AnalysisContext context)
}

static bool IsDelegateTypeWithInvokeMethod(INamedTypeSymbol namedType) =>
#pragma warning disable IDE0078 // Use pattern matching - https://github.com/dotnet/roslyn/issues/51691
namedType.TypeKind == TypeKind.Delegate && namedType.DelegateInvokeMethod != null;
Copy link
Member

Choose a reason for hiding this comment

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

❔ Is this not the same?

Suggested change
namedType.TypeKind == TypeKind.Delegate && namedType.DelegateInvokeMethod != null;
namedType is { TypeKind: TypeKind.Delegate, DelegateInvokeMethod: not null };

Copy link
Member Author

Choose a reason for hiding this comment

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

@sharwell There are actually more IDE0078 violations than the ones I suppressed. Some of them are actionable like you suggested, and others doesn't seem to be actionable. I'm thinking of setting the rule to silent for now until the analyzer and codefix bugs are fixed to avoid the many suppressions and manual fixes. Would you be okay with that?

Copy link
Member

Choose a reason for hiding this comment

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

Certainly

@@ -193,7 +193,9 @@ public static ImmutableArray<IOperation> GetTopmostExplicitDescendants(this IOpe
/// </summary>
public static bool IsOperationNoneRoot(this IOperation operation)
{
#pragma warning disable IDE0078 // Use pattern matching - https://github.com/dotnet/roslyn/issues/51691
return operation.Kind == OperationKind.None && operation.Parent == null;
Copy link
Member

Choose a reason for hiding this comment

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

💭

Suggested change
return operation.Kind == OperationKind.None && operation.Parent == null;
return operation is { Kind: OperationKind.None, Parent: null };

@Youssef1313 Youssef1313 closed this Apr 8, 2021
@Youssef1313 Youssef1313 reopened this Apr 8, 2021
@Youssef1313 Youssef1313 marked this pull request as ready for review April 8, 2021 08:13
@codecov
Copy link

codecov bot commented Apr 8, 2021

Codecov Report

Merging #4919 (c7c62aa) into main (2841e98) will decrease coverage by 0.03%.
The diff coverage is 68.11%.

@@            Coverage Diff             @@
##             main    #4919      +/-   ##
==========================================
- Coverage   95.57%   95.54%   -0.04%     
==========================================
  Files        1185     1198      +13     
  Lines      271566   274801    +3235     
  Branches    16427    16735     +308     
==========================================
+ Hits       259561   262549    +2988     
- Misses       9866    10092     +226     
- Partials     2139     2160      +21     

@mavasani
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mavasani mavasani merged commit a72e252 into dotnet:main Apr 14, 2021
@Youssef1313 Youssef1313 deleted the patch-14 branch April 15, 2021 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bump code style package to v3.9.x after VS2019 16.9 is released
4 participants