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

Update Microsoft.CodeAnalysis.CSharp.CodeStyle package #74445

Merged
merged 3 commits into from
Aug 24, 2022

Conversation

jkoritzinsky
Copy link
Member

Update the Microsoft.CodeAnalysis.CSharp.CodeStyle package to the .NET 7 Preview 7 version and fix the various failures.

Update the Microsoft.CodeAnalysis.CSharp.CodeStyle package to the .NET 7 Preview 7 version and fix the various failures.
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@ghost
Copy link

ghost commented Aug 23, 2022

Tagging subscribers to this area: @dotnet/area-meta
See info in area-owners.md if you want to be subscribed.

Issue Details

Update the Microsoft.CodeAnalysis.CSharp.CodeStyle package to the .NET 7 Preview 7 version and fix the various failures.

Author: jkoritzinsky
Assignees: jkoritzinsky
Labels:

area-Meta

Milestone: -

if (func == null)
{
func = s_GetTempPathWFunc = GetGetTempPathWFunc();
}
#pragma warning restore IDE0074
Copy link
Member

Choose a reason for hiding this comment

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

Can we make sure there's an open issue in dotnet/roslyn for this? The analyzer shouldn't fire for function pointers unless ??= can be used with function pointers.

Copy link
Member Author

Choose a reason for hiding this comment

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

I need to check this one again, but I think the reason I didn't want to use the fix was for clarity around the call to GetGetTempPathWFunc(). I'll try using ??= here and see if it works for function pointers.

Copy link
Member

Choose a reason for hiding this comment

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

I'll try using ??= here and see if it works for function pointers.

I don't believe it does.

Copy link
Member Author

Choose a reason for hiding this comment

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

You are correct. It does not work for function pointers. I'll open an issue.

@@ -95,6 +95,7 @@ private static void InitFCN()
{
fcn = host.GetService(typeof(IFileChangeNotificationSystem)) as IFileChangeNotificationSystem;
}
#pragma warning disable IDE0074 // Use compound assignment
Copy link
Member

Choose a reason for hiding this comment

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

I think there should also be a dotnet/roslyn issue for this. It shouldn't be recommending compound assignment if there are ifdefs in the way.

// Let the InternalTransaciton know about the outcome.
if (InternalTransaction != null)
{
InternalTransaction.DistributedTransactionOutcome(InternalTransaction, Status);
}

#pragma warning restore IDE0031
Copy link
Member

Choose a reason for hiding this comment

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

Why does this one need to be suppressed rather than:

InternalTransaction?.DistributedTransactionOutcome(InternalTransaction, Status);

?

Copy link
Member Author

Choose a reason for hiding this comment

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

The method DistributedTransactionOutcome is a static method. InternalTransaction is the name of both a property and the type of that property. Fixing this to use ?. changes the invocation to try to invoke an instance method on the InternalTransaction property value instead of the InternalTransaction type.

This one definitely deserves a Roslyn issue.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, then yeah, that's a bug.

@jkoritzinsky jkoritzinsky merged commit 676a6bb into dotnet:main Aug 24, 2022
@jkoritzinsky jkoritzinsky deleted the roslyn-code-style-update branch August 24, 2022 17:19
@ghost ghost locked as resolved and limited conversation to collaborators Sep 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants