Skip to content

Commit 73ef518

Browse files
refactoring
1 parent 44c0f85 commit 73ef518

File tree

4 files changed

+66
-36
lines changed

4 files changed

+66
-36
lines changed

code_to_optimize/few_formatting_errors.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import os
22

3-
class BadlyFormattedClass(object):
3+
class UnformattedExampleClass(object):
44
def __init__(
55
self,
66
name,

code_to_optimize/many_formatting_errors.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
# This is a poorly formatted Python file with many style violations
55

6-
class BadlyFormattedClass( object ):
6+
class UnformattedExampleClass( object ):
77
def __init__(self,name,age=None,email=None,phone=None,address=None,city=None,state=None,zip_code=None):
88
self.name=name;self.age=age;self.email=email;self.phone=phone
99
self.address=address;self.city=city;self.state=state;self.zip_code=zip_code
@@ -84,7 +84,7 @@ def generate_report(data,include_stats=True,include_charts=False,format_type='js
8484

8585
return result
8686

87-
class DataProcessor ( BadlyFormattedClass ) :
87+
class DataProcessor ( UnformattedExampleClass ) :
8888
def __init__(self,data_source,config=None,debug=False):
8989
super().__init__("DataProcessor")
9090
self.data_source=data_source;self.config=config or{};self.debug=debug

codeflash/code_utils/formatter.py

Lines changed: 56 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import os
44
import shlex
55
import subprocess
6-
from typing import TYPE_CHECKING
6+
from typing import TYPE_CHECKING, Optional
77

88
import isort
99

@@ -12,45 +12,68 @@
1212
if TYPE_CHECKING:
1313
from pathlib import Path
1414

15-
16-
def should_format_file(filepath, max_lines_changed=100):
17-
try:
18-
# check if black is installed
19-
subprocess.run(['black', '--version'], check=True, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
20-
21-
result = subprocess.run(
22-
['black', '--diff', filepath],
23-
capture_output=True,
24-
text=True
25-
)
26-
27-
diff_lines = [line for line in result.stdout.split('\n')
28-
if line.startswith(('+', '-')) and not line.startswith(('+++', '---'))]
29-
30-
changes_count = len(diff_lines)
31-
32-
if changes_count > max_lines_changed:
33-
logger.debug(f"Skipping {filepath}: {changes_count} lines would change (max: {max_lines_changed})")
34-
return False
35-
36-
return True
37-
38-
except subprocess.CalledProcessError:
39-
logger.warning(f"black --diff command failed for {filepath}")
40-
return False
41-
except FileNotFoundError:
42-
logger.warning("black formatter is not installed. Skipping formatting diff check.")
43-
return False
44-
45-
15+
def get_diff_lines_output_by_black(filepath: str) -> Optional[str]:
16+
try:
17+
subprocess.run(['black', '--version'], check=True,
18+
stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
19+
result = subprocess.run(
20+
['black', '--diff', filepath],
21+
capture_output=True,
22+
text=True
23+
)
24+
return result.stdout.strip() if result.stdout else None
25+
except (FileNotFoundError):
26+
return None
27+
28+
29+
def get_diff_lines_output_by_ruff(filepath: str) -> Optional[str]:
30+
try:
31+
subprocess.run(['ruff', '--version'], check=True,
32+
stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL)
33+
result = subprocess.run(
34+
['ruff', "format", '--diff', filepath],
35+
capture_output=True,
36+
text=True
37+
)
38+
return result.stdout.strip() if result.stdout else None
39+
except (FileNotFoundError):
40+
return None
41+
42+
43+
def get_diff_lines_count(diff_output: str) -> int:
44+
diff_lines = [line for line in diff_output.split('\n')
45+
if line.startswith(('+', '-')) and not line.startswith(('+++', '---'))]
46+
return len(diff_lines)
47+
48+
def is_safe_to_format(filepath: str, max_diff_lines: int = 100) -> bool:
49+
diff_changes_stdout = None
50+
51+
diff_changes_stdout = get_diff_lines_output_by_black(filepath)
52+
53+
if diff_changes_stdout is None:
54+
logger.warning(f"black formatter not found, trying ruff instead...")
55+
diff_changes_stdout = get_diff_lines_output_by_ruff(filepath)
56+
if diff_changes_stdout is None:
57+
msg = f"Both ruff, black formatters not found, skipping formatting diff check."
58+
logger.warning(msg)
59+
raise FileNotFoundError(msg)
60+
61+
diff_lines_count = get_diff_lines_count(diff_changes_stdout)
62+
63+
if diff_lines_count > max_diff_lines:
64+
logger.debug(f"Skipping {filepath}: {diff_lines_count} lines would change (max: {max_diff_lines})")
65+
return False
66+
else:
67+
return True
68+
4669

4770
def format_code(formatter_cmds: list[str], path: Path, print_status: bool = True) -> str: # noqa
4871
# TODO: Only allow a particular whitelist of formatters here to prevent arbitrary code execution
4972
formatter_name = formatter_cmds[0].lower()
5073
if not path.exists():
5174
msg = f"File {path} does not exist. Cannot format the file."
5275
raise FileNotFoundError(msg)
53-
if formatter_name == "disabled" or not should_format_file(path):
76+
if formatter_name == "disabled" or not is_safe_to_format(path): # few -> False, large -> True
5477
return path.read_text(encoding="utf8")
5578

5679
file_token = "$file" # noqa: S105

tests/test_formatter.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,12 +268,19 @@ def _run_formatting_test(source_filename: str, should_content_change: bool):
268268
else:
269269
assert content == original, f"Expected content to remain unchanged for {source_filename}"
270270

271+
def _ruff_or_black_installed() -> bool:
272+
return shutil.which("black") is not None or shutil.which("ruff") is not None
273+
271274

272275
def test_formatting_file_with_many_diffs():
273276
"""Test that files with many formatting errors are skipped (content unchanged)."""
277+
if not _ruff_or_black_installed():
278+
pytest.skip("Neither black nor ruff is installed, skipping formatting tests.")
274279
_run_formatting_test("many_formatting_errors.py", should_content_change=False)
275280

276281

277282
def test_formatting_file_with_few_diffs():
278283
"""Test that files with few formatting errors are formatted (content changed)."""
284+
if not _ruff_or_black_installed():
285+
pytest.skip("Neither black nor ruff is installed, skipping formatting tests.")
279286
_run_formatting_test("few_formatting_errors.py", should_content_change=True)

0 commit comments

Comments
 (0)