diff --git a/src/utils/namespaces.js b/src/utils/namespaces.js
index 2ab0b116f058..595d0f0defeb 100644
--- a/src/utils/namespaces.js
+++ b/src/utils/namespaces.js
@@ -5,4 +5,9 @@ export const xlink = 'http://www.w3.org/1999/xlink';
export const xml = 'http://www.w3.org/XML/1998/namespace';
export const xmlns = 'http://www.w3.org/2000/xmlns';
+export const validNamespaces = [
+ 'html', 'mathml', 'svg', 'xlink', 'xml', 'xmlns',
+ html, mathml, svg, xlink, xml, xmlns
+];
+
export default { html, mathml, svg, xlink, xml, xmlns };
diff --git a/src/validate/html/index.js b/src/validate/html/index.js
index 2e831ee9c101..8cad9bd849a7 100644
--- a/src/validate/html/index.js
+++ b/src/validate/html/index.js
@@ -1,7 +1,13 @@
import * as namespaces from '../../utils/namespaces.js';
+import flattenReference from '../../utils/flattenReference.js';
const svg = /^(?:altGlyph|altGlyphDef|altGlyphItem|animate|animateColor|animateMotion|animateTransform|circle|clipPath|color-profile|cursor|defs|desc|discard|ellipse|feBlend|feColorMatrix|feComponentTransfer|feComposite|feConvolveMatrix|feDiffuseLighting|feDisplacementMap|feDistantLight|feDropShadow|feFlood|feFuncA|feFuncB|feFuncG|feFuncR|feGaussianBlur|feImage|feMerge|feMergeNode|feMorphology|feOffset|fePointLight|feSpecularLighting|feSpotLight|feTile|feTurbulence|filter|font|font-face|font-face-format|font-face-name|font-face-src|font-face-uri|foreignObject|g|glyph|glyphRef|hatch|hatchpath|hkern|image|line|linearGradient|marker|mask|mesh|meshgradient|meshpatch|meshrow|metadata|missing-glyph|mpath|path|pattern|polygon|polyline|radialGradient|rect|set|solidcolor|stop|switch|symbol|text|textPath|title|tref|tspan|unknown|use|view|vkern)$/;
+function list ( items, conjunction = 'or' ) {
+ if ( items.length === 1 ) return items[0];
+ return `${items.slice( 0, -1 ).join( ', ' )} ${conjunction} ${items[ items.length - 1 ]}`;
+}
+
export default function validateHtml ( validator, html ) {
let elementDepth = 0;
@@ -12,13 +18,37 @@ export default function validateHtml ( validator, html ) {
}
elementDepth += 1;
+
+ node.attributes.forEach( attribute => {
+ if ( attribute.type === 'EventHandler' ) {
+ const { callee, start, type } = attribute.expression;
+
+ if ( type !== 'CallExpression' ) {
+ validator.error( `Expected a call expression`, start );
+ }
+
+ const { name } = flattenReference( callee );
+
+ if ( name === 'this' || name === 'event' ) return;
+ if ( callee.type === 'Identifier' && callee.name === 'set' || callee.name === 'fire' || callee.name in validator.methods ) return;
+
+ const validCallees = list( [ 'this.*', 'event.*', 'set', 'fire' ].concat( Object.keys( validator.methods ) ) );
+ let message = `'${validator.source.slice( callee.start, callee.end )}' is an invalid callee (should be one of ${validCallees})`;
+
+ if ( callee.type === 'Identifier' && callee.name in validator.helpers ) {
+ message += `. '${callee.name}' exists on 'helpers', did you put it in the wrong place?`;
+ }
+
+ validator.error( message, start );
+ }
+ });
}
if ( node.children ) {
node.children.forEach( visit );
}
- if (node.else ) {
+ if ( node.else ) {
visit( node.else );
}
diff --git a/src/validate/index.js b/src/validate/index.js
index e1e536f4bd0e..d83161168e38 100644
--- a/src/validate/index.js
+++ b/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 ) => {
@@ -36,28 +36,42 @@ export default function validate ( parsed, source, { onerror, onwarn, name, file
});
},
- namespace: null
+ source,
+
+ namespace: null,
+ defaultExport: null,
+ properties: {},
+ methods: {},
+ helpers: {}
};
- if ( name && !/^[a-zA-Z_$][a-zA-Z_$0-9]*$/.test( name ) ) {
- const error = new Error( `options.name must be a valid identifier` );
- onerror( error );
- }
+ try {
+ if ( name && !/^[a-zA-Z_$][a-zA-Z_$0-9]*$/.test( name ) ) {
+ const error = new Error( `options.name must be a valid identifier` );
+ throw error;
+ }
- if ( name && !/^[A-Z]/.test( name ) ) {
- const message = `options.name should be capitalised`;
- onwarn({
- message,
- filename,
- toString: () => message
- });
- }
+ if ( name && !/^[A-Z]/.test( name ) ) {
+ const message = `options.name should be capitalised`;
+ onwarn({
+ message,
+ filename,
+ toString: () => message
+ });
+ }
- if ( parsed.js ) {
- validateJs( validator, parsed.js );
- }
+ if ( parsed.js ) {
+ validateJs( validator, parsed.js );
+ }
- if ( parsed.html ) {
- validateHtml( validator, parsed.html );
+ if ( parsed.html ) {
+ validateHtml( validator, parsed.html );
+ }
+ } catch ( err ) {
+ if ( onerror ) {
+ onerror( err );
+ } else {
+ throw err;
+ }
}
}
diff --git a/src/validate/js/index.js b/src/validate/js/index.js
index cdef266c969b..3f7e8c0e427c 100644
--- a/src/validate/js/index.js
+++ b/src/validate/js/index.js
@@ -23,19 +23,19 @@ export default function validateJs ( validator, js ) {
checkForComputedKeys( validator, node.declaration.properties );
checkForDupes( validator, node.declaration.properties );
- const templateProperties = {};
+ const props = validator.properties;
node.declaration.properties.forEach( prop => {
- templateProperties[ prop.key.name ] = prop;
+ props[ prop.key.name ] = prop;
});
// Remove these checks in version 2
- if ( templateProperties.oncreate && templateProperties.onrender ) {
- validator.error( 'Cannot have both oncreate and onrender', templateProperties.onrender.start );
+ if ( props.oncreate && props.onrender ) {
+ validator.error( 'Cannot have both oncreate and onrender', props.onrender.start );
}
- if ( templateProperties.ondestroy && templateProperties.onteardown ) {
- validator.error( 'Cannot have both ondestroy and onteardown', templateProperties.onteardown.start );
+ if ( props.ondestroy && props.onteardown ) {
+ validator.error( 'Cannot have both ondestroy and onteardown', props.onteardown.start );
}
// ensure all exported props are valid
@@ -56,8 +56,8 @@ export default function validateJs ( validator, js ) {
}
});
- if ( templateProperties.namespace ) {
- const ns = templateProperties.namespace.value.value;
+ if ( props.namespace ) {
+ const ns = props.namespace.value.value;
validator.namespace = namespaces[ ns ] || ns;
}
@@ -72,4 +72,12 @@ export default function validateJs ( validator, js ) {
});
}
});
+
+ [ 'methods', 'helpers' ].forEach( key => {
+ if ( validator.properties[ key ] ) {
+ validator.properties[ key ].value.properties.forEach( prop => {
+ validator[ key ][ prop.key.name ] = prop.value;
+ });
+ }
+ });
}
diff --git a/src/validate/js/propValidators/helpers.js b/src/validate/js/propValidators/helpers.js
index 6553f079729f..7e27e311344d 100644
--- a/src/validate/js/propValidators/helpers.js
+++ b/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,45 @@ 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 ) {
+ // 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 );
+ }
+
+ 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 );
+ }
+ });
}
diff --git a/src/validate/js/propValidators/namespace.js b/src/validate/js/propValidators/namespace.js
index ca664261ef60..ec8bad0a0460 100644
--- a/src/validate/js/propValidators/namespace.js
+++ b/src/validate/js/propValidators/namespace.js
@@ -1,5 +1,22 @@
+import * as namespaces from '../../../utils/namespaces.js';
+import FuzzySet from '../utils/FuzzySet.js';
+
+const fuzzySet = new FuzzySet( namespaces.validNamespaces );
+const valid = new Set( namespaces.validNamespaces );
+
export default function namespace ( validator, prop ) {
- if ( prop.value.type !== 'Literal' || typeof prop.value.value !== 'string' ) {
+ const ns = prop.value.value;
+
+ if ( prop.value.type !== 'Literal' || typeof ns !== 'string' ) {
validator.error( `The 'namespace' property must be a string literal representing a valid namespace`, prop.start );
}
+
+ if ( !valid.has( ns ) ) {
+ const matches = fuzzySet.get( ns );
+ if ( matches && matches[0] && matches[0][0] > 0.7 ) {
+ validator.error( `Invalid namespace '${ns}' (did you mean '${matches[0][1]}'?)`, prop.start );
+ } else {
+ validator.error( `Invalid namespace '${ns}'`, prop.start );
+ }
+ }
}
diff --git a/test/validator/samples/helper-purity-check-needs-arguments/input.html b/test/validator/samples/helper-purity-check-needs-arguments/input.html
new file mode 100644
index 000000000000..244c67e697b4
--- /dev/null
+++ b/test/validator/samples/helper-purity-check-needs-arguments/input.html
@@ -0,0 +1,11 @@
+{{foo()}}
+
+
\ No newline at end of file
diff --git a/test/validator/samples/helper-purity-check-needs-arguments/warnings.json b/test/validator/samples/helper-purity-check-needs-arguments/warnings.json
new file mode 100644
index 000000000000..85abc4df9223
--- /dev/null
+++ b/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
+ }
+}]
\ No newline at end of file
diff --git a/test/validator/samples/helper-purity-check-no-this/errors.json b/test/validator/samples/helper-purity-check-no-this/errors.json
new file mode 100644
index 000000000000..87c71a4034b8
--- /dev/null
+++ b/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": 95,
+ "loc": {
+ "line": 7,
+ "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
new file mode 100644
index 000000000000..795821b446de
--- /dev/null
+++ b/test/validator/samples/helper-purity-check-no-this/input.html
@@ -0,0 +1,11 @@
+
+
+
\ No newline at end of file
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 000000000000..51801fe8dd52
--- /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 000000000000..e950152de876
--- /dev/null
+++ b/test/validator/samples/helper-purity-check-this-get/input.html
@@ -0,0 +1,11 @@
+{{foo()}}
+
+
\ No newline at end of file
diff --git a/test/validator/samples/helper-purity-check-uses-arguments/input.html b/test/validator/samples/helper-purity-check-uses-arguments/input.html
new file mode 100644
index 000000000000..559f0ba0c088
--- /dev/null
+++ b/test/validator/samples/helper-purity-check-uses-arguments/input.html
@@ -0,0 +1,13 @@
+{{sum(1, 2, 3)}}
+
+
\ No newline at end of file
diff --git a/test/validator/samples/helper-purity-check-uses-arguments/warnings.json b/test/validator/samples/helper-purity-check-uses-arguments/warnings.json
new file mode 100644
index 000000000000..0637a088a01e
--- /dev/null
+++ b/test/validator/samples/helper-purity-check-uses-arguments/warnings.json
@@ -0,0 +1 @@
+[]
\ No newline at end of file
diff --git a/test/validator/samples/method-nonexistent-helper/errors.json b/test/validator/samples/method-nonexistent-helper/errors.json
new file mode 100644
index 000000000000..80183498ca9e
--- /dev/null
+++ b/test/validator/samples/method-nonexistent-helper/errors.json
@@ -0,0 +1,8 @@
+[{
+ "message": "'foo' is an invalid callee (should be one of this.*, event.*, set, fire or bar). 'foo' exists on 'helpers', did you put it in the wrong place?",
+ "pos": 18,
+ "loc": {
+ "line": 1,
+ "column": 18
+ }
+}]
diff --git a/test/validator/samples/method-nonexistent-helper/input.html b/test/validator/samples/method-nonexistent-helper/input.html
new file mode 100644
index 000000000000..abb756f6c3ab
--- /dev/null
+++ b/test/validator/samples/method-nonexistent-helper/input.html
@@ -0,0 +1,17 @@
+
+
+
diff --git a/test/validator/samples/method-nonexistent/errors.json b/test/validator/samples/method-nonexistent/errors.json
new file mode 100644
index 000000000000..2eaf6c215d83
--- /dev/null
+++ b/test/validator/samples/method-nonexistent/errors.json
@@ -0,0 +1,8 @@
+[{
+ "message": "'foo' is an invalid callee (should be one of this.*, event.*, set, fire or bar)",
+ "pos": 18,
+ "loc": {
+ "line": 1,
+ "column": 18
+ }
+}]
diff --git a/test/validator/samples/method-nonexistent/input.html b/test/validator/samples/method-nonexistent/input.html
new file mode 100644
index 000000000000..75f2028b92f7
--- /dev/null
+++ b/test/validator/samples/method-nonexistent/input.html
@@ -0,0 +1,11 @@
+
+
+
diff --git a/test/validator/samples/namespace-invalid/errors.json b/test/validator/samples/namespace-invalid/errors.json
new file mode 100644
index 000000000000..b7f6d4d8981c
--- /dev/null
+++ b/test/validator/samples/namespace-invalid/errors.json
@@ -0,0 +1,8 @@
+[{
+ "message": "Invalid namespace 'http://www.w3.org/1999/svg' (did you mean 'http://www.w3.org/2000/svg'?)",
+ "pos": 29,
+ "loc": {
+ "line": 3,
+ "column": 2
+ }
+}]
diff --git a/test/validator/samples/namespace-invalid/input.html b/test/validator/samples/namespace-invalid/input.html
new file mode 100644
index 000000000000..a2e5baf39b01
--- /dev/null
+++ b/test/validator/samples/namespace-invalid/input.html
@@ -0,0 +1,5 @@
+
\ No newline at end of file