From 7bc8882b19dcb47fd84916cc7be32bf13228add1 Mon Sep 17 00:00:00 2001 From: lverma14 Date: Fri, 3 Mar 2023 16:40:18 -0800 Subject: [PATCH] Reducing logic, checks and fixing tests --- annotated.go | 112 +++++++++++++++----------------- annotated_test.go | 161 ++++++++++++++++++++-------------------------- 2 files changed, 121 insertions(+), 152 deletions(-) diff --git a/annotated.go b/annotated.go index 4420b2f72..5e508f2d6 100644 --- a/annotated.go +++ b/annotated.go @@ -145,83 +145,77 @@ type paramTagsAnnotation struct { var _ Annotation = paramTagsAnnotation{} var ( - errTagSyntaxSpace = (` multiple tags are not separated by space`) - errTagKeySyntax = (" tag key is invalid, Use group, name or optional as tag keys") - errTagSyntaxColon = (`tag key and value not separated by colon. Did you mean `) - errTagValueSyntaxQuote = (`tag value should start with double quote. Did you mean `) - errTagValueSyntaxEndingQuote = (`tag value should end in double quote. Did you mean `) - errTagValueSpace = (`tag key value separated by extra space. Use format` + " `tag_key:\"value\"`") + errTagSyntaxSpace = errors.New(` multiple tags are not separated by space`) + errTagKeySyntax = errors.New(" tag key is invalid, Use group, name or optional as tag keys") + errTagValueSyntaxQuote = errors.New(`tag value should start with double quote. i.e. key:"value" `) + errTagValueSyntaxEndingQuote = errors.New(`tag value should end in double quote. i.e. key:"value" `) ) +// Given a tag with key, value pair, they must be separaed by a space. +// Eg: `group:"some" optional:"true"` +// return an error if in the case of multiple tags, they aren't separated +// space. Return nil on success. +func verifyTagsSpaceSeparated(numTags int, tag string) error { + if numTags > 0 && tag != "" && tag[0] != ' ' { + return errTagSyntaxSpace + } + return nil +} + +// Given a value from a tag verify that it is contained between double quotes +// Return an error if the invalid quote format is used for tag value. +// On sucess return the remaining tag with nil error +func verifyValueQuote(value string) (string, error) { + // starting quote should be a double quote + if value[0] != '"' { + return "", errTagValueSyntaxQuote + } + // validate tag value is within quotes + i := 1 + for i < len(value) && value[i] != '"' { + if value[i] == '\\' { + i++ + } + i++ + } + if i >= len(value) { + return "", errTagValueSyntaxEndingQuote + } + return value[i+1:], nil + +} + // Given a string tag the function checks if the tag follows valid struct // format and returns an error if it's invalid. (i.e. not following // tag:"value" space-separated list ) +// Currently dig accepts only 'name', 'group', 'optional' as valid tag keys func verifyAnnotateTag(tag string) error { numTags := 0 for ; tag != ""; numTags++ { - if numTags > 0 && tag != "" && tag[0] != ' ' { - // catch invalid cases where tags aren't separated by - // space - return errors.New(errTagSyntaxSpace) + validKeys := map[string]bool{"group": true, "optional": true, "name": true} + if err := verifyTagsSpaceSeparated(numTags, tag); err != nil { + return err } i := 0 - //ignoring leading space - for i < len(tag) && tag[i] == ' ' { - i++ - } - tag = tag[i:] // return if tag is empty - if tag == "" { - break + if strings.TrimSpace(tag) == "" { + return nil } // parsing the key i.e. till reaching colon : - // Currently, dig only accepts group, name and optinal tag - // keys. However, to ease exension of valid tag keys in dig in - // the future, restricting key strings to contain only - // lowercase alphabets. - // return an error if any other character is sent as tag key - i = 0 - correctedTag := "" - for i < len(tag) && tag[i] >= 'a' && tag[i] != ':' && tag[i] != '"' && tag[i] <= 'z' { + for i < len(tag) && tag[i] != ':' { i++ } - if i == 0 { - return errors.New(errTagKeySyntax) - } - key := tag[:i] - if i+1 >= len(tag) || tag[i] != ':' { - if (key == "group" || key == "name" || key == "optional") && tag[i] == '"' { - correctedTag = tag[:i] + ":" + tag[i:] - return errors.New(errTagSyntaxColon + "`" + correctedTag + "` ?") - } - return errors.New(errTagKeySyntax) - } - if key != "group" && key != "name" && key != "optional" { - return errors.New(errTagKeySyntax) - } - if tag[i+1] == ' ' { - return errors.New(errTagValueSpace) + key := strings.TrimSpace(tag[:i]) + // if key is not equal to group name or optional + if _, ok := validKeys[key]; !ok { + return errTagKeySyntax } - if tag[i+1] != '"' { - correctedTag = tag[:i+1] + `"` + tag[i+2:] - return errors.New(errTagValueSyntaxQuote + "`" + correctedTag + "` ?") - } - - tag = tag[i+1:] // starting from " - // validate tag value is within quotes - i = 1 - for i < len(tag) && tag[i] != '"' { - if tag[i] == '\\' { - i++ - } - i++ - } - if i >= len(tag) { - correctedTag = key + ":" + tag[:i-1] + `"` - return errors.New(errTagValueSyntaxEndingQuote + "`" + correctedTag + "` ?") + value, err := verifyValueQuote(tag[i+1:]) + if err != nil { + return err } - tag = tag[i+1:] + tag = value } return nil diff --git a/annotated_test.go b/annotated_test.go index 4518ea4e8..540f0933c 100644 --- a/annotated_test.go +++ b/annotated_test.go @@ -1474,7 +1474,7 @@ func TestAnnotate(t *testing.T) { }) } -func TestAnnotateApply(t *testing.T) { +func TestAnnotateApplyFail(t *testing.T) { t.Parallel() type a struct{} @@ -1487,90 +1487,44 @@ func TestAnnotateApply(t *testing.T) { var ( errTagSyntaxSpace = ` multiple tags are not separated by space` errTagKeySyntax = " tag key is invalid, Use group, name or optional as tag keys" - errTagValueSyntaxQuote = `tag value should start with double quote. Did you mean ` - errTagValueSyntaxEndingQuote = `tag value should end in double quote. Did you mean ` - errTagSyntaxColon = `tag key and value not separated by colon. Did you mean ` - errTagValueSpace = `tag key value separated by extra space. Use format` + " `tag_key:\"value\"`" + errTagValueSyntaxQuote = `tag value should start with double quote. i.e. key:"value" ` + errTagValueSyntaxEndingQuote = `tag value should end in double quote. i.e. key:"value" ` ) tests := []struct { - give string - wantErr string - giveAnnotation fx.Annotation + give string + wantErr string + giveAnnotationParam fx.Annotation + giveAnnotationResult fx.Annotation }{ { - give: "ParamTags wrong ending quote", - wantErr: errTagValueSyntaxEndingQuote + "`name:\"something\"` ?", - giveAnnotation: fx.ParamTags(`name:"something'`), + give: "Param/Result Tags value invalid ending quote", + wantErr: errTagValueSyntaxEndingQuote, + giveAnnotationParam: fx.ParamTags(`name:"something'`), + giveAnnotationResult: fx.ResultTags(`name:"something'`), }, { - give: "ParamTags wrong starting quote", - wantErr: errTagValueSyntaxQuote + "`optional:\"true\"` ?", - giveAnnotation: fx.ParamTags(`name:"something" optional:'true"`), + give: "Param/Result Tags value wrong starting quote", + wantErr: errTagValueSyntaxQuote, + giveAnnotationParam: fx.ParamTags(`name:"something" optional:'true"`), + giveAnnotationResult: fx.ResultTags(`name:"something" optional:'true"`), }, { - give: "ParamTags key not made of alphabets", - wantErr: errTagKeySyntax, - giveAnnotation: fx.ParamTags(`a"me1:"something"`), + give: "ParamTags multiple tags not separated by space", + wantErr: errTagSyntaxSpace, + giveAnnotationParam: fx.ParamTags(`name:"something"group:"something"`), + giveAnnotationResult: fx.ResultTags(`name:"something"group:"something"`), }, { - give: "ParamTags multiple tags not separated by space", - wantErr: errTagSyntaxSpace, - giveAnnotation: fx.ParamTags(`name:"something"group:"something"`), + give: "ParamTags key not equal to group, name or optional", + wantErr: errTagKeySyntax, + giveAnnotationParam: fx.ParamTags(`name1:"something"`), + giveAnnotationResult: fx.ResultTags(`name1:"something"`), }, { - give: "ParamTags key value not separated by colon", - wantErr: errTagSyntaxColon + "`group:\"something\"` ?", - giveAnnotation: fx.ParamTags(`group"something"`), - }, - { - give: "ParamTags key not equal to group, name or optional", - wantErr: errTagKeySyntax, - giveAnnotation: fx.ParamTags(`name1:"something"`), - }, - { - give: `ParamTags invalid space after key colon`, - wantErr: errTagValueSpace, - giveAnnotation: fx.ParamTags(`name: "something"`), - }, - { - give: "ResultTags wrong ending quote", - wantErr: errTagValueSyntaxEndingQuote + "`name:\"something\"` ?", - giveAnnotation: fx.ResultTags(`name:"something'`), - }, - { - give: "ResultTags wrong starting quote", - wantErr: errTagValueSyntaxQuote + "`optional:\"true\"` ?", - giveAnnotation: fx.ResultTags(`name:"something" optional:'true"`), - }, - { - give: "ResultTags key not made of alphabets", - wantErr: errTagKeySyntax, - giveAnnotation: fx.ResultTags(`a"me1:"something"`), - }, - { - give: "ResultTags multiple tags not separated by space", - wantErr: errTagSyntaxSpace, - giveAnnotation: fx.ResultTags(`name:"something"group:"something"`), - }, - { - give: "ResultTags key value not separated by colon", - wantErr: errTagSyntaxColon + "`group:\"something\"` ?", - giveAnnotation: fx.ResultTags(`group"something"`), - }, - { - give: "ResultTags key not equal to group, name or optional", - wantErr: errTagKeySyntax, - giveAnnotation: fx.ResultTags(`name1:"something"`), - }, - { - give: "ResultTags no key", - wantErr: errTagKeySyntax, - giveAnnotation: fx.ResultTags(`:"something"`), - }, - { - give: "ResultTags invalid space after key colon ", - wantErr: errTagValueSpace, - giveAnnotation: fx.ResultTags(`name: "something"`), + give: "ParamTags key empty", + wantErr: errTagKeySyntax, + giveAnnotationParam: fx.ParamTags(`:"something"`), + giveAnnotationResult: fx.ResultTags(`:"something"`), }, } for _, tt := range tests { @@ -1579,50 +1533,71 @@ func TestAnnotateApply(t *testing.T) { fx.Provide( fx.Annotate( newA, - tt.giveAnnotation, + tt.giveAnnotationParam, ), ), fx.Invoke(newB), ) - err := app.Err() - assert.ErrorContains(t, err, tt.wantErr) + assert.ErrorContains(t, app.Err(), tt.wantErr) + }) + t.Run(tt.give, func(t *testing.T) { + app := NewForTest(t, + fx.Provide( + fx.Annotate( + newA, + tt.giveAnnotationResult, + ), + ), + fx.Invoke(newB), + ) + assert.ErrorContains(t, app.Err(), tt.wantErr) }) } +} - tests1 := []struct { - give string - giveAnnotation fx.Annotation +func TestAnnotateApplySuccess(t *testing.T) { + t.Parallel() + type a struct{} + type b struct{ a *a } + newA := func() *a { return &a{} } + newB := func(a *a) *b { + return &b{a} + } + + tests := []struct { + give string + giveAnnotationParam fx.Annotation + giveAnnotationResult fx.Annotation }{ { - give: "ParamTags Tag Empty", - giveAnnotation: fx.ParamTags(` `), + give: "ParamTags Tag Empty", + giveAnnotationParam: fx.ParamTags(` `), + giveAnnotationResult: fx.ResultTags(` `), }, { - give: "ParamTags Tag Empty with extra spaces", - giveAnnotation: fx.ParamTags(`name:"versionNum"`, ` `), + give: "ParamTags Tag Empty with extra spaces", + giveAnnotationParam: fx.ParamTags(`name:"versionNum"`, ` `), + giveAnnotationResult: fx.ResultTags(` `, `group:"versionNum"`), }, { - give: "ParamTags Tag Empty with extra spaces", - giveAnnotation: fx.ParamTags(`name:"versionNum"`, ` `), - }, - { - give: "ParamTags Tag with \\ ", - giveAnnotation: fx.ParamTags(`name:"version\\Num"`, ` `), + give: "ParamTags Tag with \\ ", + giveAnnotationParam: fx.ParamTags(`name:"version\\Num"`, ` `), + giveAnnotationResult: fx.ResultTags(``, `group:"version\\Num"`), }, } - for _, tt := range tests1 { + for _, tt := range tests { t.Run(tt.give, func(t *testing.T) { app := NewForTest(t, fx.Provide( fx.Annotate( newA, - tt.giveAnnotation, + tt.giveAnnotationParam, + tt.giveAnnotationResult, ), ), fx.Invoke(newB), ) - err := app.Err() - require.NoError(t, err) + require.NoError(t, app.Err()) }) }