internal/encoding/gotypes: support go import aliasing #4028
Conversation
This change enables support for exporting aliased package names from source cue to go code. The special case of the "_" import alias is explicitly ignored. Fixes cue-lang#3974 Signed-off-by: Piaras Hoban <phoban01@gmail.com>
|
|
||
| #ObjectMeta: {} | ||
|
|
||
| -- cue.mod/gen/k8s.io/api/core/v1/types_go_gen.cue -- |
There was a problem hiding this comment.
could you rewrite this test so it doesn't use cue.mod/gen? e.g. make these sub-packages inside the same module.
| if ok { | ||
| quotedPath = fmt.Sprintf("%s %s", alias, quotedPath) | ||
| } | ||
| printf("\t%s\n", quotedPath) |
There was a problem hiding this comment.
I'd avoid the two-layered formatting, like:
if alias, ok := g.importAliases[path]; ok {
printf("\t%s %q\n", alias, path)
} else {
printf("\t%q\n", path)
}
| @@ -323,6 +340,7 @@ func (g *generator) emitType(val cue.Value, optionalStg optionalStrategy) (typeF | |||
| if err != nil { | |||
| return facts, fmt.Errorf("cannot parse @go type expression: %w", err) | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
please avoid whitespace changes elsewhere, as they are more likely to cause git conflicts down the line
| @@ -679,11 +697,15 @@ func goValueAttr(val cue.Value) cue.Attribute { | |||
|
|
|||
| // goPkgNameForInstance determines what to name a Go package generated from a CUE instance. | |||
| // By default this is the CUE package name, but it can be overriden by a @go() package attribute. | |||
| func goPkgNameForInstance(inst *build.Instance, instVal cue.Value) string { | |||
| // Import aliases will also checked at this point and inserted if found. | |||
| func goPkgNameForInstance(inst *build.Instance, instVal cue.Value, aliasLookup map[string]string) string { | |||
There was a problem hiding this comment.
name the parameter importAliases for consistency.
| @@ -679,11 +697,15 @@ func goValueAttr(val cue.Value) cue.Attribute { | |||
|
|
|||
| // goPkgNameForInstance determines what to name a Go package generated from a CUE instance. | |||
| // By default this is the CUE package name, but it can be overriden by a @go() package attribute. | |||
| func goPkgNameForInstance(inst *build.Instance, instVal cue.Value) string { | |||
| // Import aliases will also checked at this point and inserted if found. | |||
There was a problem hiding this comment.
I'd reword this slightly, like:
When supplying importAliases, and if no package attribute is found,
the returned package name reflects the alias name that the package is being imported as.
| @@ -100,7 +109,7 @@ func Generate(ctx *cue.Context, insts ...*build.Instance) error { | |||
| buf = fmt.Appendf(buf, format, args...) | |||
| } | |||
| printf("// Code generated by \"cue exp gengotypes\"; DO NOT EDIT.\n\n") | |||
| goPkgName := goPkgNameForInstance(inst, instVal) | |||
| goPkgName := goPkgNameForInstance(inst, instVal, g.importAliases) | |||
There was a problem hiding this comment.
this is rather crucial - you must NOT use importAliases here, so you should use a nil map instead. this tracks which Go packages (by name) we generate per directory, as we can only generate one package per directory. This is entirely unrelated to what aliases a package is imported under.
| @@ -217,6 +231,9 @@ type generator struct { | |||
|
|
|||
| // def tracks the generation state for a single CUE definition. | |||
| def *generatedDef | |||
|
|
|||
| // importAliases maps package names to the alias | |||
| importAliases map[string]string | |||
There was a problem hiding this comment.
Note that we organize fields on a per-package and per-definition basis. This should be on a per-package basis I assume.
| @@ -772,3 +794,32 @@ func emitDocs(printf func(string, ...any), name string, groups []*ast.CommentGro | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| // gatherImportAliases collects the aliases from imports across the instance. | |||
| // We explicitly ignore "_" aliases because CUE does not support. | |||
There was a problem hiding this comment.
CUE source code should never be using underscore-aliased imports, so I would delete this line as well as the check for _ below - it just shouldn't happen
| if !ok { | ||
| continue | ||
| } | ||
| for _, s := range impDecl.Specs { |
There was a problem hiding this comment.
Loop directly over f.Imports, which will save you a handful of lines
| @@ -217,6 +231,9 @@ type generator struct { | |||
|
|
|||
| // def tracks the generation state for a single CUE definition. | |||
| def *generatedDef | |||
|
|
|||
| // importAliases maps package names to the alias | |||
There was a problem hiding this comment.
this needs clarifying - is it to the first alias that we found for each package? I reckon it should be, but then we should document it as such, and update the code to ignore any second or third alias for the same package path.
but also, what happens if file f1.cue does import same "foo.com/bar/p1", and f2.cue does import same "foo.com/bar/p2"? As far as I can see, you'll generate Go code which imports both packages aliased as same.
I think collecting the original aliases is a good idea, because more often than not it will lead to better results - the user will most likely have chosen reasonable aliases. But you'll need some sort of strategy to deal with conflicts, and add a test for that edge case.
I would personally suggest to track which aliases are already taken, and when you encounter an aliased import where the alias is already taken, you can add a number at the end - e.g. the second same above could become same2.
There was a problem hiding this comment.
It's also worth noting that you can have duplicates between non-aliased package imports and aliased imports too, so that must also have a test and be checked for duplicates. For example, import "foo.com/bar/same" in f1.cue, and import same "foo.com/bar/p2" in f2.cue.
This change enables support for exporting aliased package names from source cue to go code.
The special case of the "_" import alias is explicitly ignored.
Fixes #3974