Skip to content

Commit

Permalink
tools.vet: add notice for empty strings conditions (vlang#21421)
Browse files Browse the repository at this point in the history
  • Loading branch information
ttytm authored May 19, 2024
1 parent f7e820c commit f56b2b6
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 0 deletions.
12 changes: 12 additions & 0 deletions cmd/tools/vvet/tests/empty_string.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
cmd/tools/vvet/tests/empty_string.vv:3: notice: Use `foo == ''` instead of `foo.len < 1`
cmd/tools/vvet/tests/empty_string.vv:4: notice: Use `foo != ''` instead of `foo.len > 0`
cmd/tools/vvet/tests/empty_string.vv:4: notice: Use `foo == ''` instead of `foo.len == 0`
cmd/tools/vvet/tests/empty_string.vv:4: notice: Use `foo != ''` instead of `foo.len != 0`
cmd/tools/vvet/tests/empty_string.vv:7: notice: Use `'' == bar` instead of `1 > bar.len`
cmd/tools/vvet/tests/empty_string.vv:8: notice: Use `'' != bar` instead of `0 < bar.len`
cmd/tools/vvet/tests/empty_string.vv:8: notice: Use `'' == bar` instead of `0 == bar.len`
cmd/tools/vvet/tests/empty_string.vv:8: notice: Use `'' != bar` instead of `0 != bar.len`
cmd/tools/vvet/tests/empty_string.vv:10: notice: Use `foo == ''` instead of `foo.len < 1`
cmd/tools/vvet/tests/empty_string.vv:10: notice: Use `bar != ''` instead of `bar.len > 0`
cmd/tools/vvet/tests/empty_string.vv:10: notice: Use `foo == ''` instead of `foo.len == 0`
cmd/tools/vvet/tests/empty_string.vv:10: notice: Use `bar == ''` instead of `bar.len == 0`
21 changes: 21 additions & 0 deletions cmd/tools/vvet/tests/empty_string.vv
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
fn main() {
foo := 'foo'
_ := foo.len < 1
_ := foo.len > 0 || foo.len == 0 || foo.len != 0

bar := 'bar'
_ := 1 > bar.len
_ := 0 < bar.len || 0 == bar.len || 0 != bar.len

if foo.len < 1 || bar.len > 0 || (foo.len == 0 && bar.len == 0) {
}

// Should not notify when `.len` is used with other types.
baz := ['baz']
_ := baz.len == 0

foobar := {
'foo': 'bar'
}
_ := foobar.len < 1
}
42 changes: 42 additions & 0 deletions cmd/tools/vvet/vvet.v
Original file line number Diff line number Diff line change
Expand Up @@ -289,8 +289,12 @@ fn (mut vt Vet) expr(expr ast.Expr) {
}
ast.InfixExpr {
vt.vet_in_condition(expr)
vt.vet_empty_str(expr)
vt.expr(expr.right)
}
ast.ParExpr {
vt.expr(expr.expr)
}
ast.CallExpr {
vt.expr(expr.left)
vt.exprs(expr.args.map(it.expr))
Expand Down Expand Up @@ -320,6 +324,44 @@ fn (mut vt Vet) const_decl(stmt ast.ConstDecl) {
}
}

fn (mut vt Vet) vet_empty_str(expr ast.InfixExpr) {
if expr.left is ast.InfixExpr {
vt.expr(expr.left)
} else if expr.right is ast.InfixExpr {
vt.expr(expr.right)
} else if expr.left is ast.SelectorExpr && expr.right is ast.IntegerLiteral {
operand := (expr.left as ast.SelectorExpr) // TODO: remove as-casts when multiple conds can be smart-casted.
if operand.expr is ast.Ident && operand.expr.info.typ == ast.string_type_idx
&& operand.field_name == 'len' {
if expr.op != .lt && expr.right.val == '0' {
// Case: `var.len > 0`, `var.len == 0`, `var.len != 0`
op := if expr.op == .gt { '!=' } else { expr.op.str() }
vt.notice("Use `${operand.expr.name} ${op} ''` instead of `${operand.expr.name}.len ${expr.op} 0`",
expr.pos.line_nr, .unknown)
} else if expr.op == .lt && expr.right.val == '1' {
// Case: `var.len < 1`
vt.notice("Use `${operand.expr.name} == ''` instead of `${operand.expr.name}.len ${expr.op} 1`",
expr.pos.line_nr, .unknown)
}
}
} else if expr.left is ast.IntegerLiteral && expr.right is ast.SelectorExpr {
operand := expr.right
if operand.expr is ast.Ident && operand.expr.info.typ == ast.string_type_idx
&& operand.field_name == 'len' {
if expr.op != .gt && (expr.left as ast.IntegerLiteral).val == '0' {
// Case: `0 < var.len`, `0 == var.len`, `0 != var.len`
op := if expr.op == .lt { '!=' } else { expr.op.str() }
vt.notice("Use `'' ${op} ${operand.expr.name}` instead of `0 ${expr.op} ${operand.expr.name}.len`",
expr.pos.line_nr, .unknown)
} else if expr.op == .gt && (expr.left as ast.IntegerLiteral).val == '1' {
// Case: `1 > var.len`
vt.notice("Use `'' == ${operand.expr.name}` instead of `1 ${expr.op} ${operand.expr.name}.len`",
expr.pos.line_nr, .unknown)
}
}
}
}

fn (vt &Vet) vprintln(s string) {
if !vt.opt.is_verbose {
return
Expand Down

0 comments on commit f56b2b6

Please sign in to comment.