mirror of
https://github.com/codeflash-ai/codeflash.git
synced 2026-05-04 18:25:17 +00:00
Fix: Exclude property getters/setters from function discovery
**Problem:**
When functions are defined as property getters/setters (e.g., inside
Object.defineProperty with `get: function getrouter() {...}`), they were
being discovered as optimizable functions. This caused test generation
to fail because:
1. Tests tried to call the getter function directly (e.g., `obj.getrouter()`)
2. But getters are not accessible by function name - only via property access
3. This resulted in "TypeError: Cannot read properties of undefined (reading 'bind')"
**Root Cause:**
The tree-sitter function discovery in `treesitter.py` was finding all
function_expression nodes, including those used as property getters/setters.
These are not standalone functions and cannot be called directly.
**Solution:**
Added a check in `_walk_tree_for_functions` to exclude function_expression
nodes that are:
- Inside a `pair` node (key-value pair in object literal)
- Where the key is "get" or "set" (property accessor pattern)
**Affected Trace IDs (from logs):**
- 11b5475c-91d9-43b1-a317-33b7f4811c52 (init.getrouter)
- 78d8f1de-541a-4208-85c7-77194c70ec72 (createETagGenerator.generateETag)
- And others with similar patterns
**Testing:**
- Added comprehensive test suite in test_property_getter_exclusion.py
- Verified fix on actual Express.js application.js (getrouter no longer discovered)
- All existing JavaScript function discovery tests pass
- Linting (prek) passes
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
2d2bdc7b87
commit
83bd04e0a9
2 changed files with 147 additions and 0 deletions
|
|
@ -290,6 +290,18 @@ class TreeSitterAnalyzer:
|
|||
if func_info.is_method and node.parent and node.parent.type == "object":
|
||||
should_include = False
|
||||
|
||||
# Skip property getters/setters (e.g., get: function foo() {})
|
||||
# These are defined inside Object.defineProperty or object literals
|
||||
# and cannot be called directly - they're accessed via property names.
|
||||
# Tests would fail trying to call obj.getterFuncName() instead of obj.propertyName
|
||||
if node.type == "function_expression" and node.parent and node.parent.type == "pair":
|
||||
# Check if this is a getter or setter by looking at the property name
|
||||
property_name_node = node.parent.child_by_field_name("key")
|
||||
if property_name_node:
|
||||
property_name = self.get_node_text(property_name_node, source_bytes)
|
||||
if property_name in ("get", "set"):
|
||||
should_include = False
|
||||
|
||||
if should_include:
|
||||
functions.append(func_info)
|
||||
|
||||
|
|
|
|||
135
tests/test_property_getter_exclusion.py
Normal file
135
tests/test_property_getter_exclusion.py
Normal file
|
|
@ -0,0 +1,135 @@
|
|||
"""Test that property getters are excluded from optimization.
|
||||
|
||||
Property getters defined via Object.defineProperty should not be
|
||||
optimized because they're not directly callable and tests cannot
|
||||
access them by the function name.
|
||||
|
||||
Relates to bug: Generated tests try to call getters directly
|
||||
(e.g., `obj.getterFunc()`) when they should access the property
|
||||
(e.g., `obj.propertyName`).
|
||||
"""
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
from codeflash.discovery.functions_to_optimize import find_all_functions_in_file
|
||||
|
||||
|
||||
class TestPropertyGetterExclusion:
|
||||
"""Tests for excluding property getters from function discovery."""
|
||||
|
||||
def test_object_define_property_getter_excluded(self, tmp_path: Path) -> None:
|
||||
"""Test that functions used as property getters are excluded.
|
||||
|
||||
When a function is defined as `get: function foo() {...}` inside
|
||||
Object.defineProperty, it should not be discovered as an optimizable
|
||||
function because:
|
||||
1. It's not directly accessible by the function name
|
||||
2. Generated tests would fail trying to call it directly
|
||||
3. Property access patterns are different from function calls
|
||||
|
||||
This reproduces the Express.js pattern where getrouter is defined
|
||||
as a property getter inside the init function.
|
||||
"""
|
||||
js_file = tmp_path / "app.js"
|
||||
js_file.write_text("""
|
||||
const app = {};
|
||||
|
||||
// Express pattern: getter nested inside a function
|
||||
app.init = function init() {
|
||||
var router = null;
|
||||
|
||||
// Property getter pattern (like express application.js line 72)
|
||||
Object.defineProperty(this, 'router', {
|
||||
configurable: true,
|
||||
get: function getrouter() {
|
||||
if (router === null) {
|
||||
router = { use: () => {} };
|
||||
}
|
||||
return router;
|
||||
}
|
||||
});
|
||||
};
|
||||
|
||||
// Normal exported function (should be found)
|
||||
export function normalFunction() {
|
||||
return 42;
|
||||
}
|
||||
|
||||
module.exports = app;
|
||||
""")
|
||||
|
||||
functions = find_all_functions_in_file(js_file)
|
||||
function_names = {fn.function_name for fn in functions.get(js_file, [])}
|
||||
|
||||
# Property getter should NOT be found
|
||||
assert "getrouter" not in function_names, (
|
||||
"Property getter 'getrouter' should be excluded from optimization. "
|
||||
"Tests cannot access it as init.getrouter() - they would need to access "
|
||||
"the 'router' property via an instance instead."
|
||||
)
|
||||
|
||||
# Normal exported function should be found
|
||||
assert "normalFunction" in function_names
|
||||
|
||||
def test_object_define_property_setter_excluded(self, tmp_path: Path) -> None:
|
||||
"""Test that functions used as property setters are also excluded."""
|
||||
js_file = tmp_path / "app.js"
|
||||
js_file.write_text("""
|
||||
const app = {};
|
||||
|
||||
Object.defineProperty(app, 'value', {
|
||||
set: function setvalue(val) {
|
||||
this._value = val;
|
||||
},
|
||||
get: function getvalue() {
|
||||
return this._value;
|
||||
}
|
||||
});
|
||||
|
||||
export function helper() {
|
||||
return 1;
|
||||
}
|
||||
""")
|
||||
|
||||
functions = find_all_functions_in_file(js_file)
|
||||
function_names = {fn.function_name for fn in functions.get(js_file, [])}
|
||||
|
||||
# Neither getter nor setter should be found
|
||||
assert "setvalue" not in function_names
|
||||
assert "getvalue" not in function_names
|
||||
|
||||
# Helper function should still be found
|
||||
assert "helper" in function_names
|
||||
|
||||
def test_object_literal_getter_excluded(self, tmp_path: Path) -> None:
|
||||
"""Test that getter methods in object literals are excluded."""
|
||||
js_file = tmp_path / "obj.js"
|
||||
js_file.write_text("""
|
||||
const obj = {
|
||||
get router() {
|
||||
return this._router;
|
||||
},
|
||||
|
||||
// Regular method should be excluded too (it's in an object literal)
|
||||
method() {
|
||||
return 1;
|
||||
}
|
||||
};
|
||||
|
||||
export function exported() {
|
||||
return obj;
|
||||
}
|
||||
""")
|
||||
|
||||
functions = find_all_functions_in_file(js_file)
|
||||
function_names = {fn.function_name for fn in functions.get(js_file, [])}
|
||||
|
||||
# Getter in object literal should not be found
|
||||
assert "router" not in function_names
|
||||
|
||||
# Regular method in object literal should also not be found
|
||||
# (per existing code logic)
|
||||
assert "method" not in function_names
|
||||
|
||||
# Exported function should be found
|
||||
assert "exported" in function_names
|
||||
Loading…
Reference in a new issue