-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Pull in shared source for core roslyn capacilities into roslyn-analyzer packages. #79024
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
Pull in shared source for core roslyn capacilities into roslyn-analyzer packages. #79024
Conversation
<Import Project="..\..\Dependencies\Collections\Microsoft.CodeAnalysis.Collections.projitems" Label="Shared" /> | ||
<Import Project="..\..\Dependencies\PooledObjects\Microsoft.CodeAnalysis.PooledObjects.projitems" Label="Shared" /> | ||
<Import Project="..\..\Dependencies\Threading\Microsoft.CodeAnalysis.Threading.projitems" Label="Shared" /> | ||
<Import Project="..\..\Dependencies\Contracts\Microsoft.CodeAnalysis.Contracts.projitems" Label="Shared" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: this will be repeated in a lot of projects. it would make a lot more sense if all these analyzers had a shared dll they depended on that could pull these in once. that's a change for the future.
ValidateApiList(additionalFiles, publicApiMap, shippedData.ApiList, isPublic, errors); | ||
ValidateApiList(additionalFiles, publicApiMap, unshippedData.ApiList, isPublic, errors); | ||
publicApiMap.Free(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the helper that made this possible was used only here. i felt it better to just remove it than keep it around.
@@ -285,7 +285,8 @@ or RoslynDiagnosticIds.ObliviousPublicApiRuleId | |||
} | |||
} | |||
|
|||
var symbolNamesToRemove = symbolNamesToRemoveBuilder.ToImmutableAndFree(); | |||
var symbolNamesToRemove = symbolNamesToRemoveBuilder.ToImmutableHashSet(); | |||
symbolNamesToRemoveBuilder.Free(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was an odd case i couldn't keep working. so i just inlined as well as pulling in the extensions caused too many conflicts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I was going to ask. =)
@@ -171,7 +171,7 @@ internal ValueContentAbstractValue Merge(ValueContentAbstractValue otherState) | |||
throw new ArgumentNullException(nameof(otherState)); | |||
} | |||
|
|||
ImmutableHashSet<object?> mergedLiteralValues = LiteralValues.AddRange(otherState.LiteralValues); | |||
ImmutableHashSet<object?> mergedLiteralValues = ImmutableHashSetExtensions.AddRange(LiteralValues, otherState.LiteralValues); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you use any semantic search to find opportunities like this or are you just scouring code?
This is the first in several refactorings. This just gets us on the same base set of shared source projects so that these legacy libraries don't need to ship their own copies of things.
Note: this only pulls in:
It doesn't pull in our shared symbol extensions and whatnot yet. that's a followup. this keeps the size/scope of this PR low enough.