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

feat: add constant-case strategy in building env key names #33

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
54 changes: 46 additions & 8 deletions fig.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"strconv"
"strings"
"time"
"unicode"

"github.com/mitchellh/mapstructure"
"github.com/pelletier/go-toml/v2"
Expand Down Expand Up @@ -112,14 +113,15 @@ func defaultFig() *fig {
}

type fig struct {
filename string
dirs []string
tag string
timeLayout string
useEnv bool
useStrict bool
ignoreFile bool
envPrefix string
filename string
dirs []string
tag string
timeLayout string
useEnv bool
useConstantCase bool
Copy link
Owner

Choose a reason for hiding this comment

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

We can introduce an abstraction here so that multiple strategies can coexist without cluttering this struct and formatEnvKey with each option:

func defaultFig() *fig {
	return &fig{
		envKeyTransform: strings.ToUpper
	}
}

type fig struct {
    envKeyTransform func(string) string
}

...

func (f *fig) formatEnvKey(key string) string {
   ..
   key = f.envKeyTransform(key)
}

...

func UseConstantCaseForEnvKeys() Option {
	return func(f *fig) {
		f. envKeyTransform = toConstantCase
	}
}

Copy link
Author

Choose a reason for hiding this comment

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

Should we pass the env prefix to transformers? it's needed for adding prefix to the generated env key.

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah we should add it

useStrict bool
ignoreFile bool
envPrefix string
}

func (f *fig) Load(cfg interface{}) error {
Expand Down Expand Up @@ -312,12 +314,48 @@ func (f *fig) setFromEnv(fv reflect.Value, key string) error {
func (f *fig) formatEnvKey(key string) string {
// loggers[0].level --> loggers_0_level
key = strings.NewReplacer(".", "_", "[", "_", "]", "").Replace(key)
if f.useConstantCase {
key = f.toConstantCase(key)
}
if f.envPrefix != "" {
key = fmt.Sprintf("%s_%s", f.envPrefix, key)
}
return strings.ToUpper(key)
}

func (f *fig) toConstantCase(key string) string {
Copy link
Owner

Choose a reason for hiding this comment

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

This is called constant case but it's not upper casing the output.

Copy link
Author

Choose a reason for hiding this comment

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

strings.Upper is called in its parent function.
Maybe we should put the default behaviour in another strategy and be able to completely override the envKeyTransformer by providing another transformer, right?

Copy link
Owner

Choose a reason for hiding this comment

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

Yes

var b strings.Builder
runes := []rune(key)

for i := 0; i < len(runes); i++ {
if !unicode.IsUpper(runes[i]) {
b.WriteRune(runes[i])
continue
}

if i == 0 {
b.WriteRune(runes[i])
continue
}

if unicode.IsUpper(runes[i-1]) || !unicode.IsLetter(runes[i-1]) {
if len(runes) == i+1 {
b.WriteRune(runes[i])
} else if unicode.IsLower(runes[i+1]) {
b.WriteString("_")
b.WriteRune(runes[i])
} else {
b.WriteRune(runes[i])
}
} else {
b.WriteString("_")
b.WriteRune(runes[i])
}
}

return b.String()
Copy link
Owner

Choose a reason for hiding this comment

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

With input = "helloWORLD" this algorithm will output "helloWORLD". Should it not output "hello_WORLD"?

Copy link
Author

Choose a reason for hiding this comment

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

This should be helloWorld, I did this to avoid converting USA to u_s_a.
Maybe we should rename this to camelCaseToConstantCase?

Copy link
Owner

Choose a reason for hiding this comment

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

Why would "helloWORLD" -> "hello_WORLD" imply "USA" -> "u_s_a"?

Copy link
Author

Choose a reason for hiding this comment

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

If next letter of an upper case letter is lower case, it means it has to be separated with an underline. helloWorld -> hello_World.

if an upper case letter is preceded by another upper case letter, it means it's something like USA.

Copy link
Author

Choose a reason for hiding this comment

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

Current behavior only checks next letters each letter, that's why we can convert something like USACountryCode to USA_Country_Code but we can't convert helloWORLD to hello_WORLD.

I'm gonna think about another algorithm.

Copy link
Author

@mojixcoder mojixcoder Mar 4, 2024

Choose a reason for hiding this comment

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

I updated the algorithm and added more test cases, please check if that's what you meant.

Copy link
Owner

@kkyr kkyr Mar 5, 2024

Choose a reason for hiding this comment

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

Algorithm seems to work but is a little hard to read - what do you think about this version?

func (f *fig) toConstantCase(key string) string {
	var b strings.Builder
	runes := []rune(key)

	lowerToUpper := func(i int) bool {
		return i > 0 && unicode.IsLower(runes[i-1]) && unicode.IsUpper(runes[i])
	}
	upperToLower := func(i int) bool {
		return i+1 < len(runes) && unicode.IsUpper(runes[i]) && unicode.IsLower(runes[i+1])
	}

	for i := range runes {
		if i > 0 && (lowerToUpper(i) || upperToLower(i)) {
			b.WriteByte('_')
		}
		b.WriteRune(runes[i])
	}
	
	return strings.ToUpper(b.String())
}

}

// setDefaultValue calls setValue but disallows booleans from
// being set.
func (f *fig) setDefaultValue(fv reflect.Value, val string) error {
Expand Down
29 changes: 29 additions & 0 deletions fig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1289,6 +1289,35 @@ func Test_fig_setSlice(t *testing.T) {
})
}

func Test_fig_toConstantCase(t *testing.T) {
fig := defaultFig()

testCases := []struct {
key string
expected string
}{
{key: "fig_serviceName", expected: "fig_service_Name"},
{key: "fig_USA_test", expected: "fig_USA_test"},
{key: "fig_runT", expected: "fig_run_T"},
{key: "fig_0_test", expected: "fig_0_test"},
{key: "USACountry", expected: "USA_Country"},
{key: "helloWORLD", expected: "hello_WORLD"},
{key: "USA", expected: "USA"},
{key: "___", expected: "___"},
{key: "a__b__c", expected: "a__b__c"},
{key: "a__B__c", expected: "a__B__c"},
}

for _, tc := range testCases {
t.Run(tc.key, func(t *testing.T) {
res := fig.toConstantCase(tc.key)
if res != tc.expected {
t.Errorf("expected %s, got %s", tc.expected, res)
}
})
}
}

func setenv(t *testing.T, key, value string) {
t.Helper()
t.Setenv(key, value)
Expand Down
13 changes: 13 additions & 0 deletions option.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,3 +118,16 @@ func UseStrict() Option {
f.useStrict = true
}
}

// EnvConstantCaseStrategy returns an option that configures fig to
// convert env key names to constant-case naming convention.
// e.g. converts `app_serverPort` to `APP_SERVER_PORT`.
//
// fig.Load(&cfg, fig.EnvConstantCaseStrategy())
//
// If this option is not used then fig builds env key names as explained in UseEnv option.
func EnvConstantCaseStrategy() Option {
return func(f *fig) {
f.useConstantCase = true
}
}