Skip to content

Fix panic when using EqualValues with uncomparable types #1825

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

Open
ccoVeille wants to merge 1 commit into
base: master
Choose a base branch
from
+35 −3
Open

Fix panic when using EqualValues with uncomparable types #1825

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 31 additions & 3 deletions assert/assertions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,21 +177,49 @@ func ObjectsAreEqualValues(expected, actual interface{}) bool {
return false
}

// Attempt conversion of expected to actual type
// This handles more cases than just the ConvertibleTo check above
if !expectedValue.CanConvert(actualType) {
// Types are not convertible, so they cannot be equal
// This prevents panics when calling [reflect.Value.Convert]
return false
}

expectedConverted := expectedValue.Convert(actualType)
if !expectedConverted.CanInterface() {
// Cannot interface after conversion, so cannot be equal
// This prevents panics when calling [reflect.Value.Interface]
return false
}

if !isNumericType(expectedType) || !isNumericType(actualType) {
// Attempt comparison after type conversion
return reflect.DeepEqual(
expectedValue.Convert(actualType).Interface(), actual,
expectedConverted.Interface(), actual,
)
}

// If BOTH values are numeric, there are chances of false positives due
// to overflow or underflow. So, we need to make sure to always convert
// the smaller type to a larger type before comparing.
if expectedType.Size() >= actualType.Size() {
return actualValue.Convert(expectedType).Interface() == expected
if !actualValue.CanConvert(expectedType) {
// Cannot convert actual to the expected type, so cannot be equal
// This is a hypothetical case to prevent panics when calling [reflect.Value.Convert]
return false
}

actualConverted := actualValue.Convert(expectedType)
if !actualConverted.CanInterface() {
// Cannot interface after conversion, so cannot be equal
// This is a hypothetical case to prevent panics when calling [reflect.Value.Convert]
Comment on lines +208 to +215
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

Similar to line 208, this comment incorrectly describes this as a "hypothetical case" when it's actually a real protection against runtime panics. The check is necessary and mirrors the protection added at lines 189-192. Consider removing "hypothetical" from the comment or rephrasing to clarify this is a real edge case being handled.

Suggested change
// This is a hypothetical case to prevent panics when calling [reflect.Value.Convert]
return false
}
actualConverted := actualValue.Convert(expectedType)
if !actualConverted.CanInterface() {
// Cannot interface after conversion, so cannot be equal
// This is a hypothetical case to prevent panics when calling [reflect.Value.Convert]
// This prevents panics when calling [reflect.Value.Convert]
return false
}
actualConverted := actualValue.Convert(expectedType)
if !actualConverted.CanInterface() {
// Cannot interface after conversion, so cannot be equal
// This prevents panics when calling [reflect.Value.Interface]

Copilot uses AI. Check for mistakes.
Comment on lines +208 to +215
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The comment describes this as a "hypothetical case", but it's not hypothetical. This check protects against the same real panic scenario that the fix addresses (as demonstrated in issue #1699). The check is necessary to prevent panics when converting between types where ConvertibleTo returns true but the actual conversion would fail at runtime (e.g., converting a slice to a pointer to an array of different size). Consider removing "hypothetical" from the comment or rephrasing to something like "This handles the same edge case as above to prevent panics when calling [reflect.Value.Convert]".

Suggested change
// This is a hypothetical case to prevent panics when calling [reflect.Value.Convert]
return false
}
actualConverted := actualValue.Convert(expectedType)
if !actualConverted.CanInterface() {
// Cannot interface after conversion, so cannot be equal
// This is a hypothetical case to prevent panics when calling [reflect.Value.Convert]
// This handles the same edge case as above to prevent panics when calling [reflect.Value.Convert]
return false
}
actualConverted := actualValue.Convert(expectedType)
if !actualConverted.CanInterface() {
// Cannot interface after conversion, so cannot be equal
// This handles the same edge case as above to prevent panics when calling [reflect.Value.Interface]

Copilot uses AI. Check for mistakes.
return false
}

return actualConverted.Interface() == expected
}

return expectedValue.Convert(actualType).Interface() == actual
return expectedConverted.Interface() == actual
}

// isNumericType returns true if the type is one of:
Expand Down
4 changes: 4 additions & 0 deletions assert/assertions_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,10 @@ func TestObjectsAreEqualValues(t *testing.T) {
{3.14, complex128(1e+100 + 1e+100i), false},
{complex128(1e+10 + 1e+10i), complex64(1e+10 + 1e+10i), true},
{complex64(1e+10 + 1e+10i), complex128(1e+10 + 1e+10i), true},

// panics should be caught and treated as inequality
// https://github.com/stretchr/testify/issues/1699
{[]int{1, 2}, (*[3]int)(nil), false},
}

for _, c := range cases {
Expand Down