Skip to content

Commit

Permalink
Add ability to enable/disable checks: (#41)
Browse files Browse the repository at this point in the history
There is now a new "enabled" flag which can be used to skip the loading of a
check unless the user explictly opts into the check. By default, all checks
are enabled unless the author disables them.

I only added --enable, since adding --disable will require some additional
logic for parsing. Either way, --ignore does almost the same thing, but --disable
would prevent the check from even being loaded, whereas --ignore only suppresses
an error.
  • Loading branch information
dosisod authored Oct 4, 2022
1 parent 6958d81 commit 1ede2fe
Show file tree
Hide file tree
Showing 7 changed files with 103 additions and 8 deletions.
1 change: 1 addition & 0 deletions refurb/error.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ def __str__(self) -> str:

@dataclass
class Error:
enabled: ClassVar[bool] = True
prefix: ClassVar[str] = "FURB"
code: ClassVar[int]
line: int
Expand Down
16 changes: 12 additions & 4 deletions refurb/loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

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

Check = Callable[[Node, list[Error]], None]

Expand Down Expand Up @@ -40,15 +41,22 @@ def get_error_class(module: ModuleType) -> type[Error] | None:
return None


def load_checks(
ignore: set[ErrorCode], paths: list[str]
) -> defaultdict[Type[Node], list[Check]]:
def load_checks(settings: Settings) -> defaultdict[Type[Node], list[Check]]:
found: defaultdict[Type[Node], list[Check]] = defaultdict(list)
ignore = settings.ignore or set()
paths = settings.load or []
enabled = settings.enable or set()

for module in get_modules(paths):
error = get_error_class(module)
if not error:
continue

error_code = ErrorCode.from_error(error)

is_enabled = error.enabled or error_code in enabled

if not error or ErrorCode.from_error(error) in ignore:
if not is_enabled or error_code in ignore:
continue

if func := getattr(module, "check", None):
Expand Down
2 changes: 1 addition & 1 deletion refurb/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def run_refurb(settings: Settings) -> Sequence[Error | str]:
if settings.debug:
errors.append(str(tree))

checks = load_checks(settings.ignore or set(), settings.load or [])
checks = load_checks(settings)
visitor = RefurbVisitor(checks)

tree.accept(visitor)
Expand Down
19 changes: 19 additions & 0 deletions refurb/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class Settings:
explain: ErrorCode | None = None
ignore: set[ErrorCode] | None = None
load: list[str] | None = None
enable: set[ErrorCode] | None = None
debug: bool = False
generate: bool = False
help: bool = False
Expand Down Expand Up @@ -45,8 +46,13 @@ def parse_config_file(contents: str) -> Settings:
parse_error_id(str(x)) for x in settings.get("ignore", [])
)

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

return Settings(
ignore=ignore or None,
enable=enable or None,
load=settings.get("load"),
quiet=settings.get("quiet", False),
)
Expand All @@ -67,6 +73,7 @@ def parse_command_line_args(args: list[str]) -> Settings:
iargs = iter(args)
files: list[str] = []
ignore: set[ErrorCode] = set()
enable: set[ErrorCode] = set()
load: list[str] = []
explain: ErrorCode | None = None
debug = False
Expand Down Expand Up @@ -95,6 +102,14 @@ def parse_command_line_args(args: list[str]) -> Settings:

ignore.add(parse_error_id(value))

elif arg == "--enable":
value = next(iargs, None)

if value is None:
raise ValueError(f'refurb: missing argument after "{arg}"')

enable.add(parse_error_id(value))

elif arg == "--load":
value = next(iargs, None)

Expand All @@ -112,6 +127,7 @@ def parse_command_line_args(args: list[str]) -> Settings:
return Settings(
files=files or None,
ignore=ignore or None,
enable=enable or None,
load=load or None,
debug=debug,
explain=explain,
Expand All @@ -123,6 +139,9 @@ def merge_settings(command_line: Settings, config_file: Settings) -> Settings:
if not command_line.ignore:
command_line.ignore = config_file.ignore

if not command_line.enable:
command_line.enable = config_file.enable

if not command_line.load:
command_line.load = config_file.load

Expand Down
17 changes: 17 additions & 0 deletions test/custom_checks/disabled_check.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
from dataclasses import dataclass

from mypy.nodes import MypyFile

from refurb.error import Error


@dataclass
class ErrorInfo(Error):
enabled = False
prefix = "XYZ"
code = 999
msg: str = "This message is disabled by default"


def check(node: MypyFile, errors: list[Error]) -> None:
errors.append(ErrorInfo(node.line, node.column))
30 changes: 27 additions & 3 deletions test/test_arg_parsing.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,20 @@ def test_parse_ignore_check_missing_arg() -> None:
parse_args(["--ignore"])


def test_parse_enable() -> None:
got = parse_args(["--enable", "FURB123", "--enable", "321"])
expected = Settings(enable=set((ErrorCode(123), ErrorCode(321))))

assert got == expected


def test_parse_enable_check_missing_arg() -> None:
with pytest.raises(
ValueError, match='refurb: missing argument after "--enable"'
):
parse_args(["--enable"])


def test_debug_parsing() -> None:
assert parse_args(["--debug", "file"]) == Settings(
files=["file"], debug=True
Expand Down Expand Up @@ -98,12 +112,15 @@ def test_parse_config_file() -> None:
[tool.refurb]
load = ["some", "folders"]
ignore = [100, "FURB101"]
enable = ["FURB111", "FURB222"]
"""

config = parse_config_file(contents)

assert config == Settings(
load=["some", "folders"], ignore=set((ErrorCode(100), ErrorCode(101)))
load=["some", "folders"],
ignore=set((ErrorCode(100), ErrorCode(101))),
enable=set((ErrorCode(111), ErrorCode(222))),
)


Expand Down Expand Up @@ -131,14 +148,21 @@ def test_command_line_args_override_config_file() -> None:
[tool.refurb]
load = ["some", "folders"]
ignore = [100, "FURB101"]
enable = ["FURB111", "FURB222"]
"""

command_line_args = parse_args(["--load", "x", "--ignore", "123"])
command_line_args = parse_args(
["--load", "x", "--ignore", "123", "--enable", "FURB200"]
)
config_file = parse_config_file(contents)

config = merge_settings(command_line_args, config_file)

assert config == Settings(load=["x"], ignore=set((ErrorCode(123),)))
assert config == Settings(
load=["x"],
ignore=set((ErrorCode(123),)),
enable=set((ErrorCode(200),)),
)


def test_config_missing_ignore_option_is_allowed() -> None:
Expand Down
26 changes: 26 additions & 0 deletions test/test_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,29 @@ def test_system_exit_is_caught() -> None:
assert errors == [
"refurb: There are no .py[i] files in directory 'test/e2e/empty_package'"
]


DISABLED_CHECK = "test.custom_checks.disabled_check"


def test_disabled_check_is_not_ran_by_default() -> None:
errors = run_refurb(
Settings(files=["test/e2e/dummy.py"], load=[DISABLED_CHECK])
)

assert not errors


def test_disabled_check_ran_if_explicitly_enabled() -> None:
errors = run_refurb(
Settings(
files=["test/e2e/dummy.py"],
load=[DISABLED_CHECK],
enable=set((ErrorCode(prefix="XYZ", id=999),)),
)
)

expected = "test/e2e/dummy.py:1:1 [XYZ999]: This message is disabled by default" # noqa: E501

assert len(errors) == 1
assert str(errors[0]) == expected

0 comments on commit 1ede2fe

Please sign in to comment.