From 670c18782cb5a12ecb98772aebc974af45b7c444 Mon Sep 17 00:00:00 2001
From: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
Date: Tue, 29 Aug 2023 16:28:32 -0400
Subject: [PATCH 1/6] add unified way to validate river identifiers

add a configurable way to sanitize a string into a river identifier.

Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
---
 parser/internal.go            |   8 +--
 scanner/identifier.go         | 102 ++++++++++++++++++++++++++++++++++
 scanner/identifier_test.go    |  68 +++++++++++++++++++++++
 token/builder/value_tokens.go |   8 +--
 4 files changed, 172 insertions(+), 14 deletions(-)
 create mode 100644 scanner/identifier.go
 create mode 100644 scanner/identifier_test.go

diff --git a/parser/internal.go b/parser/internal.go
index b524a9e..986731c 100644
--- a/parser/internal.go
+++ b/parser/internal.go
@@ -344,7 +344,7 @@ func (p *parser) parseBlockName() *blockName {
 			// label to be a valid identifier.
 			if len(p.lit) > 2 {
 				bn.Label = p.lit[1 : len(p.lit)-1]
-				if !isValidIdentifier(bn.Label) {
+				if !scanner.IsValidIdentifier(bn.Label) {
 					p.addErrorf("expected block label to be a valid identifier, but got %q", bn.Label)
 				}
 			}
@@ -707,9 +707,3 @@ func (p *parser) parseField() *ast.ObjectField {
 	field.Value = p.ParseExpression()
 	return &field
 }
-
-func isValidIdentifier(in string) bool {
-	s := scanner.New(nil, []byte(in), nil, 0)
-	_, tok, lit := s.Scan()
-	return tok == token.IDENT && lit == in
-}
diff --git a/scanner/identifier.go b/scanner/identifier.go
new file mode 100644
index 0000000..6f05eee
--- /dev/null
+++ b/scanner/identifier.go
@@ -0,0 +1,102 @@
+package scanner
+
+import (
+	"fmt"
+
+	"github.com/grafana/river/token"
+)
+
+// IsValidIdentifier returns true if the given string is a valid river
+// identifier.
+func IsValidIdentifier(in string) bool {
+	s := New(nil, []byte(in), nil, 0)
+	_, tok, lit := s.Scan()
+	return tok == token.IDENT && lit == in
+}
+
+type SanitizeIdentifierOptions struct {
+	// EmptyValue is what to use as the identifier when the input is empty.
+	// This must be a valid river identifier.
+	EmptyValue string
+
+	// 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{
+		EmptyValue:  "empty",
+		Prefix:      "id_",
+		Replacement: "_",
+	}
+}
+
+// validate will return an error if the options are invalid.
+func (opts *SanitizeIdentifierOptions) validate() error {
+	if !IsValidIdentifier(opts.EmptyValue) {
+		return fmt.Errorf("emptyValue `%q` is not a valid river identifier", opts.EmptyValue)
+	}
+
+	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
+	}
+
+	if opts == nil {
+		opts = sanitizeIdentifierOptionsDefault()
+	}
+
+	if err := opts.validate(); err != nil {
+		return "", err
+	}
+
+	return generateNewIdentifier(in, opts.EmptyValue, opts.Prefix, opts.Replacement)
+}
+
+// generateNewIdentifier expects a valid river prefix and replacement
+// string and returns a new identifier based on the given input.
+func generateNewIdentifier(in string, emptyValue string, prefix string, replacement string) (string, error) {
+	if in == "" {
+		return emptyValue, nil
+	}
+
+	newValue := ""
+	for i, c := range in {
+		if i == 0 {
+			if !isLetter(c) {
+				newValue = prefix
+			}
+		}
+
+		if !(isLetter(c) || isDigit(c)) {
+			newValue += replacement
+			continue
+		}
+
+		newValue += string(c)
+	}
+
+	return newValue, nil
+}
diff --git a/scanner/identifier_test.go b/scanner/identifier_test.go
new file mode 100644
index 0000000..221c721
--- /dev/null
+++ b/scanner/identifier_test.go
@@ -0,0 +1,68 @@
+package scanner_test
+
+import (
+	"testing"
+
+	"github.com/grafana/river/scanner"
+	"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},
+	}
+
+	for _, tc := range tt {
+		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", "", "empty", "", nil},
+		{"start_number", "0identifier_1", "id_0identifier_1", "", nil},
+		{"start_char", "identifier_1", "identifier_1", "", nil},
+		{"start_underscore", "_identifier_1", "_identifier_1", "", nil},
+		{"special_chars", "!@#$%^&*()", "id___________", "", nil},
+		{"special_char", "identifier_1!", "identifier_1_", "", nil},
+		{"spaces", "identifier _ 1", "identifier___1", "", nil},
+		{"bad emptyValue", "", "", "emptyValue `\"\"` is not a valid river identifier", &scanner.SanitizeIdentifierOptions{EmptyValue: "", Prefix: "", Replacement: ""}},
+		{"bad prefix", "", "", "prefix `\"123\"` is not a valid river identifier", &scanner.SanitizeIdentifierOptions{EmptyValue: "empty2", Prefix: "123", Replacement: ""}},
+		{"bad replacement", "", "", "replacement `\"!\"` must be either a valid river identifier or empty", &scanner.SanitizeIdentifierOptions{EmptyValue: "empty2", Prefix: "prefix2_", Replacement: "!"}},
+		{"different empty", "", "empty2", "", &scanner.SanitizeIdentifierOptions{EmptyValue: "empty2", Prefix: "prefix2_", Replacement: ""}},
+		{"different prefix", "0identifier_1", "prefix2_0identifier_1", "", &scanner.SanitizeIdentifierOptions{EmptyValue: "empty2", Prefix: "prefix2_", Replacement: ""}},
+		{"different replacement", "identifier _ 1", "identifiera_a1", "", &scanner.SanitizeIdentifierOptions{EmptyValue: "empty2", Prefix: "prefix2_", Replacement: "a"}},
+		{"empty replacement", "identifier _ 1", "identifier_1", "", &scanner.SanitizeIdentifierOptions{EmptyValue: "empty2", Prefix: "prefix2_", Replacement: ""}},
+	}
+
+	for _, tc := range tt {
+		t.Run(tc.name, func(t *testing.T) {
+			newIdentifier, err := scanner.SanitizeIdentifier(tc.identifier, tc.opts)
+			if tc.expectErr != "" {
+				require.EqualError(t, err, tc.expectErr)
+				return
+			}
+
+			require.NoError(t, err)
+			require.Equal(t, tc.expectIdentifier, newIdentifier)
+		})
+	}
+}
diff --git a/token/builder/value_tokens.go b/token/builder/value_tokens.go
index 8fd87de..c73e34f 100644
--- a/token/builder/value_tokens.go
+++ b/token/builder/value_tokens.go
@@ -68,7 +68,7 @@ func valueTokens(v value.Value) []Token {
 		}
 
 		for i := 0; i < len(keys); i++ {
-			if isValidIdentifier(keys[i]) {
+			if scanner.IsValidIdentifier(keys[i]) {
 				toks = append(toks, Token{token.IDENT, keys[i]})
 			} else {
 				toks = append(toks, Token{token.STRING, fmt.Sprintf("%q", keys[i])})
@@ -93,9 +93,3 @@ func valueTokens(v value.Value) []Token {
 
 	return toks
 }
-
-func isValidIdentifier(in string) bool {
-	s := scanner.New(nil, []byte(in), nil, 0)
-	_, tok, lit := s.Scan()
-	return tok == token.IDENT && lit == in
-}

From c85cf96f3f00fa76237894043aba0d1f17b47c27 Mon Sep 17 00:00:00 2001
From: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
Date: Wed, 30 Aug 2023 09:26:37 -0400
Subject: [PATCH 2/6] remove emptyValue

Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
---
 scanner/identifier.go      | 15 +++------------
 scanner/identifier_test.go | 14 ++++++--------
 2 files changed, 9 insertions(+), 20 deletions(-)

diff --git a/scanner/identifier.go b/scanner/identifier.go
index 6f05eee..da8f7c4 100644
--- a/scanner/identifier.go
+++ b/scanner/identifier.go
@@ -15,10 +15,6 @@ func IsValidIdentifier(in string) bool {
 }
 
 type SanitizeIdentifierOptions struct {
-	// EmptyValue is what to use as the identifier when the input is empty.
-	// This must be a valid river identifier.
-	EmptyValue string
-
 	// 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
@@ -30,7 +26,6 @@ type SanitizeIdentifierOptions struct {
 
 func sanitizeIdentifierOptionsDefault() *SanitizeIdentifierOptions {
 	return &SanitizeIdentifierOptions{
-		EmptyValue:  "empty",
 		Prefix:      "id_",
 		Replacement: "_",
 	}
@@ -38,10 +33,6 @@ func sanitizeIdentifierOptionsDefault() *SanitizeIdentifierOptions {
 
 // validate will return an error if the options are invalid.
 func (opts *SanitizeIdentifierOptions) validate() error {
-	if !IsValidIdentifier(opts.EmptyValue) {
-		return fmt.Errorf("emptyValue `%q` is not a valid river identifier", opts.EmptyValue)
-	}
-
 	if !IsValidIdentifier(opts.Prefix) {
 		return fmt.Errorf("prefix `%q` is not a valid river identifier", opts.Prefix)
 	}
@@ -72,14 +63,14 @@ func SanitizeIdentifier(in string, opts *SanitizeIdentifierOptions) (string, err
 		return "", err
 	}
 
-	return generateNewIdentifier(in, opts.EmptyValue, opts.Prefix, opts.Replacement)
+	return generateNewIdentifier(in, opts.Prefix, opts.Replacement)
 }
 
 // generateNewIdentifier expects a valid river prefix and replacement
 // string and returns a new identifier based on the given input.
-func generateNewIdentifier(in string, emptyValue string, prefix string, replacement string) (string, error) {
+func generateNewIdentifier(in string, prefix string, replacement string) (string, error) {
 	if in == "" {
-		return emptyValue, nil
+		return "", fmt.Errorf("cannot generate a new identifier for an empty string")
 	}
 
 	newValue := ""
diff --git a/scanner/identifier_test.go b/scanner/identifier_test.go
index 221c721..28ba8e8 100644
--- a/scanner/identifier_test.go
+++ b/scanner/identifier_test.go
@@ -37,20 +37,18 @@ func TestSanitizeIdentifierOptions(t *testing.T) {
 		expectErr        string
 		opts             *scanner.SanitizeIdentifierOptions
 	}{
-		{"empty", "", "empty", "", nil},
+		{"empty", "", "", "cannot generate a new identifier for an empty string", nil},
 		{"start_number", "0identifier_1", "id_0identifier_1", "", nil},
 		{"start_char", "identifier_1", "identifier_1", "", nil},
 		{"start_underscore", "_identifier_1", "_identifier_1", "", nil},
 		{"special_chars", "!@#$%^&*()", "id___________", "", nil},
 		{"special_char", "identifier_1!", "identifier_1_", "", nil},
 		{"spaces", "identifier _ 1", "identifier___1", "", nil},
-		{"bad emptyValue", "", "", "emptyValue `\"\"` is not a valid river identifier", &scanner.SanitizeIdentifierOptions{EmptyValue: "", Prefix: "", Replacement: ""}},
-		{"bad prefix", "", "", "prefix `\"123\"` is not a valid river identifier", &scanner.SanitizeIdentifierOptions{EmptyValue: "empty2", Prefix: "123", Replacement: ""}},
-		{"bad replacement", "", "", "replacement `\"!\"` must be either a valid river identifier or empty", &scanner.SanitizeIdentifierOptions{EmptyValue: "empty2", Prefix: "prefix2_", Replacement: "!"}},
-		{"different empty", "", "empty2", "", &scanner.SanitizeIdentifierOptions{EmptyValue: "empty2", Prefix: "prefix2_", Replacement: ""}},
-		{"different prefix", "0identifier_1", "prefix2_0identifier_1", "", &scanner.SanitizeIdentifierOptions{EmptyValue: "empty2", Prefix: "prefix2_", Replacement: ""}},
-		{"different replacement", "identifier _ 1", "identifiera_a1", "", &scanner.SanitizeIdentifierOptions{EmptyValue: "empty2", Prefix: "prefix2_", Replacement: "a"}},
-		{"empty replacement", "identifier _ 1", "identifier_1", "", &scanner.SanitizeIdentifierOptions{EmptyValue: "empty2", Prefix: "prefix2_", Replacement: ""}},
+		{"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: ""}},
 	}
 
 	for _, tc := range tt {

From 56af61e03af781a870b72b84ef6c6c3edb3512aa Mon Sep 17 00:00:00 2001
From: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
Date: Wed, 30 Aug 2023 09:55:45 -0400
Subject: [PATCH 3/6] change prefix default

Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
---
 scanner/identifier.go      | 2 +-
 scanner/identifier_test.go | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/scanner/identifier.go b/scanner/identifier.go
index da8f7c4..f11658f 100644
--- a/scanner/identifier.go
+++ b/scanner/identifier.go
@@ -26,7 +26,7 @@ type SanitizeIdentifierOptions struct {
 
 func sanitizeIdentifierOptionsDefault() *SanitizeIdentifierOptions {
 	return &SanitizeIdentifierOptions{
-		Prefix:      "id_",
+		Prefix:      "_",
 		Replacement: "_",
 	}
 }
diff --git a/scanner/identifier_test.go b/scanner/identifier_test.go
index 28ba8e8..59d5e19 100644
--- a/scanner/identifier_test.go
+++ b/scanner/identifier_test.go
@@ -38,10 +38,10 @@ func TestSanitizeIdentifierOptions(t *testing.T) {
 		opts             *scanner.SanitizeIdentifierOptions
 	}{
 		{"empty", "", "", "cannot generate a new identifier for an empty string", nil},
-		{"start_number", "0identifier_1", "id_0identifier_1", "", nil},
+		{"start_number", "0identifier_1", "_0identifier_1", "", nil},
 		{"start_char", "identifier_1", "identifier_1", "", nil},
 		{"start_underscore", "_identifier_1", "_identifier_1", "", nil},
-		{"special_chars", "!@#$%^&*()", "id___________", "", 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: ""}},

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 4/6] 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))
+	})
+}

From 304bc1c624ad1ac62679d7a5b78fe57603381d73 Mon Sep 17 00:00:00 2001
From: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
Date: Wed, 30 Aug 2023 15:15:43 -0400
Subject: [PATCH 5/6] clean up temporary code

Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
---
 scanner/identifier.go | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/scanner/identifier.go b/scanner/identifier.go
index e896e69..ed2239e 100644
--- a/scanner/identifier.go
+++ b/scanner/identifier.go
@@ -9,8 +9,7 @@ import (
 // IsValidIdentifier returns true if the given string is a valid river
 // identifier.
 func IsValidIdentifier(in string) bool {
-	f := token.NewFile("test")
-	s := New(f, []byte(in), nil, 0)
+	s := New(token.NewFile(""), []byte(in), nil, 0)
 	_, tok, lit := s.Scan()
 	return tok == token.IDENT && lit == in
 }

From 859d5fbdee648a2d547d2f30a55dd9c6883b8390 Mon Sep 17 00:00:00 2001
From: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
Date: Wed, 30 Aug 2023 15:19:15 -0400
Subject: [PATCH 6/6] improve fuzzer

Signed-off-by: erikbaranowski <39704712+erikbaranowski@users.noreply.github.com>
---
 scanner/identifier_test.go | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/scanner/identifier_test.go b/scanner/identifier_test.go
index f0ffaf4..e1dfead 100644
--- a/scanner/identifier_test.go
+++ b/scanner/identifier_test.go
@@ -75,7 +75,11 @@ func BenchmarkSanitizeIdentifier(b *testing.B) {
 	}
 }
 
-func FuzzTestSanitizeIdentifier(f *testing.F) {
+func FuzzSanitizeIdentifier(f *testing.F) {
+	for _, tc := range sanitizeTestCases {
+		f.Add(tc.identifier)
+	}
+
 	f.Fuzz(func(t *testing.T, input string) {
 		newIdentifier, err := scanner.SanitizeIdentifier(input)
 		if input == "" {