Skip to content

Commit 9738147

Browse files
committed
perf: read JS/TS files once during discovery export checks
The _is_js_ts_function_exported and _is_js_ts_function_exists_but_not_exported helpers each read the file from disk independently, causing up to 2 extra reads per file during get_functions_to_optimize. Add an optional `source` parameter to both functions so callers can pass pre-read content. The main call site now reads the file once and passes it to both helpers. Also fixes pre-existing mypy error in _find_all_functions_via_language_support where discover_functions was called with wrong argument order. Signatures changed: - _is_js_ts_function_exported(file_path, function_name, source=None) - _is_js_ts_function_exists_but_not_exported(file_path, function_name, source=None)
1 parent 9164a8d commit 9738147

2 files changed

Lines changed: 165 additions & 15 deletions

File tree

codeflash/discovery/functions_to_optimize.py

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,9 @@ def get_files_for_language(
176176
return files
177177

178178

179-
def _is_js_ts_function_exported(file_path: Path, function_name: str) -> tuple[bool, str | None]:
179+
def _is_js_ts_function_exported(
180+
file_path: Path, function_name: str, source: str | None = None
181+
) -> tuple[bool, str | None]:
180182
"""Check if a JavaScript/TypeScript function is exported from its module.
181183
182184
For JS/TS, functions that are not exported cannot be imported by tests,
@@ -185,6 +187,7 @@ def _is_js_ts_function_exported(file_path: Path, function_name: str) -> tuple[bo
185187
Args:
186188
file_path: Path to the source file.
187189
function_name: Name of the function to check.
190+
source: Pre-read file content. If None, reads from disk.
188191
189192
Returns:
190193
Tuple of (is_exported, export_name). export_name may be 'default' for default exports.
@@ -193,16 +196,16 @@ def _is_js_ts_function_exported(file_path: Path, function_name: str) -> tuple[bo
193196
from codeflash.languages.javascript.treesitter import get_analyzer_for_file
194197

195198
try:
196-
source = read_file_cached(file_path)
199+
if source is None:
200+
source = read_file_cached(file_path)
197201
analyzer = get_analyzer_for_file(file_path)
198202
return analyzer.is_function_exported(source, function_name)
199203
except Exception as e:
200204
logger.debug(f"Failed to check export status for {function_name}: {e}")
201-
# Return True to avoid blocking in case of errors
202205
return True, None
203206

204207

205-
def _is_js_ts_function_exists_but_not_exported(file_path: Path, function_name: str) -> bool:
208+
def _is_js_ts_function_exists_but_not_exported(file_path: Path, function_name: str, source: str | None = None) -> bool:
206209
"""Check if a JS/TS function exists in the file but is not exported.
207210
208211
Returns True only if the function name is found as a defined function
@@ -211,7 +214,8 @@ def _is_js_ts_function_exists_but_not_exported(file_path: Path, function_name: s
211214
from codeflash.languages.javascript.treesitter import get_analyzer_for_file
212215

213216
try:
214-
source = read_file_cached(file_path)
217+
if source is None:
218+
source = read_file_cached(file_path)
215219
analyzer = get_analyzer_for_file(file_path)
216220
all_funcs = analyzer.find_functions(
217221
source, include_methods=True, include_arrow_functions=True, require_name=True
@@ -258,6 +262,13 @@ def get_functions_to_optimize(
258262
console.rule()
259263
file = Path(file) if isinstance(file, str) else file
260264
functions = find_all_functions_in_file(file)
265+
# Pre-read JS/TS file content once to avoid redundant disk reads in export checks
266+
_js_ts_source: str | None = None
267+
if only_get_this_function is not None and is_language_supported(file):
268+
_lang = get_language_support(file)
269+
if _lang.language in (Language.JAVASCRIPT, Language.TYPESCRIPT):
270+
with contextlib.suppress(OSError):
271+
_js_ts_source = file.read_text(encoding="utf-8")
261272
if only_get_this_function is not None:
262273
split_function = only_get_this_function.split(".")
263274
if len(split_function) > 2:
@@ -282,15 +293,13 @@ def get_functions_to_optimize(
282293
return functions, 0, None
283294

284295
# For JS/TS: check if the function exists but is not exported
285-
if is_language_supported(file):
286-
lang_support = get_language_support(file)
287-
if lang_support.language in (Language.JAVASCRIPT, Language.TYPESCRIPT):
288-
if _is_js_ts_function_exists_but_not_exported(file, only_function_name):
289-
exit_with_message(
290-
f"Function '{only_function_name}' exists in {file} but is not exported.\n"
291-
f"In JavaScript/TypeScript, only exported functions can be optimized.\n"
292-
f"Add: export {{ {only_function_name} }}"
293-
)
296+
if _js_ts_source is not None:
297+
if _is_js_ts_function_exists_but_not_exported(file, only_function_name, source=_js_ts_source):
298+
exit_with_message(
299+
f"Function '{only_function_name}' exists in {file} but is not exported.\n"
300+
f"In JavaScript/TypeScript, only exported functions can be optimized.\n"
301+
f"Add: export {{ {only_function_name} }}"
302+
)
294303

295304
found = closest_matching_file_function_name(only_get_this_function, functions)
296305
if found is not None:
@@ -317,7 +326,7 @@ def get_functions_to_optimize(
317326
# It's a standalone function - check if the function is exported
318327
name_to_check = found_function.function_name
319328

320-
is_exported, _ = _is_js_ts_function_exported(file, name_to_check)
329+
is_exported, _ = _is_js_ts_function_exported(file, name_to_check, source=_js_ts_source)
321330
if not is_exported:
322331
if found_function.parents:
323332
logger.debug(

tests/test_js_ts_export_helpers.py

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
"""Tests for JS/TS export helper functions with pre-read source content.
2+
3+
Verifies that _is_js_ts_function_exported and _is_js_ts_function_exists_but_not_exported
4+
work correctly both with and without pre-read source content passed in.
5+
"""
6+
7+
from pathlib import Path
8+
9+
import pytest
10+
11+
from codeflash.discovery.functions_to_optimize import (
12+
_is_js_ts_function_exists_but_not_exported,
13+
_is_js_ts_function_exported,
14+
)
15+
16+
17+
class TestIsJsTsFunctionExported:
18+
def test_named_export_detected(self, tmp_path: Path) -> None:
19+
js_file = tmp_path / "module.js"
20+
js_file.write_text(
21+
"export function add(a, b) {\n return a + b;\n}\n",
22+
encoding="utf-8",
23+
)
24+
is_exported, export_name = _is_js_ts_function_exported(js_file, "add")
25+
assert is_exported is True
26+
assert export_name == "add"
27+
28+
def test_named_export_with_source_param(self, tmp_path: Path) -> None:
29+
js_file = tmp_path / "module.js"
30+
content = "export function add(a, b) {\n return a + b;\n}\n"
31+
js_file.write_text(content, encoding="utf-8")
32+
is_exported, export_name = _is_js_ts_function_exported(js_file, "add", source=content)
33+
assert is_exported is True
34+
assert export_name == "add"
35+
36+
def test_default_export_detected(self, tmp_path: Path) -> None:
37+
js_file = tmp_path / "module.js"
38+
content = "function compute(x) {\n return x * 2;\n}\nexport default compute;\n"
39+
js_file.write_text(content, encoding="utf-8")
40+
is_exported, export_name = _is_js_ts_function_exported(js_file, "compute", source=content)
41+
assert is_exported is True
42+
assert export_name == "default"
43+
44+
def test_non_exported_function(self, tmp_path: Path) -> None:
45+
js_file = tmp_path / "module.js"
46+
content = "function helper(x) {\n return x + 1;\n}\n\nexport function main() {\n return helper(5);\n}\n"
47+
js_file.write_text(content, encoding="utf-8")
48+
is_exported, export_name = _is_js_ts_function_exported(js_file, "helper", source=content)
49+
assert is_exported is False
50+
assert export_name is None
51+
52+
def test_separate_export_clause(self, tmp_path: Path) -> None:
53+
js_file = tmp_path / "module.js"
54+
content = "function process(data) {\n return data;\n}\n\nexport { process };\n"
55+
js_file.write_text(content, encoding="utf-8")
56+
is_exported, export_name = _is_js_ts_function_exported(js_file, "process", source=content)
57+
assert is_exported is True
58+
assert export_name == "process"
59+
60+
def test_aliased_export(self, tmp_path: Path) -> None:
61+
js_file = tmp_path / "module.js"
62+
content = "function internalName(x) {\n return x;\n}\n\nexport { internalName as publicName };\n"
63+
js_file.write_text(content, encoding="utf-8")
64+
is_exported, export_name = _is_js_ts_function_exported(js_file, "internalName", source=content)
65+
assert is_exported is True
66+
assert export_name == "publicName"
67+
68+
def test_typescript_export(self, tmp_path: Path) -> None:
69+
ts_file = tmp_path / "module.ts"
70+
content = "export function greet(name: string): string {\n return `Hello, ${name}`;\n}\n"
71+
ts_file.write_text(content, encoding="utf-8")
72+
is_exported, export_name = _is_js_ts_function_exported(ts_file, "greet", source=content)
73+
assert is_exported is True
74+
assert export_name == "greet"
75+
76+
def test_fallback_reads_from_disk_when_source_is_none(self, tmp_path: Path) -> None:
77+
js_file = tmp_path / "module.js"
78+
js_file.write_text(
79+
"export function fromDisk(x) {\n return x;\n}\n",
80+
encoding="utf-8",
81+
)
82+
is_exported, export_name = _is_js_ts_function_exported(js_file, "fromDisk", source=None)
83+
assert is_exported is True
84+
assert export_name == "fromDisk"
85+
86+
87+
class TestIsJsTsFunctionExistsButNotExported:
88+
def test_unexported_function_detected(self, tmp_path: Path) -> None:
89+
js_file = tmp_path / "module.js"
90+
content = "function secret(x) {\n return x * 2;\n}\n\nexport function pub() {\n return 1;\n}\n"
91+
js_file.write_text(content, encoding="utf-8")
92+
assert _is_js_ts_function_exists_but_not_exported(js_file, "secret", source=content) is True
93+
94+
def test_exported_function_returns_false(self, tmp_path: Path) -> None:
95+
js_file = tmp_path / "module.js"
96+
content = "export function pub(x) {\n return x;\n}\n"
97+
js_file.write_text(content, encoding="utf-8")
98+
assert _is_js_ts_function_exists_but_not_exported(js_file, "pub", source=content) is False
99+
100+
def test_nonexistent_function_returns_false(self, tmp_path: Path) -> None:
101+
js_file = tmp_path / "module.js"
102+
content = "export function exists() {\n return 1;\n}\n"
103+
js_file.write_text(content, encoding="utf-8")
104+
assert _is_js_ts_function_exists_but_not_exported(js_file, "nonexistent", source=content) is False
105+
106+
def test_fallback_reads_from_disk(self, tmp_path: Path) -> None:
107+
js_file = tmp_path / "module.js"
108+
js_file.write_text(
109+
"function localOnly() {\n return 42;\n}\n",
110+
encoding="utf-8",
111+
)
112+
assert _is_js_ts_function_exists_but_not_exported(js_file, "localOnly", source=None) is True
113+
114+
def test_typescript_unexported(self, tmp_path: Path) -> None:
115+
ts_file = tmp_path / "utils.ts"
116+
content = (
117+
"function internal(x: number): number {\n return x;\n}\n\n"
118+
"export function external(y: number): number {\n return y + 1;\n}\n"
119+
)
120+
ts_file.write_text(content, encoding="utf-8")
121+
assert _is_js_ts_function_exists_but_not_exported(ts_file, "internal", source=content) is True
122+
assert _is_js_ts_function_exists_but_not_exported(ts_file, "external", source=content) is False
123+
124+
def test_arrow_function_unexported(self, tmp_path: Path) -> None:
125+
js_file = tmp_path / "arrows.js"
126+
content = "const helper = (x) => {\n return x + 1;\n};\n\nexport const main = () => {\n return 2;\n};\n"
127+
js_file.write_text(content, encoding="utf-8")
128+
assert _is_js_ts_function_exists_but_not_exported(js_file, "helper", source=content) is True
129+
assert _is_js_ts_function_exists_but_not_exported(js_file, "main", source=content) is False
130+
131+
def test_source_param_matches_disk_read(self, tmp_path: Path) -> None:
132+
js_file = tmp_path / "consistent.js"
133+
content = "function local() {\n return 'hi';\n}\n\nexport function exported() {\n return 'bye';\n}\n"
134+
js_file.write_text(content, encoding="utf-8")
135+
# Results should be identical whether source is passed or read from disk
136+
assert _is_js_ts_function_exists_but_not_exported(js_file, "local", source=content) == (
137+
_is_js_ts_function_exists_but_not_exported(js_file, "local", source=None)
138+
)
139+
assert _is_js_ts_function_exists_but_not_exported(js_file, "exported", source=content) == (
140+
_is_js_ts_function_exists_but_not_exported(js_file, "exported", source=None)
141+
)

0 commit comments

Comments
 (0)