Invalid code generation for top-level singletons

[vallahaye]

It seems that the code generated for top-level singletons with [email protected] is invalid. Specifically, the fmt and strings packages are imported but not used, resulting in compile errors.

For example, the following proto:

message Config {
  option (google.api.resource) = {
    type: "test1.testdata/Config"
    singular: "config"
    plural: "configs"
    pattern: "config"
  };

  // The resource name of the config.
  string name = 1;
}

generates the following code:

// Code generated by protoc-gen-go-aip. DO NOT EDIT.
//
// versions:
// 	protoc-gen-go-aip development
// 	protoc (unknown)
// source: test/toplevelsingleton/testdata.proto

package toplevelsingleton

import (
	fmt "fmt"
	resourcename "go.einride.tech/aip/resourcename"
	strings "strings
)

type ConfigResourceName struct {
}

func (n ConfigResourceName) Validate() error {
	return nil
}

func (n ConfigResourceName) ContainsWildcard() bool {
	return false
}

func (n ConfigResourceName) String() string {
	return resourcename.Sprint(
		"config",
	)
}

func (n ConfigResourceName) MarshalString() (string, error) {
	if err := n.Validate(); err != nil {
		return "", err
	}
	return n.String(), nil
}

func (n *ConfigResourceName) UnmarshalString(name string) error {
	err := resourcename.Sscan(
		name,
		"config",
	)
	if err != nil {
		return err
	}
	return n.Validate()
}

which, when compiled, produces the following errors:

test/toplevelsingleton/testdata_aip.go:11:2: "fmt" imported and not used
test/toplevelsingleton/testdata_aip.go:13:2: "strings" imported and not used

[radhus]

Hi @vallahaye , thanks for your report and sorry for the delay!

In AIP-156, it says:

A singleton resource must always exist by virtue of the existence of its parent, with one and exactly one per parent.

I wonder if this means that top-level singletons aren't allowed in general. WDYT?

Anwyay, I don't think we should generate broken code, and it should be up to the user to follow this rule, so I'll review your PR shortly!

[vallahaye]

I wonder if this means that top-level singletons aren't allowed in general. WDYT?

Well, I think we're in undefined territory here. While it's not strictly forbidden, I haven't found a single sentence that describes singleton resources without referring to a parent.

On the other hand, I can definitely see a use case for a top-level configuration singleton in the management plane (which is what led to me opening this issue).