From cdb1b0344c54cbcbce77d254ea30c7336188987d Mon Sep 17 00:00:00 2001
From: Ruofan XU <47302112+SleepyRoy@users.noreply.github.com>
Date: Tue, 21 Apr 2020 02:56:05 +0800
Subject: [PATCH] checker: improve type check for arithmetic assignment op and
 fix error pos

---
 vlib/v/ast/ast.v                              |  1 +
 vlib/v/checker/checker.v                      | 69 ++++++++++---------
 .../tests/inout/assign_expr_type_err_a.out    |  8 +--
 .../tests/inout/assign_expr_type_err_a.vv     |  4 +-
 .../tests/inout/assign_expr_type_err_b.out    |  8 +--
 .../tests/inout/assign_expr_type_err_b.vv     |  4 +-
 .../tests/inout/assign_expr_type_err_c.out    |  8 +--
 .../tests/inout/assign_expr_type_err_c.vv     |  4 +-
 .../tests/inout/assign_expr_type_err_d.out    | 12 ++--
 .../tests/inout/assign_expr_type_err_d.vv     |  6 +-
 .../tests/inout/assign_expr_type_err_e.out    |  6 ++
 .../tests/inout/assign_expr_type_err_e.vv     |  4 ++
 .../tests/inout/assign_expr_type_err_f.out    |  6 ++
 .../tests/inout/assign_expr_type_err_f.vv     |  4 ++
 .../tests/inout/assign_expr_type_err_g.out    |  6 ++
 .../tests/inout/assign_expr_type_err_g.vv     |  4 ++
 .../tests/inout/assign_expr_type_err_h.out    |  6 ++
 .../tests/inout/assign_expr_type_err_h.vv     |  4 ++
 .../tests/inout/assign_expr_type_err_i.out    |  6 ++
 .../tests/inout/assign_expr_type_err_i.vv     |  4 ++
 vlib/v/parser/assign.v                        |  2 +-
 21 files changed, 116 insertions(+), 60 deletions(-)
 create mode 100644 vlib/v/checker/tests/inout/assign_expr_type_err_e.out
 create mode 100644 vlib/v/checker/tests/inout/assign_expr_type_err_e.vv
 create mode 100644 vlib/v/checker/tests/inout/assign_expr_type_err_f.out
 create mode 100644 vlib/v/checker/tests/inout/assign_expr_type_err_f.vv
 create mode 100644 vlib/v/checker/tests/inout/assign_expr_type_err_g.out
 create mode 100644 vlib/v/checker/tests/inout/assign_expr_type_err_g.vv
 create mode 100644 vlib/v/checker/tests/inout/assign_expr_type_err_h.out
 create mode 100644 vlib/v/checker/tests/inout/assign_expr_type_err_h.vv
 create mode 100644 vlib/v/checker/tests/inout/assign_expr_type_err_i.out
 create mode 100644 vlib/v/checker/tests/inout/assign_expr_type_err_i.vv

diff --git a/vlib/v/ast/ast.v b/vlib/v/ast/ast.v
index 152ae6cfdc..aefc4b360b 100644
--- a/vlib/v/ast/ast.v
+++ b/vlib/v/ast/ast.v
@@ -751,6 +751,7 @@ fn (expr Expr) position() token.Position {
 		CharLiteral { return it.pos }
 		EnumVal { return it.pos }
 		FloatLiteral { return it.pos }
+		Ident { return it.pos }
 		IfExpr { return it.pos }
 		// ast.IfGuardExpr { }
 		IndexExpr { return it.pos }
diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v
index 03ee8b5eb0..dde892dccf 100644
--- a/vlib/v/checker/checker.v
+++ b/vlib/v/checker/checker.v
@@ -364,43 +364,50 @@ fn (c mut Checker) assign_expr(assign_expr mut ast.AssignExpr) {
 		}
 		else {}
 	}
