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

[Blocked] Check unique default before using the value #26

Closed
wants to merge 3 commits into from
Closed
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
8 changes: 5 additions & 3 deletions encoder/generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,17 +78,19 @@ func loadCases(dir string) ([]Case, error) {
if len(a.Files) != 2 {
return nil, fmt.Errorf("Malformed test case '%s': Must contain exactly two files (CUE and TS/ERR), but has %d", file, len(a.Files))
}
if strings.HasSuffix(fi.Name(), "error") {

fname := strings.TrimSuffix(fi.Name(), filepath.Ext(fi.Name()))
if strings.HasSuffix(fname, "error") {
cases = append(cases, Case{
CaseType: ErrorType,
Name: fi.Name(),
Name: fname,
CUE: string(a.Files[0].Data),
ERROR: strings.TrimSuffix(string(a.Files[1].Data), "\n"),
})
} else {
cases = append(cases, Case{
CaseType: TSType,
Name: fi.Name(),
Name: fname,
CUE: string(a.Files[0].Data),
TS: string(a.Files[1].Data),
})
Expand Down
34 changes: 29 additions & 5 deletions encoder/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,13 +181,16 @@ func (g *generator) genType(name string, v cue.Value) {

tvars["tokens"] = tokens

d, ok := v.Default()
d, ok, err := validateAndGetDefault(v)
if err != nil {
g.addErr(err)
}

if ok {
dStr, err := tsprintField(d)
g.addErr(err)
tvars["default"] = dStr
}

// TODO comments
// TODO maturity marker (@alpha, etc.)
g.exec(typeCode, tvars)
Expand Down Expand Up @@ -220,7 +223,7 @@ func (g *generator) genEnum(name string, v cue.Value) {
g.addErr(err)
}
pairs = orPairs
defaultValue, err = getDefaultValue(v)
defaultValue, err = getDefaultEnumValue(v)
if err != nil {
g.addErr(err)
}
Expand All @@ -243,8 +246,25 @@ func (g *generator) genEnum(name string, v cue.Value) {
g.exec(enumCode, tvars)
}

func getDefaultValue(v cue.Value) (string, error) {
func validateAndGetDefault(v cue.Value) (cue.Value, bool, error) {
def, ok := v.Default()
if ok {
label, _ := def.Label()
op, dvals := def.Expr()
if len(dvals) > 1 && op == cue.OrOp {
return def, true, valError(v, "%s has multiple defaults which is not allowed", label)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message should print out the defaults, to improve failure locality (the cost of mapping a failure to a particular place in code). Ideally, it'd even emit line numbers, like native CUE errors do, but that's more than we need to worry about right now.

}
return def, true, nil
}
return def, false, nil
}

func getDefaultEnumValue(v cue.Value) (string, error) {
def, ok, err := validateAndGetDefault(v)
if err != nil {
return "", err
}

if ok {
if v.IncompleteKind() == cue.StringKind {
dStr, err := tsprintField(def)
Expand Down Expand Up @@ -475,7 +495,11 @@ func (g *generator) genInterface(name string, v cue.Value) {

kv := KV{K: k, V: vstr}

d, ok := fields.Value().Default()
// When default exist but have sevarals, we need to fail
d, ok, err := validateAndGetDefault(fields.Value())
if err != nil {
g.addErr(err)
}
// [...number] results in [], which is not desired
// TODO: There must be a better way to handle this
if ok && d.IncompleteKind() != cue.ListKind {
Expand Down
29 changes: 29 additions & 0 deletions encoder/tests/conflictEnumDef.txtar
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@

This would be the typical cases we should fail, since Bar is refereced to a type with default
and we are trying to assign another default

Seems CUE is picking the default before eval it, it seems we can get the reference first and
check whether default exist there, if yes if the default also exist for the current cue object error out

Or should we just forbidden the disjunction between Enum and other types??
-- cue --
package cuetsy

AOrEnum: "foo" | "bar" | *"baz" @cuetsy(kind="enum")
Foo: {
Bar: AOrEnum | *"ohai"
} @cuetsy(kind="interface")

-- err --
export enum AOrEnum {
Bar = 'bar',
Baz = 'baz',
Foo = 'foo',
}
export const aOrEnumDefault: AOrEnum = AOrEnum.Baz
export interface Foo {
Bar: AOrEnum | 'ohai';
}
export const fooDefault: Foo = {
Bar: 'ohai',
}
8 changes: 8 additions & 0 deletions encoder/tests/conflictInterfaceDeferror.txtar
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
-- cue --
Foo: {
Bar: string | *"ohai"
Baz: int | *4 | *7
} @cuetsy(kind="interface")

-- err --
Baz has multiple defaults which is not allowed
8 changes: 8 additions & 0 deletions encoder/tests/conflictTypeDeferror.txtar
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
-- cue --
AType: "foo" | *"bar" | "baz" @cuetsy(kind="type")
BType: "foo" | "bar" | *"baz" @cuetsy(kind="type")
CType: AType | BType @cuetsy(kind="type")


-- err --
CType has multiple defaults which is not allowed
Copy link
Collaborator Author

@ying-jeanne ying-jeanne Sep 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is making sense and erring out now

File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.