Skip to content

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 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 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

@michaelnebel michaelnebel merged commit 87b52cc into github:main Jun 17, 2025
27 checks passed
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