+	// Single side check
+	match assign_expr.op {
+		.assign { } // No need to do single side check for =. But here put it first for speed.
+		.plus_assign {
+			if !left.is_number() && left_type != table.string_type && !left.is_pointer() {
+				c.error('operator += not defined on left operand type `$left.name`', assign_expr.left.position())
+			}
+			else if !right.is_number() && right_type != table.string_type && !right.is_pointer() {
+				c.error('operator += not defined on right operand type `$right.name`', assign_expr.val.position())
+			}
+		}
+		.minus_assign {
+			if !left.is_number() && !left.is_pointer() {
+				c.error('operator -= not defined on left operand type `$left.name`', assign_expr.left.position())
+			}
+			else if !right.is_number() && !right.is_pointer() {
+				c.error('operator -= not defined on right operand type `$right.name`', assign_expr.val.position())
+			}
+		}
+		.mult_assign, .div_assign {
+			if !left.is_number() {
+				c.error('operator ${assign_expr.op.str()} not defined on left operand type `$left.name`', assign_expr.left.position())
+			}
+			else if !right.is_number() {
+				c.error('operator ${assign_expr.op.str()} not defined on right operand type `$right.name`', assign_expr.val.position())
+			}
+		}
+		.and_assign, .or_assign, .xor_assign, .mod_assign, .left_shift_assign, .right_shift_assign {
+			if !left.is_int() {
+				c.error('operator ${assign_expr.op.str()} not defined on left operand type `$left.name`', assign_expr.left.position())
+			}
+			else if !right.is_int() {
+				c.error('operator ${assign_expr.op.str()} not defined on right operand type `$right.name`', assign_expr.val.position())
+			}
+		}
+		else { }
+	}
+	// Dual sides check (compatibility check)
 	if !c.table.check(right_type, left_type) {
 		left_type_sym := c.table.get_type_symbol(left_type)
 		right_type_sym := c.table.get_type_symbol(right_type)
 		c.error('cannot assign `$right_type_sym.name` to variable `${assign_expr.left.str()}` of type `$left_type_sym.name`',
 			assign_expr.val.position())
 	}
-	else if assign_expr.op == .plus_assign {
-		no_str_related_err := left_type == table.string_type && right_type == table.string_type
-		no_ptr_related_err := (left.is_pointer() || left.is_int()) && (right.is_pointer() || right.is_int())
-		no_num_related_err := left.is_number() && right.is_number()
-		if !no_str_related_err && !no_ptr_related_err && !no_num_related_err {
-			c.error('operator += not defined on left type `$left.name` and right type `$right.name`', assign_expr.pos)
-		}
-	}
-	else if assign_expr.op == .minus_assign {
-		no_ptr_related_err := (left.is_pointer() || left.is_int()) && (right.is_pointer() || right.is_int())
-		no_num_related_err := left.is_number() && right.is_number()
-		if !no_ptr_related_err && !no_num_related_err {
-			c.error('operator -= not defined on left type `$left.name` and right type `$right.name`', assign_expr.pos)
-		}
-	}
-	else if assign_expr.op in [.mult_assign, .div_assign] {
-		if !left.is_number() {
-			c.error('operator ${assign_expr.op.str()} not defined on left type `$left.name`', assign_expr.pos)
-		}
-		else if !right.is_number() {
-			c.error('operator ${assign_expr.op.str()} not defined on right type `$right.name`', assign_expr.pos)
-		}
-	}
-	else if assign_expr.op in [.and_assign, .or_assign, .xor_assign, .mod_assign, .left_shift_assign, .right_shift_assign] {
-		if !left.is_int() {
-			c.error('operator ${assign_expr.op.str()} not defined on left type `$left.name`', assign_expr.pos)
-		}
-		else if !right.is_int() {
-			c.error('operator ${assign_expr.op.str()} not defined on right type `$right.name`', assign_expr.pos)
-		}
-	}
 	c.check_expr_opt_call(assign_expr.val, right_type, true)
 }
 
diff --git a/vlib/v/checker/tests/inout/assign_expr_type_err_a.out b/vlib/v/checker/tests/inout/assign_expr_type_err_a.out
index 81bfe8f1b4..8e03a9c366 100644
--- a/vlib/v/checker/tests/inout/assign_expr_type_err_a.out
+++ b/vlib/v/checker/tests/inout/assign_expr_type_err_a.out
@@ -1,6 +1,6 @@
-vlib/v/checker/tests/inout/assign_expr_type_err_a.v:3:8: error: operator <<= not defined on right type `f64`
+vlib/v/checker/tests/inout/assign_expr_type_err_a.v:3:2: error: operator <<= not defined on left operand type `f64`
     1| fn main() {
-    2|     mut a := 10
-    3|     a <<= 0.5
-                 ~~~
+    2|     mut foo := 0.5
+    3|     foo <<= 1
+           ~~~
     4| }
diff --git a/vlib/v/checker/tests/inout/assign_expr_type_err_a.vv b/vlib/v/checker/tests/inout/assign_expr_type_err_a.vv
index 969ea5268e..3e8816fa74 100644
--- a/vlib/v/checker/tests/inout/assign_expr_type_err_a.vv
+++ b/vlib/v/checker/tests/inout/assign_expr_type_err_a.vv
@@ -1,4 +1,4 @@
 fn main() {
-	mut a := 10
-	a <<= 0.5
+	mut foo := 0.5
+	foo <<= 1
 }
\ No newline at end of file
diff --git a/vlib/v/checker/tests/inout/assign_expr_type_err_b.out b/vlib/v/checker/tests/inout/assign_expr_type_err_b.out
index 532cebf21b..03f55bf78d 100644
--- a/vlib/v/checker/tests/inout/assign_expr_type_err_b.out
+++ b/vlib/v/checker/tests/inout/assign_expr_type_err_b.out
@@ -1,6 +1,6 @@
-vlib/v/checker/tests/inout/assign_expr_type_err_b.v:3:7: error: operator *= not defined on left type `bool`
+vlib/v/checker/tests/inout/assign_expr_type_err_b.v:3:9: error: operator %= not defined on right operand type `string`
     1| fn main() {
-    2|     mut a := true
-    3|     a *= false
-                ~~~~~
+    2|     mut foo := 10
+    3|     foo %= 'hello'
+                  ~~~~~~~
     4| }
diff --git a/vlib/v/checker/tests/inout/assign_expr_type_err_b.vv b/vlib/v/checker/tests/inout/assign_expr_type_err_b.vv
index 382939285a..0fe4d81dda 100644
--- a/vlib/v/checker/tests/inout/assign_expr_type_err_b.vv
+++ b/vlib/v/checker/tests/inout/assign_expr_type_err_b.vv
@@ -1,4 +1,4 @@
 fn main() {
-	mut a := true
-	a *= false
+	mut foo := 10
+	foo %= 'hello'
 }
\ No newline at end of file
diff --git a/vlib/v/checker/tests/inout/assign_expr_type_err_c.out b/vlib/v/checker/tests/inout/assign_expr_type_err_c.out
index 6cb3ab2dc4..72fb961c5b 100644
--- a/vlib/v/checker/tests/inout/assign_expr_type_err_c.out
+++ b/vlib/v/checker/tests/inout/assign_expr_type_err_c.out
@@ -1,6 +1,6 @@
-vlib/v/checker/tests/inout/assign_expr_type_err_c.v:3:7: error: operator -= not defined on left type `string` and right type `string`
+vlib/v/checker/tests/inout/assign_expr_type_err_c.v:3:2: error: operator *= not defined on left operand type `string`
     1| fn main() {
-    2|     mut a := 'hello'
-    3|     a -= 'world'
-                ~~~~~~~
+    2|     mut foo := 'hello'
+    3|     foo *= 10
+           ~~~
     4| }
diff --git a/vlib/v/checker/tests/inout/assign_expr_type_err_c.vv b/vlib/v/checker/tests/inout/assign_expr_type_err_c.vv
index 651e60339d..51be8d748f 100644
--- a/vlib/v/checker/tests/inout/assign_expr_type_err_c.vv
+++ b/vlib/v/checker/tests/inout/assign_expr_type_err_c.vv
@@ -1,4 +1,4 @@
 fn main() {
-	mut a := 'hello'
-	a -= 'world'
+	mut foo := 'hello'
+	foo *= 10
 }
