diff --git a/docs/categories.md b/docs/categories.md new file mode 100644 index 0000000..c97feac --- /dev/null +++ b/docs/categories.md @@ -0,0 +1,100 @@ +# Categories + +Here is a list of the built-in categories in Refurb, and their meanings. + +## `builtin` + +Checks that have the `builtin` category cover a few different topics: + +* Built-in functions such as `print()`, `open()`, `str()`, and so on +* Statements such as `del` +* File system related operations such as `open()` and `readlines()` + +## `control-flow` + +These checks deal with the control flow of a program, such as optimizing usage +of `return` and `continue`, removing `if` statements under certain conditions, +and so on. + +## `contextlib` + +These checks are for the [contextlib](https://docs.python.org/3/library/contextlib.html) +standard library module. + +## `dict` + +These checks cover: + +* Usage of `dict` objects +* In some cases, objects supporting the [Mapping](https://docs.python.org/3/library/collections.abc.html#collections.abc.Mapping) protocol + +## `fstring` + +These checks relate to Python's [f-strings](https://fstring.help/). + +## `functools` + +These checks relate to the [functools](https://docs.python.org/3/library/functools.html) +standard library module. + +## `iterable` + +These checks cover: + +* Iterable types such as `list` and `tuple` +* Standard library objects which are commonly iterated over such as `dict` keys + +## `itertools` + +These checks relate to the [itertools](https://docs.python.org/3/library/itertools.html) +standard library module. + +## `operator` + +These checks relate to the [operator](https://docs.python.org/3/library/operator.html) +standard library module. + +## `logical` + +These checks relate to logical cleanups and optimizations, primarily in `if` statements, +but also in boolean expressions. + +## `list` + +These checks cover usage of the built-in `list` object. + +## `pathlib` + +These checks relate to the [pathlib](https://docs.python.org/3/library/pathlib.html) +standard library module. + +## `pythonic` + +This is a general catch-all for things which are "unpythonic". It differs from the +`readability` category because "unreadable" code can still be pythonic. + +## `readability` + +These checks aim to make existing code more readable. This can be subjective, but in general, +they reduce the horizontal or vertical length of your code, or make the underlying meaning +of the code more apparent. + +## `scoping` + +These checks have to do with Python's scoping rules. For more info on how Python's scoping +rules work, read [this article](https://realpython.com/python-scope-legb-rule/). + +## `string` + +These checks deal with usage of [`str`](https://docs.python.org/3/library/stdtypes.html#string-methods) +objects in Python. + +## `set` + +These checks deal with usage of [`set`](https://docs.python.org/3/tutorial/datastructures.html#sets) +objects in Python. + +## `truthy` + +These checks cover truthy and falsy operations in Python, primarily in the context of `assert` and `if` +statements. diff --git a/refurb/checks/builtin/list_extend.py b/refurb/checks/builtin/list_extend.py index ad091ad..9e0fd69 100644 --- a/refurb/checks/builtin/list_extend.py +++ b/refurb/checks/builtin/list_extend.py @@ -42,6 +42,7 @@ class ErrorInfo(Error): code = 113 msg: str = "Use `x.extend(...)` instead of repeatedly calling `x.append()`" + categories = ["list"] @dataclass diff --git a/refurb/checks/builtin/no_del.py b/refurb/checks/builtin/no_del.py index 87f916c..18a7913 100644 --- a/refurb/checks/builtin/no_del.py +++ b/refurb/checks/builtin/no_del.py @@ -39,6 +39,7 @@ class ErrorInfo(Error): """ code = 131 + categories = ["builtin", "readability"] def check(node: DelStmt, errors: list[Error]) -> None: diff --git a/refurb/checks/builtin/no_ignored_dict_items.py b/refurb/checks/builtin/no_ignored_dict_items.py index 7f43011..9e80394 100644 --- a/refurb/checks/builtin/no_ignored_dict_items.py +++ b/refurb/checks/builtin/no_ignored_dict_items.py @@ -48,6 +48,7 @@ class ErrorInfo(Error): """ code = 135 + categories = ["dict"] def check( diff --git a/refurb/checks/builtin/print.py b/refurb/checks/builtin/print.py index d506a28..d8a437a 100644 --- a/refurb/checks/builtin/print.py +++ b/refurb/checks/builtin/print.py @@ -13,6 +13,7 @@ class ErrorInfo(Error): code = 105 msg: str = 'Replace `print("")` with `print()`' + categories = ["builtin", "readability"] def check(node: CallExpr, errors: list[Error]) -> None: diff --git a/refurb/checks/builtin/set_discard.py b/refurb/checks/builtin/set_discard.py index e42e018..967619f 100644 --- a/refurb/checks/builtin/set_discard.py +++ b/refurb/checks/builtin/set_discard.py @@ -41,6 +41,7 @@ class ErrorInfo(Error): code = 132 msg: str = "Replace `if x in s: s.remove(x)` with `s.discard(x)`" + categories = ["readability", "set"] def check(node: IfStmt, errors: list[Error]) -> None: diff --git a/refurb/checks/builtin/use_isinstance_tuple.py b/refurb/checks/builtin/use_isinstance_tuple.py index 8a8a8ba..2981076 100644 --- a/refurb/checks/builtin/use_isinstance_tuple.py +++ b/refurb/checks/builtin/use_isinstance_tuple.py @@ -38,6 +38,7 @@ class ErrorInfo(Error): """ code = 121 + categories = ["readability"] def check(node: OpExpr, errors: list[Error], settings: Settings) -> None: diff --git a/refurb/checks/builtin/writelines.py b/refurb/checks/builtin/writelines.py index 07c2807..7223771 100644 --- a/refurb/checks/builtin/writelines.py +++ b/refurb/checks/builtin/writelines.py @@ -49,6 +49,7 @@ class ErrorInfo(Error): code = 122 msg: str = "Replace `for line in lines: f.write(line)` with `f.writelines(lines)`" # noqa: E501 + categories = ["builtin", "readability"] def check(node: WithStmt, errors: list[Error]) -> None: diff --git a/refurb/checks/contextlib/with_suppress.py b/refurb/checks/contextlib/with_suppress.py index c0604d3..8ff8500 100644 --- a/refurb/checks/contextlib/with_suppress.py +++ b/refurb/checks/contextlib/with_suppress.py @@ -32,6 +32,7 @@ class ErrorInfo(Error): """ code = 107 + categories = ["contextlib", "readability"] def check(node: TryStmt, errors: list[Error]) -> None: diff --git a/refurb/checks/flow/no_trailing_continue.py b/refurb/checks/flow/no_trailing_continue.py index 999dc7d..4c63b8d 100644 --- a/refurb/checks/flow/no_trailing_continue.py +++ b/refurb/checks/flow/no_trailing_continue.py @@ -61,6 +61,7 @@ def func2(x): code = 133 msg: str = "Continue is redundant here" + categories = ["control-flow", "readability"] def get_trailing_continue(node: Statement) -> Generator[Statement, None, None]: diff --git a/refurb/checks/flow/no_trailing_return.py b/refurb/checks/flow/no_trailing_return.py index 91469d4..3afb4b8 100644 --- a/refurb/checks/flow/no_trailing_return.py +++ b/refurb/checks/flow/no_trailing_return.py @@ -56,6 +56,7 @@ def func2(x): code = 125 msg: str = "Return is redundant here" + categories = ["control-flow", "readability"] def get_trailing_return(node: Statement) -> Generator[Statement, None, None]: diff --git a/refurb/checks/flow/no_with_assign.py b/refurb/checks/flow/no_with_assign.py index 47568f9..eca9be8 100644 --- a/refurb/checks/flow/no_with_assign.py +++ b/refurb/checks/flow/no_with_assign.py @@ -41,6 +41,7 @@ class ErrorInfo(Error): code = 127 msg: str = "This variable is redeclared later, and can be removed here" + categories = ["readability", "scoping"] def check(node: Block | MypyFile, errors: list[Error]) -> None: diff --git a/refurb/checks/flow/simplify_return.py b/refurb/checks/flow/simplify_return.py index 1c14e2d..c137a9c 100644 --- a/refurb/checks/flow/simplify_return.py +++ b/refurb/checks/flow/simplify_return.py @@ -57,6 +57,7 @@ def is_on_axis(position: tuple[int, int]) -> bool: """ code = 126 + categories = ["control-flow", "readability"] def get_trailing_return(node: Statement) -> Statement | None: diff --git a/refurb/checks/functools/use_cache.py b/refurb/checks/functools/use_cache.py index 7e9a0ac..0d403fd 100644 --- a/refurb/checks/functools/use_cache.py +++ b/refurb/checks/functools/use_cache.py @@ -35,6 +35,7 @@ def f(x: int) -> int: code = 134 msg: str = "Replace `@lru_cache(maxsize=None)` with `@cache`" + categories = ["functools", "readability"] def check(node: Decorator, errors: list[Error], settings: Settings) -> None: diff --git a/refurb/checks/iterable/implicit_readlines.py b/refurb/checks/iterable/implicit_readlines.py index 0c24c93..c262a37 100644 --- a/refurb/checks/iterable/implicit_readlines.py +++ b/refurb/checks/iterable/implicit_readlines.py @@ -39,6 +39,7 @@ class ErrorInfo(Error): code = 129 msg: str = "Replace `f.readlines()` with `f`" + categories = ["builtin", "readability"] def get_readline_file_object(expr: Expression) -> NameExpr | None: diff --git a/refurb/checks/iterable/in_tuple.py b/refurb/checks/iterable/in_tuple.py index 314099d..da16ecc 100644 --- a/refurb/checks/iterable/in_tuple.py +++ b/refurb/checks/iterable/in_tuple.py @@ -31,6 +31,7 @@ class ErrorInfo(Error): """ code = 109 + categories = ["iterable", "readability"] def error_msg(oper: str) -> str: diff --git a/refurb/checks/logical/use_equal_chain.py b/refurb/checks/logical/use_equal_chain.py index 65bbb5c..8049d5c 100644 --- a/refurb/checks/logical/use_equal_chain.py +++ b/refurb/checks/logical/use_equal_chain.py @@ -32,6 +32,7 @@ class ErrorInfo(Error): code = 124 msg: str = "Replace `x == y and x == z` with `x == y == z`" + categories = ["logical", "readability"] def has_common_expr(*exprs: Expression) -> bool: diff --git a/refurb/checks/logical/use_in.py b/refurb/checks/logical/use_in.py index 670cdba..95a6e57 100644 --- a/refurb/checks/logical/use_in.py +++ b/refurb/checks/logical/use_in.py @@ -33,6 +33,7 @@ class ErrorInfo(Error): code = 108 msg: str = "Replace `x == y or x == z` with `x in (y, z)`" + categories = ["logical", "readability"] def check(node: OpExpr, errors: list[Error]) -> None: diff --git a/refurb/checks/logical/use_or.py b/refurb/checks/logical/use_or.py index 9bb648c..bee2960 100644 --- a/refurb/checks/logical/use_or.py +++ b/refurb/checks/logical/use_or.py @@ -29,6 +29,7 @@ class ErrorInfo(Error): code = 110 msg: str = "Replace `x if x else y` with `x or y`" + categories = ["logical", "readability"] def check(node: ConditionalExpr, errors: list[Error]) -> None: diff --git a/refurb/checks/pathlib/cwd.py b/refurb/checks/pathlib/cwd.py index 65827d9..f94aeda 100644 --- a/refurb/checks/pathlib/cwd.py +++ b/refurb/checks/pathlib/cwd.py @@ -25,6 +25,7 @@ class ErrorInfo(Error): code = 104 msg: str = "Replace `os.getcwd()` with `Path.cwd()`" + categories = ["pathlib"] def check(node: CallExpr, errors: list[Error]) -> None: diff --git a/refurb/checks/pathlib/open.py b/refurb/checks/pathlib/open.py index e583d4f..c1befba 100644 --- a/refurb/checks/pathlib/open.py +++ b/refurb/checks/pathlib/open.py @@ -32,6 +32,7 @@ class ErrorInfo(Error): """ code = 117 + categories = ["pathlib"] def check(node: CallExpr, errors: list[Error]) -> None: diff --git a/refurb/checks/pathlib/read_text.py b/refurb/checks/pathlib/read_text.py index 766f7b5..9f97d26 100644 --- a/refurb/checks/pathlib/read_text.py +++ b/refurb/checks/pathlib/read_text.py @@ -35,6 +35,7 @@ class ErrorInfo(Error): """ code = 101 + categories = ["pathlib"] def check(node: WithStmt, errors: list[Error]) -> None: diff --git a/refurb/checks/pathlib/with_suffix.py b/refurb/checks/pathlib/with_suffix.py index 66481f6..9f3495f 100644 --- a/refurb/checks/pathlib/with_suffix.py +++ b/refurb/checks/pathlib/with_suffix.py @@ -36,6 +36,7 @@ class ErrorInfo(Error): code = 100 msg: str = "Use `Path(x).with_suffix(y)` instead of slice and concat" # noqa: E501 + categories = ["pathlib"] def check(node: OpExpr, errors: list[Error]) -> None: diff --git a/refurb/checks/pathlib/write_text.py b/refurb/checks/pathlib/write_text.py index 45e6e49..636f77f 100644 --- a/refurb/checks/pathlib/write_text.py +++ b/refurb/checks/pathlib/write_text.py @@ -34,6 +34,7 @@ class ErrorInfo(Error): """ code = 103 + categories = ["pathlib"] def check(node: WithStmt, errors: list[Error]) -> None: diff --git a/refurb/checks/readability/in_keys.py b/refurb/checks/readability/in_keys.py index 3e7b3a8..8fe739d 100644 --- a/refurb/checks/readability/in_keys.py +++ b/refurb/checks/readability/in_keys.py @@ -31,6 +31,7 @@ class ErrorInfo(Error): """ code = 130 + categories = ["dict", "readability"] def check(node: ComparisonExpr, errors: list[Error]) -> None: diff --git a/refurb/checks/readability/no_double_not.py b/refurb/checks/readability/no_double_not.py index b99ef0f..d4d5a7b 100644 --- a/refurb/checks/readability/no_double_not.py +++ b/refurb/checks/readability/no_double_not.py @@ -27,6 +27,7 @@ class ErrorInfo(Error): code = 114 msg: str = "Replace `not not x` with `bool(x)`" + categories = ["builtin", "readability", "truthy"] def check(node: UnaryExpr, errors: list[Error]) -> None: diff --git a/refurb/checks/readability/no_len_cmp.py b/refurb/checks/readability/no_len_cmp.py index 4fe5d2c..fdf6bd2 100644 --- a/refurb/checks/readability/no_len_cmp.py +++ b/refurb/checks/readability/no_len_cmp.py @@ -60,6 +60,7 @@ class ErrorInfo(Error): """ code = 115 + categories = ["iterable", "truthy"] CONTAINER_TYPES = { diff --git a/refurb/checks/readability/no_unnecessary_cast.py b/refurb/checks/readability/no_unnecessary_cast.py index 7e659e4..ad763eb 100644 --- a/refurb/checks/readability/no_unnecessary_cast.py +++ b/refurb/checks/readability/no_unnecessary_cast.py @@ -53,6 +53,7 @@ class ErrorInfo(Error): """ code = 123 + categories = ["readability"] FUNC_NAMES = { diff --git a/refurb/checks/readability/use_func_name.py b/refurb/checks/readability/use_func_name.py index d381a60..7f5b905 100644 --- a/refurb/checks/readability/use_func_name.py +++ b/refurb/checks/readability/use_func_name.py @@ -39,6 +39,7 @@ class ErrorInfo(Error): code = 111 msg: str = "Replace `lambda x: f(x)` with `f`" + categories = ["readability"] def get_lambda_arg_names(args: list[Argument]) -> list[str]: diff --git a/refurb/checks/readability/use_literal.py b/refurb/checks/readability/use_literal.py index 318c7b6..25286e2 100644 --- a/refurb/checks/readability/use_literal.py +++ b/refurb/checks/readability/use_literal.py @@ -27,6 +27,7 @@ class ErrorInfo(Error): """ code = 112 + categories = ["pythonic", "readability"] FUNC_NAMES = { diff --git a/refurb/checks/readability/use_operators.py b/refurb/checks/readability/use_operators.py index 368cef6..8c2258c 100644 --- a/refurb/checks/readability/use_operators.py +++ b/refurb/checks/readability/use_operators.py @@ -44,6 +44,7 @@ class ErrorInfo(Error): """ code = 118 + categories = ["operator"] BINARY_OPERATORS = { diff --git a/refurb/checks/readability/use_tuple_swap.py b/refurb/checks/readability/use_tuple_swap.py index 9843137..0a46625 100644 --- a/refurb/checks/readability/use_tuple_swap.py +++ b/refurb/checks/readability/use_tuple_swap.py @@ -29,6 +29,7 @@ class ErrorInfo(Error): code = 128 msg: str = "Use tuple unpacking instead of temporary variables to swap values" # noqa: E501 + categories = ["readability"] def check(node: Block | MypyFile, errors: list[Error]) -> None: diff --git a/refurb/checks/string/expandtabs.py b/refurb/checks/string/expandtabs.py index f17c94b..5486779 100644 --- a/refurb/checks/string/expandtabs.py +++ b/refurb/checks/string/expandtabs.py @@ -38,6 +38,7 @@ class ErrorInfo(Error): enabled = False code = 106 + categories = ["string"] def check_str(node: CallExpr, errors: list[Error]) -> None: diff --git a/refurb/checks/string/fstring_number.py b/refurb/checks/string/fstring_number.py index c8a4e9b..84e322d 100644 --- a/refurb/checks/string/fstring_number.py +++ b/refurb/checks/string/fstring_number.py @@ -27,6 +27,7 @@ class ErrorInfo(Error): """ code = 116 + categories = ["builtin", "fstring"] FUNC_CONVERSIONS = { diff --git a/refurb/checks/string/startswith.py b/refurb/checks/string/startswith.py index c5a2ae7..4a2ea54 100644 --- a/refurb/checks/string/startswith.py +++ b/refurb/checks/string/startswith.py @@ -31,6 +31,7 @@ class ErrorInfo(Error): """ code = 102 + categories = ["string"] def check(node: OpExpr, errors: list[Error]) -> None: diff --git a/refurb/checks/string/use_fstring_fmt.py b/refurb/checks/string/use_fstring_fmt.py index d48fced..624ec5f 100644 --- a/refurb/checks/string/use_fstring_fmt.py +++ b/refurb/checks/string/use_fstring_fmt.py @@ -33,6 +33,7 @@ class ErrorInfo(Error): """ code = 119 + categories = ["builtin", "fstring"] CONVERSIONS = { diff --git a/refurb/error.py b/refurb/error.py index d75e97a..b1a26ec 100644 --- a/refurb/error.py +++ b/refurb/error.py @@ -1,7 +1,7 @@ from __future__ import annotations from dataclasses import dataclass -from typing import ClassVar, Optional +from typing import ClassVar @dataclass(frozen=True) @@ -25,7 +25,8 @@ class Error: line: int column: int msg: str - filename: Optional[str] = None + 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 diff --git a/test/test_category_naming.py b/test/test_category_naming.py new file mode 100644 index 0000000..5a71baf --- /dev/null +++ b/test/test_category_naming.py @@ -0,0 +1,37 @@ +import re +from pathlib import Path + +import refurb +from refurb.loader import get_error_class, get_modules + + +def test_check_categories_are_valid() -> None: + # By "valid" I mean that they are well-defined (in the documentation), and + # are sorted. Basically, parse the documentation file for the categories, + # which includes a list of all categories, and make sure each check only + # uses categories defined in that list. This prevents typos from causing + # a check to have an incorrect category. + + category_docs = Path(refurb.__file__).parent.parent / "docs/categories.md" + + with category_docs.open() as f: + categories = [] + + for line in f: + if name := re.search("## `([a-z-]+)`", line): + categories.append(name.group(1)) + + for module in get_modules([]): + error = get_error_class(module) + + if not error or not error.categories: + continue + + error_msg = f'{module.__file__}: categories for "{error.__name__}" class are not sorted' + + assert sorted(error.categories) == error.categories, error_msg + + for category in error.categories: + assert ( + category in categories + ), f'{module.__file__}: category "{category}" is invalid'