From 6ecb5b32dd5eeff96e6d21e35c5d25ee10afd450 Mon Sep 17 00:00:00 2001 From: Patrick Steele-Idem Date: Wed, 2 Dec 2015 23:01:23 -0700 Subject: [PATCH] Record error for invalid attributes --- compiler/CodeGenerator.js | 13 ++++- compiler/CompileContext.js | 58 ++++++++++++++++++- compiler/Compiler.js | 13 +++++ compiler/HtmlJsParser.js | 9 +-- compiler/Parser.js | 31 +++++++++- compiler/ast/HtmlElement.js | 6 +- compiler/ast/Node.js | 14 ++++- test/autotest.js | 4 ++ test/codegen-test.js | 1 - test/fixtures/marko-taglib.json | 3 + .../autotest/error-invalid-attr/test.js | 14 +++-- test/render-test.js | 27 +++++++-- 12 files changed, 166 insertions(+), 27 deletions(-) diff --git a/compiler/CodeGenerator.js b/compiler/CodeGenerator.js index f829f3dc0..56f63ce4f 100644 --- a/compiler/CodeGenerator.js +++ b/compiler/CodeGenerator.js @@ -419,9 +419,18 @@ class Generator { return false; } - addError(message) { + addError(message, code) { + ok('"message" is required'); + var node = this._currentNode; - this.context.addError(node, message); + + if (typeof message === 'object') { + var errorInfo = message; + errorInfo.node = node; + this.context.addError(errorInfo); + } else { + this.context.addError({node, code, message}); + } } } diff --git a/compiler/CompileContext.js b/compiler/CompileContext.js index 3bf95bd9a..6fe64c39c 100644 --- a/compiler/CompileContext.js +++ b/compiler/CompileContext.js @@ -8,6 +8,51 @@ var deresolve = require('./util/deresolve'); var UniqueVars = require('./util/UniqueVars'); var PosInfo = require('./util/PosInfo'); +class CompileError { + constructor(errorInfo, context) { + this.context = context; + this.node = errorInfo.node; + this.message = errorInfo.message; + this.code = errorInfo.code; + + var pos = errorInfo.pos; + var endPos = errorInfo.endPos; + + if (pos == null) { + pos = this.node && this.node.pos; + } + + if (endPos == null) { + endPos = this.node && this.node.endPos; + } + + if (pos != null) { + pos = context.getPosInfo(pos); + } + + if (endPos != null) { + endPos = context.getPosInfo(endPos); + } + + this.pos = pos; + this.endPos = endPos; + } + + toString() { + var pos = this.pos; + if (pos) { + pos = '[' + pos + '] '; + } else { + pos = ''; + } + var str = pos + this.message; + if (this.node) { + str += ' (' + this.node.toString() + ')'; + } + return str; + } +} + class CompileContext { constructor(src, filename, builder) { ok(typeof src === 'string', '"src" string is required'); @@ -25,6 +70,7 @@ class CompileContext { this._uniqueVars = new UniqueVars(); this._srcCharProps = null; this._flags = {}; + this._errors = []; } getPosInfo(pos) { @@ -46,8 +92,16 @@ class CompileContext { return this._flags.hasOwnProperty(name); } - addError(node, message) { - throw new Error('addError() not fully implemented. Error: ' + message); // TODO + addError(errorInfo) { + this._errors.push(new CompileError(errorInfo, this)); + } + + hasErrors() { + return this._errors.length !== 0; + } + + getErrors() { + return this._errors; } getRequirePath(targetFilename) { diff --git a/compiler/Compiler.js b/compiler/Compiler.js index aedad2f3a..f0ed7d63b 100644 --- a/compiler/Compiler.js +++ b/compiler/Compiler.js @@ -84,6 +84,19 @@ class Compiler { var codeGenerator = new CodeGenerator(context); codeGenerator.generateCode(transformedAST); + if (context.hasErrors()) { + var errors = context.getErrors(); + + var message = 'An error occurred while trying to compile template at path "' + filename + '". Error(s) in template:\n'; + for (var i = 0, len = errors.length; i < len; i++) { + let error = errors[i]; + message += (i + 1) + ') ' + error.toString() + '\n'; + } + var error = new Error(message); + error.errors = errors; + throw error; + } + var compiledSrc = codeGenerator.getCode(); return compiledSrc; } diff --git a/compiler/HtmlJsParser.js b/compiler/HtmlJsParser.js index c35d4446d..1e7189875 100644 --- a/compiler/HtmlJsParser.js +++ b/compiler/HtmlJsParser.js @@ -27,10 +27,7 @@ class HtmlJsParser { }, onopentag(event) { - var tagName = event.tagName; - var argument = event.argument; - var attributes = event.attributes; - handlers.handleStartElement({tagName, argument, attributes}); + handlers.handleStartElement(event); }, onclosetag(event) { @@ -60,10 +57,6 @@ class HtmlJsParser { parser.parse(src); } - - getPos() { - return 0; - } } module.exports = HtmlJsParser; \ No newline at end of file diff --git a/compiler/Parser.js b/compiler/Parser.js index ed8d44e00..ff7a76b29 100644 --- a/compiler/Parser.js +++ b/compiler/Parser.js @@ -1,5 +1,6 @@ 'use strict'; var ok = require('assert').ok; +var path = require('path'); var COMPILER_ATTRIBUTE_HANDLERS = { whitespace: function(attr, compilerOptions) { @@ -25,6 +26,14 @@ function parseExpression(expression) { return expression; } +function getTaglibPath(taglibPath) { + if (typeof window === 'undefined') { + return path.relative(process.cwd(), taglibPath); + } else { + return taglibPath; + } +} + class Parser { constructor(parserImpl) { @@ -135,8 +144,8 @@ class Parser { }) }; - - var tagDef = context.taglibLookup.getTag(tagName); + var taglibLookup = context.taglibLookup; + var tagDef = taglibLookup.getTag(tagName); if (tagDef) { var nodeFactoryFunc = tagDef.getNodeFactory(); if (nodeFactoryFunc) { @@ -150,6 +159,24 @@ class Parser { node.pos = el.pos; + // Validate the attributes + attributes.forEach((attr) => { + let attrName = attr.name; + let attrDef = taglibLookup.getAttribute(tagName, attrName); + if (!attrDef) { + if (tagDef) { + // var isAttrForTaglib = compiler.taglibs.isTaglib(attrUri); + //Tag doesn't allow dynamic attributes + context.addError({ + node: node, + message: 'The tag "' + tagName + '" in taglib "' + getTaglibPath(tagDef.taglibId) + '" does not support attribute "' + attrName + '"' + }); + + } + return; + } + }); + this.parentNode.appendChild(node); this.stack.push({ diff --git a/compiler/ast/HtmlElement.js b/compiler/ast/HtmlElement.js index d85df6516..58f5c6016 100644 --- a/compiler/ast/HtmlElement.js +++ b/compiler/ast/HtmlElement.js @@ -39,7 +39,6 @@ class HtmlElement extends Node { let attrName = attr.name; let attrValue = attr.value; - if (attr.isLiteralValue()) { var literalValue = attrValue.value; if (typeof literalValue === 'boolean') { @@ -137,6 +136,11 @@ class HtmlElement extends Node { callback.call(thisObj, attributes[i]); } } + + toString() { + var tagName = this.tagName; + return '<' + tagName + '>'; + } } module.exports = HtmlElement; \ No newline at end of file diff --git a/compiler/ast/Node.js b/compiler/ast/Node.js index 0b1700c42..28557e883 100644 --- a/compiler/ast/Node.js +++ b/compiler/ast/Node.js @@ -3,6 +3,7 @@ var Container = require('./Container'); var ArrayContainer = require('./ArrayContainer'); var ok = require('assert').ok; var extend = require('raptor-util/extend'); +var inspect = require('util').inspect; class Node { constructor(type) { @@ -56,7 +57,7 @@ class Node { } toString() { - return JSON.stringify(this, null, 2); + return inspect(this); } toJSON() { @@ -75,6 +76,17 @@ class Node { isDetached() { return this.container == null; } + + /** + * Used by the Node.js require('util').inspect function. + * We default to inspecting on the simplified version + * of this node that is the same version we use when + * serializing to JSON. + */ + inspect(depth, opts) { + // We inspect in the simplified version of this object t + return this.toJSON(); + } } module.exports = Node; \ No newline at end of file diff --git a/test/autotest.js b/test/autotest.js index fe126fab0..a85ccabf3 100644 --- a/test/autotest.js +++ b/test/autotest.js @@ -11,6 +11,10 @@ function autoTest(name, dir, run, options) { var expectedPath = path.join(dir, 'expected' + compareExtension); var actual = run(dir); + if (actual === '$PASS$') { + return; + } + var actualJSON = isJSON ? JSON.stringify(actual, null, 2) : null; fs.writeFileSync( diff --git a/test/codegen-test.js b/test/codegen-test.js index 6f69f74c7..540ec27ed 100644 --- a/test/codegen-test.js +++ b/test/codegen-test.js @@ -24,7 +24,6 @@ describe('compiler/codegen', function() { var main = require(path.join(dir, 'index.js')); var generateCodeFunc = main; var ast = generateCodeFunc(builder); - var generator = createGenerator(); generator.generateCode(ast); return generator.getCode(); diff --git a/test/fixtures/marko-taglib.json b/test/fixtures/marko-taglib.json index f91095253..9126f0d22 100644 --- a/test/fixtures/marko-taglib.json +++ b/test/fixtures/marko-taglib.json @@ -2,5 +2,8 @@ "": { "renderer": "./taglib/test-declared-attributes/renderer.js", "@name": "string" + }, + "": { + "@foo": "string" } } \ No newline at end of file diff --git a/test/fixtures/render/autotest/error-invalid-attr/test.js b/test/fixtures/render/autotest/error-invalid-attr/test.js index 01675821d..1baf36eab 100644 --- a/test/fixtures/render/autotest/error-invalid-attr/test.js +++ b/test/fixtures/render/autotest/error-invalid-attr/test.js @@ -2,10 +2,12 @@ var expect = require('chai').expect; exports.templateData = {}; -exports.options = { - handleCompileError: function(e) { - expect(e.toString()).to.contain('template.marko:1:0'); - expect(e.toString()).to.contain('The tag "test-invalid-attr" in taglib'); - expect(e.toString()).to.contain('does not support attribute "invalid"'); - } +exports.checkError = function(e) { + expect(Array.isArray(e.errors)).to.equal(true); + expect(e.errors.length).to.equal(1); + + var message = e.toString(); + expect(message).to.contain('template.marko:1:0'); + expect(message).to.contain('The tag "test-invalid-attr" in taglib'); + expect(message).to.contain('does not support attribute "invalid"'); }; \ No newline at end of file diff --git a/test/render-test.js b/test/render-test.js index 17d6f856f..6a272089c 100644 --- a/test/render-test.js +++ b/test/render-test.js @@ -13,11 +13,30 @@ describe('render', function() { function run(dir) { var templatePath = path.join(dir, 'template.marko'); var mainPath = path.join(dir, 'test.js'); - var template = marko.load(templatePath); + var main = require(mainPath); - var templateData = main.templateData || {}; - var html = template.renderSync(templateData); - return html; + + if (main.checkError) { + var e; + + try { + marko.load(templatePath); + } catch(_e) { + e = _e; + } + + if (!e) { + throw new Error('Error expected'); + } + + main.checkError(e); + return '$PASS$'; + } else { + var template = marko.load(templatePath); + var templateData = main.templateData || {}; + var html = template.renderSync(templateData); + return html; + } }, { compareExtension: '.html'