\ No newline at end of file
diff --git a/vlib/v/checker/tests/inout/assign_expr_type_err_d.out b/vlib/v/checker/tests/inout/assign_expr_type_err_d.out
index e1dacee6d0..bb3dfc85a0 100644
--- a/vlib/v/checker/tests/inout/assign_expr_type_err_d.out
+++ b/vlib/v/checker/tests/inout/assign_expr_type_err_d.out
@@ -1,6 +1,6 @@
-vlib/v/checker/tests/inout/assign_expr_type_err_d.v:5:7: error: operator += not defined on left type `Foo` and right type `Foo`
-    3| fn main() {
-    4|     mut a := Foo{ }
-    5|     a += Foo{ }
-                ~~~
-    6| }
+vlib/v/checker/tests/inout/assign_expr_type_err_d.v:3:9: error: operator /= not defined on right operand type `bool`
+    1| fn main() {
+    2|     mut foo := 1.5
+    3|     foo /= true
+                  ~~~~
+    4| }
diff --git a/vlib/v/checker/tests/inout/assign_expr_type_err_d.vv b/vlib/v/checker/tests/inout/assign_expr_type_err_d.vv
index 237d5d83b6..ce3c670298 100644
--- a/vlib/v/checker/tests/inout/assign_expr_type_err_d.vv
+++ b/vlib/v/checker/tests/inout/assign_expr_type_err_d.vv
@@ -1,6 +1,4 @@
-struct Foo { }
-
 fn main() {
-	mut a := Foo{ }
-	a += Foo{ }
+	mut foo := 1.5
+	foo /= true
 }
