Support overriding param types for rule code

[thejcannon]

The main use-case is runtime-typed parameters (for use in "helper"/"inner" rules).

def make_rule(sometype: type[BaseType]):
    @rule(_param_type_overrides={"request": sometype.var_type})
    async def helper_rule(request: WhateverVarTypeShouldBe) -> str:
        ...

    return helper_rule

[ci skip-rust]
[ci skip-build-wheels]

[Eric-Arellano]

Sg, given that this is very niche

[thejcannon]

Will wait for another maintainers review since this is a "wide" change

[kaos]

This looks safe to me, as if you don't use this feature it should do the same as before.

Only thing I perhaps don't understand is the value of the overriden type.

[kaos]

I think this bypasses the type check performed by _ensure_type_annotation, maybe check that in the previous for loop?

for parameter, value in param_type_overrides.items():
  ...

[thejcannon]

@kaos by "value" do you mean the benefit, or the concrete value type of the mapping?

[kaos]

@kaos by "value" do you mean the benefit, or the concrete value type of the mapping?

the concrete type I think. Also I'm not sure I've come across the need for the benefit, but my mind is crazy enough to appreciate the flexibility up-front and find the use for it later :P (I can imagine there's one when you're trying to implement PluginOption and the like)

[thejcannon]

I can give two examples:

1. Most lint partitioners look like this. I want to provide a generic "rule_maker" for that however the Subsystem param can't be known statically and is only known at runtime. So I'd use @rule(param_type_overrides({"subsystem": cls.subsystem_type}) def default_rule_impl(request: ..., subsystem: BaseSubsystemType): ...
2. See Helper to make linters #16412 (comment)

[stuhood]

1. Most lint partitioners look like this. I want to provide a generic "rule_maker" for that however the Subsystem param can't be known statically and is only known at runtime. So I'd use @rule(param_type_overrides({"subsystem": cls.subsystem_type}) def default_rule_impl(request: ..., subsystem: BaseSubsystemType): ...

Is this effectively using a @union type in an argument position? You'd declare the @rule once with a @union as a positional argument, and we'd install a copy of the @rule per @union member?

[thejcannon]

1. Most lint partitioners look like this. I want to provide a generic "rule_maker" for that however the Subsystem param can't be known statically and is only known at runtime. So I'd use @rule(param_type_overrides({"subsystem": cls.subsystem_type}) def default_rule_impl(request: ..., subsystem: BaseSubsystemType): ...

Is this effectively using a @union type in an argument position? You'd declare the @rule once with a @union as a positional argument, and we'd install a copy of the @rule per @union member?

Bingo! Although it'd be a subset of @union members, as requested by plugin authors.

[stuhood]

Bingo! Although it'd be a subset of @union members, as requested by plugin authors.

It could be a separate @union probably: LinterWhichUsesADefaultPartitioner?

Is that worth sketching out as an alternative? It should be easy to implement: similar to this line (which installs a Get per @union member), if we found a @union in the positional arguments a few lines up, we'd install multiple copies of the @rule.

[thejcannon]

I think this is much more general and likely easier to grok? See also use-case 2.

[stuhood]

See also use-case 2.

Use case 2 also appears to be a @union use case, but maybe I'm misreading it.

I think this is much more general and likely easier to grok?

Possibly... note that this is only actually type safe if the substituted parameter is a subclass of the actual declared type. It might be worth validating that?

[thejcannon]

Possibly... note that this is only actually type safe if the substituted parameter is a subclass of the actual declared type. It might be worth validating that?

That's definitely the sane way to think about it. However, much like @unions themselves, we just make assumptions about the member types and hope that our @union's (protocol) "interface" is implemented by the members.

It's not any more-or-less typesafe than, say:

if TYPE_CHECKING:
    MyType = int
else:
    MyType = str

@rule
async def (request: MyType): ...

I'm not against it, but we're forcing our hand on some inheritance (which #16926 is particularly relevant as it will be the first "type" we do this substitution on)

[thejcannon]

@stuhood would you prefer I prefix with underscore so we can explore it further in Pants repo (and feel comfortable removing it possibly later?)

[stuhood]

@stuhood would you prefer I prefix with underscore so we can explore it further in Pants repo (and feel comfortable removing it possibly later?)

Yes. Please leave a comment explaining the connection to @unions and subtyping.