From 4faa0f848740a061929439c6f6f585c517231804 Mon Sep 17 00:00:00 2001
From: Delyan Angelov <delian66@gmail.com>
Date: Thu, 9 Sep 2021 09:15:21 +0300
Subject: [PATCH] v.checker: add checks for `.free()` methods

---
 vlib/builtin/js/array.js.v                  |  2 +-
 vlib/builtin/js/string.js.v                 |  2 +-
 vlib/sokol/gfx/gfx_structs.c.v              | 20 +++++++++----------
 vlib/v/checker/checker.v                    | 13 ++++++++++++
 vlib/v/checker/tests/free_method_errors.out | 21 ++++++++++++++++++++
 vlib/v/checker/tests/free_method_errors.vv  | 22 +++++++++++++++++++++
 6 files changed, 68 insertions(+), 12 deletions(-)
 create mode 100644 vlib/v/checker/tests/free_method_errors.out
 create mode 100644 vlib/v/checker/tests/free_method_errors.vv

diff --git a/vlib/builtin/js/array.js.v b/vlib/builtin/js/array.js.v
index e041b75d1e..457517ca97 100644
--- a/vlib/builtin/js/array.js.v
+++ b/vlib/builtin/js/array.js.v
@@ -234,7 +234,7 @@ pub fn (mut a array) delete_last() {
 }
 
 [unsafe]
-pub fn (a array) free() {
+pub fn (a &array) free() {
 }
 
 // todo: once (a []byte) will work rewrite this
diff --git a/vlib/builtin/js/string.js.v b/vlib/builtin/js/string.js.v
index 1a7f96a624..49c9537efa 100644
--- a/vlib/builtin/js/string.js.v
+++ b/vlib/builtin/js/string.js.v
@@ -140,7 +140,7 @@ pub fn (s string) find_between(start string, end string) string {
 }
 
 // unnecessary in the JS backend, implemented for api parity.
-pub fn (s string) free() {}
+pub fn (s &string) free() {}
 
 pub fn (s string) hash() int {
 	mut h := u32(0)
diff --git a/vlib/sokol/gfx/gfx_structs.c.v b/vlib/sokol/gfx/gfx_structs.c.v
index 7ff172f624..22c39bac64 100644
--- a/vlib/sokol/gfx/gfx_structs.c.v
+++ b/vlib/sokol/gfx/gfx_structs.c.v
@@ -94,8 +94,8 @@ pub:
 	id u32
 }
 
-pub fn (p C.sg_pipeline) free() {
-	C.sg_destroy_pipeline(p)
+pub fn (mut p C.sg_pipeline) free() {
+	C.sg_destroy_pipeline(*p)
 }
 
 pub struct C.sg_bindings {
@@ -276,8 +276,8 @@ pub:
 	id u32
 }
 
-pub fn (s C.sg_shader) free() {
-	C.sg_destroy_shader(s)
+pub fn (mut s C.sg_shader) free() {
+	C.sg_destroy_shader(*s)
 }
 
 pub struct C.sg_pass_desc {
@@ -306,8 +306,8 @@ pub struct C.sg_pass {
 	id u32
 }
 
-pub fn (p C.sg_pass) free() {
-	C.sg_destroy_pass(p)
+pub fn (mut p C.sg_pass) free() {
+	C.sg_destroy_pass(*p)
 }
 
 pub struct C.sg_buffer_desc {
@@ -334,8 +334,8 @@ pub struct C.sg_buffer {
 	id u32
 }
 
-pub fn (b C.sg_buffer) free() {
-	C.sg_destroy_buffer(b)
+pub fn (mut b C.sg_buffer) free() {
+	C.sg_destroy_buffer(*b)
 }
 
 pub struct DepthLayers {
@@ -392,8 +392,8 @@ pub:
 	id u32
 }
 
-pub fn (i C.sg_image) free() {
-	C.sg_destroy_image(i)
+pub fn (mut i C.sg_image) free() {
+	C.sg_destroy_image(*i)
 }
 
 pub const sg_cubeface_num = 6
diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v
index 5d746a8d8f..50f6e09a1d 100644
--- a/vlib/v/checker/checker.v
+++ b/vlib/v/checker/checker.v
@@ -8147,6 +8147,19 @@ fn (mut c Checker) fn_decl(mut node ast.FnDecl) {
 						node.pos)
 				}
 			}
+			if node.name == 'free' {
+				if node.return_type != ast.void_type {
+					c.error('`.free()` methods should not have a return type', node.return_type_pos)
+				}
+				if !node.receiver.typ.is_ptr() {
+					tname := sym.name.after_char(`.`)
+					c.error('`.free()` methods should be defined on either a `(mut x &$tname)`, or a `(x &$tname)` receiver',
+						node.receiver_pos)
+				}
+				if node.params.len != 1 {
+					c.error('`.free()` methods should have 0 arguments', node.pos)
+				}
+			}
 		}
 		// needed for proper error reporting during vweb route checking
 		if node.method_idx < sym.methods.len {
diff --git a/vlib/v/checker/tests/free_method_errors.out b/vlib/v/checker/tests/free_method_errors.out
new file mode 100644
index 0000000000..2b3376d031
--- /dev/null
+++ b/vlib/v/checker/tests/free_method_errors.out
@@ -0,0 +1,21 @@
+vlib/v/checker/tests/free_method_errors.vv:3:5: error: `.free()` methods should be defined on either a `(mut x &Error1)`, or a `(x &Error1)` receiver
+    1 | struct Error1 {}
+    2 | 
+    3 | fn (a Error1) free() {}
+      |     ~~~~~~~~
+    4 | 
+    5 | struct Error2 {}
+vlib/v/checker/tests/free_method_errors.vv:7:23: error: `.free()` methods should not have a return type
+    5 | struct Error2 {}
+    6 | 
+    7 | fn (a &Error2) free() f64 {}
+      |                       ~~~
+    8 | 
+    9 | struct Error3 {}
+vlib/v/checker/tests/free_method_errors.vv:11:1: error: `.free()` methods should have 0 arguments
+    9 | struct Error3 {}
+   10 | 
+   11 | fn (a &Error3) free(x int) {}
+      | ~~~~~~~~~~~~~~~~~~~~~~~~~~
+   12 | 
+   13 | struct Ok {}
diff --git a/vlib/v/checker/tests/free_method_errors.vv b/vlib/v/checker/tests/free_method_errors.vv
new file mode 100644
index 0000000000..4fe47d3dce
--- /dev/null
+++ b/vlib/v/checker/tests/free_method_errors.vv
@@ -0,0 +1,22 @@
+struct Error1 {}
+
+fn (a Error1) free() {}
+
+struct Error2 {}
+
+fn (a &Error2) free() f64 {}
+
+struct Error3 {}
+
+fn (a &Error3) free(x int) {}
+
+struct Ok {}
+
+fn (a &Ok) free() {}
+
+fn main() {
+	dump(Error1{})
+	dump(Error2{})
+	dump(Error3{})
+	dump(Ok{})
+}