Skip to content
New issue

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

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

Already on GitHub? # to your account

Add ability to enable/disable errors based on category #123

Merged
merged 1 commit into from
Nov 15, 2022
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
7 changes: 7 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,13 @@ fields are applied afterwards.
> Note that `disable_all` and `enable_all` are mutually exclusive, both on the command line and in
> the config file. You will get an error if you try to specify both.

You can also disable checks by category using the `#category` syntax. For example, `--disable "#readability"`
will disable all checks with the `readability` category. The same applies for `enable` and `ignore`.
Also, if you disable an entire category you can still explicitly re-enable a check in that category.

> Note that `#readability` is wrapped in quotes because your shell will interpret the `#` as the
> start of a comment.

## Setting Python Version

Use the `--python-version` flag to tell Refurb which version of Python your codebase is using. This
Expand Down
1 change: 1 addition & 0 deletions refurb/checks/builtin/use_max.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class ErrorInfo(Error):
"""

code = 136
categories = ["builtin", "logical", "readability"]


FUNC_TABLE = {
Expand Down
9 changes: 7 additions & 2 deletions refurb/error.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from __future__ import annotations

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


@dataclass(frozen=True)
Expand All @@ -17,16 +17,21 @@ def __str__(self) -> str:
return f"{self.prefix}{self.id}"


ErrorCategory = NewType("ErrorCategory", str)

ErrorClassifier = ErrorCategory | ErrorCode


@dataclass
class Error:
enabled: ClassVar[bool] = True
prefix: ClassVar[str] = "FURB"
categories: ClassVar[list[str]] = []
code: ClassVar[int]
line: int
column: int
msg: str
filename: str | None = None
categories: list[str] | None = None

def __str__(self) -> str:
return f"{self.filename}:{self.line}:{self.column + 1} [{self.prefix}{self.code}]: {self.msg}" # noqa: E501
36 changes: 24 additions & 12 deletions refurb/loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,14 @@ def get_modules(paths: list[str]) -> Generator[ModuleType, None, None]:


def is_valid_error_class(obj: Any) -> TypeGuard[type[Error]]: # type: ignore
return issubclass(obj, Error)
name = obj.__name__
ignored_names = ("Error", "ErrorCode", "ErrorCategory")

return (
name.startswith("Error")
and name not in ignored_names
and issubclass(obj, Error)
)


def get_error_class(module: ModuleType) -> type[Error] | None:
Expand All @@ -68,15 +75,22 @@ def get_error_class(module: ModuleType) -> type[Error] | None:
return None


def should_skip_loading_check(settings: Settings, error: type[Error]) -> bool:
def should_load_check(settings: Settings, error: type[Error]) -> bool:
error_code = ErrorCode.from_error(error)

error_is_disabled = settings.disable_all or not error.enabled
should_enable = settings.enable_all or error_code in settings.enable
if error_code in settings.enable:
return True

if error_code in (settings.disable | settings.ignore):
return False

if settings.enable.intersection(error.categories):
return True

in_disable_list = error_code in (settings.ignore | settings.disable)
if settings.disable.intersection(error.categories) or settings.disable_all:
return False

return (error_is_disabled and not should_enable) or in_disable_list
return error.enabled or settings.enable_all


VALID_NODE_TYPES = set(METHOD_NODE_MAPPINGS.values())
Expand Down Expand Up @@ -155,11 +169,9 @@ def load_checks(settings: Settings) -> defaultdict[type[Node], list[Check]]:
for module in get_modules(settings.load):
error = get_error_class(module)

if not error or should_skip_loading_check(settings, error):
continue

if func := getattr(module, "check", None):
for ty in extract_function_types(func):
found[ty].append(func)
if error and should_load_check(settings, error):
if func := getattr(module, "check", None):
for ty in extract_function_types(func):
found[ty].append(func)

return found
33 changes: 23 additions & 10 deletions refurb/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,17 @@
else:
import tomli as tomllib # pragma: no cover

from .error import ErrorCode
from .error import ErrorCategory, ErrorClassifier, ErrorCode


@dataclass
class Settings:
files: list[str] = field(default_factory=list)
explain: ErrorCode | None = None
ignore: set[ErrorCode] = field(default_factory=set)
ignore: set[ErrorClassifier] = field(default_factory=set)
load: list[str] = field(default_factory=list)
enable: set[ErrorCode] = field(default_factory=set)
disable: set[ErrorCode] = field(default_factory=set)
enable: set[ErrorClassifier] = field(default_factory=set)
disable: set[ErrorClassifier] = field(default_factory=set)
debug: bool = False
generate: bool = False
help: bool = False
Expand Down Expand Up @@ -76,6 +76,14 @@ def merge(old: Settings, new: Settings) -> Settings:
ERROR_ID_REGEX = re.compile("^([A-Z]{3,4})?(\\d{3})$")


def parse_error_classifier(err: str) -> ErrorCategory | ErrorCode:
return parse_error_category(err) or parse_error_id(err)


def parse_error_category(err: str) -> ErrorCategory | None:
return ErrorCategory(err[1:]) if err.startswith("#") else None


def parse_error_id(err: str) -> ErrorCode:
if match := ERROR_ID_REGEX.match(err):
groups = match.groups()
Expand All @@ -100,15 +108,18 @@ def parse_config_file(contents: str) -> Settings:
if tool := config.get("tool"):
if config := tool.get("refurb"):
ignore = set(
parse_error_id(str(x)) for x in config.get("ignore", [])
parse_error_classifier(str(x))
for x in config.get("ignore", [])
)

enable = set(
parse_error_id(str(x)) for x in config.get("enable", [])
parse_error_classifier(str(x))
for x in config.get("enable", [])
)

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

version = config.get("python_version")
Expand Down Expand Up @@ -169,16 +180,18 @@ def get_next_arg(arg: str, args: Iterator[str]) -> str:
settings.explain = parse_error_id(get_next_arg(arg, iargs))

elif arg == "--ignore":
settings.ignore.add(parse_error_id(get_next_arg(arg, iargs)))
settings.ignore.add(
parse_error_classifier(get_next_arg(arg, iargs))
)

elif arg == "--enable":
error_code = parse_error_id(get_next_arg(arg, iargs))
error_code = parse_error_classifier(get_next_arg(arg, iargs))

settings.enable.add(error_code)
settings.disable.discard(error_code)

elif arg == "--disable":
error_code = parse_error_id(get_next_arg(arg, iargs))
error_code = parse_error_classifier(get_next_arg(arg, iargs))

settings.disable.add(error_code)
settings.enable.discard(error_code)
Expand Down
39 changes: 38 additions & 1 deletion test/test_arg_parsing.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import pytest

from refurb.error import ErrorCode
from refurb.error import ErrorCategory, ErrorCode
from refurb.settings import Settings
from refurb.settings import parse_command_line_args as parse_args
from refurb.settings import parse_config_file, parse_error_id
Expand Down Expand Up @@ -57,6 +57,13 @@ def test_parse_ignore() -> None:
assert got == expected


def test_parse_ignore_category() -> None:
got = parse_args(["--ignore", "#category"])
expected = Settings(ignore=set((ErrorCategory("category"),)))

assert got == expected


def test_parse_ignore_check_missing_arg() -> None:
with pytest.raises(
ValueError, match='refurb: missing argument after "--ignore"'
Expand All @@ -71,6 +78,13 @@ def test_parse_enable() -> None:
assert got == expected


def test_parse_enable_category() -> None:
got = parse_args(["--enable", "#category"])
expected = Settings(enable=set((ErrorCategory("category"),)))

assert got == expected


def test_parse_enable_check_missing_arg() -> None:
with pytest.raises(
ValueError, match='refurb: missing argument after "--enable"'
Expand Down Expand Up @@ -228,6 +242,12 @@ def test_disable_error() -> None:
assert settings == Settings(disable=set((ErrorCode(100),)))


def test_disable_error_category() -> None:
settings = parse_args(["--disable", "#category"])

assert settings == Settings(disable=set((ErrorCategory("category"),)))


def test_disable_existing_enabled_error() -> None:
settings = parse_args(["--enable", "FURB100", "--disable", "FURB100"])

Expand Down Expand Up @@ -411,6 +431,23 @@ def test_merging_enable_all_field() -> None:
)


def test_parse_config_file_categories() -> None:
config = """\
[tool.refurb]
enable = ["#category-a"]
disable = ["#category-b"]
ignore = ["#category-c"]
"""

config_file = parse_config_file(config)

assert config_file == Settings(
enable=set((ErrorCategory("category-a"),)),
disable=set((ErrorCategory("category-b"),)),
ignore=set((ErrorCategory("category-c"),)),
)


def test_parse_mypy_extra_args() -> None:
settings = parse_args(["--", "mypy", "args", "here"])

Expand Down
25 changes: 24 additions & 1 deletion test/test_checks.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from pathlib import Path

from refurb.error import Error, ErrorCode
from refurb.error import Error, ErrorCategory, ErrorCode
from refurb.main import run_refurb
from refurb.settings import Settings, parse_command_line_args

Expand Down Expand Up @@ -198,6 +198,29 @@ def test_explicitly_disabled_check_is_ignored_when_enable_all_is_set() -> None:
assert not errors


def test_explicitly_enabled_check_from_disabled_category_is_ran() -> None:
errors = run_refurb(
Settings(
files=["test/data/err_123.py"],
disable=set((ErrorCategory("readability"),)),
enable=set((ErrorCode(123),)),
)
)

assert errors


def test_explicitly_enabled_category_still_runs() -> None:
errors = run_refurb(
Settings(
files=["test/data/err_123.py"],
enable=set((ErrorCategory("readability"),)),
)
)

assert errors


def test_checks_with_python_version_dependant_error_msgs() -> None:
run_checks_in_folder(Path("test/data_3.10"), version=(3, 10))

Expand Down