fix: defer validation for unresolved conditional branches

[Stefanqn]

No description provided.

[Tishka17]

Actually, I would check as much as possible at graph building stage. Probably, we should have a toggle to enable this behavior

[Stefanqn]

both is fine for me

[Tishka17]

Looks like we have 2 options here:

1. validate disabled
2. validate dynamically calculated

[Stefanqn]

like that?

[Stefanqn]

I also added a toggle per factory in my latest commit

Should alias(...) and from_context(...) support the same override as well?

[Tishka17]

Isn't per factory setting too much? Do we have any UseCase for that?

[Stefanqn]

if it's too much, drop the last commit :)
I think it's kinda useful if you'd like to keep strict / fail-fast behavior and want to explicitly mark or patch a certain case.

[Stefanqn]

Can I accelerate merging any kind of bugfix for when=Has()? I'm really looking forward to use this feature

[Tishka17]

Do you have any particular usecase for this specific PR? I am not sure if I am satisfied with the API changes here though it has some sense here. Do you need this together with #696 at this moment?

[Tishka17]

In terms of configuring validation is see these cases:

1. Factory is overridden
2. Factory is just disabled
3. Factory is disabled by static evaluation
4. Factory can be dynamically disabled
5. Factory is enable by static evaluation
6. Factory is always enabled

For me it is not clear how should we let user decide whether to skip checks. In #696 I went with simple logic: cases 1,2,3 disable validation, others are always validated.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[Stefanqn]

#696 still fails the valid use case of 1 "a" False in

@pytest.mark.parametrize( ("number", "expected", "raises"), [
    (1, "a", False),
    (0, None, True),
])
def test_unresolved_conditional_branch_is_validated_at_runtime(
        *,
        number: int,
        expected: str | None,
        raises: bool,
):
    provider = Provider(scope=Scope.APP)
    provider.activate(is_zero, Marker("ZERO"))
    provider.provide(lambda: number, provides=int)
    provider.provide(fallback, provides=str)
    provider.provide(needs_float, provides=str, when=Marker("ZERO"))
    container = make_container(provider)
    if raises:
        with pytest.raises(NoFactoryError):
            container.get(str)
    else:
        assert container.get(str) == expected

I see the validation as a guardrail. If we don't lower the bar, so that arbitrary activation logic isn't validated, false positives seem inevitable (I doubt we can properly validate arbitrary activation logic). Because false positives can block legit code, the only option I see is to provide a toggle to disable validation in that case.

Please inspect following tests in my PR:

• test_validate_unconditional_when_setting
• test_validate_unconditional_when_factory_override

[Tishka17]

    provider.provide(lambda: number, provides=int)

I do not see this as a good way for providing constants. We should either recommend using from_context here or implement something like provide_instance.

For arbitrary factories it is not possible if they produce the same value every time. We allow calling container.close() and still using container afterwards, som factory may be called multiple times and graph should be still valid with new value.

[Stefanqn]

    provider.provide(lambda: number, provides=int)

I do not see this as a good way for providing constants. We should either recommend using from_context here or implement something like provide_instance.

I agree, but personally I think from_context is worse for providing instances, because it would further split the implementation logic.

For arbitrary factories it is not possible if they produce the same value every time. We allow calling container.close() and still using container afterwards, som factory may be called multiple times and graph should be still valid with new value.

provide_instance would mitigate the problem, yet the use-case to disable validation still exists.

[Tishka17]

Another possible option: allow specific factories to be called on static evaluation step.