Allow timestamp-tagged scalars to unmarshal into strings

[oguzkilcan]

constructTimestamp only handled time.Time and interface{} targets. A plain scalar resolving to the timestamp tag left a string field at its zero value instead of the raw text. This adds a string case that writes the scalar directly, the same fallback other scalar tags already have.

Fixes #376

[Copilot]

Pull request overview

This PR fixes a gap in the YAML constructor logic where scalars implicitly resolved as !!timestamp would not populate Go string targets, leaving them at the zero value. It aligns !!timestamp behavior with other scalar tags by allowing a string fallback that assigns the scalar text directly.

Changes:

• Extend constructTimestamp to support reflect.String targets by setting the scalar value (Node.Value) into the destination string.
• Add a regression test ensuring an implicitly timestamp-tagged scalar unmarshals into map[string]string as the original timestamp text.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
yaml_test.go	Adds a regression test covering implicit !!timestamp resolution into a string field.
internal/libyaml/constructor.go	Adds reflect.String handling to constructTimestamp so timestamp scalars can populate string targets.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[ccoVeille]

Thanks @oguzkilcan

I feel like the fix is good.

But I'm doubtful about the tests.

Note: I edited your PR description to mention the issue you opened

[oguzkilcan]

Thank you for the review. I incorporated your suggestions into the test suite. It run successfully.

❯ go test -run TestUnmarshal
PASS
ok      go.yaml.in/yaml/v4      0.194s

[ingydotnet]

- !!timestamp foo - I'm wondering if this should error.

I probably need to consult with the core team.

[ingydotnet]

The 1.1 and 1.2 specs allow it to load as a string. It's really up to us what to do.

!!timestamp isn't part of the YAML 1.2 spec's Core yaml-schema, so I'm a bit loathe to make this a default.

Honestly I'd like all of tag resolution and construction to be handled at the RuleSet plugin level. Then a RuleSet would be made for the current behavior inherited from go-yaml v3 and that would be the default, and there would become myriad other RuleSets to use or extend.

That said I just noticed this is a fix for a v4 regression, so I'll take a closer look now.

[ingydotnet]

Looks like the regression was here:

go-yaml/internal/libyaml/constructor.go

Lines 653 to 666 in 082f4f4

	// constructTimestamp constructs a !!timestamp tagged value into various Go types.
	func (c *Constructor) constructTimestamp(n *Node, resolved any, out reflect.Value) bool {
	switch out.Kind() {
	case reflect.Struct:
	if resolvedv := reflect.ValueOf(resolved); out.Type() == resolvedv.Type() {
	out.Set(resolvedv)
	return true
	}
	case reflect.Interface:
	out.Set(reflect.ValueOf(resolved))
	return true
	}
	c.tagError(n, timestampTag, out)
	return false

and also regressed !!float but that got fixed here: fcab72b

[ingydotnet]

@oguzkilcan Can you add a test for an implicit timestamp, as that is what was reported by #376.