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

Remove IChecksummedObject type. #74829

Merged
merged 8 commits into from
Aug 21, 2024

Conversation

CyrusNajmabadi
Copy link
Member

This type existed only for testing purposes. But there's ahn existing pattern we already have to allow customized test behavior here. Having two mechanisms for this was super confusing and led me down a painful debugging hole in one of my branches.

@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner August 20, 2024 22:03
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Aug 20, 2024
protected override Checksum CreateChecksum(AnalyzerReference reference, CancellationToken cancellationToken)
=> reference is TestGeneratorReference generatorReference
? generatorReference.Checksum
: base.CreateChecksum(reference, cancellationToken);
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 new, and replaced the existing hijacking point that was using IChecksummedObject.

@CyrusNajmabadi
Copy link
Member Author

@ToddGrun @dibarbet ptal.

@@ -13,10 +13,10 @@ namespace Roslyn.Test.Utilities
/// A simple deriviation of <see cref="AnalyzerReference"/> that returns the source generator
/// passed, for ease in unit tests.
/// </summary>
public class TestGeneratorReference : AnalyzerReference, IChecksummedObject
public class TestGeneratorReference : AnalyzerReference
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 only impl of this interface.

if (value is IChecksummedObject checksummedObject)
{
return checksummedObject.Checksum;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

existed only to hijack. but we can already do that throuhg normal virtual methods.

@@ -11,9 +11,6 @@ namespace Microsoft.CodeAnalysis.Serialization;
internal interface ISerializerService : IWorkspaceService
{
void Serialize(object value, ObjectWriter writer, CancellationToken cancellationToken);

void SerializeParseOptions(ParseOptions options, ObjectWriter writer);
Copy link
Member Author

Choose a reason for hiding this comment

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

didn't need to be on the itnerface. was never called externally.

@CyrusNajmabadi CyrusNajmabadi merged commit c63e7b6 into dotnet:main Aug 21, 2024
25 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Aug 21, 2024
@CyrusNajmabadi CyrusNajmabadi deleted the checksummedObject branch August 21, 2024 02:20
@dibarbet dibarbet modified the milestones: Next, 17.12 P2 Aug 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead VSCode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants