more validation

compiler/index.js

@@ -5,19 +5,13 @@ import generate from './generate/index.js';
 export function compile ( source, options = {} ) {
 	const parsed = parse( source, options );
 
-	const { errors, warnings } = validate( parsed, source, options );
-
-	if ( errors.length ) {
-		// TODO optionally show all errors?
-		throw errors[0];
+	if ( !options.onwarn ) {
+		options.onwarn = warning => {
+			console.warn( `(${warning.loc.line}:${warning.loc.column}) – ${warning.message}` ); // eslint-disable-line no-console
+		};
 	}
 
-	if ( warnings.length ) {
-		console.warn( `Svelte: ${warnings.length} ${warnings.length === 1 ? 'error' : 'errors'} in ${options.filename || 'template'}:` );
-		warnings.forEach( warning => {
-			console.warn( `(${warning.loc.line}:${warning.loc.column}) – ${warning.message}` );
-		});
-	}
+	validate( parsed, source, options );
 
 	return generate( parsed, source, options );
 }

compiler/parse/index.js

@@ -8,15 +8,18 @@ import hash from './utils/hash.js';
 function ParseError ( message, template, index ) {
 	const { line, column } = locate( template, index );
 
-	const frame = getCodeFrame( template, line, column );
-
 	this.name = 'ParseError';
-	this.message = `${message} (${line + 1}:${column})\n${frame}`;
+	this.message = message;
+	this.frame = getCodeFrame( template, line, column );
+
 	this.loc = { line: line + 1, column };
 	this.pos = index;
-	this.shortMessage = message;
 }
 
+ParseError.prototype.toString = function () {
+	return `${this.message} (${this.loc.line}:${this.loc.column})\n${this.frame}`;
+};
+
 export default function parse ( template ) {
 	if ( typeof template !== 'string' ) {
 		throw new TypeError( 'Template must be a string' );

compiler/validate/index.js

@@ -1,27 +1,39 @@
 import validateJs from './js/index.js';
 import { getLocator } from 'locate-character';
+import getCodeFrame from '../utils/getCodeFrame.js';
 
-export default function validate ( parsed, source ) {
+export default function validate ( parsed, source, options ) {
 	const locator = getLocator( source );
 
 	const validator = {
 		error: ( message, pos ) => {
 			const { line, column } = locator( pos );
 
-			validator.errors.push({
-				message,
-				pos,
-				loc: { line: line + 1, column }
-			});
+			const error = new Error( message );
+			error.frame = getCodeFrame( source, line, column );
+			error.loc = { line: line + 1, column };
+			error.pos = pos;
+
+			error.toString = () => `${error.message} (${error.loc.line}:${error.loc.column})\n${error.frame}`;
+
+			if ( options.onerror ) {
+				options.onerror( error );
+			} else {
+				throw error;
+			}
 		},
 
 		warn: ( message, pos ) => {
 			const { line, column } = locator( pos );
 
-			validator.warnings.push({
+			const frame = getCodeFrame( source, line, column );
+
+			options.onwarn({
 				message,
+				frame,
+				loc: { line: line + 1, column },
 				pos,
-				loc: { line: line + 1, column }
+				toString: () => `${message} (${line + 1}:${column})\n${frame}`
 			});
 		},
 
@@ -32,7 +44,7 @@ export default function validate ( parsed, source ) {
 	};
 
 	if ( parsed.js ) {
-		validateJs( validator, parsed.js, source );
+		validateJs( validator, parsed.js );
 	}
 
 	return {

compiler/validate/js/index.js

@@ -7,7 +7,7 @@ const validPropList = Object.keys( propValidators );
 
 const fuzzySet = new FuzzySet( validPropList );
 
-export default function validateJs ( validator, js, source ) {
+export default function validateJs ( validator, js ) {
 	js.content.body.forEach( node => {
 		// check there are no named exports
 		if ( node.type === 'ExportNamedDeclaration' ) {

compiler/validate/js/propValidators/components.js

@@ -1,3 +1,19 @@
+import checkForDupes from '../utils/checkForDupes.js';
+import checkForComputedKeys from '../utils/checkForComputedKeys.js';
+
 export default function components ( validator, prop ) {
+	if ( prop.value.type !== 'ObjectExpression' ) {
+		validator.error( `The 'components' property must be an object literal`, prop.start );
+		return;
+	}
+
+	checkForDupes( validator, prop.value.properties );
+	checkForComputedKeys( validator, prop.value.properties );
 
+	prop.value.properties.forEach( component => {
+		const char = component.key.name[0];
+		if ( char === char.toLowerCase() ) {
+			validator.warn( `Component names should be capitalised`, component.start );
+		}
+	});
 }

compiler/validate/js/propValidators/data.js

@@ -1,3 +1,15 @@
+const disallowed = {
+	Literal: true,
+	ObjectExpression: true,
+	ArrayExpression: true
+};
+
 export default function data ( validator, prop ) {
+	while ( prop.type === 'ParenthesizedExpression' ) prop = prop.expression;
+
+	// TODO should we disallow references and expressions as well?
 
+	if ( disallowed[ prop.value.type ] ) {
+		validator.error( `'data' must be a function`, prop.value.start );
+	}
 }

compiler/validate/js/propValidators/events.js

@@ -1,3 +1,12 @@
+import checkForDupes from '../utils/checkForDupes.js';
+import checkForComputedKeys from '../utils/checkForComputedKeys.js';
+
 export default function events ( validator, prop ) {
+	if ( prop.value.type !== 'ObjectExpression' ) {
+		validator.error( `The 'events' property must be an object literal`, prop.start );
+		return;
+	}
 
+	checkForDupes( validator, prop.value.properties );
+	checkForComputedKeys( validator, prop.value.properties );
 }

compiler/validate/js/propValidators/helpers.js

@@ -1,3 +1,12 @@
+import checkForDupes from '../utils/checkForDupes.js';
+import checkForComputedKeys from '../utils/checkForComputedKeys.js';
+
 export default function helpers ( validator, prop ) {
+	if ( prop.value.type !== 'ObjectExpression' ) {
+		validator.error( `The 'helpers' property must be an object literal`, prop.start );
+		return;
+	}
 
+	checkForDupes( validator, prop.value.properties );
+	checkForComputedKeys( validator, prop.value.properties );
 }

compiler/validate/js/propValidators/methods.js

@@ -1,3 +1,27 @@
+import checkForDupes from '../utils/checkForDupes.js';
+import checkForComputedKeys from '../utils/checkForComputedKeys.js';
+
+const builtin = {
+	set: true,
+	get: true,
+	on: true,
+	fire: true,
+	observe: true,
+	teardown: true
+};
+
 export default function methods ( validator, prop ) {
+	if ( prop.value.type !== 'ObjectExpression' ) {
+		validator.error( `The 'methods' property must be an object literal`, prop.start );
+		return;
+	}
+
+	checkForDupes( validator, prop.value.properties );
+	checkForComputedKeys( validator, prop.value.properties );
 
+	prop.value.properties.forEach( prop => {
+		if ( builtin[ prop.key.name ] ) {
+			validator.error( `Cannot overwrite built-in method '${prop.key.name}'` );
+		}
+	});
 }

test/test.js

@@ -49,7 +49,7 @@ describe( 'svelte', () => {
 					try {
 						const expected = require( `./parser/${dir}/error.json` );
 
-						assert.equal( err.shortMessage, expected.message );
+						assert.equal( err.message, expected.message );
 						assert.deepEqual( err.loc, expected.loc );
 						assert.equal( err.pos, expected.pos );
 					} catch ( err2 ) {
@@ -80,7 +80,27 @@ describe( 'svelte', () => {
 
 				try {
 					const parsed = parse( input );
-					const { errors, warnings } = validate( parsed, input );
+
+					const errors = [];
+					const warnings = [];
+
+					validate( parsed, input, {
+						onerror ( error ) {
+							errors.push({
+								message: error.message,
+								pos: error.pos,
+								loc: error.loc
+							});
+						},
+
+						onwarn ( warning ) {
+							warnings.push({
+								message: warning.message,
+								pos: warning.pos,
+								loc: warning.loc
+							});
+						}
+					});
 
 					const expectedErrors = tryToLoadJson( `test/validator/${dir}/errors.json` ) || [];
 					const expectedWarnings = tryToLoadJson( `test/validator/${dir}/warnings.json` ) || [];

test/validator/properties-components-should-be-capitalised/input.html

@@ -0,0 +1,9 @@
+<div></div>
+
+<script>
+	export default {
+		components: {
+			foo
+		}
+	};
+</script>

test/validator/properties-components-should-be-capitalised/warnings.json

@@ -0,0 +1,8 @@
+[{
+	"message": "Component names should be capitalised",
+	"loc": {
+		"line": 6,
+		"column": 3
+	},
+	"pos": 59
+}]

test/validator/properties-data-must-be-function/errors.json

@@ -0,0 +1,8 @@
+[{
+	"message": "'data' must be a function",
+	"loc": {
+		"line": 5,
+		"column": 8
+	},
+	"pos": 48
+}]

test/validator/properties-data-must-be-function/input.html

@@ -0,0 +1,9 @@
+<div></div>
+
+<script>
+	export default {
+		data: {
+			foo: 42
+		}
+	};
+</script>