Fix false positives in SS008, SS050, SS057#408
Merged
Conversation
- GetHashCodeRefersToMutableMember: exempt readonly System.Tuple<...> fields since Tuple is an immutable sealed class - ParameterAssignedInConstructor: suppress when the parameter is later stored in a field/property (normalization pattern) - CollectionManipulatedDuringTraversal: skip modifications in for/foreach loops immediately followed by return or break Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…comparison Simpler heuristic than checking for downstream field assignment: if the parameter is used in any binary comparison in the constructor, the reassignment is intentional normalization/defaulting. Unwrap implicit conversions when inspecting operands (needed for nullable-enabled projects where reference-type operands are wrapped). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GetHashCodeRefersToMutableMember(#406):readonlyfields typed asSystem.Tuple<...>are no longer flagged.Tupleis a sealed immutable class so areadonlyreference to it cannot change state.ParameterAssignedInConstructor(#389): The analyzer no longer fires when the suspect parameter is subsequently stored into a field or property in the same constructor. This covers the normalization/defaulting pattern (if (x == null) x = fallback; this.field = x;) which was triggering a 100% false-positive rate across 560 evaluated repos.CollectionManipulatedDuringTraversal(#383): Modifications insidefor/foreachloops are no longer flagged when the modification statement is immediately followed byreturnorbreakin the same block. These are safe because the loop does not continue after the modification.All three fixes follow TDD — failing tests were added first, then minimal implementation changes to make them pass. The full suite of 1345 tests continues to pass.
Test plan
GetHashCodeRefersToMutableMember_ReadonlyTupleField_NoDiagnostic— no warning onreadonly Tuple<DateTime, Guid>GetHashCodeRefersToMutableMember_NonReadonlyTupleField_Diagnostic— still warns on non-readonly TupleCollectionManipulatedDuringTraversal_ForLoop_RemoveAtFollowedByReturn_NoDiagnosticCollectionManipulatedDuringTraversal_ForLoop_RemoveAtFollowedByBreak_NoDiagnosticCollectionManipulatedDuringTraversal_ForEachLoop_RemoveAtFollowedByReturn_NoDiagnosticCollectionManipulatedDuringTraversal_ForLoop_RemoveAtWithoutReturn_Diagnostic— still warns when no early exitParameterAssignedInConstructor_NormalizationBeforeFieldAssignment_NoDiagnosticParameterAssignedInConstructor_NormalizationWithNullCheck_NoDiagnosticParameterAssignedInConstructor_AssignedFromFieldWithoutStoringToField_Diagnostic— still warns when param is never stored🤖 Generated with Claude Code