From: Bjørn Erik Pedersen Date: Thu, 23 Oct 2025 09:00:38 +0000 (+0200) Subject: all: Simplify the reflect usage X-Git-Url: http://git.maquefel.me/?a=commitdiff_plain;h=3893e70510af80e6ef9fff46c1922404706b1c2b;p=brevno-suite%2Fhugo all: Simplify the reflect usage * Use helper funcs in hreflect package when possible. * Use hreflect.ConvertIfPossible to handle conversions when possible. * Move scratch.go from common/maps to common/hstore to clear cyclic import in the next step. * Move Indirect to hreflect and reimplementing it and adusting the behavior to preserve struct pointers. * Adjust evaluateSubElem used by where and others making the struct pointer method case slightly faster. --- diff --git a/common/collections/append.go b/common/collections/append.go index db9db8bf3..8a5aee40c 100644 --- a/common/collections/append.go +++ b/common/collections/append.go @@ -16,6 +16,8 @@ package collections import ( "fmt" "reflect" + + "github.com/gohugoio/hugo/common/hreflect" ) // Append appends from to a slice to and returns the resulting slice. @@ -25,7 +27,7 @@ func Append(to any, from ...any) (any, error) { if len(from) == 0 { return to, nil } - tov, toIsNil := indirect(reflect.ValueOf(to)) + tov, toIsNil := hreflect.Indirect(reflect.ValueOf(to)) toIsNil = toIsNil || to == nil var tot reflect.Type @@ -100,7 +102,7 @@ func Append(to any, from ...any) (any, error) { fv := reflect.ValueOf(f) if !fv.IsValid() || !fv.Type().AssignableTo(tot) { // Fall back to a []interface{} slice. - tov, _ := indirect(reflect.ValueOf(to)) + tov, _ := hreflect.Indirect(reflect.ValueOf(to)) return appendToInterfaceSlice(tov, from...) } tov = reflect.Append(tov, fv) @@ -136,17 +138,3 @@ func appendToInterfaceSlice(tov reflect.Value, from ...any) ([]any, error) { return tos, nil } - -// indirect is borrowed from the Go stdlib: 'text/template/exec.go' -// TODO(bep) consolidate -func indirect(v reflect.Value) (rv reflect.Value, isNil bool) { - for ; v.Kind() == reflect.Ptr || v.Kind() == reflect.Interface; v = v.Elem() { - if v.IsNil() { - return v, true - } - if v.Kind() == reflect.Interface && v.NumMethod() > 0 { - break - } - } - return v, false -} diff --git a/common/collections/append_test.go b/common/collections/append_test.go index 62d9015ce..c198ca09c 100644 --- a/common/collections/append_test.go +++ b/common/collections/append_test.go @@ -15,7 +15,6 @@ package collections import ( "html/template" - "reflect" "testing" qt "github.com/frankban/quicktest" @@ -148,66 +147,3 @@ func TestAppendShouldMakeACopyOfTheInputSlice(t *testing.T) { c.Assert(result, qt.DeepEquals, []string{"a", "b", "c"}) c.Assert(slice, qt.DeepEquals, []string{"d", "b"}) } - -func TestIndirect(t *testing.T) { - t.Parallel() - c := qt.New(t) - - type testStruct struct { - Field string - } - - var ( - nilPtr *testStruct - nilIface interface{} = nil - nonNilIface interface{} = &testStruct{Field: "hello"} - ) - - tests := []struct { - name string - input any - wantKind reflect.Kind - wantNil bool - }{ - { - name: "nil pointer", - input: nilPtr, - wantKind: reflect.Ptr, - wantNil: true, - }, - { - name: "nil interface", - input: nilIface, - wantKind: reflect.Invalid, - wantNil: false, - }, - { - name: "non-nil pointer to struct", - input: &testStruct{Field: "abc"}, - wantKind: reflect.Struct, - wantNil: false, - }, - { - name: "non-nil interface holding pointer", - input: nonNilIface, - wantKind: reflect.Struct, - wantNil: false, - }, - { - name: "plain value", - input: testStruct{Field: "xyz"}, - wantKind: reflect.Struct, - wantNil: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - v := reflect.ValueOf(tt.input) - got, isNil := indirect(v) - - c.Assert(got.Kind(), qt.Equals, tt.wantKind) - c.Assert(isNil, qt.Equals, tt.wantNil) - }) - } -} diff --git a/common/hreflect/convert.go b/common/hreflect/convert.go new file mode 100644 index 000000000..cb5233482 --- /dev/null +++ b/common/hreflect/convert.go @@ -0,0 +1,222 @@ +// Copyright 2025 The Hugo Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package hreflect + +import ( + "fmt" + "math" + "reflect" +) + +var ( + typeInt64 = reflect.TypeFor[int64]() + typeFloat64 = reflect.TypeFor[float64]() + typeString = reflect.TypeFor[string]() +) + +// ToInt64 converts v to int64 if possible, returning an error if not. +func ToInt64E(v reflect.Value) (int64, error) { + if v, ok := ConvertIfPossible(v, typeInt64); ok { + return v.Int(), nil + } + return 0, errConvert(v, "int64") +} + +// ToInt64 converts v to int64 if possible. It panics if the conversion is not possible. +func ToInt64(v reflect.Value) int64 { + vv, err := ToInt64E(v) + if err != nil { + panic(err) + } + return vv +} + +// ToFloat64E converts v to float64 if possible, returning an error if not. +func ToFloat64E(v reflect.Value) (float64, error) { + if v, ok := ConvertIfPossible(v, typeFloat64); ok { + return v.Float(), nil + } + return 0, errConvert(v, "float64") +} + +// ToFloat64 converts v to float64 if possible, panicking if not. +func ToFloat64(v reflect.Value) float64 { + vv, err := ToFloat64E(v) + if err != nil { + panic(err) + } + return vv +} + +// ToStringE converts v to string if possible, returning an error if not. +func ToStringE(v reflect.Value) (string, error) { + vv, err := ToStringValueE(v) + if err != nil { + return "", err + } + return vv.String(), nil +} + +func ToStringValueE(v reflect.Value) (reflect.Value, error) { + if v, ok := ConvertIfPossible(v, typeString); ok { + return v, nil + } + return reflect.Value{}, errConvert(v, "string") +} + +// ToString converts v to string if possible, panicking if not. +func ToString(v reflect.Value) string { + vv, err := ToStringE(v) + if err != nil { + panic(err) + } + return vv +} + +func errConvert(v reflect.Value, s string) error { + return fmt.Errorf("unable to convert value of type %q to %q", v.Type().String(), s) +} + +// ConvertIfPossible tries to convert val to typ if possible. +// This is currently only implemented for int kinds, +// added to handle the move to a new YAML library which produces uint64 for unsigned integers. +// We can expand on this later if needed. +// This conversion is lossless. +// See Issue 14079. +func ConvertIfPossible(val reflect.Value, typ reflect.Type) (reflect.Value, bool) { + switch val.Kind() { + case reflect.Ptr, reflect.Interface: + if val.IsNil() { + // Return typ's zero value. + return reflect.Zero(typ), true + } + val = val.Elem() + } + + if val.Type().AssignableTo(typ) { + // No conversion needed. + return val, true + } + + if IsInt(typ.Kind()) { + return convertToIntIfPossible(val, typ) + } + if IsFloat(typ.Kind()) { + return convertToFloatIfPossible(val, typ) + } + if IsUint(typ.Kind()) { + return convertToUintIfPossible(val, typ) + } + if IsString(typ.Kind()) && IsString(val.Kind()) { + return val.Convert(typ), true + } + + return reflect.Value{}, false +} + +func convertToUintIfPossible(val reflect.Value, typ reflect.Type) (reflect.Value, bool) { + if IsInt(val.Kind()) { + i := val.Int() + if i < 0 { + return reflect.Value{}, false + } + u := uint64(i) + if typ.OverflowUint(u) { + return reflect.Value{}, false + } + return reflect.ValueOf(u).Convert(typ), true + } + if IsUint(val.Kind()) { + if typ.OverflowUint(val.Uint()) { + return reflect.Value{}, false + } + return val.Convert(typ), true + } + if IsFloat(val.Kind()) { + f := val.Float() + if f < 0 || f > float64(math.MaxUint64) { + return reflect.Value{}, false + } + if f != math.Trunc(f) { + return reflect.Value{}, false + } + u := uint64(f) + if typ.OverflowUint(u) { + return reflect.Value{}, false + } + return reflect.ValueOf(u).Convert(typ), true + } + return reflect.Value{}, false +} + +func convertToFloatIfPossible(val reflect.Value, typ reflect.Type) (reflect.Value, bool) { + if IsInt(val.Kind()) { + i := val.Int() + f := float64(i) + if typ.OverflowFloat(f) { + return reflect.Value{}, false + } + return reflect.ValueOf(f).Convert(typ), true + } + if IsUint(val.Kind()) { + u := val.Uint() + f := float64(u) + if typ.OverflowFloat(f) { + return reflect.Value{}, false + } + return reflect.ValueOf(f).Convert(typ), true + } + if IsFloat(val.Kind()) { + if typ.OverflowFloat(val.Float()) { + return reflect.Value{}, false + } + return val.Convert(typ), true + } + + return reflect.Value{}, false +} + +func convertToIntIfPossible(val reflect.Value, typ reflect.Type) (reflect.Value, bool) { + if IsInt(val.Kind()) { + if typ.OverflowInt(val.Int()) { + return reflect.Value{}, false + } + return val.Convert(typ), true + } + if IsUint(val.Kind()) { + if val.Uint() > uint64(math.MaxInt64) { + return reflect.Value{}, false + } + if typ.OverflowInt(int64(val.Uint())) { + return reflect.Value{}, false + } + return val.Convert(typ), true + } + if IsFloat(val.Kind()) { + f := val.Float() + if f < float64(math.MinInt64) || f > float64(math.MaxInt64) { + return reflect.Value{}, false + } + if f != math.Trunc(f) { + return reflect.Value{}, false + } + if typ.OverflowInt(int64(f)) { + return reflect.Value{}, false + } + return reflect.ValueOf(int64(f)).Convert(typ), true + + } + + return reflect.Value{}, false +} diff --git a/common/hreflect/convert_test.go b/common/hreflect/convert_test.go new file mode 100644 index 000000000..a130c4f1f --- /dev/null +++ b/common/hreflect/convert_test.go @@ -0,0 +1,284 @@ +// Copyright 2025 The Hugo Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package hreflect + +import ( + "math" + "reflect" + "testing" + + qt "github.com/frankban/quicktest" + "github.com/gohugoio/hugo/htesting/hqt" +) + +func TestToFuncs(t *testing.T) { + c := qt.New(t) + + c.Assert(ToInt64(reflect.ValueOf(int(42))), qt.Equals, int64(42)) + c.Assert(ToFloat64(reflect.ValueOf(float32(3.14))), hqt.IsSameFloat64, float64(3.14)) + c.Assert(ToString(reflect.ValueOf("hello")), qt.Equals, "hello") +} + +func TestConvertIfPossible(t *testing.T) { + c := qt.New(t) + + for _, test := range []struct { + name string + value any + typ any + expected any + ok bool + }{ + // From uint to int. + { + name: "uint64(math.MaxUint64) to int16", + value: uint64(math.MaxUint64), + typ: int16(0), + ok: false, // overflow + }, + + { + name: "uint64(math.MaxUint64) to int64", + value: uint64(math.MaxUint64), + typ: int64(0), + ok: false, // overflow + }, + { + name: "uint64(math.MaxInt16) to int16", + value: uint64(math.MaxInt16), + typ: int64(0), + ok: true, + expected: int64(math.MaxInt16), + }, + // From int to int. + { + name: "int64(math.MaxInt64) to int16", + value: int64(math.MaxInt64), + typ: int16(0), + ok: false, // overflow + }, + { + name: "int64(math.MaxInt16) to int", + value: int64(math.MaxInt16), + typ: int(0), + ok: true, + expected: int(math.MaxInt16), + }, + + { + name: "int64(math.MaxInt16) to int", + value: int64(math.MaxInt16), + typ: int(0), + ok: true, + expected: int(math.MaxInt16), + }, + // From float64 to int. + { + name: "float64(1.5) to int", + value: float64(1.5), + typ: int(0), + ok: false, // loss of precision + }, + { + name: "float64(1.0) to int", + value: float64(1.0), + typ: int(0), + ok: true, + expected: int(1), + }, + { + name: "float64(math.MaxInt16+1) to int16", + value: float64(math.MaxInt16 + 1), + typ: int16(0), + ok: false, // overflow + }, + { + name: "float64(math.MaxFloat64) to int64", + value: float64(math.MaxFloat64), + typ: int64(0), + ok: false, // overflow + }, + { + name: "float64(32767) to int16", + value: float64(32767), + typ: int16(0), + ok: true, + expected: int16(32767), + }, + // From float32 to int. + { + name: "float32(1.5) to int", + value: float32(1.5), + typ: int(0), + ok: false, // loss of precision + }, + { + name: "float32(1.0) to int", + value: float32(1.0), + typ: int(0), + ok: true, + expected: int(1), + }, + { + name: "float32(math.MaxFloat32) to int16", + value: float32(math.MaxFloat32), + typ: int16(0), + ok: false, // overflow + }, + { + name: "float32(math.MaxFloat32) to int64", + value: float32(math.MaxFloat32), + typ: int64(0), + ok: false, // overflow + }, + { + name: "float32(math.MaxInt16) to int16", + value: float32(math.MaxInt16), + typ: int16(0), + ok: true, + expected: int16(32767), + }, + { + name: "float32(math.MaxInt16+1) to int16", + value: float32(math.MaxInt16 + 1), + typ: int16(0), + ok: false, // overflow + }, + // Int to float. + { + name: "int16(32767) to float32", + value: int16(32767), + typ: float32(0), + ok: true, + expected: float32(32767), + }, + { + name: "int64(32767) to float32", + value: int64(32767), + typ: float32(0), + ok: true, + expected: float32(32767), + }, + { + name: "int64(math.MaxInt64) to float32", + value: int64(math.MaxInt64), + typ: float32(0), + ok: true, + expected: float32(math.MaxInt64), + }, + { + name: "int64(math.MaxInt64) to float64", + value: int64(math.MaxInt64), + typ: float64(0), + ok: true, + expected: float64(math.MaxInt64), + }, + // Int to uint. + { + name: "int16(32767) to uint16", + value: int16(32767), + typ: uint16(0), + ok: true, + expected: uint16(32767), + }, + { + name: "int16(32767) to uint8", + value: int16(32767), + typ: uint8(0), + ok: false, + }, + { + name: "float64(3.14) to uint64", + value: float64(3.14), + typ: uint64(0), + ok: false, + }, + { + name: "float64(3.0) to uint64", + value: float64(3.0), + typ: uint64(0), + ok: true, + expected: uint64(3), + }, + // From uint to float. + { + name: "uint64(math.MaxInt16) to float64", + value: uint64(math.MaxInt16), + typ: float64(0), + ok: true, + expected: float64(math.MaxInt16), + }, + // Float to float. + { + name: "float64(3.14) to float32", + value: float64(3.14), + typ: float32(0), + ok: true, + expected: float32(3.14), + }, + { + name: "float32(3.14) to float64", + value: float32(3.14), + typ: float64(0), + ok: true, + expected: float64(3.14), + }, + { + name: "float64(3.14) to float64", + value: float64(3.14), + typ: float64(0), + ok: true, + expected: float64(3.14), + }, + } { + + v, ok := ConvertIfPossible(reflect.ValueOf(test.value), reflect.TypeOf(test.typ)) + c.Assert(ok, qt.Equals, test.ok, qt.Commentf("test case: %s", test.name)) + if test.ok { + c.Assert(v.Interface(), hqt.IsSameNumber, test.expected, qt.Commentf("test case: %s", test.name)) + } + } +} + +func TestConvertIfPossibleMisc(t *testing.T) { + c := qt.New(t) + type s string + + var ( + i = int32(42) + i64 = int64(i) + iv any = i + ip = &i + inil any = (*int32)(nil) + shello = s("hello") + ) + + convertOK := func(v any, typ any) any { + rv, ok := ConvertIfPossible(reflect.ValueOf(v), reflect.TypeOf(typ)) + c.Assert(ok, qt.IsTrue) + return rv.Interface() + } + + c.Assert(convertOK(shello, ""), qt.Equals, "hello") + c.Assert(convertOK(ip, int64(0)), qt.Equals, i64) + c.Assert(convertOK(iv, int64(0)), qt.Equals, i64) + c.Assert(convertOK(inil, int64(0)), qt.Equals, int64(0)) +} + +func BenchmarkToInt64(b *testing.B) { + v := reflect.ValueOf(int(42)) + for i := 0; i < b.N; i++ { + ToInt64(v) + } +} diff --git a/common/hreflect/helpers.go b/common/hreflect/helpers.go index 51be0d1cf..a0a5c0382 100644 --- a/common/hreflect/helpers.go +++ b/common/hreflect/helpers.go @@ -1,6 +1,4 @@ -// Copyright 2024 The Hugo Authors. All rights reserved. -// Some functions in this file (see comments) is based on the Go source code, -// copyright The Go Authors and governed by a BSD-style license. +// Copyright 2025 The Hugo Authors. All rights reserved. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -18,7 +16,6 @@ package hreflect import ( "context" - "math" "reflect" "sync" "time" @@ -28,6 +25,11 @@ import ( "github.com/gohugoio/hugo/common/types" ) +// IsInterfaceOrPointer returns whether the given kind is an interface or a pointer. +func IsInterfaceOrPointer(kind reflect.Kind) bool { + return kind == reflect.Interface || kind == reflect.Ptr +} + // TODO(bep) replace the private versions in /tpl with these. // IsNumber returns whether the given kind is a number. func IsNumber(kind reflect.Kind) bool { @@ -64,6 +66,11 @@ func IsFloat(kind reflect.Kind) bool { } } +// IsString returns whether the given kind is a string. +func IsString(kind reflect.Kind) bool { + return kind == reflect.String +} + // IsTruthful returns whether in represents a truthful value. // See IsTruthfulValue func IsTruthful(in any) bool { @@ -107,14 +114,14 @@ func implementsIsZero(tp reflect.Type) bool { // Based on: // https://github.com/golang/go/blob/178a2c42254166cffed1b25fb1d3c7a5727cada6/src/text/template/exec.go#L306 func IsTruthfulValue(val reflect.Value) (truth bool) { - val = indirectInterface(val) + val, isNil := Indirect(val) if !val.IsValid() { - // Something like var x interface{}, never set. It's a form of nil. + // Something like: var x any, never set. It's a form of nil. return } - if val.Kind() == reflect.Pointer && val.IsNil() { + if val.Kind() == reflect.Pointer && isNil { return } @@ -260,6 +267,7 @@ func ToSliceAny(v any) ([]any, bool) { return nil, false } +// CallMethodByName calls the method with the given name on v. func CallMethodByName(cxt context.Context, name string, v reflect.Value) []reflect.Value { fn := v.MethodByName(name) var args []reflect.Value @@ -277,15 +285,39 @@ func CallMethodByName(cxt context.Context, name string, v reflect.Value) []refle return fn.Call(args) } -// Based on: https://github.com/golang/go/blob/178a2c42254166cffed1b25fb1d3c7a5727cada6/src/text/template/exec.go#L931 -func indirectInterface(v reflect.Value) reflect.Value { - if v.Kind() != reflect.Interface { - return v +// Indirect unwraps interfaces and pointers until it finds a non-interface/pointer value. +// If a nil is encountered, the second return value is true. +// If a pointer to a struct is encountered, it is not unwrapped. +func Indirect(v reflect.Value) (vv reflect.Value, isNil bool) { + for ; IsInterfaceOrPointer(v.Kind()); v = v.Elem() { + if IsNil(v) { + return v, true + } + if v.Kind() != reflect.Interface { + // A pointer. + if v.NumMethod() > 0 { + break + } + if v.Elem().Kind() == reflect.Struct { + // Avoid unwrapping pointers to structs. + break + } + } } - if v.IsNil() { - return reflect.Value{} + return v, false +} + +// IsNil reports whether v is nil. +// Based on reflect.Value.IsNil, but also considers invalid values as nil. +func IsNil(v reflect.Value) bool { + if !v.IsValid() { + return true } - return v.Elem() + switch v.Kind() { + case reflect.Chan, reflect.Func, reflect.Interface, reflect.Map, reflect.Ptr, reflect.Slice: + return v.IsNil() + } + return false } var contextInterface = reflect.TypeOf((*context.Context)(nil)).Elem() @@ -310,45 +342,3 @@ func IsContextType(tp reflect.Type) bool { }) return isContext } - -// ConvertIfPossible tries to convert val to typ if possible. -// This is currently only implemented for int kinds, -// added to handle the move to a new YAML library which produces uint64 for unsigned integers. -// We can expand on this later if needed. -// This conversion is lossless. -// See Issue 14079. -func ConvertIfPossible(val reflect.Value, typ reflect.Type) (reflect.Value, bool) { - if IsInt(typ.Kind()) { - if IsInt(val.Kind()) { - if typ.OverflowInt(val.Int()) { - return reflect.Value{}, false - } - return val.Convert(typ), true - } - if IsUint(val.Kind()) { - if val.Uint() > uint64(math.MaxInt64) { - return reflect.Value{}, false - } - if typ.OverflowInt(int64(val.Uint())) { - return reflect.Value{}, false - } - return val.Convert(typ), true - } - if IsFloat(val.Kind()) { - f := val.Float() - if f < float64(math.MinInt64) || f > float64(math.MaxInt64) { - return reflect.Value{}, false - } - i := int64(f) - if typ.OverflowInt(i) { - return reflect.Value{}, false - } - // Check for lossless conversion. - if float64(i) != f { - return reflect.Value{}, false - } - return val.Convert(typ), true - } - } - return reflect.Value{}, false -} diff --git a/common/hreflect/helpers_test.go b/common/hreflect/helpers_test.go index 28d3519cf..cd1826ec3 100644 --- a/common/hreflect/helpers_test.go +++ b/common/hreflect/helpers_test.go @@ -1,4 +1,4 @@ -// Copyright 2019 The Hugo Authors. All rights reserved. +// Copyright 2025 The Hugo Authors. All rights reserved. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -15,12 +15,12 @@ package hreflect import ( "context" - "math" "reflect" "testing" "time" qt "github.com/frankban/quicktest" + "github.com/gohugoio/hugo/htesting/hqt" ) type zeroStruct struct { @@ -80,6 +80,77 @@ func TestToSliceAny(t *testing.T) { checkOK([]int{1, 2, 3}, []any{1, 2, 3}) } +type testIndirectStruct struct { + S string +} + +func (t *testIndirectStruct) GetS() string { + return t.S +} + +func (t testIndirectStruct) Foo() string { + return "bar" +} + +type testIndirectStructNoMethods struct { + S string +} + +func TestIsNil(t *testing.T) { + c := qt.New(t) + + var ( + nilPtr *testIndirectStruct + nilIface any = nil + nonNilIface any = &testIndirectStruct{S: "hello"} + ) + + c.Assert(IsNil(reflect.ValueOf(nilPtr)), qt.Equals, true) + c.Assert(IsNil(reflect.ValueOf(nilIface)), qt.Equals, true) + c.Assert(IsNil(reflect.ValueOf(nonNilIface)), qt.Equals, false) +} + +func TestIndirectInterface(t *testing.T) { + c := qt.New(t) + + var ( + structWithMethods = testIndirectStruct{S: "hello"} + structWithMethodsPointer = &testIndirectStruct{S: "hello"} + structWithMethodsPointerAny any = structWithMethodsPointer + structPointerToPointer = &structWithMethodsPointer + structNoMethodsPtr = &testIndirectStructNoMethods{S: "no methods"} + structNoMethods = testIndirectStructNoMethods{S: "no methods"} + intValue = 32 + intPtr = &intValue + nilPtr *testIndirectStruct + nilIface any = nil + ) + + ind := func(v any) any { + c.Helper() + vv, isNil := Indirect(reflect.ValueOf(v)) + c.Assert(isNil, qt.IsFalse) + return vv.Interface() + } + + c.Assert(ind(intValue), hqt.IsSameType, 32) + c.Assert(ind(intPtr), hqt.IsSameType, 32) + c.Assert(ind(structNoMethodsPtr), hqt.IsSameType, structNoMethodsPtr) + c.Assert(ind(structWithMethods), hqt.IsSameType, structWithMethods) + c.Assert(ind(structNoMethods), hqt.IsSameType, structNoMethods) + c.Assert(ind(structPointerToPointer), hqt.IsSameType, &testIndirectStruct{}) + c.Assert(ind(structWithMethodsPointer), hqt.IsSameType, &testIndirectStruct{}) + c.Assert(ind(structWithMethodsPointerAny), hqt.IsSameType, structWithMethodsPointer) + + vv, isNil := Indirect(reflect.ValueOf(nilPtr)) + c.Assert(isNil, qt.IsTrue) + c.Assert(vv, qt.Equals, reflect.ValueOf(nilPtr)) + + vv, isNil = Indirect(reflect.ValueOf(nilIface)) + c.Assert(isNil, qt.IsFalse) + c.Assert(vv, qt.Equals, reflect.ValueOf(nilIface)) +} + func BenchmarkIsContextType(b *testing.B) { type k string b.Run("value", func(b *testing.B) { @@ -177,93 +248,3 @@ func BenchmarkGetMethodByNamePara(b *testing.B) { } }) } - -func TestCastIfPossible(t *testing.T) { - c := qt.New(t) - - for _, test := range []struct { - name string - value any - typ any - expected any - ok bool - }{ - // From uint to int. - { - name: "uint64(math.MaxUint64) to int16", - value: uint64(math.MaxUint64), - typ: int16(0), - ok: false, // overflow - }, - - { - name: "uint64(math.MaxUint64) to int64", - value: uint64(math.MaxUint64), - typ: int64(0), - ok: false, // overflow - }, - { - name: "uint64(math.MaxInt16) to int16", - value: uint64(math.MaxInt16), - typ: int64(0), - ok: true, - expected: int64(math.MaxInt16), - }, - // From int to int. - { - name: "int64(math.MaxInt64) to int16", - value: int64(math.MaxInt64), - typ: int16(0), - ok: false, // overflow - }, - { - name: "int64(math.MaxInt16) to int", - value: int64(math.MaxInt16), - typ: int(0), - ok: true, - expected: int(math.MaxInt16), - }, - - { - name: "int64(math.MaxInt16) to int", - value: int64(math.MaxInt16), - typ: int(0), - ok: true, - expected: int(math.MaxInt16), - }, - // From float to int. - { - name: "float64(1.5) to int", - value: float64(1.5), - typ: int(0), - ok: false, // loss of precision - }, - { - name: "float64(1.0) to int", - value: float64(1.0), - typ: int(0), - ok: true, - expected: int(1), - }, - { - name: "float64(math.MaxFloat64) to int16", - value: float64(math.MaxFloat64), - typ: int16(0), - ok: false, // overflow - }, - { - name: "float64(32767) to int16", - value: float64(32767), - typ: int16(0), - ok: true, - expected: int16(32767), - }, - } { - - v, ok := ConvertIfPossible(reflect.ValueOf(test.value), reflect.TypeOf(test.typ)) - c.Assert(ok, qt.Equals, test.ok, qt.Commentf("test case: %s", test.name)) - if test.ok { - c.Assert(v.Interface(), qt.Equals, test.expected, qt.Commentf("test case: %s", test.name)) - } - } -} diff --git a/common/hstore/scratch.go b/common/hstore/scratch.go new file mode 100644 index 000000000..4b7a14bd1 --- /dev/null +++ b/common/hstore/scratch.go @@ -0,0 +1,160 @@ +// Copyright 2025 The Hugo Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package hstore + +import ( + "reflect" + "sort" + "sync" + + "github.com/gohugoio/hugo/common/collections" + "github.com/gohugoio/hugo/common/math" +) + +type StoreProvider interface { + // Store returns a Scratch that can be used to store temporary state. + // Store is not reset on server rebuilds. + Store() *Scratch +} + +// Scratch is a writable context used for stateful build operations +type Scratch struct { + values map[string]any + mu sync.RWMutex +} + +// Add will, for single values, add (using the + operator) the addend to the existing addend (if found). +// Supports numeric values and strings. +// +// If the first add for a key is an array or slice, then the next value(s) will be appended. +func (c *Scratch) Add(key string, newAddend any) (string, error) { + var newVal any + c.mu.RLock() + existingAddend, found := c.values[key] + c.mu.RUnlock() + if found { + var err error + + addendV := reflect.TypeOf(existingAddend) + + if addendV.Kind() == reflect.Slice || addendV.Kind() == reflect.Array { + newVal, err = collections.Append(existingAddend, newAddend) + if err != nil { + return "", err + } + } else { + newVal, err = math.DoArithmetic(existingAddend, newAddend, '+') + if err != nil { + return "", err + } + } + } else { + newVal = newAddend + } + c.mu.Lock() + c.values[key] = newVal + c.mu.Unlock() + return "", nil // have to return something to make it work with the Go templates +} + +// Set stores a value with the given key in the Node context. +// This value can later be retrieved with Get. +func (c *Scratch) Set(key string, value any) string { + c.mu.Lock() + c.values[key] = value + c.mu.Unlock() + return "" +} + +// Delete deletes the given key. +func (c *Scratch) Delete(key string) string { + c.mu.Lock() + delete(c.values, key) + c.mu.Unlock() + return "" +} + +// Get returns a value previously set by Add or Set. +func (c *Scratch) Get(key string) any { + c.mu.RLock() + val := c.values[key] + c.mu.RUnlock() + + return val +} + +// Values returns the raw backing map. Note that you should just use +// this method on the locally scoped Scratch instances you obtain via newScratch, not +// .Page.Scratch etc., as that will lead to concurrency issues. +func (c *Scratch) Values() map[string]any { + c.mu.RLock() + defer c.mu.RUnlock() + return c.values +} + +// SetInMap stores a value to a map with the given key in the Node context. +// This map can later be retrieved with GetSortedMapValues. +func (c *Scratch) SetInMap(key string, mapKey string, value any) string { + c.mu.Lock() + _, found := c.values[key] + if !found { + c.values[key] = make(map[string]any) + } + + c.values[key].(map[string]any)[mapKey] = value + c.mu.Unlock() + return "" +} + +// DeleteInMap deletes a value to a map with the given key in the Node context. +func (c *Scratch) DeleteInMap(key string, mapKey string) string { + c.mu.Lock() + _, found := c.values[key] + if found { + delete(c.values[key].(map[string]any), mapKey) + } + c.mu.Unlock() + return "" +} + +// GetSortedMapValues returns a sorted map previously filled with SetInMap. +func (c *Scratch) GetSortedMapValues(key string) any { + c.mu.RLock() + + if c.values[key] == nil { + c.mu.RUnlock() + return nil + } + + unsortedMap := c.values[key].(map[string]any) + c.mu.RUnlock() + var keys []string + for mapKey := range unsortedMap { + keys = append(keys, mapKey) + } + + sort.Strings(keys) + + sortedArray := make([]any, len(unsortedMap)) + for i, mapKey := range keys { + sortedArray[i] = unsortedMap[mapKey] + } + + return sortedArray +} + +// NewScratch returns a new instance of Scratch. +func NewScratch() *Scratch { + return &Scratch{values: make(map[string]any)} +} diff --git a/common/hstore/scratch_test.go b/common/hstore/scratch_test.go new file mode 100644 index 000000000..b7253ab2a --- /dev/null +++ b/common/hstore/scratch_test.go @@ -0,0 +1,221 @@ +// Copyright 2025 The Hugo Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package hstore + +import ( + "reflect" + "sync" + "testing" + + qt "github.com/frankban/quicktest" +) + +func TestScratchAdd(t *testing.T) { + t.Parallel() + c := qt.New(t) + + scratch := NewScratch() + scratch.Add("int1", 10) + scratch.Add("int1", 20) + scratch.Add("int2", 20) + + c.Assert(scratch.Get("int1"), qt.Equals, int64(30)) + c.Assert(scratch.Get("int2"), qt.Equals, 20) + + scratch.Add("float1", float64(10.5)) + scratch.Add("float1", float64(20.1)) + + c.Assert(scratch.Get("float1"), qt.Equals, float64(30.6)) + + scratch.Add("string1", "Hello ") + scratch.Add("string1", "big ") + scratch.Add("string1", "World!") + + c.Assert(scratch.Get("string1"), qt.Equals, "Hello big World!") + + scratch.Add("scratch", scratch) + _, err := scratch.Add("scratch", scratch) + + m := scratch.Values() + c.Assert(m, qt.HasLen, 5) + + if err == nil { + t.Errorf("Expected error from invalid arithmetic") + } +} + +func TestScratchAddSlice(t *testing.T) { + t.Parallel() + c := qt.New(t) + + scratch := NewScratch() + + _, err := scratch.Add("intSlice", []int{1, 2}) + c.Assert(err, qt.IsNil) + _, err = scratch.Add("intSlice", 3) + c.Assert(err, qt.IsNil) + + sl := scratch.Get("intSlice") + expected := []int{1, 2, 3} + + if !reflect.DeepEqual(expected, sl) { + t.Errorf("Slice difference, go %q expected %q", sl, expected) + } + _, err = scratch.Add("intSlice", []int{4, 5}) + + c.Assert(err, qt.IsNil) + + sl = scratch.Get("intSlice") + expected = []int{1, 2, 3, 4, 5} + + if !reflect.DeepEqual(expected, sl) { + t.Errorf("Slice difference, go %q expected %q", sl, expected) + } +} + +// https://github.com/gohugoio/hugo/issues/5275 +func TestScratchAddTypedSliceToInterfaceSlice(t *testing.T) { + t.Parallel() + c := qt.New(t) + + scratch := NewScratch() + scratch.Set("slice", []any{}) + + _, err := scratch.Add("slice", []int{1, 2}) + c.Assert(err, qt.IsNil) + c.Assert(scratch.Get("slice"), qt.DeepEquals, []int{1, 2}) +} + +// https://github.com/gohugoio/hugo/issues/5361 +func TestScratchAddDifferentTypedSliceToInterfaceSlice(t *testing.T) { + t.Parallel() + c := qt.New(t) + + scratch := NewScratch() + scratch.Set("slice", []string{"foo"}) + + _, err := scratch.Add("slice", []int{1, 2}) + c.Assert(err, qt.IsNil) + c.Assert(scratch.Get("slice"), qt.DeepEquals, []any{"foo", 1, 2}) +} + +func TestScratchSet(t *testing.T) { + t.Parallel() + c := qt.New(t) + + scratch := NewScratch() + scratch.Set("key", "val") + c.Assert(scratch.Get("key"), qt.Equals, "val") +} + +func TestScratchDelete(t *testing.T) { + t.Parallel() + c := qt.New(t) + + scratch := NewScratch() + scratch.Set("key", "val") + scratch.Delete("key") + scratch.Add("key", "Lucy Parsons") + c.Assert(scratch.Get("key"), qt.Equals, "Lucy Parsons") +} + +// Issue #2005 +func TestScratchInParallel(t *testing.T) { + var wg sync.WaitGroup + scratch := NewScratch() + + key := "counter" + scratch.Set(key, int64(1)) + for i := 1; i <= 10; i++ { + wg.Add(1) + go func(j int) { + for k := range 10 { + newVal := int64(k + j) + + _, err := scratch.Add(key, newVal) + if err != nil { + t.Errorf("Got err %s", err) + } + + scratch.Set(key, newVal) + + val := scratch.Get(key) + + if counter, ok := val.(int64); ok { + if counter < 1 { + t.Errorf("Got %d", counter) + } + } else { + t.Errorf("Got %T", val) + } + } + wg.Done() + }(i) + } + wg.Wait() +} + +func TestScratchGet(t *testing.T) { + t.Parallel() + scratch := NewScratch() + nothing := scratch.Get("nothing") + if nothing != nil { + t.Errorf("Should not return anything, but got %v", nothing) + } +} + +func TestScratchSetInMap(t *testing.T) { + t.Parallel() + c := qt.New(t) + + scratch := NewScratch() + scratch.SetInMap("key", "lux", "Lux") + scratch.SetInMap("key", "abc", "Abc") + scratch.SetInMap("key", "zyx", "Zyx") + scratch.SetInMap("key", "abc", "Abc (updated)") + scratch.SetInMap("key", "def", "Def") + c.Assert(scratch.GetSortedMapValues("key"), qt.DeepEquals, any([]any{"Abc (updated)", "Def", "Lux", "Zyx"})) +} + +func TestScratchDeleteInMap(t *testing.T) { + t.Parallel() + c := qt.New(t) + + scratch := NewScratch() + scratch.SetInMap("key", "lux", "Lux") + scratch.SetInMap("key", "abc", "Abc") + scratch.SetInMap("key", "zyx", "Zyx") + scratch.DeleteInMap("key", "abc") + scratch.SetInMap("key", "def", "Def") + scratch.DeleteInMap("key", "lmn") // Do nothing + c.Assert(scratch.GetSortedMapValues("key"), qt.DeepEquals, any([]any{"Def", "Lux", "Zyx"})) +} + +func TestScratchGetSortedMapValues(t *testing.T) { + t.Parallel() + scratch := NewScratch() + nothing := scratch.GetSortedMapValues("nothing") + if nothing != nil { + t.Errorf("Should not return anything, but got %v", nothing) + } +} + +func BenchmarkScratchGet(b *testing.B) { + scratch := NewScratch() + scratch.Add("A", 1) + b.ResetTimer() + for i := 0; i < b.N; i++ { + scratch.Get("A") + } +} diff --git a/common/hugo/hugo.go b/common/hugo/hugo.go index 38f78bc35..d0345c3c4 100644 --- a/common/hugo/hugo.go +++ b/common/hugo/hugo.go @@ -30,8 +30,8 @@ import ( "github.com/bep/godartsass/v2" "github.com/gohugoio/hugo/common/hexec" + "github.com/gohugoio/hugo/common/hstore" "github.com/gohugoio/hugo/common/loggers" - "github.com/gohugoio/hugo/common/maps" "github.com/gohugoio/hugo/hugofs/files" "github.com/bep/helpers/contexthelpers" @@ -55,7 +55,7 @@ var ( vendorInfo string ) -var _ maps.StoreProvider = (*HugoInfo)(nil) +var _ hstore.StoreProvider = (*HugoInfo)(nil) // HugoInfo contains information about the current Hugo environment type HugoInfo struct { @@ -74,7 +74,7 @@ type HugoInfo struct { conf ConfigProvider deps []*Dependency - store *maps.Scratch + store *hstore.Scratch // Context gives access to some of the context scoped variables. Context Context @@ -120,7 +120,7 @@ func (i HugoInfo) Deps() []*Dependency { return i.deps } -func (i HugoInfo) Store() *maps.Scratch { +func (i HugoInfo) Store() *hstore.Scratch { return i.store } @@ -197,7 +197,7 @@ func NewInfo(conf ConfigProvider, deps []*Dependency) HugoInfo { Environment: conf.Environment(), conf: conf, deps: deps, - store: maps.NewScratch(), + store: hstore.NewScratch(), GoVersion: goVersion, } } diff --git a/common/maps/scratch.go b/common/maps/scratch.go deleted file mode 100644 index cf5231783..000000000 --- a/common/maps/scratch.go +++ /dev/null @@ -1,160 +0,0 @@ -// Copyright 2019 The Hugo Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package maps - -import ( - "reflect" - "sort" - "sync" - - "github.com/gohugoio/hugo/common/collections" - "github.com/gohugoio/hugo/common/math" -) - -type StoreProvider interface { - // Store returns a Scratch that can be used to store temporary state. - // Store is not reset on server rebuilds. - Store() *Scratch -} - -// Scratch is a writable context used for stateful build operations -type Scratch struct { - values map[string]any - mu sync.RWMutex -} - -// Add will, for single values, add (using the + operator) the addend to the existing addend (if found). -// Supports numeric values and strings. -// -// If the first add for a key is an array or slice, then the next value(s) will be appended. -func (c *Scratch) Add(key string, newAddend any) (string, error) { - var newVal any - c.mu.RLock() - existingAddend, found := c.values[key] - c.mu.RUnlock() - if found { - var err error - - addendV := reflect.TypeOf(existingAddend) - - if addendV.Kind() == reflect.Slice || addendV.Kind() == reflect.Array { - newVal, err = collections.Append(existingAddend, newAddend) - if err != nil { - return "", err - } - } else { - newVal, err = math.DoArithmetic(existingAddend, newAddend, '+') - if err != nil { - return "", err - } - } - } else { - newVal = newAddend - } - c.mu.Lock() - c.values[key] = newVal - c.mu.Unlock() - return "", nil // have to return something to make it work with the Go templates -} - -// Set stores a value with the given key in the Node context. -// This value can later be retrieved with Get. -func (c *Scratch) Set(key string, value any) string { - c.mu.Lock() - c.values[key] = value - c.mu.Unlock() - return "" -} - -// Delete deletes the given key. -func (c *Scratch) Delete(key string) string { - c.mu.Lock() - delete(c.values, key) - c.mu.Unlock() - return "" -} - -// Get returns a value previously set by Add or Set. -func (c *Scratch) Get(key string) any { - c.mu.RLock() - val := c.values[key] - c.mu.RUnlock() - - return val -} - -// Values returns the raw backing map. Note that you should just use -// this method on the locally scoped Scratch instances you obtain via newScratch, not -// .Page.Scratch etc., as that will lead to concurrency issues. -func (c *Scratch) Values() map[string]any { - c.mu.RLock() - defer c.mu.RUnlock() - return c.values -} - -// SetInMap stores a value to a map with the given key in the Node context. -// This map can later be retrieved with GetSortedMapValues. -func (c *Scratch) SetInMap(key string, mapKey string, value any) string { - c.mu.Lock() - _, found := c.values[key] - if !found { - c.values[key] = make(map[string]any) - } - - c.values[key].(map[string]any)[mapKey] = value - c.mu.Unlock() - return "" -} - -// DeleteInMap deletes a value to a map with the given key in the Node context. -func (c *Scratch) DeleteInMap(key string, mapKey string) string { - c.mu.Lock() - _, found := c.values[key] - if found { - delete(c.values[key].(map[string]any), mapKey) - } - c.mu.Unlock() - return "" -} - -// GetSortedMapValues returns a sorted map previously filled with SetInMap. -func (c *Scratch) GetSortedMapValues(key string) any { - c.mu.RLock() - - if c.values[key] == nil { - c.mu.RUnlock() - return nil - } - - unsortedMap := c.values[key].(map[string]any) - c.mu.RUnlock() - var keys []string - for mapKey := range unsortedMap { - keys = append(keys, mapKey) - } - - sort.Strings(keys) - - sortedArray := make([]any, len(unsortedMap)) - for i, mapKey := range keys { - sortedArray[i] = unsortedMap[mapKey] - } - - return sortedArray -} - -// NewScratch returns a new instance of Scratch. -func NewScratch() *Scratch { - return &Scratch{values: make(map[string]any)} -} diff --git a/common/maps/scratch_test.go b/common/maps/scratch_test.go deleted file mode 100644 index f07169e61..000000000 --- a/common/maps/scratch_test.go +++ /dev/null @@ -1,221 +0,0 @@ -// Copyright 2018 The Hugo Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package maps - -import ( - "reflect" - "sync" - "testing" - - qt "github.com/frankban/quicktest" -) - -func TestScratchAdd(t *testing.T) { - t.Parallel() - c := qt.New(t) - - scratch := NewScratch() - scratch.Add("int1", 10) - scratch.Add("int1", 20) - scratch.Add("int2", 20) - - c.Assert(scratch.Get("int1"), qt.Equals, int64(30)) - c.Assert(scratch.Get("int2"), qt.Equals, 20) - - scratch.Add("float1", float64(10.5)) - scratch.Add("float1", float64(20.1)) - - c.Assert(scratch.Get("float1"), qt.Equals, float64(30.6)) - - scratch.Add("string1", "Hello ") - scratch.Add("string1", "big ") - scratch.Add("string1", "World!") - - c.Assert(scratch.Get("string1"), qt.Equals, "Hello big World!") - - scratch.Add("scratch", scratch) - _, err := scratch.Add("scratch", scratch) - - m := scratch.Values() - c.Assert(m, qt.HasLen, 5) - - if err == nil { - t.Errorf("Expected error from invalid arithmetic") - } -} - -func TestScratchAddSlice(t *testing.T) { - t.Parallel() - c := qt.New(t) - - scratch := NewScratch() - - _, err := scratch.Add("intSlice", []int{1, 2}) - c.Assert(err, qt.IsNil) - _, err = scratch.Add("intSlice", 3) - c.Assert(err, qt.IsNil) - - sl := scratch.Get("intSlice") - expected := []int{1, 2, 3} - - if !reflect.DeepEqual(expected, sl) { - t.Errorf("Slice difference, go %q expected %q", sl, expected) - } - _, err = scratch.Add("intSlice", []int{4, 5}) - - c.Assert(err, qt.IsNil) - - sl = scratch.Get("intSlice") - expected = []int{1, 2, 3, 4, 5} - - if !reflect.DeepEqual(expected, sl) { - t.Errorf("Slice difference, go %q expected %q", sl, expected) - } -} - -// https://github.com/gohugoio/hugo/issues/5275 -func TestScratchAddTypedSliceToInterfaceSlice(t *testing.T) { - t.Parallel() - c := qt.New(t) - - scratch := NewScratch() - scratch.Set("slice", []any{}) - - _, err := scratch.Add("slice", []int{1, 2}) - c.Assert(err, qt.IsNil) - c.Assert(scratch.Get("slice"), qt.DeepEquals, []int{1, 2}) -} - -// https://github.com/gohugoio/hugo/issues/5361 -func TestScratchAddDifferentTypedSliceToInterfaceSlice(t *testing.T) { - t.Parallel() - c := qt.New(t) - - scratch := NewScratch() - scratch.Set("slice", []string{"foo"}) - - _, err := scratch.Add("slice", []int{1, 2}) - c.Assert(err, qt.IsNil) - c.Assert(scratch.Get("slice"), qt.DeepEquals, []any{"foo", 1, 2}) -} - -func TestScratchSet(t *testing.T) { - t.Parallel() - c := qt.New(t) - - scratch := NewScratch() - scratch.Set("key", "val") - c.Assert(scratch.Get("key"), qt.Equals, "val") -} - -func TestScratchDelete(t *testing.T) { - t.Parallel() - c := qt.New(t) - - scratch := NewScratch() - scratch.Set("key", "val") - scratch.Delete("key") - scratch.Add("key", "Lucy Parsons") - c.Assert(scratch.Get("key"), qt.Equals, "Lucy Parsons") -} - -// Issue #2005 -func TestScratchInParallel(t *testing.T) { - var wg sync.WaitGroup - scratch := NewScratch() - - key := "counter" - scratch.Set(key, int64(1)) - for i := 1; i <= 10; i++ { - wg.Add(1) - go func(j int) { - for k := range 10 { - newVal := int64(k + j) - - _, err := scratch.Add(key, newVal) - if err != nil { - t.Errorf("Got err %s", err) - } - - scratch.Set(key, newVal) - - val := scratch.Get(key) - - if counter, ok := val.(int64); ok { - if counter < 1 { - t.Errorf("Got %d", counter) - } - } else { - t.Errorf("Got %T", val) - } - } - wg.Done() - }(i) - } - wg.Wait() -} - -func TestScratchGet(t *testing.T) { - t.Parallel() - scratch := NewScratch() - nothing := scratch.Get("nothing") - if nothing != nil { - t.Errorf("Should not return anything, but got %v", nothing) - } -} - -func TestScratchSetInMap(t *testing.T) { - t.Parallel() - c := qt.New(t) - - scratch := NewScratch() - scratch.SetInMap("key", "lux", "Lux") - scratch.SetInMap("key", "abc", "Abc") - scratch.SetInMap("key", "zyx", "Zyx") - scratch.SetInMap("key", "abc", "Abc (updated)") - scratch.SetInMap("key", "def", "Def") - c.Assert(scratch.GetSortedMapValues("key"), qt.DeepEquals, any([]any{"Abc (updated)", "Def", "Lux", "Zyx"})) -} - -func TestScratchDeleteInMap(t *testing.T) { - t.Parallel() - c := qt.New(t) - - scratch := NewScratch() - scratch.SetInMap("key", "lux", "Lux") - scratch.SetInMap("key", "abc", "Abc") - scratch.SetInMap("key", "zyx", "Zyx") - scratch.DeleteInMap("key", "abc") - scratch.SetInMap("key", "def", "Def") - scratch.DeleteInMap("key", "lmn") // Do nothing - c.Assert(scratch.GetSortedMapValues("key"), qt.DeepEquals, any([]any{"Def", "Lux", "Zyx"})) -} - -func TestScratchGetSortedMapValues(t *testing.T) { - t.Parallel() - scratch := NewScratch() - nothing := scratch.GetSortedMapValues("nothing") - if nothing != nil { - t.Errorf("Should not return anything, but got %v", nothing) - } -} - -func BenchmarkScratchGet(b *testing.B) { - scratch := NewScratch() - scratch.Add("A", 1) - b.ResetTimer() - for i := 0; i < b.N; i++ { - scratch.Get("A") - } -} diff --git a/htesting/hqt/checkers.go b/htesting/hqt/checkers.go index b06185756..f58382fb5 100644 --- a/htesting/hqt/checkers.go +++ b/htesting/hqt/checkers.go @@ -44,6 +44,46 @@ var IsSameFloat64 = qt.CmpEquals(cmp.Comparer(func(a, b float64) bool { return math.Abs(a-b) < 0.0001 })) +// IsSameNumber asserts that two number values are equal within a small delta. +var IsSameNumber = qt.CmpEquals( + cmp.Comparer(func(a, b float64) bool { + return math.Abs(a-b) < 0.0001 + }), + cmp.Comparer(func(a, b float32) bool { + return math.Abs(float64(a)-float64(b)) < 0.0001 + }), + cmp.Comparer(func(a, b int) bool { + return a == b + }), + cmp.Comparer(func(a, b int8) bool { + return a == b + }), + cmp.Comparer(func(a, b int16) bool { + return a == b + }), + cmp.Comparer(func(a, b int32) bool { + return a == b + }), + cmp.Comparer(func(a, b int64) bool { + return a == b + }), + cmp.Comparer(func(a, b uint) bool { + return a == b + }), + cmp.Comparer(func(a, b uint8) bool { + return a == b + }), + cmp.Comparer(func(a, b uint16) bool { + return a == b + }), + cmp.Comparer(func(a, b uint32) bool { + return a == b + }), + cmp.Comparer(func(a, b uint64) bool { + return a == b + }), +) + type argNames []string func (a argNames) ArgNames() []string { diff --git a/hugolib/page__common.go b/hugolib/page__common.go index 0b9cf0200..d7584155c 100644 --- a/hugolib/page__common.go +++ b/hugolib/page__common.go @@ -16,7 +16,7 @@ package hugolib import ( "sync" - "github.com/gohugoio/hugo/common/maps" + "github.com/gohugoio/hugo/common/hstore" "github.com/gohugoio/hugo/compare" "github.com/gohugoio/hugo/lazy" "github.com/gohugoio/hugo/markup/converter" @@ -52,7 +52,7 @@ type pageCommon struct { init *lazy.Init // Store holds state that survives server rebuilds. - store *maps.Scratch + store *hstore.Scratch // All of these represents the common parts of a page.Page navigation.PageMenusProvider @@ -103,11 +103,11 @@ type pageCommon struct { contentConverter converter.Converter } -func (p *pageCommon) Store() *maps.Scratch { +func (p *pageCommon) Store() *hstore.Scratch { return p.store } // See issue 13016. -func (p *pageCommon) Scratch() *maps.Scratch { +func (p *pageCommon) Scratch() *hstore.Scratch { return p.Store() } diff --git a/hugolib/page__new.go b/hugolib/page__new.go index 80115cc72..6b78dd083 100644 --- a/hugolib/page__new.go +++ b/hugolib/page__new.go @@ -22,6 +22,7 @@ import ( "github.com/gohugoio/hugo/resources" "github.com/gohugoio/hugo/common/constants" + "github.com/gohugoio/hugo/common/hstore" "github.com/gohugoio/hugo/common/maps" "github.com/gohugoio/hugo/common/paths" @@ -202,7 +203,7 @@ func (h *HugoSites) doNewPage(m *pageMeta) (*pageState, *paths.Path, error) { dependencyManager: m.s.Conf.NewIdentityManager(m.Path()), pageCommon: &pageCommon{ FileProvider: m, - store: maps.NewScratch(), + store: hstore.NewScratch(), Positioner: page.NopPage, InSectionPositioner: page.NopPage, ResourceNameTitleProvider: m, diff --git a/hugolib/pagesfromdata/pagesfromgotmpl.go b/hugolib/pagesfromdata/pagesfromgotmpl.go index 6b369567b..2cb01ae1d 100644 --- a/hugolib/pagesfromdata/pagesfromgotmpl.go +++ b/hugolib/pagesfromdata/pagesfromgotmpl.go @@ -20,6 +20,7 @@ import ( "path/filepath" "github.com/gohugoio/hugo/common/hashing" + "github.com/gohugoio/hugo/common/hstore" "github.com/gohugoio/hugo/common/maps" "github.com/gohugoio/hugo/common/paths" "github.com/gohugoio/hugo/helpers" @@ -48,7 +49,7 @@ type PagesFromDataTemplateContext interface { // The same template may be executed multiple times for multiple languages. // The Store can be used to store state between these invocations. - Store() *maps.Scratch + Store() *hstore.Scratch // By default, the template will be executed for the language // defined by the _content.gotmpl file (e.g. its mount definition). @@ -133,7 +134,7 @@ func (p *pagesFromDataTemplateContext) Site() page.Site { return p.p.Site } -func (p *pagesFromDataTemplateContext) Store() *maps.Scratch { +func (p *pagesFromDataTemplateContext) Store() *hstore.Scratch { return p.p.store } @@ -149,7 +150,7 @@ func NewPagesFromTemplate(opts PagesFromTemplateOptions) *PagesFromTemplate { buildState: &BuildState{ sourceInfosCurrent: maps.NewCache[string, *sourceInfo](), }, - store: maps.NewScratch(), + store: hstore.NewScratch(), } } @@ -177,7 +178,7 @@ type PagesFromTemplate struct { PagesFromTemplateOptions PagesFromTemplateDeps buildState *BuildState - store *maps.Scratch + store *hstore.Scratch } func (b *PagesFromTemplate) AddChange(id identity.Identity) { diff --git a/hugolib/shortcode.go b/hugolib/shortcode.go index 56bf1ff9e..6a5316682 100644 --- a/hugolib/shortcode.go +++ b/hugolib/shortcode.go @@ -28,13 +28,13 @@ import ( "sync" "github.com/gohugoio/hugo/common/herrors" + "github.com/gohugoio/hugo/common/hstore" "github.com/gohugoio/hugo/common/types" "github.com/gohugoio/hugo/tpl/tplimpl" "github.com/gohugoio/hugo/parser/pageparser" "github.com/gohugoio/hugo/resources/page" - "github.com/gohugoio/hugo/common/maps" "github.com/gohugoio/hugo/common/text" "github.com/gohugoio/hugo/common/urls" @@ -43,10 +43,10 @@ import ( ) var ( - _ urls.RefLinker = (*ShortcodeWithPage)(nil) - _ types.Unwrapper = (*ShortcodeWithPage)(nil) - _ text.Positioner = (*ShortcodeWithPage)(nil) - _ maps.StoreProvider = (*ShortcodeWithPage)(nil) + _ urls.RefLinker = (*ShortcodeWithPage)(nil) + _ types.Unwrapper = (*ShortcodeWithPage)(nil) + _ text.Positioner = (*ShortcodeWithPage)(nil) + _ hstore.StoreProvider = (*ShortcodeWithPage)(nil) ) // ShortcodeWithPage is the "." context in a shortcode template. @@ -73,7 +73,7 @@ type ShortcodeWithPage struct { posOffset int pos text.Position - store *maps.Scratch + store *hstore.Scratch } // InnerDeindent returns the (potentially de-indented) inner content of the shortcode. @@ -126,9 +126,9 @@ func (scp *ShortcodeWithPage) RelRef(args map[string]any) (string, error) { } // Store returns this shortcode's Store. -func (scp *ShortcodeWithPage) Store() *maps.Scratch { +func (scp *ShortcodeWithPage) Store() *hstore.Scratch { if scp.store == nil { - scp.store = maps.NewScratch() + scp.store = hstore.NewScratch() } return scp.store } @@ -136,7 +136,7 @@ func (scp *ShortcodeWithPage) Store() *maps.Scratch { // Scratch returns a scratch-pad scoped for this shortcode. This can be used // as a temporary storage for variables, counters etc. // Deprecated: Use Store instead. Note that from the templates this should be considered a "soft deprecation". -func (scp *ShortcodeWithPage) Scratch() *maps.Scratch { +func (scp *ShortcodeWithPage) Scratch() *hstore.Scratch { return scp.Store() } diff --git a/hugolib/site.go b/hugolib/site.go index 3c89b1ee3..ff9d0310a 100644 --- a/hugolib/site.go +++ b/hugolib/site.go @@ -31,6 +31,7 @@ import ( "github.com/bep/logg" "github.com/gohugoio/hugo/cache/dynacache" + "github.com/gohugoio/hugo/common/hstore" "github.com/gohugoio/hugo/common/htime" "github.com/gohugoio/hugo/common/hugio" "github.com/gohugoio/hugo/common/hugo" @@ -103,7 +104,7 @@ type Site struct { language *langs.Language languagei int pageMap *pageMap - store *maps.Scratch + store *hstore.Scratch // The owning container. h *HugoSites @@ -267,7 +268,7 @@ func NewHugoSites(cfg deps.DepsCfg) (*HugoSites, error) { language: language, languagei: i, frontmatterHandler: frontmatterHandler, - store: maps.NewScratch(), + store: hstore.NewScratch(), } if i == 0 { @@ -639,7 +640,7 @@ func (s *Site) AllRegularPages() page.Pages { return s.h.RegularPages() } -func (s *Site) Store() *maps.Scratch { +func (s *Site) Store() *hstore.Scratch { return s.store } diff --git a/resources/page/page.go b/resources/page/page.go index 490afd890..5eceaf56f 100644 --- a/resources/page/page.go +++ b/resources/page/page.go @@ -25,7 +25,7 @@ import ( "github.com/gohugoio/hugo/config" - "github.com/gohugoio/hugo/common/maps" + "github.com/gohugoio/hugo/common/hstore" "github.com/gohugoio/hugo/common/paths" "github.com/gohugoio/hugo/compare" @@ -384,9 +384,9 @@ type PageWithoutContent interface { // Note that this Scratch gets reset on server rebuilds. See Store() for a variant that survives. // Scratch returns a "scratch pad" that can be used to store state. // Deprecated: From Hugo v0.138.0 this is just an alias for Store. - Scratch() *maps.Scratch + Scratch() *hstore.Scratch - maps.StoreProvider + hstore.StoreProvider RelatedKeywordsProvider diff --git a/resources/page/page_nop.go b/resources/page/page_nop.go index 0fc9628c9..865470479 100644 --- a/resources/page/page_nop.go +++ b/resources/page/page_nop.go @@ -28,6 +28,7 @@ import ( "github.com/gohugoio/hugo/navigation" + "github.com/gohugoio/hugo/common/hstore" "github.com/gohugoio/hugo/common/hugo" "github.com/gohugoio/hugo/common/maps" "github.com/gohugoio/hugo/common/paths" @@ -414,11 +415,11 @@ func (p *nopPage) Resources() resource.Resources { return nil } -func (p *nopPage) Scratch() *maps.Scratch { +func (p *nopPage) Scratch() *hstore.Scratch { return nil } -func (p *nopPage) Store() *maps.Scratch { +func (p *nopPage) Store() *hstore.Scratch { return nil } diff --git a/resources/page/site.go b/resources/page/site.go index 3c9e9e78c..4b48ad538 100644 --- a/resources/page/site.go +++ b/resources/page/site.go @@ -16,6 +16,7 @@ package page import ( "time" + "github.com/gohugoio/hugo/common/hstore" "github.com/gohugoio/hugo/common/maps" "github.com/gohugoio/hugo/config/privacy" "github.com/gohugoio/hugo/config/services" @@ -122,7 +123,7 @@ type Site interface { // LanguagePrefix returns the language prefix for this site. LanguagePrefix() string - maps.StoreProvider + hstore.StoreProvider // For internal use only. // This will panic if the site is not fully initialized. @@ -296,7 +297,7 @@ func (s *siteWrapper) LanguagePrefix() string { return s.s.LanguagePrefix() } -func (s *siteWrapper) Store() *maps.Scratch { +func (s *siteWrapper) Store() *hstore.Scratch { return s.s.Store() } @@ -444,8 +445,8 @@ func (s testSite) Param(key any) (any, error) { return nil, nil } -func (s testSite) Store() *maps.Scratch { - return maps.NewScratch() +func (s testSite) Store() *hstore.Scratch { + return hstore.NewScratch() } func (s testSite) CheckReady() { diff --git a/resources/page/testhelpers_test.go b/resources/page/testhelpers_test.go index b9ccee004..4c6cfbd30 100644 --- a/resources/page/testhelpers_test.go +++ b/resources/page/testhelpers_test.go @@ -27,6 +27,7 @@ import ( "github.com/gohugoio/hugo/navigation" + "github.com/gohugoio/hugo/common/hstore" "github.com/gohugoio/hugo/common/hugo" "github.com/gohugoio/hugo/common/maps" "github.com/gohugoio/hugo/common/paths" @@ -493,11 +494,11 @@ func (p *testPage) Resources() resource.Resources { panic("testpage: not implemented") } -func (p *testPage) Scratch() *maps.Scratch { +func (p *testPage) Scratch() *hstore.Scratch { panic("testpage: not implemented") } -func (p *testPage) Store() *maps.Scratch { +func (p *testPage) Store() *hstore.Scratch { panic("testpage: not implemented") } diff --git a/tpl/collections/apply.go b/tpl/collections/apply.go index caa51f149..5b3cbe54d 100644 --- a/tpl/collections/apply.go +++ b/tpl/collections/apply.go @@ -34,7 +34,7 @@ func (ns *Namespace) Apply(ctx context.Context, c any, fname string, args ...any } seqv := reflect.ValueOf(c) - seqv, isNil := indirect(seqv) + seqv, isNil := hreflect.Indirect(seqv) if isNil { return nil, errors.New("can't iterate over a nil value") } @@ -135,28 +135,3 @@ func (ns *Namespace) lookupFunc(ctx context.Context, fname string) (reflect.Valu } return m, true } - -// indirect is borrowed from the Go stdlib: 'text/template/exec.go' -func indirect(v reflect.Value) (rv reflect.Value, isNil bool) { - for ; v.Kind() == reflect.Ptr || v.Kind() == reflect.Interface; v = v.Elem() { - if v.IsNil() { - return v, true - } - if v.Kind() == reflect.Interface && v.NumMethod() > 0 { - break - } - } - return v, false -} - -func indirectInterface(v reflect.Value) (rv reflect.Value, isNil bool) { - for ; v.Kind() == reflect.Interface; v = v.Elem() { - if v.IsNil() { - return v, true - } - if v.Kind() == reflect.Interface && v.NumMethod() > 0 { - break - } - } - return v, false -} diff --git a/tpl/collections/collections.go b/tpl/collections/collections.go index b678f0c1e..47f4f139a 100644 --- a/tpl/collections/collections.go +++ b/tpl/collections/collections.go @@ -25,6 +25,8 @@ import ( "time" "github.com/gohugoio/hugo/common/collections" + "github.com/gohugoio/hugo/common/hreflect" + "github.com/gohugoio/hugo/common/hstore" "github.com/gohugoio/hugo/common/maps" "github.com/gohugoio/hugo/common/types" "github.com/gohugoio/hugo/deps" @@ -75,7 +77,7 @@ func (ns *Namespace) After(n any, l any) (any, error) { } lv := reflect.ValueOf(l) - lv, isNil := indirect(lv) + lv, isNil := hreflect.Indirect(lv) if isNil { return nil, errors.New("can't iterate over a nil value") } @@ -114,7 +116,7 @@ func (ns *Namespace) Delimit(ctx context.Context, l, sep any, last ...any) (stri } lv := reflect.ValueOf(l) - lv, isNil := indirect(lv) + lv, isNil := hreflect.Indirect(lv) if isNil { return "", errors.New("can't iterate over a nil value") } @@ -207,7 +209,7 @@ func (ns *Namespace) First(limit any, l any) (any, error) { } lv := reflect.ValueOf(l) - lv, isNil := indirect(lv) + lv, isNil := hreflect.Indirect(lv) if isNil { return nil, errors.New("can't iterate over a nil value") } @@ -240,7 +242,7 @@ func (ns *Namespace) In(l any, v any) (bool, error) { switch lv.Kind() { case reflect.Array, reflect.Slice: for i := range lv.Len() { - lvv, isNil := indirectInterface(lv.Index(i)) + lvv, isNil := hreflect.Indirect(lv.Index(i)) if isNil { continue } @@ -367,7 +369,7 @@ func (ns *Namespace) Last(limit any, l any) (any, error) { } seqv := reflect.ValueOf(l) - seqv, isNil := indirect(seqv) + seqv, isNil := hreflect.Indirect(seqv) if isNil { return nil, errors.New("can't iterate over a nil value") } @@ -497,7 +499,7 @@ func (ns *Namespace) Shuffle(l any) (any, error) { } lv := reflect.ValueOf(l) - lv, isNil := indirect(lv) + lv, isNil := hreflect.Indirect(lv) if isNil { return nil, errors.New("can't iterate over a nil value") } @@ -574,13 +576,13 @@ func (i *intersector) appendIfNotSeen(v reflect.Value) { func (i *intersector) handleValuePair(l1vv, l2vv reflect.Value) { switch kind := l1vv.Kind(); { case kind == reflect.String: - l2t, err := toString(l2vv) + l2t, err := hreflect.ToStringE(l2vv) if err == nil && l1vv.String() == l2t { i.appendIfNotSeen(l1vv) } - case isNumber(kind): - f1, err1 := numberToFloat(l1vv) - f2, err2 := numberToFloat(l2vv) + case hreflect.IsNumber(kind): + f1, err1 := hreflect.ToFloat64E(l1vv) + f2, err2 := hreflect.ToFloat64E(l2vv) if err1 == nil && err2 == nil && f1 == f2 { i.appendIfNotSeen(l1vv) } @@ -629,7 +631,7 @@ func (ns *Namespace) Union(l1, l2 any) (any, error) { ) for i := range l1v.Len() { - l1vv, isNil = indirectInterface(l1v.Index(i)) + l1vv, isNil = hreflect.Indirect(l1v.Index(i)) if !l1vv.Type().Comparable() { return []any{}, errors.New("union does not support slices or arrays of uncomparable types") @@ -650,16 +652,17 @@ func (ns *Namespace) Union(l1, l2 any) (any, error) { for j := range l2v.Len() { l2vv := l2v.Index(j) + typ := l1vv.Type() switch kind := l1vv.Kind(); { case kind == reflect.String: - l2t, err := toString(l2vv) + l2t, err := hreflect.ToStringE(l2vv) if err == nil { ins.appendIfNotSeen(reflect.ValueOf(l2t)) } - case isNumber(kind): + case hreflect.IsNumber(kind): var err error - l2vv, err = convertNumber(l2vv, kind) + l2vv, err = convertNumber(l2vv, typ) if err == nil { ins.appendIfNotSeen(l2vv) } @@ -700,7 +703,7 @@ func (ns *Namespace) Uniq(l any) (any, error) { seen := make(map[any]bool) for i := range v.Len() { - ev, _ := indirectInterface(v.Index(i)) + ev, _ := hreflect.Indirect(v.Index(i)) key := normalize(ev) @@ -720,6 +723,6 @@ func (ns *Namespace) KeyVals(key any, values ...any) (types.KeyValues, error) { // NewScratch creates a new Scratch which can be used to store values in a // thread safe way. -func (ns *Namespace) NewScratch() *maps.Scratch { - return maps.NewScratch() +func (ns *Namespace) NewScratch() *hstore.Scratch { + return hstore.NewScratch() } diff --git a/tpl/collections/complement.go b/tpl/collections/complement.go index 606d77dde..93a365a20 100644 --- a/tpl/collections/complement.go +++ b/tpl/collections/complement.go @@ -17,6 +17,8 @@ import ( "errors" "fmt" "reflect" + + "github.com/gohugoio/hugo/common/hreflect" ) // Complement gives the elements in the last element of ls that are not in @@ -45,7 +47,7 @@ func (ns *Namespace) Complement(ls ...any) (any, error) { case reflect.Array, reflect.Slice: sl := reflect.MakeSlice(v.Type(), 0, 0) for i := range v.Len() { - ev, _ := indirectInterface(v.Index(i)) + ev, _ := hreflect.Indirect(v.Index(i)) if _, found := aset[normalize(ev)]; !found { sl = reflect.Append(sl, ev) } diff --git a/tpl/collections/index.go b/tpl/collections/index.go index a319ea298..7de27931e 100644 --- a/tpl/collections/index.go +++ b/tpl/collections/index.go @@ -20,6 +20,7 @@ import ( "github.com/spf13/cast" + "github.com/gohugoio/hugo/common/hreflect" "github.com/gohugoio/hugo/common/maps" ) @@ -70,7 +71,7 @@ func (ns *Namespace) doIndex(item any, args ...any) (any, error) { for _, i := range indices { index := reflect.ValueOf(i) var isNil bool - if v, isNil = indirect(v); isNil { + if v, isNil = hreflect.Indirect(v); isNil { // See issue 10489 // This used to be an error. return nil, nil diff --git a/tpl/collections/reflect_helpers.go b/tpl/collections/reflect_helpers.go index 05816a009..14b10c32f 100644 --- a/tpl/collections/reflect_helpers.go +++ b/tpl/collections/reflect_helpers.go @@ -19,6 +19,7 @@ import ( "reflect" "github.com/gohugoio/hugo/common/hashing" + "github.com/gohugoio/hugo/common/hreflect" "github.com/gohugoio/hugo/common/types" "github.com/gohugoio/hugo/resources/resource" ) @@ -28,21 +29,6 @@ var ( errorType = reflect.TypeFor[error]() ) -func numberToFloat(v reflect.Value) (float64, error) { - switch kind := v.Kind(); { - case isFloat(kind): - return v.Float(), nil - case isInt(kind): - return float64(v.Int()), nil - case isUint(kind): - return float64(v.Uint()), nil - case kind == reflect.Interface: - return numberToFloat(v.Elem()) - default: - return 0, fmt.Errorf("invalid kind %s in numberToFloat", kind) - } -} - // normalizes different numeric types if isNumber // or get the hash values if not Comparable (such as map or struct) // to make them comparable @@ -51,8 +37,8 @@ func normalize(v reflect.Value) any { switch { case !v.Type().Comparable(): return hashing.HashUint64(v.Interface()) - case isNumber(k): - f, err := numberToFloat(v) + case hreflect.IsNumber(k): + f, err := hreflect.ToFloat64E(v) if err == nil { return f } @@ -75,7 +61,7 @@ func collectIdentities(seqs ...any) (map[any]bool, error) { switch v.Kind() { case reflect.Array, reflect.Slice: for i := range v.Len() { - ev, _ := indirectInterface(v.Index(i)) + ev, _ := hreflect.Indirect(v.Index(i)) if !ev.Type().Comparable() { return nil, errors.New("elements must be comparable") @@ -99,72 +85,19 @@ func convertValue(v reflect.Value, to reflect.Type) (reflect.Value, error) { } switch kind := to.Kind(); { case kind == reflect.String: - s, err := toString(v) - return reflect.ValueOf(s), err - case isNumber(kind): - return convertNumber(v, kind) + return hreflect.ToStringValueE(v) + case hreflect.IsNumber(kind): + return convertNumber(v, to) default: return reflect.Value{}, fmt.Errorf("%s is not assignable to %s", v.Type(), to) } } -// There are potential overflows in this function, but the downconversion of -// int64 etc. into int8 etc. is coming from the synthetic unit tests for Union etc. -// TODO(bep) We should consider normalizing the slices to int64 etc. -func convertNumber(v reflect.Value, to reflect.Kind) (reflect.Value, error) { - var n reflect.Value - if isFloat(to) { - f, err := toFloat(v) - if err != nil { - return n, err - } - switch to { - case reflect.Float32: - n = reflect.ValueOf(float32(f)) - default: - n = reflect.ValueOf(float64(f)) - } - } else if isInt(to) { - i, err := toInt(v) - if err != nil { - return n, err - } - switch to { - case reflect.Int: - n = reflect.ValueOf(int(i)) - case reflect.Int8: - n = reflect.ValueOf(int8(i)) - case reflect.Int16: - n = reflect.ValueOf(int16(i)) - case reflect.Int32: - n = reflect.ValueOf(int32(i)) - case reflect.Int64: - n = reflect.ValueOf(int64(i)) - } - } else if isUint(to) { - i, err := toUint(v) - if err != nil { - return n, err - } - switch to { - case reflect.Uint: - n = reflect.ValueOf(uint(i)) - case reflect.Uint8: - n = reflect.ValueOf(uint8(i)) - case reflect.Uint16: - n = reflect.ValueOf(uint16(i)) - case reflect.Uint32: - n = reflect.ValueOf(uint32(i)) - case reflect.Uint64: - n = reflect.ValueOf(uint64(i)) - } - } - - if !n.IsValid() { - return n, errors.New("invalid values") +func convertNumber(v reflect.Value, typ reflect.Type) (reflect.Value, error) { + if v, ok := hreflect.ConvertIfPossible(v, typ); ok { + return v, nil } - - return n, nil + return reflect.Value{}, fmt.Errorf("unable to convert value of type %q to %q", v.Type().String(), typ.String()) } func newSliceElement(items any) any { @@ -183,34 +116,3 @@ func newSliceElement(items any) any { } return nil } - -func isNumber(kind reflect.Kind) bool { - return isInt(kind) || isUint(kind) || isFloat(kind) -} - -func isInt(kind reflect.Kind) bool { - switch kind { - case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: - return true - default: - return false - } -} - -func isUint(kind reflect.Kind) bool { - switch kind { - case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: - return true - default: - return false - } -} - -func isFloat(kind reflect.Kind) bool { - switch kind { - case reflect.Float32, reflect.Float64: - return true - default: - return false - } -} diff --git a/tpl/collections/sort.go b/tpl/collections/sort.go index 0c09f6af4..544cb0013 100644 --- a/tpl/collections/sort.go +++ b/tpl/collections/sort.go @@ -20,6 +20,7 @@ import ( "sort" "strings" + "github.com/gohugoio/hugo/common/hreflect" "github.com/gohugoio/hugo/common/maps" "github.com/gohugoio/hugo/langs" "github.com/gohugoio/hugo/tpl/compare" @@ -32,7 +33,7 @@ func (ns *Namespace) Sort(ctx context.Context, l any, args ...any) (any, error) return nil, errors.New("sequence must be provided") } - seqv, isNil := indirect(reflect.ValueOf(l)) + seqv, isNil := hreflect.Indirect(reflect.ValueOf(l)) if isNil { return nil, errors.New("can't iterate over a nil value") } diff --git a/tpl/collections/symdiff.go b/tpl/collections/symdiff.go index 4b9dc6e42..5cd862609 100644 --- a/tpl/collections/symdiff.go +++ b/tpl/collections/symdiff.go @@ -16,6 +16,8 @@ package collections import ( "fmt" "reflect" + + "github.com/gohugoio/hugo/common/hreflect" ) // SymDiff returns the symmetric difference of s1 and s2. @@ -45,7 +47,7 @@ func (ns *Namespace) SymDiff(s2, s1 any) (any, error) { } for i := range v.Len() { - ev, _ := indirectInterface(v.Index(i)) + ev, _ := hreflect.Indirect(v.Index(i)) key := normalize(ev) // Append if the key is not in their intersection. diff --git a/tpl/collections/symdiff_test.go b/tpl/collections/symdiff_test.go index 548f91b6c..e3ee879d8 100644 --- a/tpl/collections/symdiff_test.go +++ b/tpl/collections/symdiff_test.go @@ -54,7 +54,6 @@ func TestSymDiff(t *testing.T) { } { errMsg := qt.Commentf("[%d]", i) - result, err := ns.SymDiff(test.s2, test.s1) if b, ok := test.expected.(bool); ok && !b { diff --git a/tpl/collections/where.go b/tpl/collections/where.go index 34bfa057a..5726cc8ce 100644 --- a/tpl/collections/where.go +++ b/tpl/collections/where.go @@ -27,7 +27,7 @@ import ( // Where returns a filtered subset of collection c. func (ns *Namespace) Where(ctx context.Context, c, key any, args ...any) (any, error) { - seqv, isNil := indirect(reflect.ValueOf(c)) + seqv, isNil := hreflect.Indirect(reflect.ValueOf(c)) if isNil { return nil, errors.New("can't iterate over a nil value of type " + reflect.ValueOf(c).Type().String()) } @@ -56,12 +56,12 @@ func (ns *Namespace) Where(ctx context.Context, c, key any, args ...any) (any, e } func (ns *Namespace) checkCondition(v, mv reflect.Value, op string) (bool, error) { - v, vIsNil := indirect(v) + v, vIsNil := hreflect.Indirect(v) if !v.IsValid() { vIsNil = true } - mv, mvIsNil := indirect(mv) + mv, mvIsNil := hreflect.Indirect(mv) if !mv.IsValid() { mvIsNil = true } @@ -126,13 +126,13 @@ func (ns *Namespace) checkCondition(v, mv reflect.Value, op string) (bool, error slv = v.Interface() slmv = mv.Interface() } - } else if isNumber(v.Kind()) && isNumber(mv.Kind()) { - fv, err := toFloat(v) + } else if hreflect.IsNumber(v.Kind()) && hreflect.IsNumber(mv.Kind()) { + fv, err := hreflect.ToFloat64E(v) if err != nil { return false, err } fvp = &fv - fmv, err := toFloat(mv) + fmv, err := hreflect.ToFloat64E(mv) if err != nil { return false, err } @@ -157,7 +157,7 @@ func (ns *Namespace) checkCondition(v, mv reflect.Value, op string) (bool, error iv := v.Int() ivp = &iv for i := range mv.Len() { - if anInt, err := toInt(mv.Index(i)); err == nil { + if anInt, err := hreflect.ToInt64E(mv.Index(i)); err == nil { ima = append(ima, anInt) } } @@ -165,7 +165,7 @@ func (ns *Namespace) checkCondition(v, mv reflect.Value, op string) (bool, error sv := v.String() svp = &sv for i := range mv.Len() { - if aString, err := toString(mv.Index(i)); err == nil { + if aString, err := hreflect.ToStringE(mv.Index(i)); err == nil { sma = append(sma, aString) } } @@ -173,7 +173,7 @@ func (ns *Namespace) checkCondition(v, mv reflect.Value, op string) (bool, error fv := v.Float() fvp = &fv for i := range mv.Len() { - if aFloat, err := toFloat(mv.Index(i)); err == nil { + if aFloat, err := hreflect.ToFloat64E(mv.Index(i)); err == nil { fma = append(fma, aFloat) } } @@ -304,21 +304,13 @@ func evaluateSubElem(ctx, obj reflect.Value, elemName string) (reflect.Value, er } typ := obj.Type() - obj, isNil := indirect(obj) - - if obj.Kind() == reflect.Interface { - // If obj is an interface, we need to inspect the value it contains - // to see the full set of methods and fields. - // Indirect returns the value that it points to, which is what's needed - // below to be able to reflect on its fields. - obj = reflect.Indirect(obj.Elem()) - } + obj, isNil := hreflect.Indirect(obj) // first, check whether obj has a method. In this case, obj is // a struct or its pointer. If obj is a struct, // to check all T and *T method, use obj pointer type Value objPtr := obj - if objPtr.Kind() != reflect.Interface && objPtr.CanAddr() { + if !hreflect.IsInterfaceOrPointer(objPtr.Kind()) && objPtr.CanAddr() { objPtr = objPtr.Addr() } @@ -360,6 +352,7 @@ func evaluateSubElem(ctx, obj reflect.Value, elemName string) (reflect.Value, er if isNil { return zero, fmt.Errorf("can't evaluate a nil pointer of type %s by a struct field or map key name %s", typ, elemName) } + obj = reflect.Indirect(obj) switch obj.Kind() { case reflect.Struct: ft, ok := obj.Type().FieldByName(elemName) @@ -431,7 +424,7 @@ func (ns *Namespace) checkWhereArray(ctxv, seqv, kv, mv reflect.Value, path []st } } } else { - vv, _ := indirect(rvv) + vv, _ := hreflect.Indirect(rvv) if vv.Kind() == reflect.Map && kv.Type().AssignableTo(vv.Type().Key()) { vvv = vv.MapIndex(kv) } @@ -469,7 +462,7 @@ func (ns *Namespace) checkWhereMap(ctxv, seqv, kv, mv reflect.Value, path []stri } } case reflect.Interface: - elemvv, isNil := indirect(elemv) + elemvv, isNil := hreflect.Indirect(elemv) if isNil { continue } @@ -493,53 +486,7 @@ func (ns *Namespace) checkWhereMap(ctxv, seqv, kv, mv reflect.Value, path []stri return rv.Interface(), nil } -// toFloat returns the float value if possible. -func toFloat(v reflect.Value) (float64, error) { - switch v.Kind() { - case reflect.Float32, reflect.Float64: - return v.Float(), nil - case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: - return v.Convert(reflect.TypeOf(float64(0))).Float(), nil - case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: - return v.Convert(reflect.TypeOf(float64(0))).Float(), nil - case reflect.Interface: - return toFloat(v.Elem()) - } - return -1, errors.New("unable to convert value to float") -} - -// toInt returns the int value if possible, -1 if not. -// TODO(bep) consolidate all these reflect funcs. -func toInt(v reflect.Value) (int64, error) { - switch v.Kind() { - case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: - return v.Int(), nil - case reflect.Interface: - return toInt(v.Elem()) - } - return -1, errors.New("unable to convert value to int") -} - -func toUint(v reflect.Value) (uint64, error) { - switch v.Kind() { - case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: - return v.Uint(), nil - case reflect.Interface: - return toUint(v.Elem()) - } - return 0, errors.New("unable to convert value to uint") -} - // toString returns the string value if possible, "" if not. -func toString(v reflect.Value) (string, error) { - switch v.Kind() { - case reflect.String: - return v.String(), nil - case reflect.Interface: - return toString(v.Elem()) - } - return "", errors.New("unable to convert value to string") -} func (ns *Namespace) toTimeUnix(v reflect.Value) int64 { t, ok := hreflect.AsTime(v, ns.loc) diff --git a/tpl/collections/where_test.go b/tpl/collections/where_test.go index 9d441b030..6d53fc1ff 100644 --- a/tpl/collections/where_test.go +++ b/tpl/collections/where_test.go @@ -23,7 +23,6 @@ import ( "testing" "time" - qt "github.com/frankban/quicktest" "github.com/gohugoio/hugo/common/maps" ) @@ -863,32 +862,6 @@ func TestEvaluateSubElem(t *testing.T) { } } -func TestToFloat(t *testing.T) { - t.Parallel() - - c := qt.New(t) - - to := func(v any) float64 { - f, err := toFloat(reflect.ValueOf(v)) - c.Assert(err, qt.IsNil) - return f - } - - c.Assert(to(uint64(32)), qt.Equals, 32.0) - c.Assert(to(int64(32)), qt.Equals, 32.0) - c.Assert(to(uint32(32)), qt.Equals, 32.0) - c.Assert(to(int32(32)), qt.Equals, 32.0) - - c.Assert(to(uint16(32)), qt.Equals, 32.0) - c.Assert(to(int16(32)), qt.Equals, 32.0) - - c.Assert(to(uint8(32)), qt.Equals, 32.0) - c.Assert(to(int8(32)), qt.Equals, 32.0) - - c.Assert(to(uint(32)), qt.Equals, 32.0) - c.Assert(to(int(32)), qt.Equals, 32.0) -} - func BenchmarkWhereOps(b *testing.B) { ns := newNs() var seq []map[string]string @@ -946,3 +919,23 @@ func BenchmarkWhereMap(b *testing.B) { } } } + +func BenchmarkWhereSliceOfStructPointersWithMethod(b *testing.B) { + // TstRv2 + ns := newNs() + seq := []*TstX{} + + for i := range 1000 { + seq = append(seq, &TstX{A: "foo", B: "bar"}) + if i%2 == 0 { + seq = append(seq, &TstX{A: "baz", B: "qux"}) + } + } + b.ResetTimer() + for range b.N { + _, err := ns.Where(context.Background(), seq, "TstRv2", "eq", "bar") + if err != nil { + b.Fatal(err) + } + } +}