From 01e8b85293f2ae8f6de6bef1e420bb5e0f78c100 Mon Sep 17 00:00:00 2001 From: trag1c Date: Mon, 31 Jul 2023 20:49:16 +0200 Subject: [PATCH 1/7] Fixed grammar mistakes --- docs/check-ideas.md | 16 ++++++++-------- docs/checks.md | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/docs/check-ideas.md b/docs/check-ideas.md index 0591cc6..54323ff 100644 --- a/docs/check-ideas.md +++ b/docs/check-ideas.md @@ -5,7 +5,7 @@ be picked up by anyone looking to add new checks to Refurb! ## Pathlib -### Dont use `+` for path traversal, use `/` operator +### Don't use `+` for path traversal, use `/` operator Bad: @@ -66,7 +66,7 @@ class Person: ## String -### Use fstring instead of `+` +### Use f-string instead of `+` Disable by default, will be noisy @@ -78,7 +78,7 @@ We probably only want to check this in `in` expressions, since `x in "abc"` retu as `x in "cba"`, but `x == "abc"[0]` and `x == "cba"[0]` do not. Perhaps there should be a flag for detecting any strings which contains a (permutated) version of a built-in charset. -### Dont use `" ".join(x.capitalize() for x in s.split())`, use `string.capwords(x)` +### Don't use `" ".join(x.capitalize() for x in s.split())`, use `string.capwords(x)` Notes: @@ -146,13 +146,13 @@ See https://docs.python.org/3/library/stdtypes.html See https://docs.python.org/3/library/io.html * Use `frozenset` when `set` is never appended to -* Dont roll your own max/min/sum functions, use `max`/`min`/`sum` instead +* Don't roll your own max/min/sum functions, use `max`/`min`/`sum` instead * `print(f"{x} {y}")` -> `print(x, y)` -* Dont use `_` in expressions +* Don't use `_` in expressions * Use `x ** y` instead of `pow(x, y)` * Unless the `mod` param of `pow` is being used -* Dont call `print()` repeatedly, call `print()` once with multi line string -* Dont call `sys.stderr.write("asdf\n")`, use `print("asdf", file=sys.stderr)` +* Don't call `print()` repeatedly, call `print()` once with multi line string +* Don't call `sys.stderr.write("asdf\n")`, use `print("asdf", file=sys.stderr)` ## Enum @@ -197,4 +197,4 @@ for item in itertools.chain(list1, list2, list3): ## Iteration -### Dont use `x = x[::-1]`, use `x.reverse()` +### Don't use `x = x[::-1]`, use `x.reverse()` diff --git a/docs/checks.md b/docs/checks.md index 228d0cd..875c15d 100644 --- a/docs/checks.md +++ b/docs/checks.md @@ -45,7 +45,7 @@ contents = Path(filename).read_text() Categories: `string` -`startswith()` and `endswith()` both takes a tuple, so instead of calling +`startswith()` and `endswith()` both take a tuple, so instead of calling `startswith()` multiple times on the same string, you can check them all at once: @@ -1928,7 +1928,7 @@ def add_defaults(settings: dict[str, str]) -> dict[str, str]: Categories: `readability` `secrets` -Depending on how you are using the `secrets` module there might be a more +Depending on how you are using the `secrets` module, there might be more expressive ways of writing what it is you're trying to write. Bad: From 3bfad295f6c4adef03204cfc7bb580724bed5794 Mon Sep 17 00:00:00 2001 From: trag1c Date: Mon, 31 Jul 2023 20:49:52 +0200 Subject: [PATCH 2/7] Changed pip3 to pip as it is more common --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 1feedb4..f3e6624 100644 --- a/README.md +++ b/README.md @@ -38,7 +38,7 @@ main.py:16:9 [FURB105]: Use `print() instead of `print("")` Before installing, it is recommended that you setup a [virtual environment](https://docs.python.org/3/tutorial/venv.html). ``` -$ pip3 install refurb +$ pip install refurb $ refurb file.py folder/ ``` @@ -253,7 +253,7 @@ let `pre-commit` find the most recent one for you). Installing plugins for Refurb is very easy: ``` -$ pip3 install refurb-plugin-example +$ pip install refurb-plugin-example ``` Where `refurb-plugin-example` is the name of the plugin. Refurb will automatically load From 83985283675a34958172cb2c7bb06bcf986e1ccd Mon Sep 17 00:00:00 2001 From: trag1c Date: Mon, 31 Jul 2023 20:50:10 +0200 Subject: [PATCH 3/7] Removed implemented idea --- docs/check-ideas.md | 8 -------- 1 file changed, 8 deletions(-) diff --git a/docs/check-ideas.md b/docs/check-ideas.md index 54323ff..cc14ce1 100644 --- a/docs/check-ideas.md +++ b/docs/check-ideas.md @@ -70,14 +70,6 @@ class Person: Disable by default, will be noisy -### Dont hard code charsets (ie, `string.digits` vs `"0123456789"`) - -Python has many built-in charsets in the [string](https://docs.python.org/3/library/string.html) library, and can be used instead of defining them yourself. - -We probably only want to check this in `in` expressions, since `x in "abc"` returns the same thing -as `x in "cba"`, but `x == "abc"[0]` and `x == "cba"[0]` do not. Perhaps there should be a flag for -detecting any strings which contains a (permutated) version of a built-in charset. - ### Don't use `" ".join(x.capitalize() for x in s.split())`, use `string.capwords(x)` Notes: From 12ee83d4543d4438f308c30aa5c9fa34c1266e2f Mon Sep 17 00:00:00 2001 From: trag1c Date: Mon, 31 Jul 2023 20:50:31 +0200 Subject: [PATCH 4/7] Corrected Python version --- docs/check-ideas.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/check-ideas.md b/docs/check-ideas.md index cc14ce1..88b5653 100644 --- a/docs/check-ideas.md +++ b/docs/check-ideas.md @@ -33,9 +33,9 @@ These should be opt-in, since they can be quite noisy. - Include `dict`, `set`, `frozenset`, `defaultdict`, etc - See `TypeApplication` Mypy node -* Convert `Optional[x]` -> `x | None` (python 3.9+?) +* Convert `Optional[x]` -> `x | None` (python 3.10+) -* Convert `Union[x, y]` -> `x | y` (python 3.9+) +* Convert `Union[x, y]` -> `x | y` (python 3.10+) ## Dataclasses From 2705cddd93b6e9609604bcc8cb87211b4b456ebb Mon Sep 17 00:00:00 2001 From: trag1c Date: Mon, 31 Jul 2023 20:51:30 +0200 Subject: [PATCH 5/7] Corrected function/method wording --- docs/checks.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/checks.md b/docs/checks.md index 875c15d..979b571 100644 --- a/docs/checks.md +++ b/docs/checks.md @@ -6,7 +6,7 @@ Categories: `pathlib` A common operation is changing the extension of a file. If you have an existing `Path` object, you don't need to convert it to a string, slice -it, and append a new extension. Instead, use the `with_suffix()` function: +it, and append a new extension. Instead, use the `with_suffix()` method: Bad: @@ -70,7 +70,7 @@ if name.startswith(("b", "B")): Categories: `pathlib` When you just want to save some contents to a file, using a `with` block is -a bit overkill. Instead you can use pathlib's `write_text()` function: +a bit overkill. Instead you can use pathlib's `write_text()` method: Bad: @@ -89,7 +89,7 @@ Path(filename).write_text("hello world") Categories: `pathlib` -A modern alternative to `os.getcwd()` is the `Path.cwd()` function: +A modern alternative to `os.getcwd()` is the `Path.cwd()` method: Bad: @@ -139,7 +139,7 @@ Categories: `contextlib` `readability` Often times you want to handle an exception, and just ignore it. You can do this with a `try/except` block, using a single `pass` in the `except` block, but there is a simpler and more concise way using the `suppress()` -method from `contextlib`: +function from `contextlib`: Bad: @@ -1331,7 +1331,7 @@ if failed: Categories: `pathlib` -Use the `mkdir` function from the pathlib library instead of using the +Use the `mkdir` method from the pathlib library instead of using the `mkdir` and `makedirs` functions from the `os` library: the pathlib library is more modern and provides better flexibility over the construction and manipulation of file paths. @@ -1561,7 +1561,7 @@ In some situations the `.lstrip()`, `.rstrip()` and `.strip()` string methods can be written more succinctly: `strip()` is the same thing as calling both `lstrip()` and `rstrip()` together, and all the strip functions take an iterable argument of the characters to strip, meaning -you don't need to call a strip function multiple times with different +you don't need to call strip methods multiple times with different arguments, you can just concatenate them and call it once. Bad: From dfa2a0d4803101d57745af3e967fd5ef5bdfb003 Mon Sep 17 00:00:00 2001 From: trag1c Date: Mon, 31 Jul 2023 20:53:08 +0200 Subject: [PATCH 6/7] Improved wording --- docs/checks.md | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/docs/checks.md b/docs/checks.md index 979b571..f30a4a5 100644 --- a/docs/checks.md +++ b/docs/checks.md @@ -162,8 +162,8 @@ with suppress(FileNotFoundError): Categories: `logical` `readability` -When comparing a value to multiple possible options, don't use multiple -`or` checks, use a single `in` expr: +When comparing a value to multiple possible options, don't `or` multiple +comparison checks, use a single `in` expr: Bad: @@ -193,7 +193,7 @@ is best to pick one and stick with it. Bad: ```python -for x in [1, 2, 3]: +for x in (1, 2, 3): pass nums = [str(x) for x in [1, 2, 3]] @@ -212,8 +212,8 @@ nums = [str(x) for x in (1, 2, 3)] Categories: `logical` `readability` -Sometimes ternary (aka, inline if statements) can be simplified to a single -`or` expression. +Sometimes the ternary operator (aka, inline if statements) can be simplified to +a single `or` expression. Bad: @@ -357,7 +357,7 @@ Categories: `builtin` `fstring` The `bin()`, `oct()`, and `hex()` functions return the string representation of a number but with a prefix attached. If you don't want the prefix, you might be tempted to just slice it off, but using an -f-string will give you more flexibility: +f-string will give you more flexibility and let you work with negative numbers: Bad: @@ -944,7 +944,7 @@ for book in books.values(): Categories: `builtin` `logical` `readability` Certain ternary expressions can be written more succinctly using the -builtin `max()` function: +builtin `min`/`max` functions: Bad: @@ -968,9 +968,9 @@ highest_score = max(score1, score2) Categories: `builtin` `iterable` `readability` -Often times generator and comprehension expressions can be written more -succinctly. For example, passing a list comprehension to a function when -a generator expression would suffice, or using the shorthand notation +Often times generator expressions and list/set/dict comprehensions can be +written more succinctly. For example, passing a list comprehension to a function +when a generator expression would suffice, or using the shorthand notation in the case of `list` and `set`. For example: Bad: @@ -1873,7 +1873,7 @@ if name == "bob": Categories: `pathlib` -When checking the file extension for a pathlib object don't call +When checking the file extension for a Path object don't call `endswith()` on the `name` field, directly check against `suffix` instead. Bad: From d7d2fa467633ad80fe8186a2610541e6a26d79a8 Mon Sep 17 00:00:00 2001 From: trag1c Date: Wed, 2 Aug 2023 22:10:16 +0200 Subject: [PATCH 7/7] Moved changes to docstrings --- docs/checks.md | 13 +++++++------ refurb/checks/builtin/simplify_comprehension.py | 8 ++++---- refurb/checks/builtin/use_max.py | 2 +- refurb/checks/contextlib/with_suppress.py | 2 +- refurb/checks/iterable/in_tuple.py | 2 +- refurb/checks/logical/use_in.py | 4 ++-- refurb/checks/logical/use_or.py | 4 ++-- refurb/checks/pathlib/cwd.py | 2 +- refurb/checks/pathlib/mkdir.py | 2 +- refurb/checks/pathlib/use_suffix.py | 2 +- refurb/checks/pathlib/with_suffix.py | 2 +- refurb/checks/pathlib/write_text.py | 2 +- refurb/checks/secrets/simplify_token_function.py | 2 +- refurb/checks/string/fstring_number.py | 3 ++- refurb/checks/string/simplify_strip.py | 2 +- refurb/checks/string/startswith.py | 2 +- 16 files changed, 28 insertions(+), 26 deletions(-) diff --git a/docs/checks.md b/docs/checks.md index f30a4a5..e293aff 100644 --- a/docs/checks.md +++ b/docs/checks.md @@ -212,8 +212,8 @@ nums = [str(x) for x in (1, 2, 3)] Categories: `logical` `readability` -Sometimes the ternary operator (aka, inline if statements) can be simplified to -a single `or` expression. +Sometimes the ternary operator (aka, inline if statements) can be +simplified to a single `or` expression. Bad: @@ -357,7 +357,8 @@ Categories: `builtin` `fstring` The `bin()`, `oct()`, and `hex()` functions return the string representation of a number but with a prefix attached. If you don't want the prefix, you might be tempted to just slice it off, but using an -f-string will give you more flexibility and let you work with negative numbers: +f-string will give you more flexibility and let you work with negative +numbers: Bad: @@ -969,9 +970,9 @@ highest_score = max(score1, score2) Categories: `builtin` `iterable` `readability` Often times generator expressions and list/set/dict comprehensions can be -written more succinctly. For example, passing a list comprehension to a function -when a generator expression would suffice, or using the shorthand notation -in the case of `list` and `set`. For example: +written more succinctly. For example, passing a list comprehension to a +function when a generator expression would suffice, or using the shorthand +notation in the case of `list` and `set`. For example: Bad: diff --git a/refurb/checks/builtin/simplify_comprehension.py b/refurb/checks/builtin/simplify_comprehension.py index 12e982e..03eca1f 100644 --- a/refurb/checks/builtin/simplify_comprehension.py +++ b/refurb/checks/builtin/simplify_comprehension.py @@ -14,10 +14,10 @@ @dataclass class ErrorInfo(Error): """ - Often times generator and comprehension expressions can be written more - succinctly. For example, passing a list comprehension to a function when - a generator expression would suffice, or using the shorthand notation - in the case of `list` and `set`. For example: + Often times generator expressions and list/set/dict comprehensions can be + written more succinctly. For example, passing a list comprehension to a + function when a generator expression would suffice, or using the shorthand + notation in the case of `list` and `set`. For example: Bad: diff --git a/refurb/checks/builtin/use_max.py b/refurb/checks/builtin/use_max.py index bba21a3..c4a3c8f 100644 --- a/refurb/checks/builtin/use_max.py +++ b/refurb/checks/builtin/use_max.py @@ -10,7 +10,7 @@ class ErrorInfo(Error): """ Certain ternary expressions can be written more succinctly using the - builtin `max()` function: + builtin `min`/`max` functions: Bad: diff --git a/refurb/checks/contextlib/with_suppress.py b/refurb/checks/contextlib/with_suppress.py index 8e8f23c..ade2e21 100644 --- a/refurb/checks/contextlib/with_suppress.py +++ b/refurb/checks/contextlib/with_suppress.py @@ -11,7 +11,7 @@ class ErrorInfo(Error): Often times you want to handle an exception, and just ignore it. You can do this with a `try/except` block, using a single `pass` in the `except` block, but there is a simpler and more concise way using the `suppress()` - method from `contextlib`: + function from `contextlib`: Bad: diff --git a/refurb/checks/iterable/in_tuple.py b/refurb/checks/iterable/in_tuple.py index f26e517..ae50418 100644 --- a/refurb/checks/iterable/in_tuple.py +++ b/refurb/checks/iterable/in_tuple.py @@ -14,7 +14,7 @@ class ErrorInfo(Error): Bad: ``` - for x in [1, 2, 3]: + for x in (1, 2, 3): pass nums = [str(x) for x in [1, 2, 3]] diff --git a/refurb/checks/logical/use_in.py b/refurb/checks/logical/use_in.py index bcbf459..4e06f08 100644 --- a/refurb/checks/logical/use_in.py +++ b/refurb/checks/logical/use_in.py @@ -9,8 +9,8 @@ @dataclass class ErrorInfo(Error): """ - When comparing a value to multiple possible options, don't use multiple - `or` checks, use a single `in` expr: + When comparing a value to multiple possible options, don't `or` multiple + comparison checks, use a single `in` expr: Bad: diff --git a/refurb/checks/logical/use_or.py b/refurb/checks/logical/use_or.py index e74a947..73e1603 100644 --- a/refurb/checks/logical/use_or.py +++ b/refurb/checks/logical/use_or.py @@ -9,8 +9,8 @@ @dataclass class ErrorInfo(Error): """ - Sometimes ternary (aka, inline if statements) can be simplified to a single - `or` expression. + Sometimes the ternary operator (aka, inline if statements) can be + simplified to a single `or` expression. Bad: diff --git a/refurb/checks/pathlib/cwd.py b/refurb/checks/pathlib/cwd.py index 9c640c9..98a32b4 100644 --- a/refurb/checks/pathlib/cwd.py +++ b/refurb/checks/pathlib/cwd.py @@ -8,7 +8,7 @@ @dataclass class ErrorInfo(Error): """ - A modern alternative to `os.getcwd()` is the `Path.cwd()` function: + A modern alternative to `os.getcwd()` is the `Path.cwd()` method: Bad: diff --git a/refurb/checks/pathlib/mkdir.py b/refurb/checks/pathlib/mkdir.py index 5350b0c..6d478a3 100644 --- a/refurb/checks/pathlib/mkdir.py +++ b/refurb/checks/pathlib/mkdir.py @@ -11,7 +11,7 @@ @dataclass class ErrorInfo(Error): """ - Use the `mkdir` function from the pathlib library instead of using the + Use the `mkdir` method from the pathlib library instead of using the `mkdir` and `makedirs` functions from the `os` library: the pathlib library is more modern and provides better flexibility over the construction and manipulation of file paths. diff --git a/refurb/checks/pathlib/use_suffix.py b/refurb/checks/pathlib/use_suffix.py index 1b24154..67848b6 100644 --- a/refurb/checks/pathlib/use_suffix.py +++ b/refurb/checks/pathlib/use_suffix.py @@ -10,7 +10,7 @@ @dataclass class ErrorInfo(Error): """ - When checking the file extension for a pathlib object don't call + When checking the file extension for a Path object don't call `endswith()` on the `name` field, directly check against `suffix` instead. Bad: diff --git a/refurb/checks/pathlib/with_suffix.py b/refurb/checks/pathlib/with_suffix.py index ac091a2..72db49b 100644 --- a/refurb/checks/pathlib/with_suffix.py +++ b/refurb/checks/pathlib/with_suffix.py @@ -19,7 +19,7 @@ class ErrorInfo(Error): """ A common operation is changing the extension of a file. If you have an existing `Path` object, you don't need to convert it to a string, slice - it, and append a new extension. Instead, use the `with_suffix()` function: + it, and append a new extension. Instead, use the `with_suffix()` method: Bad: diff --git a/refurb/checks/pathlib/write_text.py b/refurb/checks/pathlib/write_text.py index c210976..4185041 100644 --- a/refurb/checks/pathlib/write_text.py +++ b/refurb/checks/pathlib/write_text.py @@ -17,7 +17,7 @@ class ErrorInfo(Error): """ When you just want to save some contents to a file, using a `with` block is - a bit overkill. Instead you can use pathlib's `write_text()` function: + a bit overkill. Instead you can use pathlib's `write_text()` method: Bad: diff --git a/refurb/checks/secrets/simplify_token_function.py b/refurb/checks/secrets/simplify_token_function.py index d99ee88..f76f756 100644 --- a/refurb/checks/secrets/simplify_token_function.py +++ b/refurb/checks/secrets/simplify_token_function.py @@ -16,7 +16,7 @@ @dataclass class ErrorInfo(Error): """ - Depending on how you are using the `secrets` module there might be a more + Depending on how you are using the `secrets` module, there might be more expressive ways of writing what it is you're trying to write. Bad: diff --git a/refurb/checks/string/fstring_number.py b/refurb/checks/string/fstring_number.py index e7883a2..593effd 100644 --- a/refurb/checks/string/fstring_number.py +++ b/refurb/checks/string/fstring_number.py @@ -11,7 +11,8 @@ class ErrorInfo(Error): The `bin()`, `oct()`, and `hex()` functions return the string representation of a number but with a prefix attached. If you don't want the prefix, you might be tempted to just slice it off, but using an - f-string will give you more flexibility: + f-string will give you more flexibility and let you work with negative + numbers: Bad: diff --git a/refurb/checks/string/simplify_strip.py b/refurb/checks/string/simplify_strip.py index 0a799ab..233335e 100644 --- a/refurb/checks/string/simplify_strip.py +++ b/refurb/checks/string/simplify_strip.py @@ -12,7 +12,7 @@ class ErrorInfo(Error): methods can be written more succinctly: `strip()` is the same thing as calling both `lstrip()` and `rstrip()` together, and all the strip functions take an iterable argument of the characters to strip, meaning - you don't need to call a strip function multiple times with different + you don't need to call strip methods multiple times with different arguments, you can just concatenate them and call it once. Bad: diff --git a/refurb/checks/string/startswith.py b/refurb/checks/string/startswith.py index fbfef83..7603471 100644 --- a/refurb/checks/string/startswith.py +++ b/refurb/checks/string/startswith.py @@ -9,7 +9,7 @@ @dataclass class ErrorInfo(Error): """ - `startswith()` and `endswith()` both takes a tuple, so instead of calling + `startswith()` and `endswith()` both take a tuple, so instead of calling `startswith()` multiple times on the same string, you can check them all at once: