Skip to content

Conversation

mikeschinkel
Copy link
Contributor

@mikeschinkel mikeschinkel commented Dec 17, 2023

Using the following command line where persister contains files generated by sqlc, the latest version in master generates invalid code for results that are references, e.g. map, slice, and pointers.

This PR aims to fix that — which appear to be that the original fix for #12 was incomplete, and this PR assumes PR #65 has been committed.

ifacemaker -f ./persister/*.go -s Queries -i DataStoreQueries -p app -o ./app/query.iface.go

Here is the test code I wrote against:

package persister
type Category struct {}
func (q *Queries) Ex1() (map[int]*Category, error) {
	return nil, nil
}
func (q *Queries) Ex2() (map[int]Category, error) {
	return nil, nil
}
func (q *Queries) Ex3() ([]*Category, error) {
	return nil, nil
}
func (q *Queries) Ex4() ([]Category, error) {
	return nil, nil
}
func (q *Queries) Ex5() (*Category, error) {
	return nil, nil
}

Without the PR it generates this (and some newlines I removed), which fails to compile in Go:

// Code generated by ifacemaker; DO NOT EDIT.
package app
import (
	_ "embed"
	_ "github.com/mattn/go-sqlite3"
)
// DataStoreQueries ...
type DataStoreQueries interface {
	Ex1() (map[int]*Category, error)
	Ex2() (map[int]Category, error)
	Ex3() ([]*Category, error)
	Ex4() ([]Category, error)
	Ex5() (*Category, error)
}

With the PR, it generates this, also minus newlines, which does compile in Go:

// Code generated by ifacemaker; DO NOT EDIT.
package app
import (
	_ "embed"
	_ "github.com/mattn/go-sqlite3"
	"github.com/mikeschinkel/gerardus/persister"
)
// DataStoreQueries ...
type DataStoreQueries interface {
	Ex1() (map[int]*persister.Category, error)
	Ex2() (map[int]persister.Category, error)
	Ex3() ([]*persister.Category, error)
	Ex4() ([]persister.Category, error)
	Ex5() (*persister.Category, error)
}

@mikeschinkel mikeschinkel changed the title Fix struct validation for multiple files Fix result generation for map, slice, pointer when targeting package Dec 17, 2023
@mikeschinkel mikeschinkel changed the title Fix result generation for map, slice, pointer when targeting package Fix result generation for map, slice, pointer when targeting other package Dec 17, 2023
@gaby gaby changed the title Fix result generation for map, slice, pointer when targeting other package fix: Result generation for map, slice, pointer when targeting other package Mar 15, 2025
@grivera64
Copy link
Contributor

grivera64 commented Mar 24, 2025

From taking a look at the ifacemaker codebase, the current way to use types from external packages is by importing the external package into the current file's namespace using a . to instead of using package.Type. This is done by passing in the -m flag (short for --import-module=<full-package-name>) that was added in PR #52. For example:

// File: human.go
// Package full name is github.com/vburenin/ifacemaker/example/mypackage
package mypackage

import (
	"fmt"
)

type Food struct {
	name string
}

func (f *Food) Name() string {
	return f.name
}

type Human struct {
}

func (h *Human) Eat(food *Food) {
	fmt.Printf("Eating %s...\n", food.Name())
}

If you run the following command:

ifacemaker -f human.go -m "github.com/vburenin/ifacemaker/example/mypackage" -s Human -i IHuman -p otherpackage -o ihuman.go

You will get the following:

// Code generated by ifacemaker; DO NOT EDIT.

package otherpackage

import (
	. "github.com/vburenin/ifacemaker/example/mypackage"
)

// IHuman ...
type IHuman interface {
	Eat(food *Food)
}

While not documented in the README.md, this works for smaller cases like the above. A problem with this current technique though is that can lead to name conflicts if the package of the generated interface has the same name as the imported package. In this case, this PR helps deal with this issue.

When using this PR to generate the example above, we can then omit the -m flag and it will work the same way but by using the package name as a prefix to the type:

ifacemaker -f human.go -s Human -i IHuman -p otherpackage -o ihuman.go
// Code generated by ifacemaker; DO NOT EDIT.

package otherpackage

import "github.com/vburenin/ifacemaker/example/mypackage"

// IHuman ...
type IHuman interface {
	Eat(food *mypackage.Food)
}

If everything looks fine so far for the PR, I think all we need is to add a unit test for this functionality. What are your thoughts on this @gaby ? Also, this PR assumes that #65 was merged. Maybe we could merge #72 first into the master branch and merge here to ensure there are no issues between the two PR's?

@gaby
Copy link
Collaborator

gaby commented Mar 24, 2025

@grivera64 Can you come up with a unit-test for this?

@gaby gaby merged commit 1456818 into vburenin:master Mar 25, 2025
9 checks passed
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.

3 participants