feat: Add support to disable stringer

[bombsimon]

Currently godump always uses the stringer interface if defined. This is not always desirable if you want to check the internals of a type.

This adds a feature with the same name used for spew (DisableMethods) which can be set to true to disable this feature.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[ccoVeille]

I like the idea, but the DisableMethods name pulled out of spew is not easy to understand.

So I'm fine with the implementation, except the name.

Also, I like what you pulled from spew, because it shows that DisableMethods was about Error() string and String() string

Maybe there is something to do with Error too, but I'm unsure.

otherwise DisableStringer could be a good candidate.

Wording is tough, so let's wait for feedbacks here

[bombsimon]

Also, I like what you pulled from spew, because it shows that DisableMethods was about Error() string and String() string

otherwise DisableStringer could be a good candidate.

I actually called this DisableStringer initially but thought that it would be more future proof to chose a name that would work for any interface. If we don't do that now and end up implementing Error() as well we would have to add a new option like DisableError.

But I don't have that strong feeling, if DisableStringer is what you want I'll just change and any future methods can be handled when they occur.

[Andrei-hub11]

I actually called this DisableStringer initially but thought that it would be more future proof to chose a name that would work for any interface. If we don't do that now and end up implementing Error() as well we would have to add a new option like DisableError.

But I don't have that strong feeling, if DisableStringer is what you want I'll just change and any future methods can be handled when they occur.

@ccoVeille I'd recommend DisableMethodOutput or DisableTypeFormatters.

[Akkadius]

Also, I like what you pulled from spew, because it shows that DisableMethods was about Error() string and String() string

otherwise DisableStringer could be a good candidate.

I actually called this DisableStringer initially but thought that it would be more future proof to chose a name that would work for any interface. If we don't do that now and end up implementing Error() as well we would have to add a new option like DisableError.

But I don't have that strong feeling, if DisableStringer is what you want I'll just change and any future methods can be handled when they occur.

I echo the sentiments for DisableStringer as it is clear, simple, immediately understood by the reader.

[Akkadius]

Do we really want to return blank? I would think we would at least want the following to return the raw "non-stringer" value from the struct ? I could be wrong here

	if !val.CanInterface() {
		val = forceExported(val)
	}

[bombsimon]

That's interesting, I'm not sure I follow when and what that is supposed to work 🤔

The test for this is on a private field in a private struct and it seems to work as expected by returning the empty string to continue the processing in printValue to figure out the type.

https://github.com/bombsimon/godump/blob/781f048e52e61a9ef387b4e81c4adfe328caff51/godump_test.go#L1051-L1061

I'll do some more testing but feel free to add an example on when this would be needed if you know how to! Thanks!

[bombsimon]

Didn't get too much time, but quickly glancing at the code, it looks like the need for this forced export is so we can call val.Interface().(fmt.Stringer) on private fields. But we won't end up in this path if we're not looking for strings so we don't have a use of what's returned from forceExported.

We do something similar at printValue for struct fields which I think is what you're referring to to show private fields. But it's not important that we force a value that can call Interface() here either because we don't do that.

I don't think we need to do any change at least to get expected behavior. I updated the test to ensure we see the private field and I also removed one place where we call asStringer and look for the empty string which can be done recursively instead.