\ No newline at end of file
diff --git a/vlib/v/checker/tests/inout/assign_expr_type_err_e.out b/vlib/v/checker/tests/inout/assign_expr_type_err_e.out
new file mode 100644
index 0000000000..ae6a0441b1
--- /dev/null
+++ b/vlib/v/checker/tests/inout/assign_expr_type_err_e.out
@@ -0,0 +1,6 @@
+vlib/v/checker/tests/inout/assign_expr_type_err_e.v:3:2: error: operator -= not defined on left operand type `string`
+    1| fn main() {
+    2|     mut foo := 'hello'
+    3|     foo -= `a`
+           ~~~
+    4| }
diff --git a/vlib/v/checker/tests/inout/assign_expr_type_err_e.vv b/vlib/v/checker/tests/inout/assign_expr_type_err_e.vv
new file mode 100644
index 0000000000..e2eeba8ee3
--- /dev/null
+++ b/vlib/v/checker/tests/inout/assign_expr_type_err_e.vv
@@ -0,0 +1,4 @@
+fn main() {
+	mut foo := 'hello'
+	foo -= `a`
+}
\ No newline at end of file
diff --git a/vlib/v/checker/tests/inout/assign_expr_type_err_f.out b/vlib/v/checker/tests/inout/assign_expr_type_err_f.out
new file mode 100644
index 0000000000..960a87a532
--- /dev/null
+++ b/vlib/v/checker/tests/inout/assign_expr_type_err_f.out
@@ -0,0 +1,6 @@
+vlib/v/checker/tests/inout/assign_expr_type_err_f.v:3:9: error: operator -= not defined on right operand type `bool`
+    1| fn main() {
+    2|     mut foo := 10
+    3|     foo -= false
+                  ~~~~~
+    4| }
diff --git a/vlib/v/checker/tests/inout/assign_expr_type_err_f.vv b/vlib/v/checker/tests/inout/assign_expr_type_err_f.vv
new file mode 100644
index 0000000000..7b4ab43de9
--- /dev/null
+++ b/vlib/v/checker/tests/inout/assign_expr_type_err_f.vv
@@ -0,0 +1,4 @@
+fn main() {
+	mut foo := 10
+	foo -= false
+}
\ No newline at end of file
diff --git a/vlib/v/checker/tests/inout/assign_expr_type_err_g.out b/vlib/v/checker/tests/inout/assign_expr_type_err_g.out
new file mode 100644
index 0000000000..4d17aa0c58
--- /dev/null
+++ b/vlib/v/checker/tests/inout/assign_expr_type_err_g.out
@@ -0,0 +1,6 @@
+vlib/v/checker/tests/inout/assign_expr_type_err_g.v:3:2: error: operator += not defined on left operand type `bool`
+    1| fn main() {
+    2|     mut foo := true
+    3|     foo += false
+           ~~~
+    4| }
diff --git a/vlib/v/checker/tests/inout/assign_expr_type_err_g.vv b/vlib/v/checker/tests/inout/assign_expr_type_err_g.vv
new file mode 100644
index 0000000000..018a651364
--- /dev/null
+++ b/vlib/v/checker/tests/inout/assign_expr_type_err_g.vv
@@ -0,0 +1,4 @@
+fn main() {
+	mut foo := true
+	foo += false
+}
\ No newline at end of file
diff --git a/vlib/v/checker/tests/inout/assign_expr_type_err_h.out b/vlib/v/checker/tests/inout/assign_expr_type_err_h.out
new file mode 100644
index 0000000000..9e639b5285
--- /dev/null
+++ b/vlib/v/checker/tests/inout/assign_expr_type_err_h.out
@@ -0,0 +1,6 @@
+vlib/v/checker/tests/inout/assign_expr_type_err_h.v:3:9: error: operator += not defined on right operand type `bool`
+    1| fn main() {
+    2|     mut foo := 'hello'
+    3|     foo += false
+                  ~~~~~
+    4| }
diff --git a/vlib/v/checker/tests/inout/assign_expr_type_err_h.vv b/vlib/v/checker/tests/inout/assign_expr_type_err_h.vv
new file mode 100644
index 0000000000..565c13cbff
--- /dev/null
+++ b/vlib/v/checker/tests/inout/assign_expr_type_err_h.vv
@@ -0,0 +1,4 @@
+fn main() {
+	mut foo := 'hello'
+	foo += false
+}
\ No newline at end of file
diff --git a/vlib/v/checker/tests/inout/assign_expr_type_err_i.out b/vlib/v/checker/tests/inout/assign_expr_type_err_i.out
new file mode 100644
index 0000000000..3a056beb08
--- /dev/null
+++ b/vlib/v/checker/tests/inout/assign_expr_type_err_i.out
@@ -0,0 +1,6 @@
+vlib/v/checker/tests/inout/assign_expr_type_err_i.v:3:9: error: cannot assign `string` to variable `foo` of type `f64`
+    1| fn main() {
+    2|     mut foo := 1.5
+    3|     foo += 'hello'
+                  ~~~~~~~
+    4| }
diff --git a/vlib/v/checker/tests/inout/assign_expr_type_err_i.vv b/vlib/v/checker/tests/inout/assign_expr_type_err_i.vv
new file mode 100644
index 0000000000..da50d07c0f
--- /dev/null
+++ b/vlib/v/checker/tests/inout/assign_expr_type_err_i.vv
@@ -0,0 +1,4 @@
+fn main() {
+	mut foo := 1.5
+	foo += 'hello' 
+}
\ No newline at end of file
diff --git a/vlib/v/parser/assign.v b/vlib/v/parser/assign.v
index f606cf1b33..9107a8e881 100644
--- a/vlib/v/parser/assign.v
+++ b/vlib/v/parser/assign.v
@@ -53,8 +53,8 @@ fn (var p Parser) assign_stmt() ast.Stmt {
 // TODO: is it possible to merge with AssignStmt?
 pub fn (var p Parser) assign_expr(left ast.Expr) ast.AssignExpr {
 	op := p.tok.kind
-	p.next()
 	pos := p.tok.position()
+	p.next()
 	val := p.expr(0)
 	match left {
 		ast.IndexExpr {