Skip to content

Commit bc6444a

Browse files
mashraf-222claude
andcommitted
fix: extract shared config helper and fix 4 review bugs
Extract resolve_config_onto_args() from duplicated logic between process_pyproject_config and apply_language_config (~100 lines deduped). This fixes: - Missing benchmark validation in apply_language_config (HIGH) - Missing LSP guard in apply_language_config (MEDIUM) - --all /specific/path silently overridden in orchestration loop (MEDIUM) - Silent except Exception: pass in config discovery (MEDIUM) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 4a24900 commit bc6444a

3 files changed

Lines changed: 66 additions & 122 deletions

File tree

codeflash/cli_cmds/cli.py

Lines changed: 49 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,29 @@
1+
from __future__ import annotations
2+
13
import logging
24
import os
35
import sys
4-
from argparse import SUPPRESS, ArgumentParser, Namespace
6+
from argparse import SUPPRESS, ArgumentParser
57
from functools import lru_cache
68
from pathlib import Path
9+
from typing import TYPE_CHECKING
710

811
from codeflash.cli_cmds import logging_config
912
from codeflash.cli_cmds.console import apologize_and_exit, logger
1013
from codeflash.code_utils import env_utils
1114
from codeflash.code_utils.code_utils import exit_with_message, normalize_ignore_paths
12-
from codeflash.code_utils.config_parser import LanguageConfig, parse_config_file
15+
from codeflash.code_utils.config_parser import parse_config_file
1316
from codeflash.languages import set_current_language
1417
from codeflash.languages.language_enum import Language
1518
from codeflash.languages.test_framework import set_current_test_framework
1619
from codeflash.lsp.helpers import is_LSP_enabled
1720
from codeflash.version import __version__ as version
1821

22+
if TYPE_CHECKING:
23+
from argparse import Namespace
24+
25+
from codeflash.code_utils.config_parser import LanguageConfig
26+
1927

