refactor: Do tag input validation with __code__ variables if available (#945)

This commit is contained in:
Juro Oravec 2025-02-02 21:47:34 +01:00 committed by GitHub
parent f52f12579b
commit c7aba40252
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 526 additions and 26 deletions

View file

@ -178,8 +178,9 @@ class NodeMeta(type):
# But cause we stripped the two parameters, then the error will be:
# `render() takes from 1 positional arguments but 2 were given`
args, kwargs = validate_params(
self.tag,
orig_render,
validation_signature,
self.tag,
resolved_params_without_invalid_kwargs,
invalid_kwargs,
)

View file

@ -18,30 +18,28 @@ from django_components.util.tag_parser import TagAttr, parse_tag
# For details see https://github.com/django-components/django-components/pull/902#discussion_r1913611633
# and following comments
def validate_params(
func: Callable[..., Any],
validation_signature: inspect.Signature,
tag: str,
signature: inspect.Signature,
params: List["TagParam"],
extra_kwargs: Optional[Dict[str, Any]] = None,
) -> Tuple[Tuple[Any, ...], Dict[str, Any]]:
"""
Validates a list of TagParam objects against this tag's function signature.
The validation preserves the order of parameters as they appeared in the template.
Raises `TypeError` if the parameters don't match tfuncsignature.
Raises `TypeError` if the parameters don't match the tag's signature.
We have to have a custom validation, because if we simply spread all args and kwargs,
into `BaseNode.render()`, then we won't be able to detect duplicate kwargs or other
errors.
"""
# Create a function that uses the given signature
def validator(*args: Any, **kwargs: Any) -> Tuple[Tuple[Any, ...], Dict[str, Any]]:
return args, kwargs
validator.__signature__ = signature # type: ignore[attr-defined]
# Call the validator with our args and kwargs in the same order as they appeared
# in the template, to let the Python interpreter validate on repeated kwargs.
supports_code_objects = func is not None and hasattr(func, "__code__") and hasattr(func.__code__, "co_varnames")
try:
# Returns args, kwargs
return apply_params_in_original_order(validator, params, extra_kwargs)
if supports_code_objects:
args, kwargs = _validate_params_with_code(func, params, extra_kwargs)
else:
args, kwargs = _validate_params_with_signature(validation_signature, params, extra_kwargs)
return args, kwargs
except TypeError as e:
raise TypeError(f"Invalid parameters for tag '{tag}': {str(e)}") from None
@ -227,8 +225,8 @@ def merge_repeated_kwargs(params: List[TagParam]) -> List[TagParam]:
return resolved_params
def apply_params_in_original_order(
fn: Callable[..., Any],
def _validate_params_with_signature(
signature: inspect.Signature,
params: List[TagParam],
extra_kwargs: Optional[Dict[str, Any]] = None,
) -> Any:
@ -258,8 +256,6 @@ def apply_params_in_original_order(
Returns the result of calling fn with the validated parameters
"""
signature = inspect.signature(fn)
# Track state as we process parameters
seen_kwargs = False # To detect positional args after kwargs
used_param_names = set() # To detect duplicate kwargs
@ -294,8 +290,8 @@ def apply_params_in_original_order(
# Process parameters in their original order
for param in params:
# This is a positional argument
if param.key is None:
# This is a positional argument
if seen_kwargs:
raise TypeError("positional argument follows keyword argument")
@ -323,20 +319,141 @@ def apply_params_in_original_order(
if param.key in used_param_names:
raise TypeError(f"got multiple values for argument '{param.key}'")
# Only validate kwarg names if the function doesn't accept **kwargs
# Validate kwarg names if the function doesn't accept **kwargs
if not has_var_keyword and param.key not in valid_params:
raise TypeError(f"got an unexpected keyword argument '{param.key}'")
validated_kwargs[param.key] = param.value
used_param_names.add(param.key)
# Add any extra kwargs - These are allowed only if the function accepts **kwargs
if extra_kwargs:
if not has_var_keyword:
first_key = next(iter(extra_kwargs))
raise TypeError(f"got an unexpected keyword argument '{first_key}'")
validated_kwargs.update(extra_kwargs)
# Check for missing required arguments and apply defaults
for param_name, signature_param in params_by_name.items():
if param_name in used_param_names or param_name in validated_kwargs:
continue
if signature_param.kind in (inspect.Parameter.POSITIONAL_ONLY, inspect.Parameter.POSITIONAL_OR_KEYWORD):
if signature_param.default == inspect.Parameter.empty:
raise TypeError(f"missing a required argument: '{param_name}'")
elif len(validated_args) <= next_positional_index:
validated_kwargs[param_name] = signature_param.default
elif signature_param.kind == inspect.Parameter.KEYWORD_ONLY:
if signature_param.default == inspect.Parameter.empty:
raise TypeError(f"missing a required argument: '{param_name}'")
else:
validated_kwargs[param_name] = signature_param.default
# Return args and kwargs
return validated_args, validated_kwargs
def _validate_params_with_code(
fn: Callable[..., Any],
params: List["TagParam"],
extra_kwargs: Optional[Dict[str, Any]] = None,
) -> Tuple[Tuple[Any, ...], Dict[str, Any]]:
"""
Validate and process function parameters using __code__ attributes for better performance.
This is the preferred implementation when the necessary attributes are available.
This implementation is about 3x faster than signature-based validation.
For context, see https://github.com/django-components/django-components/issues/935
"""
code = fn.__code__
defaults = fn.__defaults__ or ()
kwdefaults = getattr(fn, "__kwdefaults__", None) or {}
# Get parameter information from code object
param_names = code.co_varnames[: code.co_argcount + code.co_kwonlyargcount]
positional_count = code.co_argcount
kwonly_count = code.co_kwonlyargcount
has_var_positional = bool(code.co_flags & 0x04) # CO_VARARGS
has_var_keyword = bool(code.co_flags & 0x08) # CO_VARKEYWORDS
# Skip self and context parameters
skip_params = 2
param_names = param_names[skip_params:]
positional_count = max(0, positional_count - skip_params)
# Calculate required counts
num_defaults = len(defaults)
required_positional = positional_count - num_defaults
# Track state
seen_kwargs = False
used_param_names = set()
validated_args = []
validated_kwargs = {}
next_positional_index = 0
# Process parameters in order
for param in params:
if param.key is None:
# This is a positional argument
if seen_kwargs:
raise TypeError("positional argument follows keyword argument")
# Check position limit for non-variadic functions
if not has_var_positional and next_positional_index >= positional_count:
if positional_count == 0:
raise TypeError("takes 0 positional arguments but 1 was given")
raise TypeError(f"takes {positional_count} positional argument(s) but more were given")
# For non-variadic arguments, get parameter name
if next_positional_index < positional_count:
param_name = param_names[next_positional_index]
if param_name in used_param_names:
raise TypeError(f"got multiple values for argument '{param_name}'")
used_param_names.add(param_name)
validated_args.append(param.value)
next_positional_index += 1
else:
# This is a keyword argument
seen_kwargs = True
# Check for duplicate kwargs
if param.key in used_param_names:
raise TypeError(f"got multiple values for argument '{param.key}'")
# Validate kwarg names
is_valid_kwarg = param.key in param_names[: positional_count + kwonly_count] or ( # Regular param
has_var_keyword and param.key not in param_names
) # **kwargs param
if not is_valid_kwarg:
raise TypeError(f"got an unexpected keyword argument '{param.key}'")
validated_kwargs[param.key] = param.value
used_param_names.add(param.key)
# Add any extra kwargs
if extra_kwargs:
if not has_var_keyword:
first_key = next(iter(extra_kwargs))
raise TypeError(f"got an unexpected keyword argument '{first_key}'")
validated_kwargs.update(extra_kwargs)
# Final validation using signature.bind()
bound_args = signature.bind(*validated_args, **validated_kwargs)
bound_args.apply_defaults()
# Check for missing required arguments and apply defaults
for i, param_name in enumerate(param_names):
if param_name in used_param_names or param_name in validated_kwargs:
continue
# Actually call the function with validated parameters
return fn(*bound_args.args, **bound_args.kwargs)
if i < positional_count: # Positional parameter
if i < required_positional:
raise TypeError(f"missing a required argument: '{param_name}'")
elif len(validated_args) <= i:
default_index = i - required_positional
validated_kwargs[param_name] = defaults[default_index]
elif i < positional_count + kwonly_count: # Keyword-only parameter
if param_name not in kwdefaults:
raise TypeError(f"missing a required argument: '{param_name}'")
else:
validated_kwargs[param_name] = kwdefaults[param_name]
return tuple(validated_args), validated_kwargs

View file

@ -1,3 +1,5 @@
import inspect
from django.template import Context, Template
from django.template.exceptions import TemplateSyntaxError
@ -676,3 +678,383 @@ class DecoratorTests(BaseTestCase):
template2.render(Context({}))
render._node.unregister(component_tags.register) # type: ignore[attr-defined]
def force_signature_validation(fn):
"""
Creates a proxy around a function that makes __code__ inaccessible,
forcing the use of signature-based validation.
"""
class SignatureOnlyFunction:
def __init__(self, fn):
self.__wrapped__ = fn
self.__signature__ = inspect.signature(fn)
def __call__(self, *args, **kwargs):
return self.__wrapped__(*args, **kwargs)
def __getattr__(self, name):
# Return None for __code__ to force signature-based validation
if name == "__code__":
return None
# For all other attributes, delegate to the wrapped function
return getattr(self.__wrapped__, name)
return SignatureOnlyFunction(fn)
class SignatureBasedValidationTests(BaseTestCase):
# Test that the template tag can be used within the template under the registered tag
def test_node_class_tags(self):
class TestNode(BaseNode):
tag = "mytag"
end_tag = "endmytag"
@force_signature_validation
def render(self, context: Context, name: str, **kwargs) -> str:
return f"Hello, {name}!"
TestNode.register(component_tags.register)
# Works with end tag and self-closing
template_str: types.django_html = """
{% load component_tags %}
{% mytag 'John' %}
{% endmytag %}
Shorthand: {% mytag 'Mary' / %}
"""
template = Template(template_str)
rendered = template.render(Context({}))
self.assertEqual(rendered.strip(), "Hello, John!\n Shorthand: Hello, Mary!")
# But raises if missing end tag
template_str2: types.django_html = """
{% load component_tags %}
{% mytag 'John' %}
"""
with self.assertRaisesMessage(TemplateSyntaxError, "Unclosed tag on line 3: 'mytag'"):
Template(template_str2)
TestNode.unregister(component_tags.register)
def test_node_class_no_end_tag(self):
class TestNode(BaseNode):
tag = "mytag"
@force_signature_validation
def render(self, context: Context, name: str, **kwargs) -> str:
return f"Hello, {name}!"
TestNode.register(component_tags.register)
# Raises with end tag or self-closing
template_str: types.django_html = """
{% load component_tags %}
{% mytag 'John' %}
{% endmytag %}
Shorthand: {% mytag 'Mary' / %}
"""
with self.assertRaisesMessage(TemplateSyntaxError, "Invalid block tag on line 4: 'endmytag'"):
Template(template_str)
# Works when missing end tag
template_str2: types.django_html = """
{% load component_tags %}
{% mytag 'John' %}
"""
template2 = Template(template_str2)
rendered2 = template2.render(Context({}))
self.assertEqual(rendered2.strip(), "Hello, John!")
TestNode.unregister(component_tags.register)
def test_node_class_flags(self):
captured = None
class TestNode(BaseNode):
tag = "mytag"
end_tag = "endmytag"
allowed_flags = ["required", "default"]
@force_signature_validation
def render(self, context: Context, name: str, **kwargs) -> str:
nonlocal captured
captured = self.allowed_flags, self.flags, self.active_flags
return f"Hello, {name}!"
TestNode.register(component_tags.register)
template_str = """
{% load component_tags %}
{% mytag 'John' required / %}
"""
template = Template(template_str)
template.render(Context({}))
allowed_flags, flags, active_flags = captured # type: ignore
self.assertEqual(allowed_flags, ["required", "default"])
self.assertEqual(flags, {"required": True, "default": False})
self.assertEqual(active_flags, ["required"])
TestNode.unregister(component_tags.register)
def test_node_render(self):
# Check that the render function is called with the context
captured = None
class TestNode(BaseNode):
tag = "mytag"
@force_signature_validation
def render(self, context: Context) -> str:
nonlocal captured
captured = context.flatten()
return f"Hello, {context['name']}!"
TestNode.register(component_tags.register)
template_str = """
{% load component_tags %}
{% mytag / %}
"""
template = Template(template_str)
rendered = template.render(Context({"name": "John"}))
self.assertEqual(captured, {"False": False, "None": None, "True": True, "name": "John"})
self.assertEqual(rendered.strip(), "Hello, John!")
TestNode.unregister(component_tags.register)
def test_node_render_raises_if_no_context_arg(self):
with self.assertRaisesMessage(TypeError, "`render()` method of TestNode must have at least two parameters"):
class TestNode(BaseNode):
tag = "mytag"
def render(self) -> str: # type: ignore
return ""
def test_node_render_accepted_params_set_by_render_signature(self):
captured = None
class TestNode1(BaseNode):
tag = "mytag"
allowed_flags = ["required", "default"]
@force_signature_validation
def render(self, context: Context, name: str, count: int = 1, *, msg: str, mode: str = "default") -> str:
nonlocal captured
captured = name, count, msg, mode
return ""
TestNode1.register(component_tags.register)
# Set only required params
template1 = Template(
"""
{% load component_tags %}
{% mytag 'John' msg='Hello' required %}
"""
)
template1.render(Context({}))
self.assertEqual(captured, ("John", 1, "Hello", "default"))
# Set all params
template2 = Template(
"""
{% load component_tags %}
{% mytag 'John2' count=2 msg='Hello' mode='custom' required %}
"""
)
template2.render(Context({}))
self.assertEqual(captured, ("John2", 2, "Hello", "custom"))
# Set no params
template3 = Template(
"""
{% load component_tags %}
{% mytag %}
"""
)
with self.assertRaisesMessage(
TypeError, "Invalid parameters for tag 'mytag': missing a required argument: 'name'"
):
template3.render(Context({}))
# Omit required arg
template4 = Template(
"""
{% load component_tags %}
{% mytag msg='Hello' %}
"""
)
with self.assertRaisesMessage(
TypeError, "Invalid parameters for tag 'mytag': missing a required argument: 'name'"
):
template4.render(Context({}))
# Omit required kwarg
template5 = Template(
"""
{% load component_tags %}
{% mytag name='John' %}
"""
)
with self.assertRaisesMessage(
TypeError, "Invalid parameters for tag 'mytag': missing a required argument: 'msg'"
):
template5.render(Context({}))
# Extra args
template6 = Template(
"""
{% load component_tags %}
{% mytag 123 count=1 name='John' %}
"""
)
with self.assertRaisesMessage(
TypeError, "Invalid parameters for tag 'mytag': got multiple values for argument 'name'"
):
template6.render(Context({}))
# Extra args after kwargs
template6 = Template(
"""
{% load component_tags %}
{% mytag count=1 name='John' 123 %}
"""
)
with self.assertRaisesMessage(TypeError, "positional argument follows keyword argument"):
template6.render(Context({}))
# Extra kwargs
template7 = Template(
"""
{% load component_tags %}
{% mytag 'John' msg='Hello' mode='custom' var=123 %}
"""
)
with self.assertRaisesMessage(
TypeError, "Invalid parameters for tag 'mytag': got an unexpected keyword argument 'var'"
):
template7.render(Context({}))
# Extra kwargs - non-identifier or kwargs
template8 = Template(
"""
{% load component_tags %}
{% mytag 'John' msg='Hello' mode='custom' data-id=123 class="pa-4" @click.once="myVar" %}
"""
)
with self.assertRaisesMessage(
TypeError, "Invalid parameters for tag 'mytag': got an unexpected keyword argument 'data-id'"
):
template8.render(Context({}))
# Extra arg after special kwargs
template9 = Template(
"""
{% load component_tags %}
{% mytag data-id=123 'John' msg='Hello' %}
"""
)
with self.assertRaisesMessage(SyntaxError, "positional argument follows keyword argument"):
template9.render(Context({}))
TestNode1.unregister(component_tags.register)
def test_node_render_extra_args_and_kwargs(self):
captured = None
class TestNode1(BaseNode):
tag = "mytag"
allowed_flags = ["required", "default"]
@force_signature_validation
def render(self, context: Context, name: str, *args, msg: str, **kwargs) -> str:
nonlocal captured
captured = name, args, msg, kwargs
return ""
TestNode1.register(component_tags.register)
template1 = Template(
"""
{% load component_tags %}
{% mytag 'John'
123 456 789 msg='Hello' a=1 b=2 c=3 required
data-id=123 class="pa-4" @click.once="myVar"
%}
"""
)
template1.render(Context({}))
self.assertEqual(
captured,
(
"John",
(123, 456, 789),
"Hello",
{"a": 1, "b": 2, "c": 3, "data-id": 123, "class": "pa-4", "@click.once": "myVar"},
),
)
TestNode1.unregister(component_tags.register)
def test_node_render_kwargs_only(self):
captured = None
class TestNode(BaseNode):
tag = "mytag"
@force_signature_validation
def render(self, context: Context, **kwargs) -> str:
nonlocal captured
captured = kwargs
return ""
TestNode.register(component_tags.register)
# Test with various kwargs including non-identifier keys
template = Template(
"""
{% load component_tags %}
{% mytag
name='John'
age=25
data-id=123
class="header"
@click="handleClick"
v-if="isVisible"
%}
"""
)
template.render(Context({}))
# All kwargs should be accepted since the function accepts **kwargs
self.assertEqual(
captured,
{
"name": "John",
"age": 25,
"data-id": 123,
"class": "header",
"@click": "handleClick",
"v-if": "isVisible",
},
)
# Test with positional args (should fail since function only accepts kwargs)
template2 = Template(
"""
{% load component_tags %}
{% mytag "John" name="Mary" %}
"""
)
with self.assertRaisesMessage(
TypeError, "Invalid parameters for tag 'mytag': takes 0 positional arguments but 1 was given"
):
template2.render(Context({}))
TestNode.unregister(component_tags.register)