Skip to content

C#: Add cs/gethashcode-is-not-defined to the Code Quality suite. #19716

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

michaelnebel
Copy link
Contributor

@michaelnebel michaelnebel commented Jun 10, 2025

In this PR we add the query cs/gethashcode-is-not-defined to the Code Quality suite.
It is worth mentioning that

  • There is compiler warning CS0659 which reports all types where the Equals method is overridden, but where the GetHashCode is not. Our query is a bit more conservative in its approach by only reporting such types, if they are used in a hashing context.
  • Only very few cases found via MRVA, but it looks like they would have been good to fix.
  • There wasn't any triage data for this query. Created some examples and ran autofix locally - it provided good fix suggestions (in all cases it correctly applied HashCode.Combine to create a decent hash function).

DCA looks good.

@github-actions github-actions bot added the C# label Jun 10, 2025
@michaelnebel michaelnebel added the no-change-note-required This PR does not need a change note label Jun 10, 2025
@michaelnebel michaelnebel marked this pull request as ready for review June 10, 2025 14:20
@Copilot Copilot AI review requested due to automatic review settings June 10, 2025 14:20
@michaelnebel michaelnebel requested a review from a team as a code owner June 10, 2025 14:20
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds the new cs/gethashcode-is-not-defined query to the Code Quality suite.

  • Updated the query metadata to include the "quality" tag.
  • Added the query to the csharp-code-quality.qls.expected integration-test listing.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
csharp/ql/src/Likely Bugs/HashedButNoHash.ql Added quality tag to query metadata
csharp/ql/integration-tests/posix/query-suite/csharp-code-quality.qls.expected Listed the new query in the expected Code Quality suite
Comments suppressed due to low confidence (3)

csharp/ql/integration-tests/posix/query-suite/csharp-code-quality.qls.expected:11

  • Add corresponding integration test cases or sample code to validate that cs/gethashcode-is-not-defined triggers as expected in the suite.
ql/csharp/ql/src/Likely Bugs/HashedButNoHash.ql

csharp/ql/integration-tests/posix/query-suite/csharp-code-quality.qls.expected:11

  • Ensure this query is also added to all other platform-specific suite definitions (e.g., Windows and macOS) to keep test coverage consistent across environments.
ql/csharp/ql/src/Likely Bugs/HashedButNoHash.ql

csharp/ql/src/Likely Bugs/HashedButNoHash.ql:10

  • [nitpick] Align the indentation of the newly added "quality" tag with the existing tags to maintain consistent formatting.
*       quality

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C# no-change-note-required This PR does not need a change note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants