Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor violation diffing into separate function #399

Merged
merged 2 commits into from
Oct 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 21 additions & 13 deletions src/fixit/engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,26 @@
LOG = logging.getLogger(__name__)


def diff_violation(path: Path, module: Module, violation: LintViolation) -> str:
"""
Generate string diff representation of a violation.
"""

orig = module.code
mod = module.deep_replace( # type:ignore # LibCST#906
violation.node, violation.replacement
)
assert isinstance(mod, Module)
change = mod.code

return unified_diff(
orig,
change,
path.name,
n=1,
)


class LintRunner:
def __init__(self, path: Path, source: FileContent) -> None:
self.path = path
Expand Down Expand Up @@ -87,19 +107,7 @@ def visit_hook(name: str) -> Iterator[None]:
count += 1

if violation.replacement:
orig = self.module.code
mod = self.module.deep_replace( # type:ignore # LibCST#906
violation.node, violation.replacement
)
assert isinstance(mod, Module)
change = mod.code

diff = unified_diff(
orig,
change,
self.path.name,
n=1,
)
diff = diff_violation(self.path, self.module, violation)
violation = replace(violation, diff=diff)

yield violation
Expand Down
8 changes: 2 additions & 6 deletions src/fixit/testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@
from pathlib import Path
from typing import Any, Callable, Collection, Dict, List, Mapping, Sequence, Type, Union

from moreorless import unified_diff

from .engine import LintRunner
from .engine import diff_violation, LintRunner
from .ftypes import Config
from .rule import Invalid, LintRule, Valid

Expand Down Expand Up @@ -112,9 +110,7 @@ def _test_method(

if len(reports) == 1:
# make sure we generated a reasonable diff
expected_diff = unified_diff(
source_code, expected_code, filename=path.name, n=1
)
expected_diff = diff_violation(path, runner.module, reports[0])
self.assertEqual(expected_diff, report.diff)


Expand Down
1 change: 1 addition & 0 deletions src/fixit/tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

from fixit.testing import add_lint_rule_tests_to_module
from .config import ConfigTest
from .engine import EngineTest
from .ftypes import TypesTest
from .rule import RuleTest, RunnerTest
from .smoke import SmokeTest
Expand Down
61 changes: 61 additions & 0 deletions src/fixit/tests/engine.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# Copyright (c) Meta Platforms, Inc. and affiliates.
#
# This source code is licensed under the MIT license found in the
# LICENSE file in the root directory of this source tree.

from pathlib import Path
from textwrap import dedent
from unittest import TestCase

from libcst import (
Call,
ensure_type,
Expr,
parse_module,
SimpleStatementLine,
SimpleString,
)
from libcst.metadata import CodePosition, CodeRange

from ..engine import diff_violation
from ..ftypes import LintViolation


class EngineTest(TestCase):
def test_diff_violation(self):
src = dedent(
"""\
import sys
print("hello world")
"""
)
path = Path("foo.py")
module = parse_module(src)
node = ensure_type(
ensure_type(
ensure_type(module.body[-1], SimpleStatementLine).body[0], Expr
).value,
Call,
).args[0]
repl = node.with_changes(value=SimpleString('"goodnight moon"'))

violation = LintViolation(
"Fake",
CodeRange(CodePosition(1, 1), CodePosition(2, 2)),
message="some error",
node=node,
replacement=repl,
)

expected = dedent(
"""\
--- a/foo.py
+++ b/foo.py
@@ -1,2 +1,2 @@
import sys
-print("hello world")
+print("goodnight moon")
"""
)
result = diff_violation(path, module, violation)
self.assertEqual(expected, result)