Merge pull request #184 from sveltejs/gh-179

error if method is an arrow function expression and uses `this` or `arguments`

src/validate/js/propValidators/methods.js

@@ -1,5 +1,6 @@
 import checkForDupes from '../utils/checkForDupes.js';
 import checkForComputedKeys from '../utils/checkForComputedKeys.js';
+import usesThisOrArguments from '../utils/usesThisOrArguments.js';
 
 const builtin = {
 	set: true,
@@ -23,5 +24,11 @@ export default function methods ( validator, prop ) {
 		if ( builtin[ prop.key.name ] ) {
 			validator.error( `Cannot overwrite built-in method '${prop.key.name}'` );
 		}
+
+		if ( prop.value.type === 'ArrowFunctionExpression' ) {
+			if ( usesThisOrArguments( prop.value.body ) ) {
+				validator.error( `Method '${prop.key.name}' should be a function expression, not an arrow function expression`, prop.start );
+			}
+		}
 	});
 }

src/validate/js/propValidators/onrender.js

@@ -1,3 +1,9 @@
-export default function onrender () {
+import usesThisOrArguments from '../utils/usesThisOrArguments.js';
 
+export default function onrender ( validator, prop ) {
+	if ( prop.value.type === 'ArrowFunctionExpression' ) {
+		if ( usesThisOrArguments( prop.value.body ) ) {
+			validator.error( `'onrender' should be a function expression, not an arrow function expression`, prop.start );
+		}
+	}
 }

src/validate/js/propValidators/onteardown.js

@@ -1,3 +1,9 @@
-export default function onteardown () {
+import usesThisOrArguments from '../utils/usesThisOrArguments.js';
 
+export default function onteardown ( validator, prop ) {
+	if ( prop.value.type === 'ArrowFunctionExpression' ) {
+		if ( usesThisOrArguments( prop.value.body ) ) {
+			validator.error( `'onteardown' should be a function expression, not an arrow function expression`, prop.start );
+		}
+	}
 }

src/validate/js/utils/usesThisOrArguments.js

@@ -0,0 +1,24 @@
+import { walk } from 'estree-walker';
+import isReference from '../../../utils/isReference.js';
+
+export default function usesThisOrArguments ( node ) {
+	let result = false;
+
+	walk( node, {
+		enter ( node ) {
+			if ( result || node.type === 'FunctionExpression' || node.type === 'FunctionDeclaration' ) {
+				return this.skip();
+			}
+
+			if ( node.type === 'ThisExpression' ) {
+				result = true;
+			}
+
+			if ( node.type === 'Identifier' && isReference( node ) && node.name === 'arguments' ) {
+				result = true;
+			}
+		}
+	});
+
+	return result;
+}

test/validator/method-arrow-no-this/errors.json

@@ -0,0 +1 @@
+[]

test/validator/method-arrow-no-this/input.html

@@ -0,0 +1,9 @@
+<button on:click='foo()'></button>
+
+<script>
+	export default {
+		methods: {
+			foo: () => console.log( 'foo' )
+		}
+	};
+</script>

test/validator/method-arrow-this/errors.json

@@ -0,0 +1,8 @@
+[{
+	"message": "Method 'foo' should be a function expression, not an arrow function expression",
+	"pos": 79,
+	"loc": {
+		"line": 6,
+		"column": 3
+	}
+}]

test/validator/method-arrow-this/input.html

@@ -0,0 +1,11 @@
+<button on:click='foo()'></button>
+
+<script>
+	export default {
+		methods: {
+			foo: () => {
+				this.set({ a: 1 });
+			}
+		}
+	};
+</script>

test/validator/onrender-arrow-no-this/errors.json

@@ -0,0 +1 @@
+[]

test/validator/onrender-arrow-no-this/input.html

@@ -0,0 +1,5 @@
+<script>
+	export default {
+		onrender: () => console.log( 'rendering' )
+	};
+</script>

test/validator/onrender-arrow-this/errors.json

@@ -0,0 +1,8 @@
+[{
+	"message": "'onrender' should be a function expression, not an arrow function expression",
+	"pos": 29,
+	"loc": {
+		"line": 3,
+		"column": 2
+	}
+}]

test/validator/onrender-arrow-this/input.html

@@ -0,0 +1,7 @@
+<script>
+	export default {
+		onrender: () => {
+			this.set({ a: 1 });
+		}
+	};
+</script>

test/validator/onteardown-arrow-no-this/errors.json

@@ -0,0 +1 @@
+[]

test/validator/onteardown-arrow-no-this/input.html

@@ -0,0 +1,5 @@
+<script>
+	export default {
+		onteardown: () => console.log( 'tearing down' )
+	};
+</script>

test/validator/onteardown-arrow-this/errors.json

@@ -0,0 +1,8 @@
+[{
+	"message": "'onteardown' should be a function expression, not an arrow function expression",
+	"pos": 29,
+	"loc": {
+		"line": 3,
+		"column": 2
+	}
+}]

test/validator/onteardown-arrow-this/input.html

@@ -0,0 +1,7 @@
+<script>
+	export default {
+		onteardown: () => {
+			this.set({ a: 1 });
+		}
+	};
+</script>