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

wire: wire.Value is not forbidding references to names introduced in function scope #34

Closed
vangent opened this issue Aug 29, 2018 · 6 comments
Assignees

Comments

@vangent
Copy link
Contributor

vangent commented Aug 29, 2018

See
https://github.com/vangent/go-cloud/tree/wirebug/wire/internal/wire/testdata/ValueFromStructField
for an example.

func newMainService(cfg *MainConfig) *MainService {
      wire.Build(
		MainService{},
		foo.New,
		wire.Value(cfg.Foo),
		bar.New,
		wire.Value(cfg.Bar),
		baz.New,
		wire.Value(cfg.Baz),
	)
        return nil
}

results in generated code

            // Injectors from wire.go:                                                                                                                
                                                                                                                                                      
            func newMainService(cfg *MainConfig) *MainService {                                                                                       
                config := _wireConfigValue                                                                                                            
                service := foo.New(config)                                                                                                            
                config2 := _wireConfigValue2                                                                                                          
                service2 := bar.New(config2, service)                                                                                                 
                config3 := _wireConfigValue3                                                                                                          
                service3 := baz.New(config3, service2)                                                                                                
                mainService := &MainService{                                                                                                          
                        Foo: service,                                                                                                                 
                        Bar: service2,                                                                                                                
                        Baz: service3,                                                                                                                
                }
                return mainService
            }

            var (
                _wireConfigValue  = cfg.Foo
                _wireConfigValue2 = cfg.Bar
                _wireConfigValue3 = cfg.Baz
            )

            // wire.go:

which fails with

    wire_test.go:110: go build check failed: build: exit status 2; output:                                                                        
        # example.com/main                                                                                                                        
        ./wire_gen.go:33:22: undefined: cfg                                                                                                       
        ./wire_gen.go:34:22: undefined: cfg                                                                                                       
        ./wire_gen.go:35:22: undefined: cfg 

cfg is out of scope in the generated code.

@vangent
Copy link
Contributor Author

vangent commented Aug 29, 2018

The README for wire says "It's important to note that the expression will be copied to the injector's package; references to variables will be evaluated during the injector package's initialization." So maybe this is not supported? The error message could be better.

@zombiezen
Copy link
Collaborator

Link for sample is broken, and the imports are really relevant here. This sounds like a legit bug, though (the Wire value initialization time stuff is pretty now). Can you check that link?

@vangent
Copy link
Contributor Author

vangent commented Sep 4, 2018

The new testcase MultipleSimilarPackages is easily modified to demonstrate this:

vangent/go-cloud@ecd11e6#diff-08ca43f68146628973a3c5efdcb374e7

@zombiezen
Copy link
Collaborator

Ohhh. The issue is that Wire is not forbidding references to names introduced in function scope. Wire should throw an error in this case and not generate the code. I modified the issue description to make this more clear.

@vangent
Copy link
Contributor Author

vangent commented Sep 4, 2018

Yea, the README explicitly says it's not supported but the error message doesn't tell you what you did wrong.

Why can't we support this?

@zombiezen
Copy link
Collaborator

It is technically feasible, but it makes the rule of "wire.Value is for constants" no longer true, and thus more complex to the user.

@vangent vangent changed the title wire: wire.Value on struct value results in out-of-scope reference wire: wire.Value is not forbidding references to names introduced in function scope Sep 7, 2018
@zombiezen zombiezen transferred this issue from google/go-cloud Nov 28, 2018
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

No branches or pull requests

2 participants