error comparison not working on type with field

[Shulhan]

Hi all,

I found an error that instantiate with type that have field cannot be
compared with the instance of the same type. Here is the test code,

----
type typeErrorWithoutField struct{}

func (te *typeErrorWithoutField) Error() string {
return "typeError"
}

type typeErrorWithPublicField struct {
P string
}

func (te *typeErrorWithPublicField) Error() string {
return "typeErrorWithPublicField"
}

type typeErrorWithPrivateField struct {
p string
}

func (te *typeErrorWithPrivateField) Error() string {
return "typeErrorWithPrivateField"
}

type typeErrorWithIs struct {
p string
}

func (te *typeErrorWithIs) Error() string {
return "typeErrorWithIs"
}

func (te *typeErrorWithIs) Is(target error) bool {
return target.Error() == te.Error()
}

func TestErrors(t *testing.T) {
cases := []struct {
desc string
err error
target error
}{{
err: &typeErrorWithoutField{},
target: &typeErrorWithoutField{},
}, {
desc: "With error created from &typeErrorWithPublicField{}",
err: &typeErrorWithPublicField{},
target: &typeErrorWithPublicField{},
}, {
desc: "With error created from &typeErrorWithPrivateField{}",
err: &typeErrorWithPrivateField{},
target: &typeErrorWithPrivateField{},
}, {
desc: "With error created from &typeErrorWithIs{}",
err: &typeErrorWithIs{},
target: &typeErrorWithIs{},
}}

for _, c := range cases {
t.Log(c.desc)
t.Logf("err == target: %v\n", c.err == c.target)
t.Logf("errors.Is(err, target): %v\n", errors.Is(c.err, c.target))
}
}
----

Running this on Go tip, Go 1.16.3, Go 1.15.11 return the same output,

----
ms@inspiro 0 % go test -v -count=1 .
=== RUN TestErrors
errors_test.go:69:
errors_test.go:70: err == target: true
errors_test.go:71: errors.Is(err, target): true
errors_test.go:69: With error created from &typeErrorWithPublicField{}
errors_test.go:70: err == target: false
errors_test.go:71: errors.Is(err, target): false
errors_test.go:69: With error created from &typeErrorWithPrivateField{}
errors_test.go:70: err == target: false
errors_test.go:71: errors.Is(err, target): false
errors_test.go:69: With error created from &typeErrorWithIs{}
errors_test.go:70: err == target: false
errors_test.go:71: errors.Is(err, target): true
--- PASS: TestErrors (0.00s)
=== RUN TestTypeErrorWithoutField
errors_test.go:81: err == target: true
errors_test.go:82: errors.Is(err, target): true
--- PASS: TestTypeErrorWithoutField (0.00s)
=== RUN TestTypeErrorWithPublicField
errors_test.go:91: err == target: false
errors_test.go:92: errors.Is(err, target): false
--- PASS: TestTypeErrorWithPublicField (0.00s)
=== RUN TestTypeErrorWithIs
errors_test.go:101: err == target: false
errors_test.go:102: errors.Is(err, target): true
--- PASS: TestTypeErrorWithIs (0.00s)
PASS
ok git.sr.ht/~shulhan/sandbox/errors 0.001s
----

The only way to make the error comparable and return true is by
implementing method Is() on the type and use the errors.Is to compare it,
or manually by using err.Error() == obj.Error().

The thing is, 1) there are some error type that does not implement the Is() type.
One that I stumbled on is ssh.PassphraseMissingError.
2) some people, like me, assume that errors.Is() working for type defined error,
but in fact it is not.

Did I miss something? Is this a bug or an expected behaviour?

[Shulhan]

Seems like defined type for error MUST implement the Error method
without pointer receiver, otherwise errors.Is is not working as
expected.

If the type use pointer receiver as in `(e *T) Error()`, one must cast
the error `err.(*T)` to check for expected error (assuming that the
consumer cannot implement Is() method on the T).

The following test show two types that implement error interface with
and without pointer receiver.

----
type typeError struct {
P int
}

func (e typeError) Error() string {
return "typeError"
}

type typeErrorWithPointer struct {
P int
}

func (e *typeErrorWithPointer) Error() string {
return "typeErrorWithPointer"
}

func TestTypeErrors(t *testing.T) {

cases := []struct {
desc string
err error
target error
}{{

desc: "typeError",
err: typeError{},
target: typeError{},
}, {
desc: "typeErrorWithPointer",
err: &typeErrorWithPointer{},
target: &typeErrorWithPointer{},

}}

for _, c := range cases {
t.Log(c.desc)

t.Log(c.err == c.target)
t.Log(errors.Is(c.err, c.target))
}
}
----

The test output,

----
=== RUN TestTypeErrors
errors_test.go:174: typeError
errors_test.go:175: true
errors_test.go:176: true
errors_test.go:174: typeErrorWithPointer
errors_test.go:175: false
errors_test.go:176: false
--- PASS: TestTypeErrors (0.00s)
----

I think I must review all of the code that use external library with
an errors.Is after this.