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

Auto set package name with goverter:output:file #180

Open
applejag opened this issue Jan 29, 2025 · 2 comments · May be fixed by #181
Open

Auto set package name with goverter:output:file #180

applejag opened this issue Jan 29, 2025 · 2 comments · May be fixed by #181
Labels
feature New feature or request

Comments

@applejag
Copy link
Contributor

Is your feature request related to a problem? Please describe.
When outputting into the same package it's quite tedious to having to specify the full package name with goverter:output:package. This is an albeit small friction point, but still feels like goverter should be able to figure this out for us instead of us having to input this.

I want to only need to specify goverter:output:file. The issue with this comes when you want to both output into the same package and reference some other functions.

For example, given this:

package sample

import "strings"

// goverter:converter
// goverter:output:file ./generated.go
type Converter interface {
	// goverter:map Moo | Uppercase
	ConvertFoo(foo Foo) Bar
}

type Foo struct {
	Moo string
}

type Bar struct {
	Moo string
}

func Uppercase(s string) string {
	return strings.ToUpper(s)
}

When generating it outputs this:

// Code generated by github.com/jmattheis/goverter, DO NOT EDIT.
//go:build !goverter

package generated

import sample "goverter-test/pkg/sample"

type ConverterImpl struct{}

func (c *ConverterImpl) ConvertFoo(source sample.Foo) sample.Bar {
	var sampleBar sample.Bar
	sampleBar.Moo = sample.Uppercase(source.Moo)
	return sampleBar
}

This fails to compile because the default package name is generated, and having two different package names in the same dir is not allowed:

$ go test ./pkg/sample
found packages generated (generated.go) and sample (sample.go) in /home/kallefagerberg/code/gh/goverter-test/pkg/sample

If you override just the package name with // goverter:output:package :sample then it correctly says package sample in the generated file, but it still fails to compile because it is still importing the sample package from the sample package itself.

$ go test ./pkg/sample
package goverter-test/pkg/sample
	imports goverter-test/pkg/sample: import cycle not allowed

The only way to get around this today is to write the fully-qualified package name (as the docs suggest).

Describe the solution you'd like
It would be much nicer if the output package name defaulted to the package name that's already used in that directory (if any). If none exist, then you can default back to generated again.

This new behavior should apply if you leave goverter:output:package unset.

By looking around Go's parsing packages, I found this which could help: https://pkg.go.dev/golang.org/x/tools/go/packages#example-package

For example if I have the following repo:

 .
 ├── pkg
 │   └── sample
 │       └── sample.go
 ├── go.mod
 ├── go.sum
 └── main.go

And in the main.go I have the following code:

func main() {
	cfg := &packages.Config{
		Mode: packages.NeedName,
	}
	pkgs, err := packages.Load(cfg, "./pkg/sample")
	if err != nil {
		log.Fatalf("Load: %s", err)
	}
	for _, pkg := range pkgs {
		log.Printf("Package ID: %s", pkg.ID)
		log.Printf("Package name: %s", pkg.Name)
	}
}

Running it results in this:

$ go run .
2025/01/29 17:39:52 Package ID: goverter-test/pkg/sample
2025/01/29 17:39:52 Package name: sample

(I named the module goverter-test with go mod init goverter-test).

Describe alternatives you've considered
An alternative could be to instead programmatically try get the module name (the goverter-test part), and then resolve what the full package name would be. This would mean it would be able to assume the full package name with this too:

// goverter:converter
// goverter:output:package mygeneratedconverter
type Converter interface {
	ConvertFoo(foo Foo) Bar
}

although I don't think it has any benefit if goverter is aware of the full package name when it's not using other files...

@applejag
Copy link
Contributor Author

By looking at goverter's code I see that you're already using the golang.org/x/tools/package package, which makes sense. Question would be how to incorporate it into this use case...?

@jmattheis jmattheis added the feature New feature or request label Jan 29, 2025
@jmattheis
Copy link
Owner

jmattheis commented Jan 29, 2025

Thanks for the detailed feature request (:.

Automatically detecting this if the output file is in the same directory as the Converter interface should be fairly easy because this is already supported for goverter:variables. E.g. https://github.com/jmattheis/goverter/tree/main/example/format/assignvariables

Generally supporting this has some edge-cases that make this flaky to use. E.g. your code example only works if there is at least one .go file in the directory, otherwise the package ID won't contain the full path and there must be no compile errors. In the case where the output package is the same as the input package, it's guaranteed that there is a go file, so this is fine.

Can be implemented, it's probably a check at the end of config/converter.go#parseConverter and if the output:file is relative to ./ then we can automatically set the output:package.

@applejag applejag linked a pull request Jan 29, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants