From f56b2b6833749a7642891111b2501028428f892f Mon Sep 17 00:00:00 2001
From: Turiiya <34311583+ttytm@users.noreply.github.com>
Date: Sun, 19 May 2024 20:40:25 +0200
Subject: [PATCH] tools.vet: add notice for empty strings conditions (#21421)

---
 cmd/tools/vvet/tests/empty_string.out | 12 ++++++++
 cmd/tools/vvet/tests/empty_string.vv  | 21 ++++++++++++++
 cmd/tools/vvet/vvet.v                 | 42 +++++++++++++++++++++++++++
 3 files changed, 75 insertions(+)
 create mode 100644 cmd/tools/vvet/tests/empty_string.out
 create mode 100644 cmd/tools/vvet/tests/empty_string.vv

diff --git a/cmd/tools/vvet/tests/empty_string.out b/cmd/tools/vvet/tests/empty_string.out
new file mode 100644
index 00000000000000..94a0449f9aecbf
--- /dev/null
+++ b/cmd/tools/vvet/tests/empty_string.out
@@ -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`
diff --git a/cmd/tools/vvet/tests/empty_string.vv b/cmd/tools/vvet/tests/empty_string.vv
new file mode 100644
index 00000000000000..64aa7b610bd2cd
--- /dev/null
+++ b/cmd/tools/vvet/tests/empty_string.vv
@@ -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
+}
diff --git a/cmd/tools/vvet/vvet.v b/cmd/tools/vvet/vvet.v
index 3d263fbcc66ebb..bf7ddc5cb80995 100644
--- a/cmd/tools/vvet/vvet.v
+++ b/cmd/tools/vvet/vvet.v
@@ -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))
@@ -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