Skip to content
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.

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

Fix Value.Type() returning Type referencing nil ptr for eight external values #2388

Merged
merged 5 commits into from
Mar 22, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
42 changes: 41 additions & 1 deletion values.go
Original file line number Diff line number Diff line change
Expand Up @@ -1426,7 +1426,7 @@ func (v Array) String() string {
// Dictionary

type Dictionary struct {
DictionaryType Type
DictionaryType *DictionaryType
Pairs []KeyValuePair
}

Expand All @@ -1453,6 +1453,11 @@ func NewMeteredDictionary(
func (Dictionary) isValue() {}

func (v Dictionary) Type() Type {
if v.DictionaryType == nil {
fxamacker marked this conversation as resolved.
Show resolved Hide resolved
// Return nil Type instead of Type referencing nil *DictionaryType,
// so caller can check if v's type is nil and also prevent nil pointer dereference.
return nil
}
return v.DictionaryType
}

Expand Down Expand Up @@ -1541,6 +1546,11 @@ func NewMeteredStruct(
func (Struct) isValue() {}

func (v Struct) Type() Type {
if v.StructType == nil {
// Return nil Type instead of Type referencing nil *StructType,
// so caller can check if v's type is nil and also prevent nil pointer dereference.
return nil
}
return v.StructType
}

Expand Down Expand Up @@ -1619,6 +1629,11 @@ func NewMeteredResource(
func (Resource) isValue() {}

func (v Resource) Type() Type {
if v.ResourceType == nil {
// Return nil Type instead of Type referencing nil *ResourceType,
// so caller can check if v's type is nil and also prevent nil pointer dereference.
return nil
}
return v.ResourceType
}

Expand Down Expand Up @@ -1676,6 +1691,11 @@ func NewMeteredAttachment(
func (Attachment) isValue() {}

func (v Attachment) Type() Type {
if v.AttachmentType == nil {
// Return nil Type instead of Type referencing nil *AttachmentType,
// so caller can check if v's type is nil and also prevent nil pointer dereference.
return nil
}
return v.AttachmentType
}

Expand Down Expand Up @@ -1733,6 +1753,11 @@ func NewMeteredEvent(
func (Event) isValue() {}

func (v Event) Type() Type {
if v.EventType == nil {
// Return nil Type instead of Type referencing nil *EventType,
// so caller can check if v's type is nil and also prevent nil pointer dereference.
return nil
}
return v.EventType
}

Expand Down Expand Up @@ -1789,6 +1814,11 @@ func NewMeteredContract(
func (Contract) isValue() {}

func (v Contract) Type() Type {
if v.ContractType == nil {
// Return nil Type instead of Type referencing nil *ContractType,
// so caller can check if v's type is nil and also prevent nil pointer dereference.
return nil
}
return v.ContractType
}

Expand Down Expand Up @@ -2012,6 +2042,11 @@ func NewMeteredEnum(
func (Enum) isValue() {}

func (v Enum) Type() Type {
if v.EnumType == nil {
// Return nil Type instead of Type referencing nil *EnumType,
// so caller can check if v's type is nil and also prevent nil pointer dereference.
return nil
}
return v.EnumType
}

Expand Down Expand Up @@ -2059,6 +2094,11 @@ func NewMeteredFunction(gauge common.MemoryGauge, functionType *FunctionType) Fu
func (Function) isValue() {}

func (v Function) Type() Type {
if v.FunctionType == nil {
// Return nil Type instead of Type referencing nil *FunctionType,
// so caller can check if v's type is nil and also prevent nil pointer dereference.
return nil
}
return v.FunctionType
}

Expand Down
158 changes: 156 additions & 2 deletions values_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -635,6 +635,8 @@ func TestGetType(t *testing.T) {
reflect.TypeOf(Event{}): {},
reflect.TypeOf(Resource{}): {},
reflect.TypeOf(Struct{}): {},
reflect.TypeOf(Enum{}): {},
reflect.TypeOf(Attachment{}): {},
turbolent marked this conversation as resolved.
Show resolved Hide resolved
}

typelessTypes := map[reflect.Type]struct{}{
Expand Down Expand Up @@ -681,14 +683,54 @@ func TestGetType(t *testing.T) {
}
})

t.Run("array value", func(t *testing.T) {
t.Run("array value without type", func(t *testing.T) {
t.Parallel()

typ := NewConstantSizedArrayType(4, TheInt8Type)
// Type is not set.
value := NewArray([]Value{})
assert.Nil(t, value.Type())

// Type is nil.
value = value.WithType(nil)
assert.Nil(t, value.Type())

// Type is ArrayType(nil).
value = value.WithType(ArrayType(nil))
assert.Nil(t, value.Type())
})

t.Run("constant sized array value", func(t *testing.T) {
t.Parallel()

typ := NewConstantSizedArrayType(1, TheInt8Type)
arrayValue := NewArray([]Value{NewInt8(0)}).WithType(typ)
assert.Equal(t, arrayValue.Type(), typ)
})

t.Run("variable sized array value", func(t *testing.T) {
t.Parallel()

typ := NewVariableSizedArrayType(TheInt8Type)
arrayValue := NewArray([]Value{}).WithType(typ)
assert.Equal(t, arrayValue.Type(), typ)
})

t.Run("contract value without type", func(t *testing.T) {
t.Parallel()

// Type is not set.
value := NewContract([]Value{})
assert.Nil(t, value.Type())

// Type is nil.
value = value.WithType(nil)
assert.Nil(t, value.Type())

// Type is (*ContractType)(nil).
value = value.WithType((*ContractType)(nil))
assert.Nil(t, value.Type())
})

t.Run("contract value", func(t *testing.T) {
t.Parallel()

Expand All @@ -697,6 +739,22 @@ func TestGetType(t *testing.T) {
assert.Equal(t, typ, value.Type())
})

t.Run("dictionary value without type", func(t *testing.T) {
t.Parallel()

// Type is not set.
value := NewDictionary([]KeyValuePair{})
assert.Nil(t, value.Type())

// Type is nil.
value = value.WithType(nil)
assert.Nil(t, value.Type())

// Type is (*DictionaryType)(nil).
value = value.WithType((*DictionaryType)(nil))
assert.Nil(t, value.Type())
})

t.Run("dictionary value", func(t *testing.T) {
t.Parallel()

Expand All @@ -705,6 +763,22 @@ func TestGetType(t *testing.T) {
assert.Equal(t, typ, value.Type())
})

t.Run("event value without type", func(t *testing.T) {
t.Parallel()

// Type is not set.
value := NewEvent([]Value{})
assert.Nil(t, value.Type())

// Type is nil.
value = value.WithType(nil)
assert.Nil(t, value.Type())

// Type is (*EventType)(nil).
value = value.WithType((*EventType)(nil))
assert.Nil(t, value.Type())
})

t.Run("event value", func(t *testing.T) {
t.Parallel()

Expand All @@ -713,6 +787,22 @@ func TestGetType(t *testing.T) {
assert.Equal(t, typ, value.Type())
})

t.Run("resource value without type", func(t *testing.T) {
t.Parallel()

// Type is not set.
value := NewResource([]Value{})
assert.Nil(t, value.Type())

// Type is nil.
value = value.WithType(nil)
assert.Nil(t, value.Type())

// Type is (*ResourceType)(nil).
value = value.WithType((*ResourceType)(nil))
assert.Nil(t, value.Type())
})

t.Run("resource value", func(t *testing.T) {
t.Parallel()

Expand All @@ -721,11 +811,75 @@ func TestGetType(t *testing.T) {
assert.Equal(t, typ, value.Type())
})

t.Run("struct value without type", func(t *testing.T) {
t.Parallel()

// Type is not set.
value := NewStruct([]Value{})
assert.Nil(t, value.Type())

// Type is nil.
value = value.WithType(nil)
assert.Nil(t, value.Type())

// Type is (*StructType)(nil).
value = value.WithType((*StructType)(nil))
assert.Nil(t, value.Type())
})

t.Run("struct value", func(t *testing.T) {
t.Parallel()

typ := NewStructType(nil, "Foo", nil, nil)
value := NewStruct([]Value{}).WithType(typ)
assert.Equal(t, typ, value.Type())
})

t.Run("enum value without type", func(t *testing.T) {
t.Parallel()

// Type is not set.
value := NewEnum([]Value{})
assert.Nil(t, value.Type())

// Type is nil.
value = value.WithType(nil)
assert.Nil(t, value.Type())

// Type is (*EnumType)(nil).
value = value.WithType((*EnumType)(nil))
assert.Nil(t, value.Type())
})

t.Run("enum value", func(t *testing.T) {
t.Parallel()

typ := NewEnumType(nil, "Foo", nil, nil, nil)
value := NewEnum([]Value{}).WithType(typ)
assert.Equal(t, typ, value.Type())
})

t.Run("attachment value without type", func(t *testing.T) {
t.Parallel()

// Type is not set.
value := NewAttachment([]Value{})
assert.Nil(t, value.Type())

// Type is nil.
value = value.WithType(nil)
assert.Nil(t, value.Type())

// Type is (*AttachmentType)(nil).
value = value.WithType((*AttachmentType)(nil))
assert.Nil(t, value.Type())
})

t.Run("attachment value", func(t *testing.T) {
t.Parallel()

typ := NewAttachmentType(nil, nil, "Foo", nil, nil)
value := NewAttachment([]Value{}).WithType(typ)
assert.Equal(t, typ, value.Type())
})
}