From c199407db255990af0ec1cbe2459aaacf8030ba9 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 26 Nov 2016 11:34:16 -0500 Subject: [PATCH] more validation --- compiler/index.js | 16 ++++------ compiler/parse/index.js | 11 ++++--- compiler/validate/index.js | 30 +++++++++++++------ compiler/validate/js/index.js | 2 +- .../validate/js/propValidators/components.js | 16 ++++++++++ compiler/validate/js/propValidators/data.js | 12 ++++++++ compiler/validate/js/propValidators/events.js | 9 ++++++ .../validate/js/propValidators/helpers.js | 9 ++++++ .../validate/js/propValidators/methods.js | 24 +++++++++++++++ test/test.js | 24 +++++++++++++-- .../input.html | 9 ++++++ .../warnings.json | 8 +++++ .../errors.json | 8 +++++ .../input.html | 9 ++++++ 14 files changed, 160 insertions(+), 27 deletions(-) create mode 100644 test/validator/properties-components-should-be-capitalised/input.html create mode 100644 test/validator/properties-components-should-be-capitalised/warnings.json create mode 100644 test/validator/properties-data-must-be-function/errors.json create mode 100644 test/validator/properties-data-must-be-function/input.html diff --git a/compiler/index.js b/compiler/index.js index 8ee6777e96..069417ed96 100644 --- a/compiler/index.js +++ b/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 ); } diff --git a/compiler/parse/index.js b/compiler/parse/index.js index 823a496b1e..87e7599258 100644 --- a/compiler/parse/index.js +++ b/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' ); diff --git a/compiler/validate/index.js b/compiler/validate/index.js index 3e47bb4790..a2ee763a2c 100644 --- a/compiler/validate/index.js +++ b/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 { diff --git a/compiler/validate/js/index.js b/compiler/validate/js/index.js index bbe518b285..026a304309 100644 --- a/compiler/validate/js/index.js +++ b/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' ) { diff --git a/compiler/validate/js/propValidators/components.js b/compiler/validate/js/propValidators/components.js index b90d9babc2..d8fc4aadde 100644 --- a/compiler/validate/js/propValidators/components.js +++ b/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 ); + } + }); } diff --git a/compiler/validate/js/propValidators/data.js b/compiler/validate/js/propValidators/data.js index 89fe0763b6..32911e328a 100644 --- a/compiler/validate/js/propValidators/data.js +++ b/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 ); + } } diff --git a/compiler/validate/js/propValidators/events.js b/compiler/validate/js/propValidators/events.js index ca7b280348..437b259132 100644 --- a/compiler/validate/js/propValidators/events.js +++ b/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 ); } diff --git a/compiler/validate/js/propValidators/helpers.js b/compiler/validate/js/propValidators/helpers.js index 71916a5623..6553f07972 100644 --- a/compiler/validate/js/propValidators/helpers.js +++ b/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 ); } diff --git a/compiler/validate/js/propValidators/methods.js b/compiler/validate/js/propValidators/methods.js index 275c47db11..daa5bd662a 100644 --- a/compiler/validate/js/propValidators/methods.js +++ b/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}'` ); + } + }); } diff --git a/test/test.js b/test/test.js index 5cc883f017..423dce5613 100644 --- a/test/test.js +++ b/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` ) || []; diff --git a/test/validator/properties-components-should-be-capitalised/input.html b/test/validator/properties-components-should-be-capitalised/input.html new file mode 100644 index 0000000000..3052ff968e --- /dev/null +++ b/test/validator/properties-components-should-be-capitalised/input.html @@ -0,0 +1,9 @@ +
+ + diff --git a/test/validator/properties-components-should-be-capitalised/warnings.json b/test/validator/properties-components-should-be-capitalised/warnings.json new file mode 100644 index 0000000000..8be7714b33 --- /dev/null +++ b/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 +}] diff --git a/test/validator/properties-data-must-be-function/errors.json b/test/validator/properties-data-must-be-function/errors.json new file mode 100644 index 0000000000..d72146f3ff --- /dev/null +++ b/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 +}] diff --git a/test/validator/properties-data-must-be-function/input.html b/test/validator/properties-data-must-be-function/input.html new file mode 100644 index 0000000000..5b40b54cdc --- /dev/null +++ b/test/validator/properties-data-must-be-function/input.html @@ -0,0 +1,9 @@ +
+ +