Skip to content

Commit

Permalink
checks: add unbraced-expr check
Browse files Browse the repository at this point in the history
  • Loading branch information
nmoroze committed Apr 1, 2024
1 parent 126a4f3 commit 99bdf37
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 4 deletions.
14 changes: 14 additions & 0 deletions docs/violations.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ This page lists all lint violations that may be reported by `tclint`.
- [`backslash-spacing`](#backslash-spacing)
- [`expr-format`](#expr-format)
- [`spaces-in-braces`](#spaces-in-braces)
- [`unbraced-expr`](#unbraced-expr)

Each of these violations is sorted into one of two coarse categories, which can
be displayed using the `--show-categories` CLI option.
Expand Down Expand Up @@ -134,3 +135,16 @@ for {set i 0} {$i < 10} {incr i} {
### Rationale

Consistent formatting enhances readability.

## `unbraced-expr`

Expressions that contain substitutions should be enclosed by braces.

### Rationale

Without braces, the Tcl parser will perform substitutions before interpreting
the expression. This is not actually the desired behavior in most cases, and may
impact functionality in some edge cases. In addition, this can reduce
performance.

See "Performance Considerations" in the [Tcl docs for `expr`](https://www.tcl.tk/man/tcl/TclCmd/expr.html).
28 changes: 27 additions & 1 deletion src/tclint/checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from tclint.commands import get_commands
from tclint.violations import Rule, Violation

from tclint.syntax_tree import Visitor, Script, List, ParenExpression
from tclint.syntax_tree import Visitor, Script, List, ParenExpression, BracedExpression


class IndentLevelChecker(Visitor):
Expand Down Expand Up @@ -728,6 +728,31 @@ def _check_braced_arg(self, node):
)


class UnbracedExprChecker(Visitor):
def check(self, _, tree, __):
self._violations = []
tree.accept(self, recurse=True)
return self._violations

def visit_command(self, command):
if command.routine != "expr":
return

if len(command.args) == 1 and isinstance(command.args[0], BracedExpression):
return

for child in command.args:
if child.contents is None:
self._violations.append(
Violation(
Rule.UNBRACED_EXPR,
"expression with substitutions should be enclosed by braces",
command.args[0].pos,
)
)
return


def get_checkers():
return (
IndentLevelChecker(),
Expand All @@ -737,4 +762,5 @@ def get_checkers():
BlankLineChecker(),
BackslashNewlineChecker(),
SpacesInBracesChecker(),
UnbracedExprChecker(),
)
2 changes: 2 additions & 0 deletions src/tclint/violations.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class Rule(Enum):
BACKSLASH_SPACING = "backslash-spacing"
EXPR_FORMAT = "expr-format"
SPACES_IN_BRACES = "spaces-in-braces"
UNBRACED_EXPR = "unbraced-expr"

def __str__(self):
return self.value
Expand Down Expand Up @@ -47,6 +48,7 @@ def __str__(self):
Rule.SPACES_IN_BRACES: Category.STYLE,
Rule.REDEFINED_BUILTIN: Category.FUNC,
Rule.COMMAND_ARGS: Category.FUNC,
Rule.UNBRACED_EXPR: Category.FUNC,
}


Expand Down
4 changes: 2 additions & 2 deletions tests/data/dirty.tcl
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
for {set i 1} {$i < 100} {incr i} {
if {[expr $i % 15] == 0} {
if {$i % 15 == 0} {
puts "FizzBuzz"
} elseif {[expr $i % 3] == 0} {
} elseif {$i % 3 == 0} {
puts "Fizz"
} elseif {[expr $i % 5] == 0} {
puts "Buzz"
Expand Down
1 change: 1 addition & 0 deletions tests/data/dirty.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@ data/dirty.tcl:3:1: expected indent of 8 spaces, got 0 [indent]
data/dirty.tcl:4:1: expected indent of 4 spaces, got 0 [indent]
data/dirty.tcl:4:2: expected 1 space between words, got 3 [spacing]
data/dirty.tcl:6:1: expected indent of 4 spaces, got 0 [indent]
data/dirty.tcl:6:17: expression with substitutions should be enclosed by braces [unbraced-expr]
data/dirty.tcl:7:13: expected 1 space between words, got 3 [spacing]
data/dirty.tcl:9:13: expected 1 space between words, got 5 [spacing]
3 changes: 2 additions & 1 deletion tests/test_tclint.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,14 @@ def test_switches():
data/dirty.tcl:4:2: expected 1 space between words, got 3 [spacing]
data/dirty.tcl:5:1: expected indent of 4 spaces, got 8 [indent]
data/dirty.tcl:6:1: expected indent of 2 spaces, got 0 [indent]
data/dirty.tcl:6:17: expression with substitutions should be enclosed by braces [unbraced-expr]
data/dirty.tcl:7:1: expected indent of 4 spaces, got 8 [indent]
data/dirty.tcl:7:13: expected 1 space between words, got 3 [spacing]
data/dirty.tcl:8:1: expected indent of 2 spaces, got 4 [indent]
data/dirty.tcl:9:1: expected indent of 4 spaces, got 8 [indent]
data/dirty.tcl:9:13: expected 1 space between words, got 5 [spacing]
data/dirty.tcl:10:1: expected indent of 2 spaces, got 4 [indent]
""".lstrip()
""".lstrip() # noqa E501

assert p.stdout.decode("utf-8") == expected
assert p.returncode == 1
Expand Down

0 comments on commit 99bdf37

Please sign in to comment.