enforce helper function purity

src/validate/index.js

@@ -18,7 +18,7 @@ export default function validate ( parsed, source, { onerror, onwarn, name, file
 
 			error.toString = () => `${error.message} (${error.loc.line}:${error.loc.column})\n${error.frame}`;
 
-			onerror( error );
+			throw error;
 		},
 
 		warn: ( message, pos ) => {
@@ -45,25 +45,33 @@ export default function validate ( parsed, source, { onerror, onwarn, name, file
 		helpers: {}
 	};
 
-	if ( name && !/^[a-zA-Z_$][a-zA-Z_$0-9]*$/.test( name ) ) {
+	try {
-		const error = new Error( `options.name must be a valid identifier` );
+		if ( name && !/^[a-zA-Z_$][a-zA-Z_$0-9]*$/.test( name ) ) {
-		onerror( error );
+			const error = new Error( `options.name must be a valid identifier` );
-	}
+			throw error;
+		}
 
-	if ( name && !/^[A-Z]/.test( name ) ) {
+		if ( name && !/^[A-Z]/.test( name ) ) {
-		const message = `options.name should be capitalised`;
+			const message = `options.name should be capitalised`;
-		onwarn({
+			onwarn({
-			message,
+				message,
-			filename,
+				filename,
-			toString: () => message
+				toString: () => message
-		});
+			});
-	}
+		}
 
-	if ( parsed.js ) {
+		if ( parsed.js ) {
-		validateJs( validator, parsed.js );
+			validateJs( validator, parsed.js );
-	}
+		}
 
-	if ( parsed.html ) {
+		if ( parsed.html ) {
-		validateHtml( validator, parsed.html );
+			validateHtml( validator, parsed.html );
+		}
+	} catch ( err ) {
+		if ( onerror ) {
+			onerror( err );
+		} else {
+			throw err;
+		}
 	}
 }

src/validate/js/propValidators/helpers.js

@@ -1,5 +1,6 @@
 import checkForDupes from '../utils/checkForDupes.js';
 import checkForComputedKeys from '../utils/checkForComputedKeys.js';
+import { walk } from 'estree-walker';
 
 export default function helpers ( validator, prop ) {
 	if ( prop.value.type !== 'ObjectExpression' ) {
@@ -9,4 +10,39 @@ export default function helpers ( validator, prop ) {
 
 	checkForDupes( validator, prop.value.properties );
 	checkForComputedKeys( validator, prop.value.properties );
+
+	prop.value.properties.forEach( prop => {
+		if ( !/FunctionExpression/.test( prop.value.type ) ) return;
+
+		let lexicalDepth = 0;
+		let usesArguments = false;
+
+		walk( prop.value.body, {
+			enter ( node ) {
+				if ( /^Function/.test( node.type ) ) {
+					lexicalDepth += 1;
+				}
+
+				else if ( lexicalDepth === 0 ) {
+					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 );
+					}
+
+					else if ( node.type === 'Identifier' && node.name === 'arguments' ) {
+						usesArguments = true;
+					}
+				}
+			},
+
+			leave ( node ) {
+				if ( /^Function/.test( node.type ) ) {
+					lexicalDepth -= 1;
+				}
+			}
+		});
+
+		if ( prop.value.params.length === 0 && !usesArguments ) {
+			validator.warn( `Helpers should be pure functions, with at least one argument`, prop.start );
+		}
+	});
 }

test/validator/samples/helper-purity-check-needs-arguments/input.html

@@ -0,0 +1,11 @@
+{{foo()}}
+
+<script>
+	export default {
+		helpers: {
+			foo () {
+				return Math.random();
+			}
+		}
+	}
+</script>

test/validator/samples/helper-purity-check-needs-arguments/warnings.json

@@ -0,0 +1,8 @@
+[{
+	"message": "Helpers should be pure functions, with at least one argument",
+	"pos": 54,
+	"loc": {
+		"line": 6,
+		"column": 3
+	}
+}]

test/validator/samples/helper-purity-check-no-this/errors.json

@@ -0,0 +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,
+	"loc": {
+		"line": 7,
+		"column": 11
+	}
+}]

test/validator/samples/helper-purity-check-no-this/input.html

@@ -0,0 +1,11 @@
+{{foo()}}
+
+<script>
+	export default {
+		helpers: {
+			foo () {
+				return this.get( 'bar' );
+			}
+		}
+	}
+</script>

test/validator/samples/helper-purity-check-uses-arguments/input.html

@@ -0,0 +1,13 @@
+{{sum(1, 2, 3)}}
+
+<script>
+	export default {
+		helpers: {
+			sum () {
+				var total = '';
+				for ( var i = 0; i < arguments.length; i += 1 ) total += arguments[i];
+				return total;
+			}
+		}
+	}
+</script>

test/validator/samples/helper-purity-check-uses-arguments/warnings.json

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