livebud / bud

The Full-Stack Web Framework for Go
MIT License
5.58k stars 179 forks source link

Unused import error in generated controller #137

Closed donovanrost closed 2 years ago

donovanrost commented 2 years ago

I'm attempting to implement persistence. So I create a new folder models. Write a struct. Update the controllers to use that new struct instead of the autogenerated one. Then run bud run. But bud errors out with

bud/.app/controller/controller.go:10:2: imported and not used: "github/donovanrost/bud-test/models"

I don't think that bud needs to do any code generation with this package. Is there a way to tell bud to ignore it?

matthewmueller commented 2 years ago

@donovanrost this should work, did you perhaps remove the Controller struct as well?

Here's a working example:

controller/controller.go

package controller

import (
    context "context"

    "github.com/livebud/issue-137/model/post"
)

type Controller struct {
    Post *post.Model
}

// Index of posts
// GET
func (c *Controller) Index(ctx context.Context) (posts []*post.Post, err error) {
    return c.Post.FindMany()
}

// Show post
// GET /:id
func (c *Controller) Show(ctx context.Context, id int) (post *post.Post, err error) {
    return c.Post.Find(id)
}

model/post/post.go

package post

import "fmt"

type Model struct {
}

type Post struct {
    ID    int
    Title string
    Body  string
}

var db = []*Post{
    {
        ID:    1,
        Title: "first post",
        Body:  "first body",
    },
    {
        ID:    2,
        Title: "second post",
        Body:  "second body",
    },
}

func (m *Model) FindMany() ([]*Post, error) {
    return db, nil
}

func (m *Model) Find(id int) (*Post, error) {
    for _, p := range db {
        if p.ID == id {
            return p, nil
        }
    }
    return nil, fmt.Errorf("post %d not found", id)
}
$ bud run
| Listening on http://127.0.0.1:3000
| Ready on http://127.0.0.1:3000

If you're still running into trouble, I'd very much welcome another video :-)

vito commented 2 years ago

Hiya, I ran into this same issue because my controller didn't have the package as a dependency. I had my models a models package (mostly generated by xo/xo) and my db dependency in another (*db.DB), so it was possible for me to use the models with the database in valid Go code but because none of the generated code actually mentioned the package by name the import would be unused.

Moving the dependency initializer to the same package fixed the issue (*models.Conn, since xo/xo already takes the DB name for an interface) because then the Controller definition referenced the dependent package.

This might be more correct code to write anyway, but it was a little hard to discover. Maybe go fmt could be run against the generated code to remove any unused imports?

matthewmueller commented 2 years ago

Hey @vito, thanks for letting me know! I'm having trouble understanding the setup though.

Can you share some code?

If I'm understanding correctly, $APP/controller never imported $APP/model, but the generated $APP/bud/.app/controller imported $APP/model and then reported it unused?

Or in diagram form something like this?

graph TD
$APP/model --> $APP/db
$APP/bud/.app/controller --> $APP/controller
$APP/bud/.app/controller --> $APP/model
$APP/bud/.app/main.go --> $APP/bud/.app/controller

If so, I don't understand how $APP/bud/.app/controller would know about the $APP/model package. I can definitely try reproing this if that appears to be the case!

vito commented 2 years ago

Sorry for not including code. :) It's late here and the broken code is a few iterations in the past without any of this committed yet. I'll circle back and add a repro if this clarification doesn't help:

The controller imported both $APP/model and $APP/db, but only used $APP/model in the action implementations, and didn't reference it as a controller dependency. The generated $APP/bud/.app/controller code brought in both imports. The $APP/db import was OK because it's part of the controller dependency injection and ends up referenced in the loadController args, but the $APP/models import was not OK because no code in that generated file references it; it imports the controller and calls its action instead.

donovanrost commented 2 years ago

@matthewmueller Your example definitely worked. Thank you so much for that. I think the issue came from not specifying my package as a dependency.

matthewmueller commented 2 years ago

Hey @vito, thanks for your response. I'm probably just being slow and I think I understand the confusion where the generated code is throwing an error when the hand-written controller code is perfectly fine, but I'm having trouble reproducing this case.

I pushed up my attempted reproduction based on your description to Github: https://github.com/livebud/issues/blob/main/137/controller/controller.go

Maybe you could have a look and let me know what I'm doing wrong?

vito commented 2 years ago

@matthewmueller Ah ha, thanks for starting on that before I could get back to it, I was able to whittle your example down and get it to fail by comparing to my branch.

It's actually much simpler: if the return type of an action contains a field from a package, the package ends up imported for some reason. I submitted a PR to update the repro: https://github.com/livebud/issues/pull/1

(There's probably a more accurate description of what's happening once the root cause is found.)

matthewmueller commented 2 years ago

Ooo. I think I see the problem now. I'll get this fixed soon! Thanks for whittling down the repro!