Skip to content

Commit

Permalink
Respond to feedback
Browse files Browse the repository at this point in the history 
- simplify example API to always require string description and valid example
- accept null as a valid example value for nullable data values
- reword error messages
  • Loading branch information
@gcheadle-vmware
gcheadle-vmware committed Jan 20, 2022
1 parent 490144b commit 5e32602
Show file tree
Hide file tree
Showing 5 changed files with 117 additions and 124 deletions.
20 changes: 9 additions & 11 deletions pkg/cmd/template/schema_author_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -778,7 +778,7 @@ schema.yml:
|
= found: missing value in @schema/examples (by schema.yml:3)
= expected: tuple with description and example
= expected: 2-tuple containing description (string) and example value (of expected type)
`

filesToProcess := files.NewSortedFiles([]*files.File{
Expand All @@ -804,9 +804,8 @@ schema.yml:
4 | key: val
|
= found: non tuple in @schema/examples (by schema.yml:3)
= expected: tuple with optional string description and required example value
= hint: use a trailing comma to construct tuple with a single value. e.g. ('example value',)
= found: string for @schema/examples (at schema.yml:3)
= expected: 2-tuple containing description (string) and example value (of expected type)
`

filesToProcess := files.NewSortedFiles([]*files.File{
Expand All @@ -833,7 +832,7 @@ schema.yml:
|
= found: empty tuple in @schema/examples (by schema.yml:3)
= expected: tuple with optional string description and required example value
= expected: 2-tuple containing description (string) and example value (of expected type)
`

filesToProcess := files.NewSortedFiles([]*files.File{
Expand All @@ -859,8 +858,8 @@ schema.yml:
4 | key: val
|
= found: 3 arguments in @schema/examples (by schema.yml:3)
= expected: no more than 2 arguments per tuple. e.g. @schema/examples ('description', exampleValue())
= found: 3-tuple argument in @schema/examples (by schema.yml:3)
= expected: 2-tuple containing description (string) and example value (of expected type)
`

filesToProcess := files.NewSortedFiles([]*files.File{
Expand All @@ -886,9 +885,8 @@ schema.yml:
4 | key: 7.3
|
= found: Non-string value in @schema/examples (by schema.yml:3)
= expected: string
= hint: @schema/examples optionally accepts a string description as the first argument in a tuple
= found: float value for @schema/examples (at schema.yml:3)
= expected: 2-tuple containing description (string) and example value (of expected type)
`

filesToProcess := files.NewSortedFiles([]*files.File{
Expand All @@ -915,7 +913,7 @@ schema.yml:
|
= found: keyword argument in @schema/examples (by schema.yml:3)
= expected: tuple with description and example
= expected: 2-tuple containing description (string) and example value (of expected type)
`

filesToProcess := files.NewSortedFiles([]*files.File{
Expand Down
7 changes: 4 additions & 3 deletions pkg/cmd/template/schema_inspect_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -713,14 +713,14 @@ db_conn:
#@schema/examples ("hostname example description", "localhost")
#@schema/desc "The hostname"
hostname: ""
#@schema/examples (8080,)
#@schema/examples ("",8080)
port: 0
#@schema/examples ("timeout example description", 4.2), ("another timeout ex desc", 5)
timeout: 1.0
#@schema/examples ("any_key example description", "anything")
#@schema/type any=True
any_key: thing
#@schema/examples ("null_key example description", "anything")
#@schema/examples ("null_key example description", None)
#@schema/nullable
null_key: ""
`
Expand Down Expand Up @@ -770,6 +770,7 @@ components:
type: string
default: ""
port:
x-example-description: ""
example: 8080
type: integer
default: 0
Expand All @@ -786,7 +787,7 @@ components:
default: thing
null_key:
x-example-description: null_key example description
example: anything
example: null
type: string
default: null
nullable: true
Expand Down
140 changes: 69 additions & 71 deletions pkg/schema/annotations.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,23 @@ type TitleAnnotation struct {
pos *filepos.Position
}

// ExampleAnnotation documents an example of a node
// ExampleAnnotation provides the Examples of a node
type ExampleAnnotation struct {
descriptions []string
examples []interface{}
pos *filepos.Position
examples []Example
pos *filepos.Position
}

// Example contains a yaml example and its description
type Example struct {
description string
example interface{}
}

// Documentation holds metadata about a Type, provided via documentation annotations
type Documentation struct {
title string
description string
*Example
}

// NewTypeAnnotation checks the keyword argument provided via @schema/type annotation, and returns wrapper for the annotated node.
Expand Down Expand Up @@ -265,7 +277,7 @@ func NewExampleAnnotation(ann template.NodeAnnotation, pos *filepos.Position) (*
annPositions: []*filepos.Position{ann.Position},
position: pos,
description: fmt.Sprintf("syntax error in @%v annotation", AnnotationExamples),
expected: fmt.Sprintf("tuple with description and example"),
expected: fmt.Sprintf("2-tuple containing description (string) and example value (of expected type)"),
found: fmt.Sprintf("keyword argument in @%v (by %v)", AnnotationExamples, ann.Position.AsCompactString()),
}
}
Expand All @@ -274,73 +286,67 @@ func NewExampleAnnotation(ann template.NodeAnnotation, pos *filepos.Position) (*
annPositions: []*filepos.Position{ann.Position},
position: pos,
description: fmt.Sprintf("syntax error in @%v annotation", AnnotationExamples),
expected: fmt.Sprintf("tuple with description and example"),
expected: fmt.Sprintf("2-tuple containing description (string) and example value (of expected type)"),
found: fmt.Sprintf("missing value in @%v (by %v)", AnnotationExamples, ann.Position.AsCompactString()),
}
}

var descriptions []string
var examples []interface{}
for _, example := range ann.Args {
var exampleDescription string
var exampleYAML interface{}
var err error

if exampleTuple, ok := example.(starlark.Tuple); ok {
switch len(exampleTuple) {
case 0:
return nil, schemaAssertionError{
annPositions: []*filepos.Position{ann.Position},
position: pos,
description: fmt.Sprintf("syntax error in @%v annotation", AnnotationExamples),
expected: fmt.Sprintf("tuple with optional string description and required example value"),
found: fmt.Sprintf("empty tuple in @%v (by %v)", AnnotationExamples, ann.Position.AsCompactString()),
}
case 1:
exampleYAML, err = core.NewStarlarkValue(exampleTuple[0]).AsGoValue()
if err != nil {
panic(err)
}
case 2:
exampleDescription, err = core.NewStarlarkValue(exampleTuple[0]).AsString()
if err != nil {
return nil, schemaAssertionError{
annPositions: []*filepos.Position{ann.Position},
position: pos,
description: fmt.Sprintf("syntax error in @%v annotation", AnnotationExamples),
expected: fmt.Sprintf("string"),
found: fmt.Sprintf("Non-string value in @%v (by %v)", AnnotationExamples, ann.Position.AsCompactString()),
hints: []string{fmt.Sprintf("@%v optionally accepts a string description as the first argument in a tuple", AnnotationExamples)},
}
}
exampleYAML, err = core.NewStarlarkValue(exampleTuple[1]).AsGoValue()
if err != nil {
panic(err)
}
default:
var examples []Example
for _, ex := range ann.Args {
exampleTuple, ok := ex.(starlark.Tuple)
if !ok {
return nil, schemaAssertionError{
annPositions: []*filepos.Position{ann.Position},
position: pos,
description: fmt.Sprintf("syntax error in @%v annotation", AnnotationExamples),
expected: fmt.Sprintf("2-tuple containing description (string) and example value (of expected type)"),
found: fmt.Sprintf("%v for @%v (at %v)", ex.Type(), AnnotationExamples, ann.Position.AsCompactString()),
}
}
switch {
case len(exampleTuple) == 0:
return nil, schemaAssertionError{
annPositions: []*filepos.Position{ann.Position},
position: pos,
description: fmt.Sprintf("syntax error in @%v annotation", AnnotationExamples),
expected: fmt.Sprintf("2-tuple containing description (string) and example value (of expected type)"),
found: fmt.Sprintf("empty tuple in @%v (by %v)", AnnotationExamples, ann.Position.AsCompactString()),
}
case len(exampleTuple) == 1:
return nil, schemaAssertionError{
annPositions: []*filepos.Position{ann.Position},
position: pos,
description: fmt.Sprintf("syntax error in @%v annotation", AnnotationExamples),
expected: fmt.Sprintf("2-tuple containing description (string) and example value (of expected type)"),
found: fmt.Sprintf("empty tuple in @%v (by %v)", AnnotationExamples, ann.Position.AsCompactString()),
}
case len(exampleTuple) > 2:
return nil, schemaAssertionError{
annPositions: []*filepos.Position{ann.Position},
position: pos,
description: fmt.Sprintf("syntax error in @%v annotation", AnnotationExamples),
expected: fmt.Sprintf("2-tuple containing description (string) and example value (of expected type)"),
found: fmt.Sprintf("%v-tuple argument in @%v (by %v)", len(exampleTuple), AnnotationExamples, ann.Position.AsCompactString()),
}
default:
description, err := core.NewStarlarkValue(exampleTuple[0]).AsString()
if err != nil {
return nil, schemaAssertionError{
annPositions: []*filepos.Position{ann.Position},
position: pos,
description: fmt.Sprintf("syntax error in @%v annotation", AnnotationExamples),
expected: fmt.Sprintf("no more than 2 arguments per tuple. e.g. @%v ('description', exampleValue())", AnnotationExamples),
found: fmt.Sprintf("%v arguments in @%v (by %v)", len(exampleTuple), AnnotationExamples, ann.Position.AsCompactString()),
expected: fmt.Sprintf("2-tuple containing description (string) and example value (of expected type)"),
found: fmt.Sprintf("%v value for @%v (at %v)", exampleTuple[0].Type(), AnnotationExamples, ann.Position.AsCompactString()),
}
}
} else {
return nil, schemaAssertionError{
annPositions: []*filepos.Position{ann.Position},
position: pos,
description: fmt.Sprintf("syntax error in @%v annotation", AnnotationExamples),
expected: fmt.Sprintf("tuple with optional string description and required example value"),
found: fmt.Sprintf("non tuple in @%v (by %v)", AnnotationExamples, ann.Position.AsCompactString()),
hints: []string{"use a trailing comma to construct tuple with a single value. e.g. ('example value',)"},
exampleVal, err := core.NewStarlarkValue(exampleTuple[1]).AsGoValue()
if err != nil {
panic(err)
}
examples = append(examples, Example{description, yamlmeta.NewASTFromInterfaceWithPosition(exampleVal, pos)})
}
descriptions = append(descriptions, exampleDescription)
examples = append(examples, yamlmeta.NewASTFromInterfaceWithPosition(exampleYAML, pos))

}
return &ExampleAnnotation{descriptions, examples, ann.Position}, nil
return &ExampleAnnotation{examples, ann.Position}, nil
}

// NewTypeFromAnn returns type information given by annotation.
Expand Down Expand Up @@ -579,15 +585,8 @@ func getTypeFromAnnotations(anns []Annotation, pos *filepos.Position) (Type, err
return typeFromAnn, nil
}

type documentation struct {
title string
description string
exampleDescription string
exampleYAML interface{}
}

func setDocumentationFromAnns(docAnns []Annotation, typeOfValue Type) error {
var documentationInfo documentation
var documentationInfo Documentation
for _, a := range docAnns {
switch ann := a.(type) {
case *TitleAnnotation:
Expand All @@ -601,8 +600,7 @@ func setDocumentationFromAnns(docAnns []Annotation, typeOfValue Type) error {
}
// display only first example
if len(ann.examples) != 0 {
documentationInfo.exampleDescription = ann.descriptions[0]
documentationInfo.exampleYAML = ann.examples[0]
documentationInfo.Example = &ann.examples[0]
}
}
}
Expand All @@ -613,15 +611,15 @@ func setDocumentationFromAnns(docAnns []Annotation, typeOfValue Type) error {
func checkExamplesValue(ann *ExampleAnnotation, typeOfValue Type) error {
var typeCheck TypeCheck
for _, ex := range ann.examples {
if node, ok := ex.(yamlmeta.Node); ok {
if node, ok := ex.example.(yamlmeta.Node); ok {
defaultValue := node.DeepCopyAsNode()
chk := typeOfValue.AssignTypeTo(defaultValue)
if !chk.HasViolations() {
chk = CheckDocument(defaultValue)
}
typeCheck.Violations = append(typeCheck.Violations, chk.Violations...)
} else {
chk := typeOfValue.CheckType(&yamlmeta.MapItem{Value: ex, Position: typeOfValue.GetDefinitionPosition()})
chk := typeOfValue.CheckType(&yamlmeta.MapItem{Value: ex.example, Position: typeOfValue.GetDefinitionPosition()})
typeCheck.Violations = append(typeCheck.Violations, chk.Violations...)
}
}
Expand Down
12 changes: 4 additions & 8 deletions pkg/schema/openapi.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,10 @@ func collectDocumentation(t Type) []*yamlmeta.MapItem {
if t.GetDescription() != "" {
annDocumentation = append(annDocumentation, &yamlmeta.MapItem{Key: descriptionProp, Value: t.GetDescription()})
}
exampleDescription, exampleYAML := t.GetExample()
if exampleYAML != nil {
if exampleDescription != "" {
annDocumentation = append(annDocumentation, &yamlmeta.MapItem{Key: exampleDescriptionProp, Value: exampleDescription})
annDocumentation = append(annDocumentation, &yamlmeta.MapItem{Key: exampleProp, Value: exampleYAML})
} else {
annDocumentation = append(annDocumentation, &yamlmeta.MapItem{Key: exampleProp, Value: exampleYAML})
}
ex := t.GetExample()
if ex != nil {
annDocumentation = append(annDocumentation, &yamlmeta.MapItem{Key: exampleDescriptionProp, Value: ex.description})
annDocumentation = append(annDocumentation, &yamlmeta.MapItem{Key: exampleProp, Value: ex.example})
}
return annDocumentation
}
Expand Down

0 comments on commit 5e32602

Please sign in to comment.