Fix WithArguments when used with IFactory bindings including Components

[sbergen]

Pull request checklist

Please check if your PR fulfills the following requirements:

• Tests for the changes have been added (for bug fixes / features)
• Docs have been reviewed and added / updated if needed (for bug fixes / features)
• No compiler errors or warnings

Pull request type

Please check the type of change your PR introduces:

• Bugfix
• Feature
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no api changes)
• Build related changes
• Documentation content changes
• Other (please describe):

Issue Number

Issue Number: #196

Does this introduce a breaking change?

• Yes
• No

Depends on the definition of "breaking":
Some unnecessary WithArguments use which was previously valid, might now produce errors about unused injected values. However, this is very unlikely to be a major issue for anyone, and points out actual problems (values being unused) in any case.

Other information

• This fix has been in use in Yousician for a long time already without issues, only now took the time to prepare a PR out of it.

On which Unity version has this been tested?

• 2020.4 LTS
• 2020.3
• 2020.2
• 2020.1
• 2019.4 LTS
• 2019.3
• 2019.2
• 2019.1
• 2018.4 LTS

Scripting backend:

• Mono
• IL2CPP

Note: Every pull request is tested on the Continuous Integration (CI) system to confirm that it works in Unity.

Ideally, the pull request will pass ("be green"). This means that all tests pass and there are no errors. However, it is not uncommon for the CI infrastructure itself to fail on specific platforms or for so-called "flaky" tests to fail ("be red"). Each CI failure must be manually inspected to determine the cause.

CI starts automatically when you open a pull request, but only Releasers/Collaborators can restart a CI run. If you believe CI is giving a false negative, ask a Releaser to restart the tests.

[RichardWepner]

@sbergen: regarding whether or not it's a breaking change: imho code for which the behavior changes due to the applied code changes relies on a bug (/broken contract), and the changes attempt to fix this bug. So if you ask me, this could be dealt with as being not a breaking change.

On the one hand, it looks like the added test cases don't cover all methods that got adjusted, on the other hand, just by looking at the code, all adjusted methods should work fine if any of them does.
On the other hand, since I'm not familiar with this part of Extenject yet, I don't know if there are any other parts in the code where the same fix might need to be applied.

I'd be fine with merging this Pull Request as it is right now.

[Mathijs-Bakker]

Merged

Thanks!

[Mathijs-Bakker]

@sbergen Yousician is an awesome music app! Should/can it be added to the apps that use (x)enject list?
You can all things you want to tell about Yousician it here. And it will be added to the list!