Skip to content

Fix: Exclude property getters/setters from function discovery#1938

Merged
Saga4 merged 1 commit intomainfrom
fix/exclude-property-getters-from-discovery
Apr 1, 2026
Merged

Fix: Exclude property getters/setters from function discovery#1938
Saga4 merged 1 commit intomainfrom
fix/exclude-property-getters-from-discovery

Conversation

@mohammedahmed18
Copy link
Copy Markdown
Contributor

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 with:

TypeError: Cannot read properties of undefined (reading 'bind')

Reproduction

In Express.js application.js:

Object.defineProperty(app, 'router', {
  get: function getrouter() {
    return router;
  }
});

Codeflash discovered getrouter as an optimizable function and generated tests like:

const r1 = init.getrouter();  // ❌ undefined - getrouter is not exported

But it should be:

const r1 = init.router;  // ✅ accesses property, triggers getter

Solution

Added check in _walk_tree_for_functions() to exclude function_expression nodes that are property getters/setters:

  • Inside a pair node (key-value in object literal)
  • Where the key is "get" or "set"

Testing

  • ✅ New test suite: test_property_getter_exclusion.py
  • ✅ Verified on actual Express.js code (getrouter no longer discovered)
  • ✅ All existing JavaScript discovery tests pass
  • ✅ Linting (prek) passes

Affected Trace IDs

From /workspace/logs analysis:

  • 11b5475c-91d9-43b1-a317-33b7f4811c52 (init.getrouter)
  • 78d8f1de-541a-4208-85c7-77194c70ec72 (createETagGenerator.generateETag)
  • And others with similar test failures

Files Changed

  • codeflash/languages/javascript/treesitter.py: Added getter/setter exclusion logic
  • tests/test_property_getter_exclusion.py: Comprehensive test coverage

🤖 Generated with Claude Code

**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>
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 1, 2026

Claude finished @mohammedahmed18's task in 1m 30s —— View job


PR Review Summary

  • Triage: SMALL (12 lines of production code)
  • Lint and type check (prek: ✅ passed)
  • Resolve stale review threads (none found)
  • Code review
  • Duplicate detection
  • Summary comment

Prek Checks

ruff check and ruff format passed. No issues.


Code Review

Fix is correct for the stated use case, but has a potential false-positive edge case worth acknowledging.

The logic (treesitter.py:297-303):

if node.type == "function_expression" and node.parent and node.parent.type == "pair":
    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

False positive concern: This check fires on any function_expression stored under a property key named "get" or "set", not only inside Object.defineProperty. For example:

// HTTP route handlers - "get" is just a property name, not a getter
const routes = {
  get: function handleGet(req, res) { return res.send("hello"); },
  set: function handleSet(req, res) { return res.send("ok"); }
};

Here handleGet is a directly callable, exportable function, but the fix would silently skip it from discovery. In HTTP frameworks (Express, Fastify, etc.), get/set/post/delete as property names for route handlers is a very common pattern.

More precise fix would be to walk up the parent chain and verify the pair is inside Object.defineProperty(...), or to check for the actual tree-sitter getter_definition node type (used for ES6 get foo() {} syntax). However, the Object.defineProperty case specifically uses function_expression in a pair — so the false positive only affects codebases that coincidentally use property keys literally named "get" or "set" to store named function expressions.

Test coverage issue: test_object_literal_getter_excluded (test_property_getter_exclusion.py:100) tests get router() { ... } syntax (ES6 getter shorthand), which tree-sitter parses as a getter_definition node — not function_expression. This test does not exercise the new code path. It would pass even without the new 12 lines. Consider replacing it with a test that confirms { get: function namedFn() {} } is excluded in a non-defineProperty context to at least document the known false-positive behavior (or add a test for the Object.defineProperty pattern specifically).

No bugs that crash at runtime. No security issues. No breaking API changes. No duplicate logic found in other language modules.


Duplicate Detection

No duplicates detected. The check is specific to languages/javascript/treesitter.py and has no parallel in the Python discovery path (Python uses AST decorators for properties, handled differently).


Test Coverage

New test file covers the primary use case (Express.js Object.defineProperty getter pattern). One test case (test_object_literal_getter_excluded) does not actually exercise the new code. Two test cases do exercise the new code path. Coverage is adequate for the bug being fixed.


Last updated: 2026-04-01T02:00:00Z

# 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":
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could create a list of such function types to avoid.

@Saga4 Saga4 merged commit 8e14914 into main Apr 1, 2026
36 of 39 checks passed
@Saga4 Saga4 deleted the fix/exclude-property-getters-from-discovery branch April 1, 2026 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants