Skip to content

chore: remove incorrectly named ToSnakeCase func#180

Open
tobikris wants to merge 1 commit intoinvopop:mainfrom
exaring:snake-kebab
Open

chore: remove incorrectly named ToSnakeCase func#180
tobikris wants to merge 1 commit intoinvopop:mainfrom
exaring:snake-kebab

Conversation

@tobikris
Copy link
Copy Markdown

The function was supposed to convert to kebab-case, but named ToSnakeCase. It was removed in favor of using the github.com/stoewer/go-strcase package's KebabCase. This package was already suggested in the documentation anyway.

I verified that this change has no unwanted effects with the following test - no changes in the meaningful cases.

package jsonschema

import (
	"regexp"
	"strings"
	"testing"

	"github.com/stretchr/testify/assert"
)

func TestSnakeCase(t *testing.T) {
        // list from https://github.com/stoewer/go-strcase/blob/master/kebab_test.go#L12
        // except test cases which were not producing useful results before
	data := map[string]string{
		"":      "",
		"F":     "f",
		"Foo":   "foo",
		"FooB":  "foo-b",
		"FooID": "foo-id",
		// " FooBar\t":      "foo-bar",
		"HTTPStatusCode": "http-status-code",
		// "ParseURL.DoParse": "parse-url.do-parse",
		// "Convert Space": "convert-space",
		"Convert-dash": "convert-dash",
		// "Skip___MultipleUnderscores": "skip-multiple-underscores",
		// "Skip   MultipleSpaces":      "skip-multiple-spaces",
		// "Skip---MultipleDashes":      "skip-multiple-dashes",
	}

	for camel, snake := range data {
		converted := ToSnakeCase(camel)
		assert.Equal(t, snake, converted)
	}
}

var (
	matchFirstCap = regexp.MustCompile("(.)([A-Z][a-z]+)")
	matchAllCap   = regexp.MustCompile("([a-z0-9])([A-Z])")
)

// ToSnakeCase converts the provided string into snake case using dashes.
// This is useful for Schema IDs and definitions to be coherent with
// common JSON Schema examples.
func ToSnakeCase(str string) string {
	snake := matchFirstCap.ReplaceAllString(str, "${1}-${2}")
	snake = matchAllCap.ReplaceAllString(snake, "${1}-${2}")
	return strings.ToLower(snake)
}

Copy link
Copy Markdown
Contributor

@samlown samlown left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good to me!

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR removes the misnamed ToSnakeCase helper (which produced kebab-case) and switches schema ID generation to use github.com/stoewer/go-strcase’s KebabCase, aligning implementation with the documented recommendation.

Changes:

  • Removed the internal ToSnakeCase implementation from utils.go.
  • Updated schema $id generation to use strcase.KebabCase.
  • Added github.com/stoewer/go-strcase to go.mod/go.sum.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

File Description
utils.go Removes the exported ToSnakeCase helper implementation.
reflect.go Uses strcase.KebabCase for schema ID generation and updates related comment wording.
go.mod Adds github.com/stoewer/go-strcase dependency.
go.sum Records checksums for the new dependency.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread utils.go
@samlown
Copy link
Copy Markdown
Contributor

samlown commented Apr 23, 2026

If you could, please update this branch with main (or grant me permissions to commit). Thanks!

@samlown samlown added the question Further information is requested label Apr 23, 2026
The function was supposed to convert to kebab-case, but named ToSnakeCase.
It was removed in favor of using the github.com/stoewer/go-strcase package's KebabCase.
This package was already suggested in the documentation anyway.
@tobikris
Copy link
Copy Markdown
Author

I have rebased this branch 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

question Further information is requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants