From 5b4d523e9bc5e4dad0d3a21940e69451f9753897 Mon Sep 17 00:00:00 2001 From: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com> Date: Wed, 30 Aug 2023 15:11:42 -0400 Subject: [PATCH] remove configurability add fuzz and benchmark testing Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com> --- scanner/identifier.go | 64 +++++++------------------ scanner/identifier_test.go | 96 +++++++++++++++++++++++--------------- 2 files changed, 75 insertions(+), 85 deletions(-) diff --git a/scanner/identifier.go b/scanner/identifier.go index f11658f..e896e69 100644 --- a/scanner/identifier.go +++ b/scanner/identifier.go @@ -9,85 +9,53 @@ import ( // IsValidIdentifier returns true if the given string is a valid river // identifier. func IsValidIdentifier(in string) bool { - s := New(nil, []byte(in), nil, 0) + f := token.NewFile("test") + s := New(f, []byte(in), nil, 0) _, tok, lit := s.Scan() return tok == token.IDENT && lit == in } -type SanitizeIdentifierOptions struct { - // Prefix is what will be prepended to the identifier if it does not start - // with a letter or underscore. This must be a valid river identifier. - Prefix string - - // Replacement is what will be used to replace invalid characters. This - // must be a valid river identifier or empty. - Replacement string -} - -func sanitizeIdentifierOptionsDefault() *SanitizeIdentifierOptions { - return &SanitizeIdentifierOptions{ - Prefix: "_", - Replacement: "_", - } -} - -// validate will return an error if the options are invalid. -func (opts *SanitizeIdentifierOptions) validate() error { - if !IsValidIdentifier(opts.Prefix) { - return fmt.Errorf("prefix `%q` is not a valid river identifier", opts.Prefix) - } - - if !(IsValidIdentifier(opts.Replacement) || opts.Replacement == "") { - return fmt.Errorf("replacement `%q` must be either a valid river identifier or empty", opts.Replacement) - } - - return nil -} - // SanitizeIdentifier will return the given string mutated into a valid river // identifier. If the given string is already a valid identifier, it will be // returned unchanged. // // This should be used with caution since the different inputs can result in // identical outputs. -func SanitizeIdentifier(in string, opts *SanitizeIdentifierOptions) (string, error) { - if IsValidIdentifier(in) { - return in, nil +func SanitizeIdentifier(in string) (string, error) { + if in == "" { + return "", fmt.Errorf("cannot generate a new identifier for an empty string") } - if opts == nil { - opts = sanitizeIdentifierOptionsDefault() + if IsValidIdentifier(in) { + return in, nil } - if err := opts.validate(); err != nil { - return "", err + newValue := generateNewIdentifier(in) + if !IsValidIdentifier(newValue) { + panic(fmt.Errorf("invalid identifier %q generated for `%q`", newValue, in)) } - return generateNewIdentifier(in, opts.Prefix, opts.Replacement) + return newValue, nil } // generateNewIdentifier expects a valid river prefix and replacement // string and returns a new identifier based on the given input. -func generateNewIdentifier(in string, prefix string, replacement string) (string, error) { - if in == "" { - return "", fmt.Errorf("cannot generate a new identifier for an empty string") - } - +func generateNewIdentifier(in string) string { newValue := "" for i, c := range in { if i == 0 { - if !isLetter(c) { - newValue = prefix + if isDigit(c) { + newValue = "_" } } if !(isLetter(c) || isDigit(c)) { - newValue += replacement + newValue += "_" continue } newValue += string(c) } - return newValue, nil + return newValue } diff --git a/scanner/identifier_test.go b/scanner/identifier_test.go index 59d5e19..f0ffaf4 100644 --- a/scanner/identifier_test.go +++ b/scanner/identifier_test.go @@ -7,53 +7,55 @@ import ( "github.com/stretchr/testify/require" ) -func TestIsValidIdentifier(t *testing.T) { - tt := []struct { - name string - identifier string - expect bool - }{ - {"empty", "", false}, - {"start_number", "0identifier_1", false}, - {"start_char", "identifier_1", true}, - {"start_underscore", "_identifier_1", true}, - {"special_chars", "!@#$%^&*()", false}, - {"special_char", "identifier_1!", false}, - {"spaces", "identifier _ 1", false}, - } +var validTestCases = []struct { + name string + identifier string + expect bool +}{ + {"empty", "", false}, + {"start_number", "0identifier_1", false}, + {"start_char", "identifier_1", true}, + {"start_underscore", "_identifier_1", true}, + {"special_chars", "!@#$%^&*()", false}, + {"special_char", "identifier_1!", false}, + {"spaces", "identifier _ 1", false}, +} - for _, tc := range tt { +func TestIsValidIdentifier(t *testing.T) { + for _, tc := range validTestCases { t.Run(tc.name, func(t *testing.T) { require.Equal(t, tc.expect, scanner.IsValidIdentifier(tc.identifier)) }) } } -func TestSanitizeIdentifierOptions(t *testing.T) { - tt := []struct { - name string - identifier string - expectIdentifier string - expectErr string - opts *scanner.SanitizeIdentifierOptions - }{ - {"empty", "", "", "cannot generate a new identifier for an empty string", nil}, - {"start_number", "0identifier_1", "_0identifier_1", "", nil}, - {"start_char", "identifier_1", "identifier_1", "", nil}, - {"start_underscore", "_identifier_1", "_identifier_1", "", nil}, - {"special_chars", "!@#$%^&*()", "___________", "", nil}, - {"special_char", "identifier_1!", "identifier_1_", "", nil}, - {"spaces", "identifier _ 1", "identifier___1", "", nil}, - {"bad prefix", "", "", "prefix `\"123\"` is not a valid river identifier", &scanner.SanitizeIdentifierOptions{Prefix: "123", Replacement: ""}}, - {"bad replacement", "", "", "replacement `\"!\"` must be either a valid river identifier or empty", &scanner.SanitizeIdentifierOptions{Prefix: "prefix2_", Replacement: "!"}}, - {"different prefix", "0identifier_1", "prefix2_0identifier_1", "", &scanner.SanitizeIdentifierOptions{Prefix: "prefix2_", Replacement: ""}}, - {"different replacement", "identifier_1%", "identifier_1_percent", "", &scanner.SanitizeIdentifierOptions{Prefix: "prefix2_", Replacement: "_percent"}}, - {"empty replacement", "identifier _ 1", "identifier_1", "", &scanner.SanitizeIdentifierOptions{Prefix: "prefix2_", Replacement: ""}}, +func BenchmarkIsValidIdentifier(b *testing.B) { + for i := 0; i < b.N; i++ { + for _, tc := range validTestCases { + _ = scanner.IsValidIdentifier(tc.identifier) + } } +} + +var sanitizeTestCases = []struct { + name string + identifier string + expectIdentifier string + expectErr string +}{ + {"empty", "", "", "cannot generate a new identifier for an empty string"}, + {"start_number", "0identifier_1", "_0identifier_1", ""}, + {"start_char", "identifier_1", "identifier_1", ""}, + {"start_underscore", "_identifier_1", "_identifier_1", ""}, + {"special_chars", "!@#$%^&*()", "__________", ""}, + {"special_char", "identifier_1!", "identifier_1_", ""}, + {"spaces", "identifier _ 1", "identifier___1", ""}, +} - for _, tc := range tt { +func TestSanitizeIdentifier(t *testing.T) { + for _, tc := range sanitizeTestCases { t.Run(tc.name, func(t *testing.T) { - newIdentifier, err := scanner.SanitizeIdentifier(tc.identifier, tc.opts) + newIdentifier, err := scanner.SanitizeIdentifier(tc.identifier) if tc.expectErr != "" { require.EqualError(t, err, tc.expectErr) return @@ -64,3 +66,23 @@ func TestSanitizeIdentifierOptions(t *testing.T) { }) } } + +func BenchmarkSanitizeIdentifier(b *testing.B) { + for i := 0; i < b.N; i++ { + for _, tc := range sanitizeTestCases { + _, _ = scanner.SanitizeIdentifier(tc.identifier) + } + } +} + +func FuzzTestSanitizeIdentifier(f *testing.F) { + f.Fuzz(func(t *testing.T, input string) { + newIdentifier, err := scanner.SanitizeIdentifier(input) + if input == "" { + require.EqualError(t, err, "cannot generate a new identifier for an empty string") + return + } + require.NoError(t, err) + require.True(t, scanner.IsValidIdentifier(newIdentifier)) + }) +}