Skip to content

exec: fix case-insensitive file name collision - #4129

Open
grzn wants to merge 2 commits into
99designs:masterfrom
grzn:master
Open

exec: fix case-insensitive file name collision#4129
grzn wants to merge 2 commits into
99designs:masterfrom
grzn:master

Conversation

@grzn

@grzn grzn commented Apr 20, 2026

Copy link
Copy Markdown

Similar to #2829.

  1. added a case under codegen/testserver/casesensitivity/ to demonstrate the problem
  2. added handing of filenames with different cases, similar to what was done in resolver: fix case-insensitive file name collision #2829

@grzn
grzn requested a review from StevenACoffman as a code owner April 20, 2026 10:32
Comment thread codegen/generate.go
}

func addBuild(filename string, p *ast.Position, data *Data, builds *map[string]*Data) {
func addBuild(fnCase string, fnKey string, p *ast.Position, data *Data, builds *map[string]*build) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
func addBuild(fnCase string, fnKey string, p *ast.Position, data *Data, builds *map[string]*build) {
func addBuild(fnCase, fnKey string, p *ast.Position, data *Data, builds *map[string]*build) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The abbreviation you chose fn for fnCase and fnKey is commonly used to mean function so it may be a little confusing here?

@StevenACoffman StevenACoffman Apr 23, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Also, even in gigantic projects, the number of files is probably not significant, as you do in a number of the other functions, so I'm not sure that we need to bother passing the same data fnCase and fnKey here, when we can just re-compute fnKey from strings.ToLower(filename)

@StevenACoffman

Copy link
Copy Markdown
Collaborator

Hi! I appreciate your PR, but the fmt-and-lint check failed with:

Error: generate: open /home/runner/work/gqlgen/gqlgen/codegen/testserver/casesensitivity/resource.generated.go: no such file or directory
Error: Process completed with exit code 1.

And when I looked at the lint failures, I questioned whether we need to change that signature at all, since the strings.ToLower() conversion of a filename is not going to make a measurable performance impact at the scale of the number of files in the vast majority of projects.

BTW, https://github.com/SimonWaldherr/golang-benchmarks#caseinsensitivecompare shows that strings.ToUpper() is faster and has fewer allocations.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants