Skip to content

Commit 8e14914

Browse files
authored
Merge pull request #1938 from codeflash-ai/fix/exclude-property-getters-from-discovery
Fix: Exclude property getters/setters from function discovery
2 parents 2d2bdc7 + 83bd04e commit 8e14914

2 files changed

Lines changed: 147 additions & 0 deletions

File tree

codeflash/languages/javascript/treesitter.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,18 @@ def _walk_tree_for_functions(
290290
if func_info.is_method and node.parent and node.parent.type == "object":
291291
should_include = False
292292

293+
# Skip property getters/setters (e.g., get: function foo() {})
294+
# These are defined inside Object.defineProperty or object literals
295+
# and cannot be called directly - they're accessed via property names.
296+
# Tests would fail trying to call obj.getterFuncName() instead of obj.propertyName
297+
if node.type == "function_expression" and node.parent and node.parent.type == "pair":
298+
# Check if this is a getter or setter by looking at the property name
299+
property_name_node = node.parent.child_by_field_name("key")
300+
if property_name_node:
301+
property_name = self.get_node_text(property_name_node, source_bytes)
302+
if property_name in ("get", "set"):
303+
should_include = False
304+
293305
if should_include:
294306
functions.append(func_info)
295307

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
"""Test that property getters are excluded from optimization.
2+
3+
Property getters defined via Object.defineProperty should not be
4+
optimized because they're not directly callable and tests cannot
5+
access them by the function name.
6+
7+
Relates to bug: Generated tests try to call getters directly
8+
(e.g., `obj.getterFunc()`) when they should access the property
9+
(e.g., `obj.propertyName`).
10+
"""
11+
12+
from pathlib import Path
13+
14+
from codeflash.discovery.functions_to_optimize import find_all_functions_in_file
15+
16+
17+
class TestPropertyGetterExclusion:
18+
"""Tests for excluding property getters from function discovery."""
19+
20+
def test_object_define_property_getter_excluded(self, tmp_path: Path) -> None:
21+
"""Test that functions used as property getters are excluded.
22+
23+
When a function is defined as `get: function foo() {...}` inside
24+
Object.defineProperty, it should not be discovered as an optimizable
25+
function because:
26+
1. It's not directly accessible by the function name
27+
2. Generated tests would fail trying to call it directly
28+
3. Property access patterns are different from function calls
29+
30+
This reproduces the Express.js pattern where getrouter is defined
31+
as a property getter inside the init function.
32+
"""
33+
js_file = tmp_path / "app.js"
34+
js_file.write_text("""
35+
const app = {};
36+
37+
// Express pattern: getter nested inside a function
38+
app.init = function init() {
39+
var router = null;
40+
41+
// Property getter pattern (like express application.js line 72)
42+
Object.defineProperty(this, 'router', {
43+
configurable: true,
44+
get: function getrouter() {
45+
if (router === null) {
46+
router = { use: () => {} };
47+
}
48+
return router;
49+
}
50+
});
51+
};
52+
53+
// Normal exported function (should be found)
54+
export function normalFunction() {
55+
return 42;
56+
}
57+
58+
module.exports = app;
59+
""")
60+
61+
functions = find_all_functions_in_file(js_file)
62+
function_names = {fn.function_name for fn in functions.get(js_file, [])}
63+
64+
# Property getter should NOT be found
65+
assert "getrouter" not in function_names, (
66+
"Property getter 'getrouter' should be excluded from optimization. "
67+
"Tests cannot access it as init.getrouter() - they would need to access "
68+
"the 'router' property via an instance instead."
69+
)
70+
71+
# Normal exported function should be found
72+
assert "normalFunction" in function_names
73+
74+
def test_object_define_property_setter_excluded(self, tmp_path: Path) -> None:
75+
"""Test that functions used as property setters are also excluded."""
76+
js_file = tmp_path / "app.js"
77+
js_file.write_text("""
78+
const app = {};
79+
80+
Object.defineProperty(app, 'value', {
81+
set: function setvalue(val) {
82+
this._value = val;
83+
},
84+
get: function getvalue() {
85+
return this._value;
86+
}
87+
});
88+
89+
export function helper() {
90+
return 1;
91+
}
92+
""")
93+
94+
functions = find_all_functions_in_file(js_file)
95+
function_names = {fn.function_name for fn in functions.get(js_file, [])}
96+
97+
# Neither getter nor setter should be found
98+
assert "setvalue" not in function_names
99+
assert "getvalue" not in function_names
100+
101+
# Helper function should still be found
102+
assert "helper" in function_names
103+
104+
def test_object_literal_getter_excluded(self, tmp_path: Path) -> None:
105+
"""Test that getter methods in object literals are excluded."""
106+
js_file = tmp_path / "obj.js"
107+
js_file.write_text("""
108+
const obj = {
109+
get router() {
110+
return this._router;
111+
},
112+
113+
// Regular method should be excluded too (it's in an object literal)
114+
method() {
115+
return 1;
116+
}
117+
};
118+
119+
export function exported() {
120+
return obj;
121+
}
122+
""")
123+
124+
functions = find_all_functions_in_file(js_file)
125+
function_names = {fn.function_name for fn in functions.get(js_file, [])}
126+
127+
# Getter in object literal should not be found
128+
assert "router" not in function_names
129+
130+
# Regular method in object literal should also not be found
131+
# (per existing code logic)
132+
assert "method" not in function_names
133+
134+
# Exported function should be found
135+
assert "exported" in function_names

0 commit comments

Comments
 (0)