Skip to content

Commit 256874b

Browse files
authored
fix: Fix caching the issues with fixes (#1623)
1 parent 0316605 commit 256874b

File tree

2 files changed

+65
-4
lines changed

2 files changed

+65
-4
lines changed

src/robocop/linter/runner.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
from robocop.linter.diagnostics import Diagnostics, RunStatistic
1414
from robocop.linter.fix import FixApplier
1515
from robocop.linter.reports import save_reports_result_to_cache
16-
from robocop.linter.rules import FixableRule
1716
from robocop.linter.utils.disablers import DisablersFinder
1817
from robocop.linter.utils.file_types import get_resource_with_lang
1918
from robocop.linter.utils.misc import is_suite_templated
@@ -105,8 +104,8 @@ def run(self) -> list[Diagnostic]:
105104
print(f"Scanning file: {source_file.path}")
106105
diagnostics = self.get_cached_diagnostics(source_file.config, source_file.path)
107106
if diagnostics is not None:
108-
fixes = [diag.fix(source_file) for diag in diagnostics if isinstance(diag.rule, FixableRule)]
109-
if not fix_applier.apply_fixes(source_file, fixes):
107+
no_fixables = all(not diag.rule.fixable for diag in diagnostics)
108+
if no_fixables or not (source_file.config.linter.fix or source_file.config.linter.diff):
110109
self.diagnostics.extend(diagnostics)
111110
files += 1
112111
cached_files += 1
@@ -116,7 +115,8 @@ def run(self) -> list[Diagnostic]:
116115
continue
117116
self.diagnostics.extend(diagnostics)
118117
files += 1
119-
self.config_manager.cache.set_linter_entry(source_file.path, source_file.config.hash(), diagnostics)
118+
if not source_file.config.linter.diff: # diff simulate fixes, so it's best to ignore the results
119+
self.config_manager.cache.set_linter_entry(source_file.path, source_file.config.hash(), diagnostics)
120120
self.config_manager.cache.save()
121121

122122
if not files and not self.config_manager.default_config.silent:

tests/cache/test_cache_integration.py

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import shutil
66
import time
77
from pathlib import Path
8+
from textwrap import dedent
89

910
import msgpack
1011

@@ -244,6 +245,66 @@ def test_cache_with_multiple_files(self, tmp_path, capsys):
244245
assert "Used cached results for 1 of 2 files" in out2
245246
assert len(second_result) == first_count # Results unchanged (just added comment)
246247

248+
def test_cache_with_fixable_rule(self, tmp_path, capsys):
249+
"""Test cache behavior with fixable rules."""
250+
# Arrange - create tedt file for fixable rule
251+
test_file = tmp_path / "test.robot"
252+
content = dedent("""
253+
*** Settings ***
254+
Suite Setup With Trailing Whitespace
255+
""").lstrip() # noqa: W291
256+
test_file.write_text(content, encoding="utf-8")
257+
258+
with working_directory(tmp_path):
259+
# Act - first run to populate cache
260+
result1 = check_files(return_result=True, verbose=True)
261+
out1, _ = capsys.readouterr()
262+
# Act - second run to regain fixable rules
263+
result2 = check_files(return_result=True, verbose=True)
264+
out2, _ = capsys.readouterr()
265+
# Act - third run that shows fix diff
266+
result3 = check_files(return_result=True, verbose=True, diff=True)
267+
out3, _ = capsys.readouterr()
268+
# Act - fourth run that has all issues (diff does not change it)
269+
result4 = check_files(return_result=True, verbose=True)
270+
out4, _ = capsys.readouterr()
271+
# Act - fifth run that fixes the issue
272+
result5 = check_files(return_result=True, verbose=True, fix=True)
273+
out5, _ = capsys.readouterr()
274+
# Act - sixth run that has only remaining issues
275+
result6 = check_files(return_result=True, verbose=True)
276+
out6, _ = capsys.readouterr()
277+
278+
# Assert - first run not yet cached, all issues
279+
assert "Used cached results" not in out1
280+
assert len(result1) == 3
281+
assert "1 fixable with the ``--fix`` option." in out1
282+
283+
# Assert - second run cached, all issues
284+
assert "Used cached results" in out2
285+
assert len(result2) == 3
286+
assert "1 fixable with the ``--fix`` option." in out2
287+
288+
# Assert - third run not cached, only unfixable issues in results (fixable goes to diff)
289+
assert "Used cached results" not in out3 # we didn't use cache since we generated diff
290+
assert len(result3) == 2
291+
assert "Found 3 issues (1 fixed, 2 remaining)." in out3 # it is printed but not applied (diff)
292+
293+
# Assert - fourth run cached, all issues (no changes from diff)
294+
assert "Used cached results" in out4
295+
assert len(result4) == 3
296+
assert "1 fixable with the ``--fix`` option." in out4
297+
298+
# Assert - fifth run not cached, only unfixable issues in results (fixable are fixed)
299+
assert "Used cached results" not in out5
300+
assert len(result5) == 2
301+
assert "Found 3 issues (1 fixed, 2 remaining)." in out5
302+
303+
# Assert - sixth run cached, only unfixable issues (fixable were fixed)
304+
assert "Used cached results" in out6
305+
assert len(result6) == 2
306+
assert "Found 2 issues." in out6
307+
247308
def test_file_with_parse_error_not_cached(self, tmp_path, capsys):
248309
"""Test that files with parse errors are not cached."""
249310
# Arrange - create file with actual encoding/binary issues that cause DataError

0 commit comments

Comments
 (0)