Skip to content

Commit

Permalink
A lotta stuff, v0.2.4
Browse files Browse the repository at this point in the history
  • Loading branch information
rdbende committed Aug 10, 2022
1 parent 1c3426a commit 9b48447
Show file tree
Hide file tree
Showing 16 changed files with 215 additions and 80 deletions.
11 changes: 9 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ Calling a function instead of passing the reference for `command` argument
+ ttk.Button(command=foo)
```

### `TK112`
Calling a function with arguments instead of using a lambda or a partial function and passing the reference for `command` argument

```diff
- ttk.Button(command=foo(bar, baz))
+ ttk.Button(command=lambda: foo(bar, baz))
```

### `TK201`
Don't use `from tkinter import *`

Expand Down Expand Up @@ -95,9 +103,8 @@ for index, foo in enumerate(foos):
- [ ] Suggest refactoring code that uses `w.update`, as it's usually pointless, [potentially harmful](https://wiki.tcl-lang.org/page/Update+considered+harmful), and considered a code smell (**TK103**)
- [ ] Warn when using a float as `Text` widget index (**TK132**)
- [ ] Infinite loop in a handler - propose to use recursive function with `w.after` (**TK122**)
- [ ] Event and callback handlers (I can't remember what I meant by this sentence, lol)
- [x] Warn when calling the function inline, instead of just referencing it (**TK111**)
- [ ] Suggest using a lambda function when args are passed to inline calls (**TK112**)
- [x] Suggest using a lambda function when args are passed to inline calls (**TK112**)

- Common best practices
- [x] Warn on `from tkinter import *`, suggest using `import tkinter` or `import tkinter as tk` instead (**TK201**)
Expand Down
2 changes: 1 addition & 1 deletion flake8_tkinter/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

from .visitor import Visitor

__version__ = "0.2.3"
__version__ = "0.2.4"


@dataclass(frozen=True)
Expand Down
58 changes: 31 additions & 27 deletions flake8_tkinter/checkers/TK111.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,34 +3,36 @@
import ast

from .base import CheckerBase
from .data import Settings
from .data import is_from_tkinter
from .TK231 import BIND_METHODS

COMMAND_ARGS = {"command", "xscrollcommand", "yscrollcommand", "postcommand"}
CONFIG_METHODS = {"config", "configure"}


class TK111(CheckerBase):
message = ""
message_on_command_arg = "Calling '{handler}()' instead of referencing it for '{argument}'. Perhaps you meant '{argument}={handler}' (without the parentheses)?"
message_on_bind = "Calling '{handler}()' instead of referencing it for bind. Perhaps you meant '{handler}' (without the parentheses)?"
message = " (without the parentheses)?"
message_on_command_arg = "Calling '{handler}()' instead of referencing it for '{argument}'. Perhaps you meant '{argument}={handler}'"
message_on_bind = (
"Calling '{handler}()' instead of referencing it for bind. Perhaps you meant '{handler}'"
)

@staticmethod
def detect(node: ast.Call) -> bool:
if isinstance(node.func.value, ast.Name):
if (
node.func.value.id in {Settings.tkinter_as, Settings.ttk_as}
or node.func.attr in CONFIG_METHODS
):
for keyword in node.keywords:
return (
keyword.arg in COMMAND_ARGS
and isinstance(keyword.value, ast.Call)
and not keyword.value.args
and not keyword.value.keywords
)
elif node.func.attr in BIND_METHODS and len(node.args) >= 2:
return isinstance(node.args[1], ast.Call)
if not isinstance(node.func.value, ast.Name):
return

if is_from_tkinter(node.func.value.id) or node.func.attr in CONFIG_METHODS:
for keyword in node.keywords:
func = keyword.value
return (
keyword.arg in COMMAND_ARGS
and isinstance(func, ast.Call)
and not (func.args or func.keywords)
)
elif node.func.attr in BIND_METHODS and len(node.args) >= 2:
func = node.args[1]
return isinstance(func, ast.Call) and not (func.args or func.keywords)

@classmethod
def get_message(cls, handler: str | None, argument: str | None = None) -> str:
Expand All @@ -39,37 +41,39 @@ def get_message(cls, handler: str | None, argument: str | None = None) -> str:
else:
msg = cls.message_on_command_arg

return f"{cls.__mro__[0].__name__} {msg.format(handler=handler, argument=argument)}"
return f"{cls.__mro__[0].__name__} {(msg + cls.message).format(handler=handler, argument=argument)}"

@staticmethod
def get_data(node: ast.Call) -> dict[str, str]:
if node.func.value.id in {Settings.tkinter_as, Settings.ttk_as} or node.func.attr in CONFIG_METHODS:
if is_from_tkinter(node.func.value.id) or node.func.attr in CONFIG_METHODS:
keyword, *_ = [kw for kw in node.keywords if kw.arg in COMMAND_ARGS]
func = keyword.value.func

if isinstance(func, ast.Name):
return {"handler": func.id, "argument": keyword.arg}
elif isinstance(func, ast.Attribute):
return {
"handler": ".".join([func.value.id, func.attr]),
"handler": f"{func.value.id}.{func.attr}",
"argument": keyword.arg,
}
elif isinstance(func, ast.Lambda):
return {"handler": "<lambda>"}

elif node.func.attr in BIND_METHODS:
func = node.args[1].func

if isinstance(func, ast.Name):
return {"handler": func.id}
elif isinstance(func, ast.Attribute):
return {"handler": ".".join([func.value.id, func.attr])}
return {"handler": f"{func.value.id}.{func.attr}"}
elif isinstance(func, ast.Lambda):
return {"handler": "<lambda>"}

raise NotImplementedError(
"Oh, crap! This is an error with flake8-tkinter.\n\
Please report this error here: https://github.com/rdbende/flake8-tkinter/issues/new"
)
return {"handler": "", "argument": ""}

@staticmethod
def get_pos(node: ast.Call) -> tuple[int, int]:
if node.func.value.id in {Settings.tkinter_as, Settings.ttk_as} or node.func.attr in CONFIG_METHODS:
if is_from_tkinter(node.func.value.id) or node.func.attr in CONFIG_METHODS:
keyword, *_ = [kw for kw in node.keywords if kw.arg in COMMAND_ARGS]
return keyword.value.lineno, keyword.value.col_offset
elif node.func.attr in BIND_METHODS:
Expand Down
52 changes: 52 additions & 0 deletions flake8_tkinter/checkers/TK112.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
from __future__ import annotations

import ast

from .data import is_from_tkinter
from .TK111 import TK111
from .TK231 import BIND_METHODS

COMMAND_ARGS = {"command", "xscrollcommand", "yscrollcommand", "postcommand"}
CONFIG_METHODS = {"config", "configure"}


def is_partial(func: ast.Name | ast.Attribute) -> bool:
if isinstance(func, ast.Name):
return func.id in ("partial", "partialmethod")
elif isinstance(func, ast.Attribute):
return f"{func.value.id}.{func.attr}" in (
"functools.partial",
"functools.partialmethod",
)

return False


class TK112(TK111):
message = " If you need to call '{handler}' with arguments, use lambda or functools.partial."
message_on_command_arg = (
"Calling '{handler}()' with arguments instead of referencing it for '{argument}'."
)
message_on_bind = "Calling '{handler}()' with arguments instead of referencing it for bind."

@staticmethod
def detect(node: ast.Call) -> bool:
if not isinstance(node.func.value, ast.Name):
return

if is_from_tkinter(node.func.value.id) or node.func.attr in CONFIG_METHODS:
for keyword in node.keywords:
func = keyword.value
return (
keyword.arg in COMMAND_ARGS
and isinstance(func, ast.Call)
and (func.args or func.keywords)
and not is_partial(func.func)
)
elif node.func.attr in BIND_METHODS and len(node.args) >= 2:
func = node.args[1]
return (
isinstance(func, ast.Call)
and (func.args or func.keywords)
and not is_partial(func.func)
)
4 changes: 3 additions & 1 deletion flake8_tkinter/checkers/TK211.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@


class TK211(CheckerBase):
message = "Using `import tkinter.ttk as ttk` is pointless. Use `from tkinter import ttk` instead."
message = (
"Using `import tkinter.ttk as ttk` is pointless. Use `from tkinter import ttk` instead."
)

@staticmethod
def detect() -> bool:
Expand Down
4 changes: 4 additions & 0 deletions flake8_tkinter/checkers/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,7 @@ class _Settings:


Settings = _Settings()


def is_from_tkinter(thing: str) -> bool:
return thing in {Settings.tkinter_as, Settings.ttk_as}
16 changes: 11 additions & 5 deletions flake8_tkinter/visitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

from .checkers.data import Settings
from .checkers.TK111 import TK111
from .checkers.TK112 import TK112
from .checkers.TK201 import TK201
from .checkers.TK202 import TK202
from .checkers.TK211 import TK211
Expand All @@ -21,6 +22,11 @@ def __init__(self) -> None:
self.problems: list[tuple[int, int, str]] = []

def visit_ImportFrom(self, node: ast.ImportFrom) -> None: # noqa: N802
for name in node.names:
if name.name == "ttk":
Settings.tkinter_used = True
Settings.ttk_as = name.asname or name.name

if node.module == "tkinter" and TK201.detect(node):
self.append(node.lineno, node.col_offset, TK201)
elif node.module == "tkinter.ttk" and TK202.detect(node):
Expand All @@ -32,12 +38,10 @@ def visit_Import(self, node: ast.Import) -> None: # noqa: N802
for name in node.names:
if name.name == "tkinter":
Settings.tkinter_used = True
Settings.tkinter_as = name.asname or "tkinter"

if name.name == "tkinter.ttk":
Settings.tkinter_as = name.asname or name.name
elif name.name == "tkinter.ttk":
Settings.tkinter_used = True
Settings.ttk_as = name.asname or "tkinter.ttk"

Settings.ttk_as = name.asname or name.name
if TK211.detect():
self.append(node.lineno, node.col_offset, TK211)

Expand All @@ -48,6 +52,8 @@ def visit_Call(self, node: ast.Call) -> None: # noqa: N802
if isinstance(node.func, ast.Attribute):
if TK111.detect(node):
self.append(*TK111.get_pos(node), TK111, node)
elif TK112.detect(node):
self.append(*TK112.get_pos(node), TK112, node)
if TK231.detect(node):
self.append(node.lineno, node.col_offset, TK231, node)

Expand Down
2 changes: 1 addition & 1 deletion tests/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from flake8_tkinter import Plugin


def _results(code_string: str) -> set[str]:
def lint(code_string: str) -> set[str]:
tree = ast.parse(code_string)
plugin = Plugin(tree)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from base import _results
from base import lint

from flake8_tkinter.checkers.TK231 import BIND_METHODS

Expand All @@ -9,7 +9,7 @@ def test_bind_add_true():
code = f"widget.{method}('Button', '<Button-1>', print, add=True)"
else:
code = f"widget.{method}('<Button-1>', print, add=True)"
assert _results(code) == set()
assert lint(code) == set()


def test_bind_add_false():
Expand All @@ -18,7 +18,7 @@ def test_bind_add_false():
code = f"widget.{method}('Button', '<Button-1>', print, add=False)"
else:
code = f"widget.{method}('<Button-1>', print, add=False)"
assert _results(code) == set()
assert lint(code) == set()


def test_bind_add_missing():
Expand All @@ -27,7 +27,7 @@ def test_bind_add_missing():
code = f"widget.{method}('Button', '<Button-1>', print)"
else:
code = f"widget.{method}('<Button-1>', print)"
assert _results(code) == {f"1:1 TK231 Using {method} without `add=True` will overwrite any existing bindings to this sequence on this widget. Either overwrite them explicitly with `add=False` or use `add=True` to keep existing bindings."}
assert lint(code) == {f"1:1 TK231 Using {method} without `add=True` will overwrite any existing bindings to this sequence on this widget. Either overwrite them explicitly with `add=False` or use `add=True` to keep existing bindings."}


def test_bind_method_without_handler_passed():
Expand All @@ -36,4 +36,4 @@ def test_bind_method_without_handler_passed():
code = f"widget.{method}('Button', '<Button-1>')"
else:
code = f"widget.{method}('<Button-1>')"
assert _results(code) == set()
assert lint(code) == set()
29 changes: 29 additions & 0 deletions tests/test_callback_function_passed_call.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
from base import lint


def test_command_arg_function_call_with_plain_tkinter():
code = "import tkinter as tk;tk.Button(command=handler())"
assert lint(code) == {
"1:40 TK111 Calling 'handler()' instead of referencing it for 'command'. Perhaps you meant 'command=handler' (without the parentheses)?"
}


def test_command_arg_function_call_with_ttk():
code = "from tkinter import ttk;ttk.Button(command=handler())"
assert lint(code) == {
"1:44 TK111 Calling 'handler()' instead of referencing it for 'command'. Perhaps you meant 'command=handler' (without the parentheses)?"
}


def test_command_arg_function_call_in_config():
code = "w.config(command=handler())"
assert lint(code) == {
"1:18 TK111 Calling 'handler()' instead of referencing it for 'command'. Perhaps you meant 'command=handler' (without the parentheses)?"
}


def test_bind_function_call():
code = "w.bind('<Button-1>', handler(), add=True)"
assert lint(code) == {
"1:22 TK111 Calling 'handler()' instead of referencing it for bind. Perhaps you meant 'handler' (without the parentheses)?"
}
29 changes: 29 additions & 0 deletions tests/test_callback_function_passed_call_with_args.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
from base import lint


def test_command_arg_function_call_with_args_with_plain_tkinter():
code = "import tkinter as tk;tk.Button(command=handler(foo, bar))"
assert lint(code) == {
"1:40 TK112 Calling 'handler()' with arguments instead of referencing it for 'command'. If you need to call 'handler' with arguments, use lambda or functools.partial."
}


def test_command_arg_function_call_with_args_with_ttk():
code = "from tkinter import ttk;ttk.Button(command=handler(foo, bar))"
assert lint(code) == {
"1:44 TK112 Calling 'handler()' with arguments instead of referencing it for 'command'. If you need to call 'handler' with arguments, use lambda or functools.partial."
}


def test_command_arg_function_call_with_args_in_config():
code = "w.config(command=handler(foo, bar))"
assert lint(code) == {
"1:18 TK112 Calling 'handler()' with arguments instead of referencing it for 'command'. If you need to call 'handler' with arguments, use lambda or functools.partial."
}


def test_bind_function_call_with_args():
code = "w.bind('<Button-1>', handler(foo, bar), add=True)"
assert lint(code) == {
"1:22 TK112 Calling 'handler()' with arguments instead of referencing it for bind. If you need to call 'handler' with arguments, use lambda or functools.partial."
}
4 changes: 2 additions & 2 deletions tests/test_dumb_bool_constants.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
from base import _results
from base import lint
from flake8_tkinter.checkers.TK221 import DUMB_CONSTANTS


def test_dumb_boolean_constants():
for constant in DUMB_CONSTANTS:
code = f"import tkinter as tk\ntk.{constant}"
assert _results(code) == {
assert lint(code) == {
f"2:1 TK221 Using tkinter.{constant} is pointless. Use an appropriate Python boolean instead."
}
Loading

0 comments on commit 9b48447

Please sign in to comment.