From 3dd570fdd0f335854a5f9da3a82f73bf2241ecf8 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 15 Nov 2025 10:42:44 -0600 Subject: [PATCH] address bot review --- esphome/cpp_generator.py | 20 ++++++------ tests/component_tests/text/test_text.py | 4 +-- tests/unit_tests/test_lambda_dedup.py | 41 +++++++++++++++++++------ 3 files changed, 42 insertions(+), 23 deletions(-) diff --git a/esphome/cpp_generator.py b/esphome/cpp_generator.py index fcc0ca2e43..2904cae796 100644 --- a/esphome/cpp_generator.py +++ b/esphome/cpp_generator.py @@ -716,18 +716,17 @@ async def get_variable_with_full_id(id_: ID) -> tuple[ID, "MockObj"]: return await CORE.get_variable_with_full_id(id_) -def _try_deduplicate_lambda(lambda_expr: LambdaExpression) -> str | None: - """Try to deduplicate a lambda expression. +def _get_shared_lambda_name(lambda_expr: LambdaExpression) -> str: + """Get the shared function name for a lambda expression. - If an identical lambda was already generated, returns the name of the - shared function. Otherwise, creates a new shared function and stores it. + If an identical lambda was already generated, returns the existing shared + function name. Otherwise, creates a new shared function and returns its name. Args: - lambda_expr: The lambda expression to potentially deduplicate + lambda_expr: The lambda expression to deduplicate Returns: - The name of the shared function if this lambda should be deduplicated, - None if this is the first occurrence (caller should use original lambda) + The name of the shared function for this lambda (either existing or newly created) """ # Create a unique key from the lambda content, parameters, and return type content = lambda_expr.content @@ -837,10 +836,9 @@ async def process_lambda( lambda_expr = LambdaExpression( parts, parameters, capture, return_type, location ) - func_name = _try_deduplicate_lambda(lambda_expr) - if func_name is not None: - # Return a shared function reference instead of inline lambda - return SharedFunctionLambdaExpression(func_name, parameters, return_type) + func_name = _get_shared_lambda_name(lambda_expr) + # Return a shared function reference instead of inline lambda + return SharedFunctionLambdaExpression(func_name, parameters, return_type) return LambdaExpression(parts, parameters, capture, return_type, location) diff --git a/tests/component_tests/text/test_text.py b/tests/component_tests/text/test_text.py index b2318bd416..ffc0fd780a 100644 --- a/tests/component_tests/text/test_text.py +++ b/tests/component_tests/text/test_text.py @@ -1,4 +1,4 @@ -"""Tests for the binary sensor component.""" +"""Tests for the text component.""" from esphome.core import CORE @@ -58,7 +58,7 @@ def test_text_config_value_mode_set(generate_main): assert "it_3->traits.set_mode(text::TEXT_MODE_PASSWORD);" in main_cpp -def test_text_config_lamda_is_set(generate_main): +def test_text_config_lambda_is_set(generate_main) -> None: """ Test if lambda is set for lambda mode (optimized with stateless lambda and deduplication) """ diff --git a/tests/unit_tests/test_lambda_dedup.py b/tests/unit_tests/test_lambda_dedup.py index ec4c0d29c9..ebc217f308 100644 --- a/tests/unit_tests/test_lambda_dedup.py +++ b/tests/unit_tests/test_lambda_dedup.py @@ -22,8 +22,8 @@ def test_deduplicate_identical_lambdas() -> None: ) # Try to deduplicate them - func_name1 = cg._try_deduplicate_lambda(lambda1) - func_name2 = cg._try_deduplicate_lambda(lambda2) + func_name1 = cg._get_shared_lambda_name(lambda1) + func_name2 = cg._get_shared_lambda_name(lambda2) # Both should get the same function name (deduplication happened) assert func_name1 == func_name2 @@ -46,8 +46,8 @@ def test_different_lambdas_not_deduplicated() -> None: return_type=cg.RawExpression("int"), ) - func_name1 = cg._try_deduplicate_lambda(lambda1) - func_name2 = cg._try_deduplicate_lambda(lambda2) + func_name1 = cg._get_shared_lambda_name(lambda1) + func_name2 = cg._get_shared_lambda_name(lambda2) # Different lambdas should get different function names assert func_name1 != func_name2 @@ -71,8 +71,8 @@ def test_different_return_types_not_deduplicated() -> None: return_type=cg.RawExpression("float"), # Different return type ) - func_name1 = cg._try_deduplicate_lambda(lambda1) - func_name2 = cg._try_deduplicate_lambda(lambda2) + func_name1 = cg._get_shared_lambda_name(lambda1) + func_name2 = cg._get_shared_lambda_name(lambda2) # Different return types = different functions assert func_name1 != func_name2 @@ -94,8 +94,8 @@ def test_different_parameters_not_deduplicated() -> None: return_type=cg.RawExpression("int"), ) - func_name1 = cg._try_deduplicate_lambda(lambda1) - func_name2 = cg._try_deduplicate_lambda(lambda2) + func_name1 = cg._get_shared_lambda_name(lambda1) + func_name2 = cg._get_shared_lambda_name(lambda2) # Different parameters = different functions assert func_name1 != func_name2 @@ -111,7 +111,7 @@ def test_flush_lambda_dedup_declarations() -> None: return_type=cg.RawExpression("int"), ) - cg._try_deduplicate_lambda(lambda1) + cg._get_shared_lambda_name(lambda1) # Check that declaration was stored assert cg._KEY_LAMBDA_DEDUP_DECLARATIONS in CORE.data @@ -154,7 +154,7 @@ def test_lambda_deduplication_counter() -> None: capture="", return_type=cg.RawExpression("int"), ) - func_name = cg._try_deduplicate_lambda(lambda_expr) + func_name = cg._get_shared_lambda_name(lambda_expr) assert func_name == f"shared_lambda_{i}" @@ -171,3 +171,24 @@ def test_lambda_format_body() -> None: assert lambda1.format_body() == "return 42;" # With source would need a proper source object, skip for now + + +def test_stateful_lambdas_not_deduplicated() -> None: + """Test that stateful lambdas (non-empty capture) are not deduplicated.""" + # _get_shared_lambda_name is only called for stateless lambdas (capture == "") + # Stateful lambdas bypass deduplication entirely in process_lambda + + # Verify that a stateful lambda would NOT get deduplicated + # by checking it's not in the stateless dedup cache + stateful_lambda = cg.LambdaExpression( + parts=["return x + y;"], + parameters=[], + capture="=", # Non-empty capture means stateful + return_type=cg.RawExpression("int"), + ) + + # Stateful lambdas should NOT be passed to _get_shared_lambda_name + # This is enforced by the `if capture == ""` check in process_lambda + # We verify the lambda has a non-empty capture + assert stateful_lambda.capture != "" + assert stateful_lambda.capture == "="