Merge pull request #4 from samueljsb/fixit-inline

fixit inline

CHANGELOG.md

@@ -6,6 +6,21 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## Unreleased
 
+### Added
+
+- Silence `fixit` errors with *inline* comments.
+
+  `fixit` does not always respect `lint-fixme` comments when they are on the
+  line above the line causing the error. This is a known bug and is reported
+  in https://github.com/Instagram/Fixit/issues/405.
+
+  In some of these cases (e.g. decorators), placing the comment on the same line
+  as the error can ensure it is respected. The `fixme-inline` linter option
+  allows the comments to be added inline instead of on the lien above.
+
+  N.B. This might prevent the comments from being successfully removed by the
+  `fix fixit` command, if there are other errors ignored on the same line.
+
 ## 1.1.0 (2023-11-24)
 
 ### Added

silence_lint_error/comments.py

@@ -0,0 +1,70 @@
+from __future__ import annotations
+
+import tokenize_rt
+
+
+def add_error_silencing_comments(
+        src: str, error_lines: set[int],
+        comment_type: str, error_code: str,
+) -> str:
+    """Add comments to some code to silence linting errors.
+
+    Args:
+        src: The content of the module to add comments to.
+        error_lines: The lines on which to silence errors.
+        comment_type: The type of comment to add (e.g. `noqa` or `lint-fixme`)
+        code: The error code to silence.
+
+    Returns:
+        The content of the module with the additional comments added.
+    """
+    tokens = tokenize_rt.src_to_tokens(src)
+
+    for idx, token in tokenize_rt.reversed_enumerate(tokens):
+        if token.line not in error_lines:
+            continue
+        if not token.src.strip():
+            continue
+
+        if token.name == 'COMMENT':
+            new_comment = add_code_to_comment(token.src, 'noqa', error_code)
+            tokens[idx] = tokens[idx]._replace(src=new_comment)
+        else:
+            tokens.insert(
+                idx+1, tokenize_rt.Token(
+                    'COMMENT', f'# {comment_type}: {error_code}',
+                ),
+            )
+            tokens.insert(idx+1, tokenize_rt.Token('UNIMPORTANT_WS', '  '))
+
+        error_lines.remove(token.line)
+
+    return tokenize_rt.tokens_to_src(tokens)
+
+
+def add_noqa_comments(src: str, lines: set[int], error_code: str) -> str:
+    """Add `noqa` comments to some code.
+
+    Args:
+        src: The content of the module to add `noqa` comments to.
+        lines: The lines on which to add `noqa` coments.
+        code: The error code to silence.
+
+    Returns:
+        The content of the module with the additional comments added.
+    """
+    return add_error_silencing_comments(src, lines, 'noqa', error_code)
+
+
+def add_code_to_comment(comment: str, comment_type: str, code: str) -> str:
+    """Add to a comment to make it a error-silencing comment.
+
+    If the comment already includes an error silencing section of the same type, this
+    will add the code to the list of silenced errors.
+    """
+    if f'{comment_type}: ' in comment:
+        return comment.replace(
+            f'{comment_type}: ', f'{comment_type}: {code},', 1,
+        )
+    else:
+        return comment + f'  # {comment_type}: {code}'

silence_lint_error/silence_lint_error.py

@@ -13,7 +13,7 @@
 
 import attrs
 
-from . import noqa
+from . import comments
 
 if TYPE_CHECKING:
     from typing import TypeAlias
@@ -114,6 +114,28 @@ def silence_violations(
         return ''.join(new_lines)
 
 
+class FixitInline(Fixit):
+    """An alternative `fixit` implementation that adds `lint-fixme` comment inline.
+
+    This is sometimes necessary because `fixit` does not always respect `lint-fixme`
+    comments when they are on the line above the line causing the error. This is a
+    known bug and is reported in https://github.com/Instagram/Fixit/issues/405.
+
+    In some of these cases, placing the comment on the same line as the error can
+    ensure it is respected (e.g. for decorators).
+    """
+
+    def silence_violations(
+        self, src: str, violations: Sequence[Violation],
+    ) -> str:
+        [rule_name] = {violation.rule_name for violation in violations}
+        linenos_to_silence = {violation.lineno for violation in violations}
+        return comments.add_error_silencing_comments(
+            src, linenos_to_silence,
+            'lint-fixme', rule_name,
+        )
+
+
 class Flake8:
     name = 'flake8'
 
@@ -147,7 +169,7 @@ def silence_violations(
     ) -> str:
         [rule_name] = {violation.rule_name for violation in violations}
         linenos_to_silence = {violation.lineno for violation in violations}
-        return noqa.add_noqa_comments(src, linenos_to_silence, rule_name)
+        return comments.add_noqa_comments(src, linenos_to_silence, rule_name)
 
 
 class Ruff:
@@ -188,11 +210,12 @@ def silence_violations(
     ) -> str:
         [rule_name] = {violation.rule_name for violation in violations}
         linenos_to_silence = {violation.lineno for violation in violations}
-        return noqa.add_noqa_comments(src, linenos_to_silence, rule_name)
+        return comments.add_noqa_comments(src, linenos_to_silence, rule_name)
 
 
 LINTERS: dict[str, type[Linter]] = {
     'fixit': Fixit,
+    'fixit-inline': FixitInline,
     'flake8': Flake8,
     'ruff': Ruff,
 }

tests/noqa_test.py → tests/comments_test.py

@@ -2,8 +2,8 @@
 
 import pytest
 
-from silence_lint_error.noqa import add_code_to_comment
-from silence_lint_error.noqa import add_noqa_comments
+from silence_lint_error.comments import add_code_to_comment
+from silence_lint_error.comments import add_noqa_comments
 
 
 def test_add_noqa_comments():
@@ -54,4 +54,4 @@ def baz(  # noqa: ABC123
     ),
 )
 def test_add_code_to_comment(original, expected):
-    assert add_code_to_comment(original, 'ABC1') == expected
+    assert add_code_to_comment(original, 'noqa', 'ABC1') == expected

tests/silence_lint_error/fixit_test.py

@@ -139,6 +139,42 @@ def f(x):
 """
 
 
+def test_main_inline(tmp_path: Path, capsys: pytest.CaptureFixture[str]):
+    python_module = tmp_path / 't.py'
+    python_module.write_text(
+        """\
+x = None
+isinstance(x, str) or isinstance(x, int)
+
+def f(x):
+    return isinstance(x, str) or isinstance(x, int)
+""",
+    )
+
+    ret = main(
+        ('fixit-inline', 'fixit.rules:CollapseIsinstanceChecks', str(python_module)),
+    )
+
+    assert ret == 1
+    assert python_module.read_text() == """\
+x = None
+isinstance(x, str) or isinstance(x, int)  # lint-fixme: CollapseIsinstanceChecks
+
+def f(x):
+    return isinstance(x, str) or isinstance(x, int)  # lint-fixme: CollapseIsinstanceChecks
+"""  # noqa: B950
+
+    captured = capsys.readouterr()
+    assert captured.out == f"""\
+{python_module}
+"""
+    assert captured.err == """\
+-> finding errors with fixit
+found errors in 1 files
+-> adding comments to silence errors
+"""
+
+
 def test_main_no_violations(
         tmp_path: Path, capsys: pytest.CaptureFixture[str],
 ):