Prevent missing fields in struct initialization

Question

Consider this example. Let's say I have this object which is ubiquitous throughout my codebase:

type Person struct {
    Name string
    Age  int
    [some other fields]
}

Somewhere deep in the codebase, I also have some code that creates a new Person struct. Maybe it's something like the following utility function (note that this is just an example of some function that creates a Person-- the point of my question is not to ask about the copy function specifically):

func copyPerson(origPerson Person) *Person {
    copy := Person{
        Name: origPerson.Name,
        Age:  origPerson.Age,
        [some other fields]
    }
    return &copy
}

Another developer comes along and adds a new field Gender to the Person struct. However, because the copyPerson function is in a distant piece of code they forget to update copyPerson. Since golang doesn't throw any warning or error if you omit a parameter when creating a struct, the code will compile and appear to work fine; the only difference is that the copyPerson method will now fail to copy over the Gender struct, and the result of copyPerson will have Gender replaced with a nil value (e.g. the empty string).

What is the best way to prevent this from happening? Is there a way to ask golang to enforce no missing parameters in a specific struct initialization? Is there a linter that can detect this type of potential error?

Answer 1

The way I would solve this is to just use NewPerson(params) and not export the person. This makes it so the only way to get a person instance is to go through your New method.

package person

// Struct is not exported
type person struct {
    Name string
    Age  int
    Gender bool
}

// We are forced to call the constructor to get an instance of person
func New(name string, age int, gender bool) person {
    return person{name, age, gender}
}

This forces everyone to get an instance from the same place. When you add a field, you can add it to the function definition and then you get compile time errors anywhere they are constructing a new instance, so you can easily find them and fix them.

Answer 2

First of all, your copyPerson() function does not live up to its name. It copies some fields of a Person, but not (necessarily) all. It should've been named copySomeFieldsOfPerson().

To copy a complete struct value, just assign the struct value. If you have a function receiving a non-pointer Person, that is already a copy, so just return its address:

func copyPerson(p Person) *Person {
    return &p
}

That's all, this will copy all present and future fields of Person.

Now there may be cases where fields are pointers or header-like values (like a slice) which should be "detached" from the original field (more precisely from the pointed object), in which case you do need to make manual adjustments, e.g.

type Person struct {
    Name string
    Age  int
    Data []byte
}

func copyPerson(p Person) *Person {
    p2 := p
    p2.Data = append(p2.Data, p.Data...)
    return &p2
}

Or an alternative solution which does not make another copy of p but still detaches Person.Data:

func copyPerson(p Person) *Person {
    var data []byte
    p.Data = append(data, p.Data...)
    return &p
}

Of course, if someone adds a field which also needs manual handling, this won't help you out.

You could also use unkeyed literal, like this:

func copyPerson(p Person) *Person {
    return &Person{
        p.Name,
        p.Age,
    }
}

This will result in a compile-time error if someone adds a new field to Person, because an unkeyed composite struct literal must list all fields. Again, this will not help you out if someone changes the fields where the new fields are assignable to the old ones (e.g. someone swaps 2 fields next to each other having the same type), also unkeyed literals are discouraged.

Best would be for the package owner to provide a copy constructor, next to the Person type definition. So if someone changes Person, he / she should be responsible keeping CopyPerson() still operational. And as others mentioned, you should already have unit tests which should fail if CopyPerson() does not live up to its name.

The best viable option?

If you can't place the CopyPerson() next to the Person type and have its author maintain it, go ahead with the struct value copying and manual handling of pointer and header-like fields.

And you can create a person2 type which is a "snapshot" of the Person type. Use a blank global variable to receive compile-time alert if the original Person type changes, in which case copyPerson()'s containing source file will refuse to compile, so you'll know it needs adjusting.

This is how it can be done:

type person2 struct {
    Name string
    Age  int
}

var _ = Person(person2{})

The blank variable declaration will not compile if fields of Person and person2 do not match.

A variation of the above compile-time check could be to use typed-nil pointers:

var _ = (*Person)((*person2)(nil))

Answer 3

I'm not aware of a language rule that enforces that.

But you can write custom checkers for Go vet if you'd like. Here's a recent post talking about that.

---

That said, I would reconsider the design here. If the Person struct is so important in your code base, centralize its creation and copying so that "distant places" don't just create and move Persons around. Refactor your code so that only a single constructor is used to build Persons (maybe something like person.New returning a person.Person), and then you'll be able to centrally control how its fields are initialized.

Answer 4

The idiomatic way would be to not do this at all, and instead make the zero value useful. The example of a copy function doesn't really make sense because it's totally unnecessary - you could just say:

copy := new(Person)
*copy = *origPerson

and not need a dedicated function nor have to keep a listing of fields up to date. If you want a constructor for new instances like NewPerson, just write one and use it as a matter of course. Linters are great for some things but nothing beats well-understood best practices and peer code review.

Answer 5

The best solution I have been able to come up with (and it's not very good) is to define a new struct tempPerson identical to the Person struct and put it nearby to any code which initializes a new Person struct, and to change the code that initializes a Person so that it instead initializes it as a tempPerson but then casts it to a Person. Like this:

type tempPerson struct {
    Name string
    Age  int
    [some other fields]
}

func copyPerson(origPerson Person) *Person {
    tempCopy := tempPerson{
        Name: orig.Name,
        Age:  orig.Age,
        [some other fields]
    }
    copy := (Person)(tempCopy)
    return &copy
}

This way if another field Gender is added to Person but not to tempPerson the code will fail at compile-time. Presumably the developer would then see the error, edit tempPerson to match their change to Person, and in doing so notice the nearby code which uses tempPerson and recognize that they should edit that code to also handle the Gender field as well.

I don't love this solution because it involves copying and pasting the struct definition everywhere that we initialize a Person struct and would like to have this safety. Is there any better way?

Answer 6

Approach 1 Add something like copy constructor:

type Person struct {
    Name string
    Age  int
}

func CopyPerson(name string, age int)(*Person, error){
    // check params passed if needed
    return &Person{Name: name, Age: age}, nil
}


p := CopyPerson(p1.Name, p1.age) // force all fields to be passed

Approach 2: (not sure if this is possible)

Can this be covered in tests say using reflection?
If we compare the number of fields initialised(initialise all the field with values different than the default values) in the original struct and the fields in copy returned by the copy function.

Answer 7

Here is how i would do it:

func copyPerson(origPerson Person) *Person { 
    newPerson := origPerson

    //proof that 'newPerson' points to a new person object
    newPerson.name = "new name"
    return &newPerson
}

Go Playground