From bdf58ee3c25317880ab5ceb0104aadc9ee162a9b Mon Sep 17 00:00:00 2001
From: Rich Harris <richard.a.harris@gmail.com>
Date: Wed, 12 Apr 2017 11:30:33 -0400
Subject: [PATCH] handle this.get special case in helpers

---
 src/validate/js/propValidators/helpers.js             |  6 ++++++
 .../samples/helper-purity-check-no-this/errors.json   |  4 ++--
 .../samples/helper-purity-check-no-this/input.html    |  4 ++--
 .../samples/helper-purity-check-this-get/errors.json  |  8 ++++++++
 .../samples/helper-purity-check-this-get/input.html   | 11 +++++++++++
 5 files changed, 29 insertions(+), 4 deletions(-)
 create mode 100644 test/validator/samples/helper-purity-check-this-get/errors.json
 create mode 100644 test/validator/samples/helper-purity-check-this-get/input.html

diff --git a/src/validate/js/propValidators/helpers.js b/src/validate/js/propValidators/helpers.js
index 8248445d44..7e27e31134 100644
--- a/src/validate/js/propValidators/helpers.js
+++ b/src/validate/js/propValidators/helpers.js
@@ -24,6 +24,12 @@ export default function helpers ( validator, prop ) {
 				}
 
 				else if ( lexicalDepth === 0 ) {
+					// handle special case that's caused some people confusion — using `this.get(...)` instead of passing argument
+					// TODO do the same thing for computed values?
+					if ( node.type === 'CallExpression' && node.callee.type === 'MemberExpression' && node.callee.object.type === 'ThisExpression' && node.callee.property.name === 'get' && !node.callee.property.computed ) {
+						validator.error( `Cannot use this.get(...) — it must be passed into the helper function as an argument`, node.start );
+					}
+
 					if ( node.type === 'ThisExpression' ) {
 						validator.error( `Helpers should be pure functions — they do not have access to the component instance and cannot use 'this'. Did you mean to put this in 'methods'?`, node.start );
 					}
diff --git a/test/validator/samples/helper-purity-check-no-this/errors.json b/test/validator/samples/helper-purity-check-no-this/errors.json
index 9de3d7465d..87c71a4034 100644
--- a/test/validator/samples/helper-purity-check-no-this/errors.json
+++ b/test/validator/samples/helper-purity-check-no-this/errors.json
@@ -1,8 +1,8 @@
 [{
 	"message": "Helpers should be pure functions — they do not have access to the component instance and cannot use 'this'. Did you mean to put this in 'methods'?",
-	"pos": 74,
+	"pos": 95,
 	"loc": {
 		"line": 7,
-		"column": 11
+		"column": 4
 	}
 }]
\ No newline at end of file
diff --git a/test/validator/samples/helper-purity-check-no-this/input.html b/test/validator/samples/helper-purity-check-no-this/input.html
index e950152de8..795821b446 100644
--- a/test/validator/samples/helper-purity-check-no-this/input.html
+++ b/test/validator/samples/helper-purity-check-no-this/input.html
@@ -1,10 +1,10 @@
-{{foo()}}
+<button on:click='foo()'>foo</button>
 
 <script>
 	export default {
 		helpers: {
 			foo () {
-				return this.get( 'bar' );
+				this.set({ foo: true });
 			}
 		}
 	}
diff --git a/test/validator/samples/helper-purity-check-this-get/errors.json b/test/validator/samples/helper-purity-check-this-get/errors.json
new file mode 100644
index 0000000000..51801fe8dd
--- /dev/null
+++ b/test/validator/samples/helper-purity-check-this-get/errors.json
@@ -0,0 +1,8 @@
+[{
+	"message": "Cannot use this.get(...) — it must be passed into the helper function as an argument",
+	"pos": 74,
+	"loc": {
+		"line": 7,
+		"column": 11
+	}
+}]
\ No newline at end of file
diff --git a/test/validator/samples/helper-purity-check-this-get/input.html b/test/validator/samples/helper-purity-check-this-get/input.html
new file mode 100644
index 0000000000..e950152de8
--- /dev/null
+++ b/test/validator/samples/helper-purity-check-this-get/input.html
@@ -0,0 +1,11 @@
+{{foo()}}
+
+<script>
+	export default {
+		helpers: {
+			foo () {
+				return this.get( 'bar' );
+			}
+		}
+	}
+</script>
\ No newline at end of file