Skip to content

Make InterceptableLocation implement IEquatable<InterceptableLocation> #77137

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.

Sign up for GitHub

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

Jump to bottom
Merged
jjonescz merged 4 commits into from
Feb 17, 2025
Merged

Make InterceptableLocation implement IEquatable<InterceptableLocation> #77137

jjonescz merged 4 commits into from
Feb 17, 2025

Conversation

@aradalvand
Copy link
Contributor

@aradalvand aradalvand commented Feb 10, 2025 •
edited
Loading

Closes #77135

@RikkiGibson Let me know if this is not the ideal setup.

@aradalvand aradalvand requested a review from a team as a code owner February 10, 2025 19:33
@ghost ghost added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Feb 10, 2025
@dotnet-policy-service dotnet-policy-service bot added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label Feb 10, 2025
@aradalvand aradalvand changed the title Make InterceptableLocation implement `IEquatable<InterceptableLocat… Make InterceptableLocation implement IEquatable<InterceptableLocation> Feb 10, 2025
RikkiGibson
RikkiGibson reviewed Feb 10, 2025
View reviewed changes
jjonescz
jjonescz reviewed Feb 11, 2025
View reviewed changes
@RikkiGibson
Copy link
Member

RikkiGibson commented Feb 11, 2025

Thanks for addressing the feedback on this PR. There is still a public API check failure:

src\Compilers\CSharp\Portable\Utilities\InterceptableLocation.cs(44,26): error RS0016: (NETCORE_ENGINEERING_TELEMETRY=Build) Symbol 'abstract Microsoft.CodeAnalysis.CSharp.InterceptableLocation.Equals(Microsoft.CodeAnalysis.CSharp.InterceptableLocation? other) -> bool' is not part of the declared public API (https://github.com/dotnet/roslyn-analyzers/blob/main/src/PublicApiAnalyzers/PublicApiAnalyzers.Help.md)

This can be resolved by adding abstract Microsoft.CodeAnalysis.CSharp.InterceptableLocation.Equals(Microsoft.CodeAnalysis.CSharp.InterceptableLocation? other) -> bool to a new line of src\Compilers\CSharp\Portable\PublicAPI.Unshipped.txt. Maybe grouping it with the other lines for InterceptableLocation would be good.

Public API fix should be verifiable locally with dotnet build src\Compilers\CSharp\Portable -p:RunAnalyzersDuringBuild=true. Thanks!

@aradalvand aradalvand requested a review from a team as a code owner February 16, 2025 12:02
@dotnet-policy-service dotnet-policy-service bot added the Needs API Review Needs to be reviewed by the API review council label Feb 16, 2025
@RikkiGibson RikkiGibson requested a review from a team February 16, 2025 21:40
@jjonescz jjonescz merged commit 80ccf3f into dotnet:main Feb 17, 2025
24 checks passed
@jjonescz
Copy link
Member

jjonescz commented Feb 17, 2025

Thanks for the contribution @aradalvand

@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Feb 17, 2025
@333fred 333fred mentioned this pull request Feb 18, 2025
Merged
333fred added a commit to 333fred/roslyn that referenced this pull request Feb 18, 2025
@333fred
Merge remote-tracking branch 'upstream/main' into merge-main
212d206 
* upstream/main: (576 commits)
  Add missing word to the documentation file (dotnet#77259)
  Make `InterceptableLocation` implement `IEquatable<InterceptableLocation>` (dotnet#77137)
  REvert
  REvert
  REvert
  rename
  Semantic document
  remove more
  inline
  inline
  Remove state type from 'move type' refactoring
  preserve behavior
  Simplify
  Remove dependency on semantics
  Simplify
  Simplify code
  Use property
  Properly pass progress object along in fix all provider
  fix syncing
  add background color for inline hints in high contrast mode
  ...
This was referenced Feb 19, 2025
Closed
Closed
Closed
@akhera99 akhera99 modified the milestones: Next, 17.14 P2 Feb 25, 2025
@dotnet-bot dotnet-bot mentioned this pull request Feb 26, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@333fred 333fred 333fred left review comments

@jjonescz jjonescz jjonescz approved these changes

@RikkiGibson RikkiGibson RikkiGibson approved these changes

Assignees

No one assigned

Labels

Area-Compilers Community The pull request was submitted by a contributor who is not a Microsoft employee. Needs API Review Needs to be reviewed by the API review council untriaged Issues and PRs which have not yet been triaged by a lead

Projects

None yet

Milestone

17.14 P2

Development

Successfully merging this pull request may close these issues.

Make InterceptableLocation implement IEquatable<InterceptableLocation>

5 participants

@aradalvand @RikkiGibson @jjonescz @333fred @akhera99