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

refactor: including package path in implicit service name #13

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Commits on Aug 4, 2022

  1. refactor: including package path in implicit service name

    During use, I found that registering services with the same name will panic,
    even if they came from different packages. Here's an minimal repro:
    
    ```go
    // a/c/a-c.go
    package c
    import ( /* ... */ )
    type MyStruct struct { /* ... */ }
    func NewMyStruct(i *do.Injector) (*MyStruct, error) { /* ... */ }
    
    // b/c/b-c.go
    package c
    import ( /* ... */ )
    type MyStruct struct { /* ... */ }
    func NewMyStruct(i *do.Injector) (*MyStruct, error) { /* ... */ }
    
    // main.go
    package main
    import ( /* ... */ )
    func main() {
        i := do.New()
        do.Provide(i, ac.NewMyStruct)
        do.Provide(i, bc.NewMyStruct)
    }
    
    // panic: DI: service `*c.MyStruct` has already been declared
    ```
    
    The problem occurs because `generateServiceName()` didn't create
    a "global-unique" qualifier for type.
    This PR refactors `generateServiceName()` to create
    unique qualifiers by prepending package path.
    
    \## Implementation
    
    This implementation uses `reflect.TypeOf()`,
    which is the same as `fmt.Sprintf("%T")`.
    It basically the same as `typeOfT.PkgPath() + "." + typeOfT.Name()`.
    
    For the following code it generates such qualifiers:
    
    ```go
    // service_test.go
    type testStruct struct{}
    type testInterface interface{}
    ```
    
    type|qualifier
    -|-
    `testStruct`|`github.com/samber/do.testStruct`
    `*testStruct`|`github.com/samber/do.*testStruct`
    `testInterface`|`github.com/samber/do.*testInterface`
    `int`|`int`
    `*int`|`*int`
    
    \## Alternatives
    
    \### `typeOfT.Name()` vs. `typeOfT.String()`
    
    `fmt.Sprintf("%T")` uses `typeOfT.String()`.
    But `typeOfT.String()`
    [says](https://pkg.go.dev/reflect#Type)
    "The string representation **may** use shortened package names
    and is not guaranteed to be unique among types."
    Therefore it cannot be guaranteed for usage as a type qualifier.
    
    \### Processing method of pointer indirect star
    
    The current `do.*test` looks strange
    but I haven't thought of a better one yet.
    
    \## Testing
    
    Tests have all been updated.
    Maybe it's necessary to create two test packages
    to test bugs mentioned above,
    but adding two new files for this is somewhat weird
    so I didn't do that.
    Is it necessary?
    
    \## Compatibility
    
    This doesn't introduce API changes and therefore
    it should be possible to add into v1.
    But it introduces internal behavior changes
    so please consider it carefully.
    ilharp committed Aug 4, 2022
    Configuration menu
    Copy the full SHA
    5641b97 View commit details
    Browse the repository at this point in the history

Commits on Aug 6, 2022

  1. refactor: use &t

    ilharp committed Aug 6, 2022
    Configuration menu
    Copy the full SHA
    c5d56ed View commit details
    Browse the repository at this point in the history

Commits on Sep 24, 2023

  1. Configuration menu
    Copy the full SHA
    477a12b View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    01131f3 View commit details
    Browse the repository at this point in the history
  3. oops

    samber committed Sep 24, 2023
    Configuration menu
    Copy the full SHA
    ac1dede View commit details
    Browse the repository at this point in the history