2028
def parse_args() -> Namespace:
2129
parser = _build_parser()
@@ -91,6 +99,21 @@ def process_pyproject_config(args: Namespace) -> Namespace:
9199
pyproject_config, pyproject_file_path = parse_config_file(args.config_file)
92100
except ValueError as e:
93101
exit_with_message(f"Error parsing config file: {e}", error_on_exit=True)
102+
103+
language = None
104+
lang_str = pyproject_config.get("language")
105+
if lang_str:
106+
language = Language(lang_str)
107+
108+
args = resolve_config_onto_args(args, pyproject_config, pyproject_file_path, language)
109+
110+
if is_LSP_enabled():
111+
args.all = None
112+
return args
113+
return handle_optimize_all_arg_parsing(args)
114+
115+
116+
def resolve_config_onto_args(args: Namespace, config: dict, config_path: Path, language: Language | None) -> Namespace:
94117
supported_keys = [
95118
"module_root",
96119
"tests_root",
@@ -104,29 +127,26 @@ def process_pyproject_config(args: Namespace) -> Namespace:
104127
"override_fixtures",
105128
]
106129
for key in supported_keys:
107-
if key in pyproject_config and (
108-
(hasattr(args, key.replace("-", "_")) and getattr(args, key.replace("-", "_")) is None)
109-
or not hasattr(args, key.replace("-", "_"))
130+
attr_key = key.replace("-", "_")
131+
if key in config and (
132+
(hasattr(args, attr_key) and getattr(args, attr_key) is None) or not hasattr(args, attr_key)
110133
):
111-
setattr(args, key.replace("-", "_"), pyproject_config[key])
134+
setattr(args, attr_key, config[key])
135+
112136
assert args.module_root is not None, "--module-root must be specified"
113137
assert Path(args.module_root).is_dir(), f"--module-root {args.module_root} must be a valid directory"
114138

115-
is_js_ts_project = pyproject_config.get("language") in ("javascript", "typescript")
116-
is_java_project = pyproject_config.get("language") == "java"
139+
is_js_ts = language in (Language.JAVASCRIPT, Language.TYPESCRIPT)
140+
is_java = language == Language.JAVA
117141

118-
# Set the language singleton early so downstream code (e.g. get_git_diff)
119-
# can use current_language_support() before function discovery.
120-
if pyproject_config.get("language"):
121-
set_current_language(pyproject_config["language"])
142+
if language is not None:
143+
set_current_language(language)
122144

123-
# Set the test framework singleton for JS/TS projects
124-
if is_js_ts_project and pyproject_config.get("test_framework"):
125-
set_current_test_framework(pyproject_config["test_framework"])
145+
if is_js_ts and config.get("test_framework"):
146+
set_current_test_framework(config["test_framework"])
126147

127148
if args.tests_root is None:
128-
if is_java_project:
129-
# Try standard Maven/Gradle test directories
149+
if is_java:
130150
for test_dir in ["src/test/java", "test", "tests"]:
131151
test_path = Path(args.module_root).parent / test_dir if "/" in test_dir else Path(test_dir)
132152
if not test_path.is_absolute():
@@ -136,29 +156,26 @@ def process_pyproject_config(args: Namespace) -> Namespace:
136156
break
137157
if args.tests_root is None:
138158
args.tests_root = str(Path.cwd() / "src" / "test" / "java")
139-
elif is_js_ts_project:
140-
# Try common JS test directories at project root first
159+
elif is_js_ts:
141160
for test_dir in ["test", "tests", "__tests__"]:
142161
if Path(test_dir).is_dir():
143162
args.tests_root = test_dir
144163
break
145-
# If not found at project root, try inside module_root (e.g., src/test, src/__tests__)
146164
if args.tests_root is None and args.module_root:
147165
module_root_path = Path(args.module_root)
148166
for test_dir in ["test", "tests", "__tests__"]:
149167
test_path = module_root_path / test_dir
150168
if test_path.is_dir():
151169
args.tests_root = str(test_path)
152170
break
153-
# Final fallback: default to module_root
154-
# Note: This may cause issues if tests are colocated with source files
155-
# In such cases, the user should explicitly configure testsRoot in package.json
156171
if args.tests_root is None:
157172
args.tests_root = args.module_root
158173
else:
159174
raise AssertionError("--tests-root must be specified")
175+
160176
assert Path(args.tests_root).is_dir(), f"--tests-root {args.tests_root} must be a valid directory"
161-
if args.benchmark:
177+
178+
if getattr(args, "benchmark", False):
162179
assert args.benchmarks_root is not None, "--benchmarks-root must be specified when running with --benchmark"
163180
assert Path(args.benchmarks_root).is_dir(), (
164181
f"--benchmarks-root {args.benchmarks_root} must be a valid directory"
@@ -184,29 +201,20 @@ def process_pyproject_config(args: Namespace) -> Namespace:
184201

185202
require_github_app_or_exit(owner, repo_name)
186203

187-
# Project root path is one level above the specified directory, because that's where the module can be imported from
188204
args.module_root = Path(args.module_root).resolve()
189205
if hasattr(args, "ignore_paths") and args.ignore_paths is not None:
190-
# Normalize ignore paths, supporting both literal paths and glob patterns
191-
# Use module_root as base path for resolving relative paths and patterns
192206
args.ignore_paths = normalize_ignore_paths(args.ignore_paths, base_path=args.module_root)
193-
# If module-root is "." then all imports are relatives to it.
194-
# in this case, the ".." becomes outside project scope, causing issues with un-importable paths
195-
args.project_root = project_root_from_module_root(Path(args.module_root), pyproject_file_path)
207+
args.project_root = project_root_from_module_root(Path(args.module_root), config_path)
196208
args.tests_root = Path(args.tests_root).resolve()
197209
if args.benchmarks_root:
198210
args.benchmarks_root = Path(args.benchmarks_root).resolve()
199-
args.test_project_root = project_root_from_module_root(args.tests_root, pyproject_file_path)
211+
args.test_project_root = project_root_from_module_root(args.tests_root, config_path)
200212

201-
if is_java_project and pyproject_file_path.is_dir():
202-
# For Java projects, pyproject_file_path IS the project root directory (not a file).
203-
# Override project_root which may have resolved to a sub-module.
204-
args.project_root = pyproject_file_path.resolve()
205-
args.test_project_root = pyproject_file_path.resolve()
206-
if is_LSP_enabled():
207-
args.all = None
208-
return args
209-
return handle_optimize_all_arg_parsing(args)
213+
if is_java and config_path.is_dir():
214+
args.project_root = config_path.resolve()
215+
args.test_project_root = config_path.resolve()
216+
217+
return args
210218

211219

212220
def project_root_from_module_root(module_root: Path, pyproject_file_path: Path) -> Path:
@@ -227,80 +235,7 @@ def project_root_from_module_root(module_root: Path, pyproject_file_path: Path)
227235

228236

229237
def apply_language_config(args: Namespace, lang_config: LanguageConfig) -> Namespace:
230-
config = lang_config.config
231-
config_path = lang_config.config_path
232-
233-
supported_keys = [
234-
"module_root",
235-
"tests_root",
236-
"benchmarks_root",
237-
"ignore_paths",
238-
"pytest_cmd",
239-
"formatter_cmds",
240-
"disable_telemetry",
241-
"disable_imports_sorting",
242-
"git_remote",
243-
"override_fixtures",
244-
]
245-
for key in supported_keys:
246-
if key in config and ((hasattr(args, key) and getattr(args, key) is None) or not hasattr(args, key)):
247-
setattr(args, key, config[key])
248-
249-
assert args.module_root is not None, "--module-root must be specified"
250-
assert Path(args.module_root).is_dir(), f"--module-root {args.module_root} must be a valid directory"
251-
252-
set_current_language(lang_config.language)
253-
254-
is_js_ts = lang_config.language in (Language.JAVASCRIPT, Language.TYPESCRIPT)
255-
if is_js_ts and config.get("test_framework"):
256-
set_current_test_framework(config["test_framework"])
257-
258-
is_java = lang_config.language == Language.JAVA
259-
if args.tests_root is None:
260-
if is_java:
261-
for test_dir in ["src/test/java", "test", "tests"]:
262-
test_path = Path(args.module_root).parent / test_dir if "/" in test_dir else Path(test_dir)
263-
if not test_path.is_absolute():
264-
test_path = Path.cwd() / test_path
265-
if test_path.is_dir():
266-
args.tests_root = str(test_path)
267-
break
268-
if args.tests_root is None:
269-
args.tests_root = str(Path.cwd() / "src" / "test" / "java")
270-
elif is_js_ts:
271-
for test_dir in ["test", "tests", "__tests__"]:
272-
if Path(test_dir).is_dir():
273-
args.tests_root = test_dir
274-
break
275-
if args.tests_root is None and args.module_root:
276-
module_root_path = Path(args.module_root)
277-
for test_dir in ["test", "tests", "__tests__"]:
278-
test_path = module_root_path / test_dir
279-
if test_path.is_dir():
280-
args.tests_root = str(test_path)
281-
break
282-
if args.tests_root is None:
283-
args.tests_root = args.module_root
284-
else:
285-
raise AssertionError("--tests-root must be specified")
286-
287-
assert Path(args.tests_root).is_dir(), f"--tests-root {args.tests_root} must be a valid directory"
288-
289-
args.module_root = Path(args.module_root).resolve()
290-
if hasattr(args, "ignore_paths") and args.ignore_paths is not None:
291-
args.ignore_paths = normalize_ignore_paths(args.ignore_paths, base_path=args.module_root)
292-
args.project_root = project_root_from_module_root(args.module_root, config_path)
293-
args.tests_root = Path(args.tests_root).resolve()
294-
if args.benchmarks_root:
295-
args.benchmarks_root = Path(args.benchmarks_root).resolve()
296-
args.test_project_root = project_root_from_module_root(args.tests_root, config_path)
297-
298-
if is_java and config_path.is_dir():
299-
# For Java projects, config_path IS the project root directory (from build-tool detection).
300-
args.project_root = config_path.resolve()
301-
args.test_project_root = config_path.resolve()
302-
303-
return args
238+
return resolve_config_onto_args(args, lang_config.config, lang_config.config_path, lang_config.language)
304239

305240

306241
def handle_optimize_all_arg_parsing(args: Namespace) -> Namespace:

codeflash/code_utils/config_parser.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from __future__ import annotations
22

3+
import logging
34
from dataclasses import dataclass
45
from pathlib import Path
56
from typing import Any
@@ -10,6 +11,8 @@
1011
from codeflash.languages.language_enum import Language
1112
from codeflash.lsp.helpers import is_LSP_enabled
1213

14+
logger = logging.getLogger("codeflash")
15+
1316
PYPROJECT_TOML_CACHE: dict[Path, Path] = {}
1417
ALL_CONFIG_FILES: dict[Path, dict[str, Path]] = {}
1518

@@ -197,7 +200,7 @@ def _check_dir_for_configs(dir_path: Path, configs: list[LanguageConfig], seen_l
197200
seen_languages.add(Language.PYTHON)
198201
configs.append(LanguageConfig(config=normalized, config_path=pyproject, language=Language.PYTHON))
199202
except Exception:
200-
pass
203+
logger.debug("Failed to parse Python config in %s", dir_path, exc_info=True)
201204

202205
if Language.JAVASCRIPT not in seen_languages and Language.TYPESCRIPT not in seen_languages:
203206
package_json = dir_path / "package.json"
@@ -210,7 +213,7 @@ def _check_dir_for_configs(dir_path: Path, configs: list[LanguageConfig], seen_l
210213
seen_languages.add(lang)
211214
configs.append(LanguageConfig(config=config, config_path=path, language=lang))
212215
except Exception:
213-
pass
216+
logger.debug("Failed to parse JS/TS config in %s", dir_path, exc_info=True)
214217

215218
if Language.JAVA not in seen_languages:
216219
if (
@@ -224,7 +227,7 @@ def _check_dir_for_configs(dir_path: Path, configs: list[LanguageConfig], seen_l
224227
seen_languages.add(Language.JAVA)
225228
configs.append(LanguageConfig(config=java_config, config_path=dir_path, language=Language.JAVA))
226229
except Exception:
227-
pass
230+
logger.debug("Failed to parse Java config in %s", dir_path, exc_info=True)
228231

229232

230233
def find_all_config_files(start_dir: Path | None = None) -> list[LanguageConfig]:

codeflash/main.py

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -129,10 +129,11 @@ def main() -> None:
129129
except UnsupportedLanguageError:
130130
pass # Unknown extension, let all configs run
131131

132-
# Track whether --all was originally requested (before handle_optimize_all_arg_parsing
133-
# resolves it — in multi-language mode, module_root isn't available yet so the resolution
134-
# produces None; we re-resolve per language inside the loop)
135-
optimize_all_requested = hasattr(args, "all") and args.all is not None
132+
# Save the raw --all value before handle_optimize_all_arg_parsing mutates it.
133+
# In multi-language mode, module_root is None at this point so the resolution
134+
# produces None for the default case; we re-resolve per language inside the loop.
135+
original_all = getattr(args, "all", None) if hasattr(args, "all") else None
136+
optimize_all_requested = hasattr(args, "all") and original_all is not None
136137

137138
# Multi-language path: run git/GitHub checks ONCE before the loop
138139
args = handle_optimize_all_arg_parsing(args)
@@ -145,7 +146,12 @@ def main() -> None:
145146
pass_args = apply_language_config(pass_args, lang_config)
146147

147148
if optimize_all_requested:
148-
pass_args.all = pass_args.module_root
149+
if original_all == "":
150+
# --all with no path: use this language's module_root
151+
pass_args.all = pass_args.module_root
152+
else:
153+
# --all /specific/path: preserve the user's path
154+
pass_args.all = Path(original_all).resolve()
149155

150156
if not env_utils.check_formatter_installed(pass_args.formatter_cmds):
151157
logger.info("Skipping %s: formatter not installed", lang_name)

0 commit comments

Comments
 (0)