fix: Multibind scopes

[eirikur-nc]

What

Change how the scope argument of the multibind method works, so that it applies to the target rather than the containing list or dictionary. Also, consider explicit class bindings when resolving multibindings, and honor the scope of the class binding when the multibinding has no scope.

Why

So that it becomes possible to control the scope of individual multi-bindings. E.g.

def configure(binder: Binder) -> None:
    binder.multibind(List[Plugin], to=PluginA, scope=singleton)
    binder.multibind(List[Plugin], to=PluginB)

injector = Injector([configure])
first_list = injector.get(List[Plugin])
second_list = injector.get(List[Plugin])

assert first_list is not second_list
assert first_list[0] is second_list[0]
assert first_list[1] is not second_list[1]

Closes #283

[codecov]

Codecov Report

❌ Patch coverage is 98.14815% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 95.98%. Comparing base (5a30e9a) to head (9faa32a).

Files with missing lines	Patch %	Lines
injector/__init__.py	98.14%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #284      +/-   ##
==========================================
+ Coverage   95.84%   95.98%   +0.13%     
==========================================
  Files           1        1              
  Lines         602      623      +21     
  Branches      103      106       +3     
==========================================
+ Hits          577      598      +21     
  Misses         19       19              
  Partials        6        6              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[davidparsson]

Thank you. This looks promising. I'll try to find time to review this properly.

[davidparsson]

Could you please elaborate a bit on in what case this is needed? Is it required for the case when multiple different types are bound with the singleton scope in the same multibound type? Just like PluginA and PluginC in the case below:

def test_multibind_dict_scopes_applies_to_the_bound_items() -> None:
    def configure(binder: Binder) -> None:
        binder.multibind(Dict[str, Plugin], to={'a': PluginA}, scope=singleton)
        binder.multibind(Dict[str, Plugin], to={'b': PluginB})
        binder.multibind(Dict[str, Plugin], to={'c': PluginC}, scope=singleton)

[davidparsson]

I quickly tried to reproduce the problem without the pseudo-type, by instead passing the resolved type, which is Plugin for the example above. I got a weird problem in that test case that I haven't yet resolved, but for multibound lists every case I can come up with seems to work as expected. This makes me suspect it's not really needed.

[davidparsson]

Thanks for one more nice contribution!

I'm planning on merging this, and tweaking a few minor things before releasing, but I would appreciate if you could take the time to respond to my comments.

[davidparsson]

I'm planning on removing the has_explicit_binding_for() condition, since an explicit binding (that should be respected) may exist on parent.parent.parent as well. That change also makes sure that classes decorated with @singleton are instantiated as far up in the hierarchy as possible, which makes sure the instance is shared among all child injectors.

Do you have any argument against doing this? All tests still pass.

[davidparsson]

I quickly tried to reproduce the problem without the pseudo-type, by instead passing the resolved type, which is Plugin for the example above. I got a weird problem in that test case that I haven't yet resolved, but for multibound lists every case I can come up with seems to work as expected. This makes me suspect it's not really needed.

[davidparsson]

I think we should respect the parent provider here as well, for the same reasons as for scopes. I'll fix this.