Skip to content

Commit

Permalink
Added checker for multiline list comprehensions ban (#48)
Browse files Browse the repository at this point in the history
  • Loading branch information
nfx authored Oct 3, 2024
1 parent c0057b8 commit 7015ef8
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 1 deletion.
19 changes: 18 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ PyLint with checks for common mistakes and issues in Python code specifically in
* [`C8915`: `spark-outside-function`](#c8915-spark-outside-function)
* [`C8917`: `use-display-instead-of-show`](#c8917-use-display-instead-of-show)
* [`W8916`: `no-spark-argument-in-function`](#w8916-no-spark-argument-in-function)
* [`readability` checker](#readability-checker)
* [`R8923`: `rewrite-as-for-loop`](#r8923-rewrite-as-for-loop)
* [`mocking` checker](#mocking-checker)
* [`R8918`: `explicit-dependency-required`](#r8918-explicit-dependency-required)
* [`R8919`: `obscure-mock`](#r8919-obscure-mock)
Expand Down Expand Up @@ -295,6 +297,21 @@ To disable this check on a specific line, add `# pylint: disable=no-spark-argume

[[back to top](#pylint-plugin-for-databricks)]

## `readability` checker
To use this checker, add `databricks.labs.pylint.readability` to `load-plugins` configuration in your `pylintrc` or `pyproject.toml` file.

[[back to top](#pylint-plugin-for-databricks)]

### `R8923`: `rewrite-as-for-loop`

List comprehension spans multiple lines, rewrite as for loop. List comprehensions in Python are typically used to create new lists by iterating over an existing
iterable in a concise, one-line syntax. However, when a list comprehension becomes too complex or spans
multiple lines, it may lose its readability and clarity, which are key advantages of Python's syntax.

To disable this check on a specific line, add `# pylint: disable=rewrite-as-for-loop` at the end of it.

[[back to top](#pylint-plugin-for-databricks)]

## `mocking` checker
To use this checker, add `databricks.labs.pylint.mocking` to `load-plugins` configuration in your `pylintrc` or `pyproject.toml` file.

Expand Down Expand Up @@ -372,7 +389,7 @@ To disable this check on a specific line, add `# pylint: disable=dead-code` at t
To test this plugin in isolation, you can use the following command:

```bash
pylint --load-plugins=databricks.labs.pylint.all --disable=all --enable=missing-data-security-mode,unsupported-runtime,dbutils-fs-cp,dbutils-fs-head,dbutils-fs-ls,dbutils-fs-mount,dbutils-credentials,dbutils-notebook-run,pat-token-leaked,internal-api,legacy-cli,incompatible-with-uc,notebooks-too-many-cells,notebooks-percent-run,spark-outside-function,use-display-instead-of-show,no-spark-argument-in-function,explicit-dependency-required,obscure-mock,mock-no-assign,mock-no-usage,dead-code .
pylint --load-plugins=databricks.labs.pylint.all --disable=all --enable=missing-data-security-mode,unsupported-runtime,dbutils-fs-cp,dbutils-fs-head,dbutils-fs-ls,dbutils-fs-mount,dbutils-credentials,dbutils-notebook-run,pat-token-leaked,internal-api,legacy-cli,incompatible-with-uc,notebooks-too-many-cells,notebooks-percent-run,spark-outside-function,use-display-instead-of-show,no-spark-argument-in-function,rewrite-as-for-loop,explicit-dependency-required,obscure-mock,mock-no-assign,mock-no-usage,dead-code .
```

[[back to top](#pylint-plugin-for-databricks)]
Expand Down
2 changes: 2 additions & 0 deletions scripts/docs.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from databricks.labs.pylint.legacy import LegacyChecker
from databricks.labs.pylint.mocking import MockingChecker
from databricks.labs.pylint.notebooks import NotebookChecker
from databricks.labs.pylint.readability import ReadabilityChecker
from databricks.labs.pylint.spark import SparkChecker


Expand All @@ -23,6 +24,7 @@ def do_something():
LegacyChecker(linter),
NotebookChecker(linter),
SparkChecker(linter),
ReadabilityChecker(linter),
MockingChecker(linter),
EradicateChecker(linter),
]:
Expand Down
2 changes: 2 additions & 0 deletions src/databricks/labs/pylint/all.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from databricks.labs.pylint.legacy import LegacyChecker
from databricks.labs.pylint.mocking import MockingChecker
from databricks.labs.pylint.notebooks import NotebookChecker
from databricks.labs.pylint.readability import ReadabilityChecker
from databricks.labs.pylint.spark import SparkChecker


Expand All @@ -15,3 +16,4 @@ def register(linter):
linter.register_checker(SparkChecker(linter))
linter.register_checker(MockingChecker(linter))
linter.register_checker(EradicateChecker(linter))
linter.register_checker(ReadabilityChecker(linter))
23 changes: 23 additions & 0 deletions src/databricks/labs/pylint/readability.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
from astroid import nodes # type: ignore
from pylint.checkers import BaseChecker


class ReadabilityChecker(BaseChecker):
name = "readability"
msgs = {
"R8923": (
"List comprehension spans multiple lines, rewrite as for loop",
"rewrite-as-for-loop",
"""List comprehensions in Python are typically used to create new lists by iterating over an existing
iterable in a concise, one-line syntax. However, when a list comprehension becomes too complex or spans
multiple lines, it may lose its readability and clarity, which are key advantages of Python's syntax.""",
),
}

def visit_listcomp(self, node: nodes.ListComp) -> None:
if node.lineno != node.end_lineno:
self.add_message("rewrite-as-for-loop", node=node)


def register(linter):
linter.register_checker(ReadabilityChecker(linter))
16 changes: 16 additions & 0 deletions tests/test_readability.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
from databricks.labs.pylint.readability import ReadabilityChecker


def test_single_line_list_comprehensions_allowed(lint_with):
messages = lint_with(ReadabilityChecker) << """[x for x in range(10) if x % 2 == 0]"""
assert not messages


def test_multi_line_list_comprehensions_not_allowed(lint_with):
messages = (
lint_with(ReadabilityChecker)
<< """[
x for x in range(10) if x % 2 == 0
]"""
)
assert messages == {"[rewrite-as-for-loop] List comprehension spans multiple lines, rewrite as for loop"}

0 comments on commit 7015ef8

Please sign in to comment.