Skip to content

Commit

Permalink
Add ability to ignore errors on a per file/folder basis (#183):
Browse files Browse the repository at this point in the history
Closes #29.

Now you can ignore errors by id and category for a specific file or folder.
This is only allowed in the config file, and uses TOML tables/arrays to specify
which ignores should apply to which paths. All paths are relative to the config
file.

Read the "Ignore Checks Per File/Folder" section of the README for more info
on how to use this new feature.
  • Loading branch information
dosisod authored Jan 10, 2023
1 parent 5ae7c06 commit b39a33f
Show file tree
Hide file tree
Showing 9 changed files with 302 additions and 42 deletions.
27 changes: 27 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,33 @@ You can use the `--config-file` flag to tell Refurb to use a different config fi
default `pyproject.toml` file. Note that it still must be in the same form as the normal
`pyproject.toml` file.

### Ignore Checks Per File/Folder

If you have a large codebase you might want to ignore errors for certain files or folders,
which allows you to incrementally fix errors as you see fit. To do that, add the following
to your `pyproject.toml` file:

```toml
# these settings will be applied globally
[tool.refurb]
enable_all = true

# these will only be applied to the "src" folder
[[tool.refurb.amend]]
path = "src"
ignore = ["FURB123", "FURB120"]

# these will only be applied to the "src/util.py" file
[[tool.refurb.amend]]
path = "src/util.py"
ignore = ["FURB125", "FURB148"]
```

> Note that only the `ignore` field is available in the `amend` sections. This is because
> a check can only be enabled/disabled for the entire codebase, and cannot be selectively
> enabled/disabled on a per-file basis. Assuming a check is enabled though, you can simply
> `ignore` the errors for the files of your choosing.
## Using Refurb With `pre-commit`

You can use Refurb with [pre-commit](https://pre-commit.com/) by adding the following
Expand Down
17 changes: 15 additions & 2 deletions refurb/error.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,22 @@
from __future__ import annotations

from dataclasses import dataclass
from typing import ClassVar, NewType
from pathlib import Path
from typing import ClassVar


@dataclass(frozen=True)
class ErrorCode:
"""
This class represents an error code id which can be used to enable and
disable errors in Refurb. The `path` field is used to tell Refurb that a
particular error should only apply to a given path instead of all paths,
which is the default.
"""

id: int
prefix: str = "FURB"
path: Path | None = None

@classmethod
def from_error(cls, err: type[Error]) -> ErrorCode:
Expand All @@ -17,7 +26,11 @@ def __str__(self) -> str:
return f"{self.prefix}{self.id}"


ErrorCategory = NewType("ErrorCategory", str)
@dataclass(frozen=True)
class ErrorCategory:
value: str
path: Path | None = None


ErrorClassifier = ErrorCategory | ErrorCode

Expand Down
11 changes: 8 additions & 3 deletions refurb/loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from refurb.visitor.mapping import METHOD_NODE_MAPPINGS

from . import checks as checks_module
from .error import Error, ErrorCode
from .error import Error, ErrorCategory, ErrorCode
from .settings import Settings
from .types import Check

Expand Down Expand Up @@ -54,6 +54,9 @@ def get_modules(paths: list[str]) -> Generator[ModuleType, None, None]:


def is_valid_error_class(obj: Any) -> TypeGuard[type[Error]]: # type: ignore
if not hasattr(obj, "__name__"):
return False

name = obj.__name__
ignored_names = ("Error", "ErrorCode", "ErrorCategory")

Expand Down Expand Up @@ -84,10 +87,12 @@ def should_load_check(settings: Settings, error: type[Error]) -> bool:
if error_code in (settings.disable | settings.ignore):
return False

if settings.enable.intersection(error.categories):
categories = {ErrorCategory(cat) for cat in error.categories}

if settings.enable & categories:
return True

if settings.disable.intersection(error.categories) or settings.disable_all:
if settings.disable & categories or settings.disable_all:
return False

return error.enabled or settings.enable_all
Expand Down
44 changes: 40 additions & 4 deletions refurb/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,8 @@ def get_source_lines(filepath: str) -> list[str]:
return Path(filepath).read_text("utf8").splitlines()


def ignored_via_comment(error: Error | str) -> bool:
if isinstance(error, str) or not error.filename:
return False
def is_ignored_via_comment(error: Error) -> bool:
assert error.filename

line = get_source_lines(error.filename)[error.line - 1]

Expand All @@ -82,6 +81,39 @@ def ignored_via_comment(error: Error | str) -> bool:
return False


def is_ignored_via_amend(error: Error, settings: Settings) -> bool:
assert error.filename

path = Path(error.filename).resolve()
error_code = ErrorCode.from_error(type(error))
config_root = (
Path(settings.config_file).parent if settings.config_file else Path()
)

for ignore in settings.ignore:
if ignore.path:
ignore_path = (config_root / ignore.path).resolve()

if path.is_relative_to(ignore_path):
if isinstance(ignore, ErrorCode):
return str(ignore) == str(error_code)

return ignore.value in error.categories

return False


def should_ignore_error(error: Error | str, settings: Settings) -> bool:
if isinstance(error, str):
return False

return (
not error.filename
or is_ignored_via_comment(error)
or is_ignored_via_amend(error, settings)
)


def run_refurb(settings: Settings) -> Sequence[Error | str]:
stdout = StringIO()
stderr = StringIO()
Expand Down Expand Up @@ -140,7 +172,11 @@ def run_refurb(settings: Settings) -> Sequence[Error | str]:
errors += visitor.errors

return sorted(
[error for error in errors if not ignored_via_comment(error)],
[
error
for error in errors
if not should_ignore_error(error, settings)
],
key=sort_errors,
)

Expand Down
92 changes: 59 additions & 33 deletions refurb/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

import re
import sys
from dataclasses import dataclass, field
from dataclasses import dataclass, field, replace
from pathlib import Path
from typing import Iterator
from typing import Any, Iterator

if sys.version_info >= (3, 11):
import tomllib # pragma: no cover
Expand Down Expand Up @@ -102,43 +102,69 @@ def parse_python_version(version: str) -> tuple[int, int]:
raise ValueError("refurb: version must be in form `x.y`")


def parse_amendment( # type: ignore
amendment: dict[str, Any]
) -> set[ErrorClassifier]:
def parse_amend_error(err: str, path: Path) -> ErrorClassifier:
classifier = parse_error_classifier(err)

return replace(classifier, path=path)

match amendment:
case {"path": str(path), "ignore": list(ignored), **extra}:
if extra:
raise ValueError(
'refurb: only "path" and "ignore" fields are supported'
)

return {parse_amend_error(error, Path(path)) for error in ignored}

raise ValueError(
'refurb: "path" or "ignore" fields are missing or malformed'
)


def parse_config_file(contents: str) -> Settings:
config = tomllib.loads(contents)
tool = tomllib.loads(contents).get("tool")

if tool := config.get("tool"):
if config := tool.get("refurb"):
ignore = {
parse_error_classifier(str(x))
for x in config.get("ignore", [])
}
if not tool:
return Settings()

enable = {
parse_error_classifier(str(x))
for x in config.get("enable", [])
}
config = tool.get("refurb")

disable = {
parse_error_classifier(str(x))
for x in config.get("disable", [])
}
if not config:
return Settings()

version = config.get("python_version")
python_version = parse_python_version(version) if version else None
mypy_args = [str(x) for x in config.get("mypy_args", [])]

return Settings(
ignore=ignore,
enable=enable - disable,
disable=disable,
load=config.get("load", []),
quiet=config.get("quiet", False),
disable_all=config.get("disable_all", False),
enable_all=config.get("enable_all", False),
python_version=python_version,
mypy_args=mypy_args,
)
ignore = {parse_error_classifier(str(x)) for x in config.get("ignore", [])}
enable = {parse_error_classifier(str(x)) for x in config.get("enable", [])}

disable = {
parse_error_classifier(str(x)) for x in config.get("disable", [])
}

version = config.get("python_version")
python_version = parse_python_version(version) if version else None
mypy_args = [str(arg) for arg in config.get("mypy_args", [])]

return Settings()
amendments: list[dict[str, Any]] = config.get("amend", []) # type: ignore

if not isinstance(amendments, list):
raise ValueError('refurb: "amend" field(s) must be a TOML table')

for amendment in amendments:
ignore.update(parse_amendment(amendment))

return Settings(
ignore=ignore,
enable=enable - disable,
disable=disable,
load=config.get("load", []),
quiet=config.get("quiet", False),
disable_all=config.get("disable_all", False),
enable_all=config.get("enable_all", False),
python_version=python_version,
mypy_args=mypy_args,
)


def parse_command_line_args(args: list[str]) -> Settings:
Expand Down
3 changes: 3 additions & 0 deletions test/config/amend_config.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[[tool.refurb.amend]]
path = "../data"
ignore = ["FURB123"]
90 changes: 90 additions & 0 deletions test/test_arg_parsing.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from pathlib import Path

import pytest

from refurb.error import ErrorCategory, ErrorCode
Expand Down Expand Up @@ -496,3 +498,91 @@ def test_flags_which_support_comma_separated_cli_args() -> None:
disable=set((ErrorCode(102), ErrorCode(103))),
ignore=set((ErrorCode(104), ErrorCode(105))),
)


def test_parse_amend_file_paths() -> None:
config = """\
[tool.refurb]
ignore = ["FURB100"]
[[tool.refurb.amend]]
path = "some/file/path"
ignore = ["FURB101", "FURB102"]
[[tool.refurb.amend]]
path = "some/other/path"
ignore = ["FURB102", "FURB103"]
"""

config_file = parse_config_file(config)

assert config_file == Settings(
ignore=set(
(
ErrorCode(100),
ErrorCode(101, path=Path("some/file/path")),
ErrorCode(102, path=Path("some/file/path")),
ErrorCode(102, path=Path("some/other/path")),
ErrorCode(103, path=Path("some/other/path")),
)
)
)


def test_invalid_amend_field_fails() -> None:
config = """\
[tool.refurb]
amend = "oops"
"""

msg = r'"amend" field\(s\) must be a TOML table'

with pytest.raises(ValueError, match=msg):
parse_config_file(config)


def test_extra_fields_in_amend_table_fails() -> None:
config = """\
[[tool.refurb.amend]]
path = "some/folder"
ignore = ["FURB123"]
extra = "data"
"""

msg = 'only "path" and "ignore" fields are supported'

with pytest.raises(ValueError, match=msg):
parse_config_file(config)


def test_missing_or_malformed_fields_in_amend_table_fails() -> None:
msg = '"path" or "ignore" fields are missing or malformed'

config = """\
[[tool.refurb.amend]]
ignore = ["FURB123"]
"""

with pytest.raises(ValueError, match=msg):
parse_config_file(config)

config = """\
[[tool.refurb.amend]]
path = "some/folder"
"""

with pytest.raises(ValueError, match=msg):
parse_config_file(config)

config = """\
[[tool.refurb.amend]]
path = true
ignore = false
"""

with pytest.raises(ValueError, match=msg):
parse_config_file(config)


def test_parse_empty_config_file() -> None:
assert parse_config_file("") == Settings()
Loading

0 comments on commit b39a33f

Please sign in to comment.