From 8aa77f89ce1ef56c98d989b03bc35fa4c6289b8a Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sun, 20 Sep 2026 17:33:05 +0200 Subject: [PATCH] fix(extract): a decorator with arguments but no path is not a route Distilled from #1245 by Andrew Hundt (a slice of d15071c9), with a different rule than upstream chose. Python's unittest.mock is used as a decorator: @patch("subprocess.run") def test_runs(self, run): ... decorator_method_name maps a bare `patch` to the HTTP method PATCH, and try_route_from_decorator_call accepts a receiver-less decorator call. When extract_route_path_from_args then finds nothing path-shaped, the function falls back to "/". So every mock-decorated test function became a Route handler, PATCH "/", and every one of those was wrong. Upstream fixed it with a has_receiver heuristic. That still admits @mock.patch("os.getcwd") (it has a receiver) and would reject the bare framework decorators Litestar and BlackSheep use, @get("/x"). The rule here is about the arguments, not the callee: a decorator call that HAS arguments but none of them path-shaped is not a route. The "/" default survives only for a zero-argument call such as @app.route(), which is the one case where "/" is what the framework means. No test on main relied on the default for a call with non-path arguments; checked before changing it. RED before the fix: extract_python_mock_patch_is_not_route FAIL tests/test_extraction.c:4023: mocked->route_path is not NULL extract_python_bare_decorator_route_rules FAIL tests/test_extraction.c:4063: mocked->route_path is not NULL The second test also pins the two shapes that must keep working: bare @get("/health") still yields a GET route, and zero-argument @app.route() still yields "/" ANY -- those assertions passed before the fix and after. GREEN after: extraction 351 passed; extraction pipeline edge_types_probe route_canon cross_repo infrascan lang_contract 754 passed, with the CALLS-breadth contract at 54 languages / 0 failures. The single remaining red in both runs is extract_spill_round_trip_keeps_every_field refusing to spill onto a disk below its 10 GB floor -- this host, not this change. Co-authored-by: Andrew Hundt Signed-off-by: Martin Vogel --- internal/cbm/extract_defs.c | 9 ++++- tests/test_extraction.c | 73 +++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/internal/cbm/extract_defs.c b/internal/cbm/extract_defs.c index 93da084be..eb32ec2a6 100644 --- a/internal/cbm/extract_defs.c +++ b/internal/cbm/extract_defs.c @@ -1591,7 +1591,11 @@ static bool try_drf_action_decorator(CBMArena *a, TSNode dchild, const char *sou } // Try to extract a route from a single decorator call node. -// Returns true if a route method was found (even with fallback path "/"). +// Returns true if a route method was found. The fallback path "/" applies only +// to a zero-argument call (`@app.route()`): a call that HAS arguments but none +// path-shaped is not a route -- unittest.mock's `@patch("subprocess.run")` +// shares its name with the HTTP verb and used to mint a PATCH "/" handler for +// every mocked test function (distilled from PR #1245). static bool try_route_from_decorator_call(CBMArena *a, TSNode dchild, const char *source, const char **out_path, const char **out_method) { TSNode fn = ts_node_child_by_field_name(dchild, TS_FIELD("function")); @@ -1616,6 +1620,9 @@ static bool try_route_from_decorator_call(CBMArena *a, TSNode dchild, const char *out_method = method; return true; } + if (ts_node_named_child_count(args) > 0) { + return false; + } } *out_path = "/"; *out_method = method; diff --git a/tests/test_extraction.c b/tests/test_extraction.c index a61083755..8b10c6448 100644 --- a/tests/test_extraction.c +++ b/tests/test_extraction.c @@ -4001,6 +4001,77 @@ TEST(extract_java_method_annotations_issue382) { PASS(); } +/* Distilled from PR #1245 (Andrew Hundt): unittest.mock's `@patch("x")` is a + * bare decorator whose name collides with the HTTP verb, and its only argument + * is a dotted target, not a path. The "/" fallback then minted a Route handler + * PATCH "/" for every mocked test function. Rule: a decorator call WITH + * arguments but no path-shaped one is not a route. */ +TEST(extract_python_mock_patch_is_not_route) { + CBMFileResult *r = extract("from unittest.mock import patch\n\n" + "@patch(\"subprocess.run\")\n" + "def test_cmd(mock_run):\n" + " pass\n\n" + "@app.patch(\"/items/{id}\")\n" + "def update_item():\n" + " pass\n", + CBM_LANG_PYTHON, "t", "test_routes.py"); + ASSERT_NOT_NULL(r); + ASSERT_FALSE(r->has_error); + + const CBMDefinition *mocked = find_def_by_name(r, "test_cmd"); + ASSERT_NOT_NULL(mocked); + ASSERT_NULL(mocked->route_path); + ASSERT_NULL(mocked->route_method); + + const CBMDefinition *route = find_def_by_name(r, "update_item"); + ASSERT_NOT_NULL(route); + ASSERT_STR_EQ(route->route_path, "/items/{id}"); + ASSERT_STR_EQ(route->route_method, "PATCH"); + + cbm_free_result(r); + PASS(); +} + +/* Companion to the mock-patch case: the narrowing must not cost real routes. + * A receiver-less framework decorator with a path-shaped argument (Litestar / + * BlackSheep `@get("/x")`) stays a route, `@mock.patch("x")` (receiver, but + * no path) is not one, and a zero-argument `@app.route()` keeps the "/" + * default main emits today. */ +TEST(extract_python_bare_decorator_route_rules) { + CBMFileResult *r = extract("from litestar import get\n" + "from unittest import mock\n\n" + "@get(\"/health\")\n" + "def health():\n" + " pass\n\n" + "@mock.patch(\"os.getcwd\")\n" + "def test_cwd(mock_cwd):\n" + " pass\n\n" + "@app.route()\n" + "def index():\n" + " pass\n", + CBM_LANG_PYTHON, "t", "routes.py"); + ASSERT_NOT_NULL(r); + ASSERT_FALSE(r->has_error); + + const CBMDefinition *health = find_def_by_name(r, "health"); + ASSERT_NOT_NULL(health); + ASSERT_STR_EQ(health->route_path, "/health"); + ASSERT_STR_EQ(health->route_method, "GET"); + + const CBMDefinition *mocked = find_def_by_name(r, "test_cwd"); + ASSERT_NOT_NULL(mocked); + ASSERT_NULL(mocked->route_path); + ASSERT_NULL(mocked->route_method); + + const CBMDefinition *index = find_def_by_name(r, "index"); + ASSERT_NOT_NULL(index); + ASSERT_STR_EQ(index->route_path, "/"); + ASSERT_STR_EQ(index->route_method, "ANY"); + + cbm_free_result(r); + PASS(); +} + /* ── ArkTS (HarmonyOS .ets) ─────────────────────────────────────── */ TEST(arkts_component_struct) { @@ -7926,6 +7997,8 @@ SUITE(extraction) { RUN_TEST(js_index_module_qn_not_collide_with_folder); RUN_TEST(python_regular_module_qn_unchanged); RUN_TEST(extract_java_method_annotations_issue382); + RUN_TEST(extract_python_mock_patch_is_not_route); + RUN_TEST(extract_python_bare_decorator_route_rules); RUN_TEST(arkts_component_struct); RUN_TEST(arkts_exported_struct_decorators); RUN_TEST(arkts_member_decorators);