From 2c79edf519a394f6324a641e3274fa1217bdfff3 Mon Sep 17 00:00:00 2001 From: ido Date: Sun, 1 Feb 2015 11:50:05 +0200 Subject: [PATCH] better error reporting, added column and enfOffset, deprecated index --- src/RTCodeError.js | 97 +++++++++++++++++++++++++ src/cli.js | 17 +---- src/context.js | 25 +++++-- src/reactTemplates.js | 144 ++++++++++++++++--------------------- test/data/invalid-brace.rt | 6 ++ test/data/invalid-style.rt | 3 + test/src/test.js | 128 ++++++++++++++++----------------- 7 files changed, 250 insertions(+), 170 deletions(-) create mode 100644 src/RTCodeError.js create mode 100644 test/data/invalid-brace.rt create mode 100644 test/data/invalid-style.rt diff --git a/src/RTCodeError.js b/src/RTCodeError.js new file mode 100644 index 0000000..980b521 --- /dev/null +++ b/src/RTCodeError.js @@ -0,0 +1,97 @@ +'use strict'; + +/** + * @param {string} html + * @param node + * @return {{line: number, col: number}}} + */ +function getLine(html, node) { + if (!node) { + return {col: 1, line: 1}; + } + var linesUntil = html.substring(0, node.startIndex).split('\n'); + return {line: linesUntil.length, col: linesUntil[linesUntil.length - 1].length + 1}; +} + +//function getLine(node) { +// if (!node) { +// return 0; +// } +// var line = 0; +// var prev = node.prev; +// while (prev) { +// var nl = prev.data.split('\n').length - 1; +// line += nl; +// prev = prev.prev; +// } +// +// line += getLine(node.parent); +// return line + 1; +//} + +//function RTCodeError(message, line) { +// this.name = 'RTCodeError'; +// this.message = message || ''; +// this.line = line || -1; +//} +//RTCodeError.prototype = Error.prototype; + +// Redefine properties on Error to be enumerable +/*eslint no-extend-native:0*/ +Object.defineProperty(Error.prototype, 'message', {configurable: true, enumerable: true}); +Object.defineProperty(Error.prototype, 'stack', {configurable: true, enumerable: true}); +//Object.defineProperty(Error.prototype, 'line', { configurable: true, enumerable: true }); + +/** + * @param {string} message + * @param {number=} startOffset + * @param {number=} endOffset + * @param {number=} line + * @param {number=} column + * @constructor + */ +function RTCodeError(message, startOffset, endOffset, line, column) { + Error.captureStackTrace(this, RTCodeError); + this.name = 'RTCodeError'; + this.message = message || ''; + this.index = norm(startOffset); + this.startOffset = norm(startOffset); + this.endOffset = norm(endOffset); + this.line = norm(line); + this.column = norm(column); +} + +function norm(n) { + return n === undefined ? -1 : n; +} + +RTCodeError.prototype = Object.create(Error.prototype); + +RTCodeError.build = buildError; +RTCodeError.norm = norm; + +RTCodeError.prototype.toIssue = function () { +}; + +/** + * @param {string} msg + * @param {*} context + * @param {*} node + * @return {RTCodeError} + */ +function buildError(msg, context, node) { + var pos = getLine(context.html, node); + var end; + if (node.data) { + end = node.startIndex + node.data.length; + } else if (node.next) { + end = node.next.startIndex; + } else { + end = context.html.length; + } + return new RTCodeError(msg, node.startIndex, end, pos.line, pos.col); +} + +module.exports = { + RTCodeError: RTCodeError +}; diff --git a/src/cli.js b/src/cli.js index 56d0417..262dda3 100755 --- a/src/cli.js +++ b/src/cli.js @@ -57,25 +57,10 @@ function handleSingleFile(currentOptions, filename) { context.error('invalid file, only handle rt files', filename); return;// only handle html files } -// var html = fs.readFileSync(filename).toString(); -// if (!html.match(/\<\!doctype jsx/)) { -// console.log('invalid file, missing header'); -// return; -// } -// var js = reactTemplates.convertTemplateToReact(html); -// fs.writeFileSync(filename + '.js', js); try { api.convertFile(filename, filename + '.js', currentOptions, context); } catch (e) { - context.error(e.message, filename, e.line || -1, -1, e.index || -1); -// if (defaultOptions.json) { -// context.error(e.message, filename, e.line || -1, -1, e.index || -1); -// console.log(JSON.stringify(context.getMessages(), undefined, 2)); -// } else { -// console.log('Error processing file: ' + filename + ', ' + e.message + ' line: ' + e.line || -1); -// } - // if (defaultOptions.stack) - // console.log(e.stack); + context.error(e.message, filename, e.line, e.column, e.startOffset, e.endOffset); } } diff --git a/src/context.js b/src/context.js index 65e8b59..fe8b795 100644 --- a/src/context.js +++ b/src/context.js @@ -3,7 +3,7 @@ * @typedef {{color: boolean, cwd: string, report: function(string), warn: function(string), getMessages: function():Array.}} CONTEXT */ /** - * @typedef {{msg: string, level: MESSAGE_LEVEL, file: string}} MESSAGE + * @typedef {{msg: string, level: MESSAGE_LEVEL, file: string,line:number,column:number,startOffset:number,endOffset:number}} MESSAGE */ /** @@ -17,13 +17,19 @@ var MESSAGE_LEVEL = { }; var _ = require('lodash'); +var err = require('./RTCodeError'); +var norm = err.RTCodeError.norm; + /** * @type {CONTEXT} */ var context = { + /** @type {Array.} */ messages: [], + /** @type {boolean} */ color: true, + /** @type {string} */ cwd: process.cwd(), report: function (msg) { console.log(msg); @@ -34,11 +40,20 @@ var context = { warn: function (msg, file, line, column) { context.issue(MESSAGE_LEVEL.WARN, msg, file, line, column); }, - error: function (msg, file, line, column, index) { - context.issue(MESSAGE_LEVEL.ERROR, msg, file, line, column, index); + error: function (msg, file, line, column, startOffset, endOffset) { + context.issue(MESSAGE_LEVEL.ERROR, msg, file, line, column, startOffset, endOffset); }, - issue: function (level, msg, file, line, column, index) { - context.messages.push({level: level, msg: msg, file: file || null, line: line || -1, column: column || -1, index: index || -1}); + /** + * @param {MESSAGE_LEVEL} level + * @param {string} msg + * @param {string} file + * @param {number} line + * @param {number} column + * @param {number=} startOffset + * @param {number=} endOffset + */ + issue: function (level, msg, file, line, column, startOffset, endOffset) { + context.messages.push({level: level, msg: msg, file: file || null, line: norm(line), column: norm(column), index: norm(startOffset), startOffset: norm(startOffset), endOffset: norm(endOffset)}); }, getMessages: function () { return context.messages; diff --git a/src/reactTemplates.js b/src/reactTemplates.js index 0fed9ba..68646bc 100644 --- a/src/reactTemplates.js +++ b/src/reactTemplates.js @@ -8,6 +8,8 @@ var esprima = require('esprima'); var escodegen = require('escodegen'); var reactDOMSupport = require('./reactDOMSupport'); var stringUtils = require('./stringUtils'); +var rtError = require('./RTCodeError'); +var RTCodeError = rtError.RTCodeError; var repeatTemplate = _.template('_.map(<%= collection %>,<%= repeatFunction %>.bind(<%= repeatBinds %>))'); var ifTemplate = _.template('((<%= condition %>)?(<%= body %>):null)'); @@ -34,6 +36,10 @@ var propsProp = 'rt-props'; var defaultOptions = {modules: 'amd', version: false, force: false, format: 'stylish', targetVersion: '0.12.2'}; +/** + * @param context + * @return {boolean} + */ function shouldUseCreateElement(context) { switch (context.options.targetVersion) { case '0.11.2': @@ -54,6 +60,10 @@ _.forEach(reactSupportedAttributes, function (attributeReactName) { } }); +/** + * @param children + * @return {string} + */ function concatChildren(children) { var res = ''; _.forEach(children, function (child) { @@ -66,8 +76,17 @@ function concatChildren(children) { return res; } +/** + * @const + */ var curlyMap = {'{': 1, '}': -1}; +/** + * @param node + * @param context + * @param {string} txt + * @return {string} + */ function convertText(node, context, txt) { var res = ''; var first = true; @@ -84,7 +103,7 @@ function convertText(node, context, txt) { curlyCounter += curlyMap[txt.charAt(end)] || 0; } if (curlyCounter !== 0) { - throw buildError("Failed to parse text '" + txt + "'", context, node); + throw RTCodeError.build("Failed to parse text '" + txt + "'", context, node); } else { var needsParens = start !== 0 || end !== txt.length - 1; res += (first ? '' : '+') + (needsParens ? '(' : '') + txt.substr(start + 1, end - start - 2) + (needsParens ? ')' : ''); @@ -99,6 +118,7 @@ function convertText(node, context, txt) { res = 'true'; } + //validateJS(res, node, context); return res; } @@ -120,81 +140,18 @@ function generateInjectedFunc(context, namePrefix, body, params) { return generatedFuncName; } -/** - * @param {string} html - * @param node - * @return {number} - */ -function getLine(html, node) { - if (!node) { - return 0; - } - return html.substring(0, node.startIndex).split('\n').length - 1; -} - -//function getLine(node) { -// if (!node) { -// return 0; -// } -// var line = 0; -// var prev = node.prev; -// while (prev) { -// var nl = prev.data.split('\n').length - 1; -// line += nl; -// prev = prev.prev; -// } -// -// line += getLine(node.parent); -// return line + 1; -//} - -//function RTCodeError(message, line) { -// this.name = 'RTCodeError'; -// this.message = message || ''; -// this.line = line || -1; -//} -//RTCodeError.prototype = Error.prototype; - -// Redefine properties on Error to be enumerable -/*eslint no-extend-native:0*/ -Object.defineProperty(Error.prototype, 'message', { configurable: true, enumerable: true }); -Object.defineProperty(Error.prototype, 'stack', { configurable: true, enumerable: true }); -//Object.defineProperty(Error.prototype, 'line', { configurable: true, enumerable: true }); - -function RTCodeError(message, index, line) { - Error.captureStackTrace(this, RTCodeError); - this.name = 'RTCodeError'; - this.message = message || ''; - this.index = index || -1; - this.line = line || -1; -} - -RTCodeError.prototype = Object.create(Error.prototype); - -/** - * @param {string} msg - * @param {*} context - * @param {*} node - * @return {RTCodeError} - */ -function buildError(msg, context, node) { - var line = getLine(context.html, node); - return new RTCodeError(msg, node.startIndex, line); -} - - function generateProps(node, context) { // console.log(node); var props = {}; _.forOwn(node.attribs, function (val, key) { var propKey = attributesMapping[key.toLowerCase()] || key; if (props.hasOwnProperty(propKey)) { - throw buildError('duplicate definition of ' + propKey + ' ' + JSON.stringify(node.attribs), context, node); + throw RTCodeError.build('duplicate definition of ' + propKey + ' ' + JSON.stringify(node.attribs), context, node); } if (key.indexOf('on') === 0 && !isStringOnlyCode(val)) { var funcParts = val.split('=>'); if (funcParts.length !== 2) { - throw buildError("when using 'on' events, use lambda '(p1,p2)=>body' notation or use {} to return a callback function. error: [" + key + "='" + val + "']", context, node); + throw RTCodeError.build("when using 'on' events, use lambda '(p1,p2)=>body' notation or use {} to return a callback function. error: [" + key + "='" + val + "']", context, node); } var evtParams = funcParts[0].replace('(', '').replace(')', '').trim(); var funcBody = funcParts[1].trim(); @@ -276,22 +233,26 @@ function convertHtmlToReact(node, context) { _.each(node.attribs[scopeProp].split(';'), function (scopePart) { var scopeSubParts = scopePart.split(' as '); if (scopeSubParts.length < 2) { - throw buildError("invalid scope part '" + scopePart + "'", context, node); + throw RTCodeError.build("invalid scope part '" + scopePart + "'", context, node); } var scopeName = scopeSubParts[1].trim(); + validateJS(scopeName, node, context); stringUtils.addIfNotThere(context.boundParams, scopeName); data.scopeName += stringUtils.capitalize(scopeName); data.scopeMapping[scopeName] = scopeSubParts[0].trim(); + validateJS(data.scopeMapping[scopeName], node, context); }); } if (node.attribs[templateProp]) { var arr = node.attribs[templateProp].split(' in '); if (arr.length !== 2) { - throw buildError("rt-repeat invalid 'in' expression '" + node.attribs[templateProp] + "'", context, node); + throw RTCodeError.build("rt-repeat invalid 'in' expression '" + node.attribs[templateProp] + "'", context, node); } data.item = arr[0].trim(); data.collection = arr[1].trim(); + validateJS(data.item, node, context); + validateJS(data.collection, node, context); stringUtils.addIfNotThere(context.boundParams, data.item); stringUtils.addIfNotThere(context.boundParams, data.item + 'Index'); } @@ -303,7 +264,9 @@ function convertHtmlToReact(node, context) { data.condition = node.attribs[ifProp].trim(); } data.children = node.children ? concatChildren(_.map(node.children, function (child) { - return convertHtmlToReact(child, context); + var code = convertHtmlToReact(child, context); + validateJS(code, child, context); + return code; })) : ''; if (hasNonSimpleChildren(node)) { @@ -344,7 +307,7 @@ function convertHtmlToReact(node, context) { // return html; //} function isTag(node) { - return node.type === 'tag'; + return node.type === 'tag'; } function handleSelfClosingHtmlTags(nodes) { @@ -375,29 +338,29 @@ function convertTemplateToReact(html, options) { options = _.defaults({}, options, defaultOptions); var defines = {'react/addons': 'React', lodash: '_'}; var context = defaultContext(html, options); - var rootTags = _.filter(rootNode.root()[0].children, function (i) { return i.type === 'tag'; }); + var rootTags = _.filter(rootNode.root()[0].children, {type: 'tag'}); rootTags = handleSelfClosingHtmlTags(rootTags); if (!rootTags || rootTags.length === 0) { throw new RTCodeError('Document should have a root element'); } var firstTag = null; - _.forEach(rootTags, function(tag) { + _.forEach(rootTags, function (tag) { if (tag.name === 'rt-require') { - if (!tag.attribs.dependency || !tag.attribs.as) { - throw buildError("rt-require needs 'dependency' and 'as' attributes", context, tag); - } else if (tag.children.length) { - throw buildError('rt-require may have no children', context, tag); - } else { - defines[tag.attribs.dependency] = tag.attribs.as; - } + if (!tag.attribs.dependency || !tag.attribs.as) { + throw RTCodeError.build("rt-require needs 'dependency' and 'as' attributes", context, tag); + } else if (tag.children.length) { + throw RTCodeError.build('rt-require may have no children', context, tag); + } else { + defines[tag.attribs.dependency] = tag.attribs.as; + } } else if (firstTag === null) { - firstTag = tag; + firstTag = tag; } else { - throw buildError('Document should have no more than a single root element', context, tag); + throw RTCodeError.build('Document should have no more than a single root element', context, tag); } }); if (firstTag === null) { - throw buildError('Document should have a single root element', context, rootNode.root()[0]); + throw RTCodeError.build('Document should have a single root element', context, rootNode.root()[0]); } var body = convertHtmlToReact(firstTag, context); var requirePaths = _(defines).keys().map(function (reqName) { return '"' + reqName + '"'; }).value().join(','); @@ -426,6 +389,19 @@ function generate(data, options) { return templatePJSTemplate(data); } +/** + * @param {string} code + * @param node + * @param context + */ +function validateJS(code, node, context) { + try { + esprima.parse(code); + } catch (e) { + throw RTCodeError.build(e.description, context, node); + } +} + /** * @param {string} name * @return {string} @@ -438,5 +414,7 @@ module.exports = { convertTemplateToReact: convertTemplateToReact, RTCodeError: RTCodeError, normalizeName: normalizeName, - _test: {} + _test: { + convertText: convertText + } }; diff --git a/test/data/invalid-brace.rt b/test/data/invalid-brace.rt new file mode 100644 index 0000000..aa40fc6 --- /dev/null +++ b/test/data/invalid-brace.rt @@ -0,0 +1,6 @@ + + + +
+ {tr('ANIMATION_PANEL_HEADER_LABEL'} +
\ No newline at end of file diff --git a/test/data/invalid-style.rt b/test/data/invalid-style.rt new file mode 100644 index 0000000..69a297a --- /dev/null +++ b/test/data/invalid-style.rt @@ -0,0 +1,3 @@ +
+
+
\ No newline at end of file diff --git a/test/src/test.js b/test/src/test.js index 9de5f8c..2bea995 100644 --- a/test/src/test.js +++ b/test/src/test.js @@ -6,7 +6,7 @@ var _ = require('lodash'); var path = require('path'); var React = require('react/addons'); var cheerio = require('cheerio'); - +var RTCodeError = reactTemplates.RTCodeError; var dataPath = path.resolve(__dirname, '..', 'data'); /** @@ -17,35 +17,23 @@ function readFileNormalized(filename) { return fs.readFileSync(filename).toString().replace(/\r/g, '').trim(); } -test('invalid tests', function (t) { - var files = [ - {file: 'invalid-scope.rt', issue: new reactTemplates.RTCodeError("invalid scope part 'a in a in a'", -1, -1)}, - {file: 'invalid-html.rt', issue: new reactTemplates.RTCodeError('Document should have a root element', -1, -1)}, - {file: 'invalid-exp.rt', issue: new reactTemplates.RTCodeError("Failed to parse text '\n {z\n'", 5, -1)}, - { - file: 'invalid-lambda.rt', - issue: new reactTemplates.RTCodeError("when using 'on' events, use lambda '(p1,p2)=>body' notation or use {} to return a callback function. error: [onClick='']", -1, -1) - }, - { - file: 'invalid-js.rt', - issue: new reactTemplates.RTCodeError('Line 7: Unexpected token ILLEGAL', 187, undefined) - }, - { - file: 'invalid-single-root.rt', - issue: new reactTemplates.RTCodeError('Document should have no more than a single root element', 12, 1) - }, - { - file: 'invalid-repeat.rt', - issue: new reactTemplates.RTCodeError('rt-repeat invalid \'in\' expression \'a in b in c\'', -1, -1) - }, - { - file: 'invalid-rt-require.rt', - issue: new reactTemplates.RTCodeError("rt-require needs 'dependency' and 'as' attributes", -1, -1) - } - ]; - t.plan(files.length); +var invalidFiles = [ + {file: 'invalid-scope.rt', issue: new RTCodeError("invalid scope part 'a in a in a'", 0, 35, 1, 1)}, + {file: 'invalid-html.rt', issue: new RTCodeError('Document should have a root element', -1, -1, -1, -1)}, + {file: 'invalid-exp.rt', issue: new RTCodeError("Failed to parse text '\n {z\n'", 5, 13, 1, 6)}, + {file: 'invalid-lambda.rt', issue: new RTCodeError("when using 'on' events, use lambda '(p1,p2)=>body' notation or use {} to return a callback function. error: [onClick='']", 0, 23, 1, 1)}, + {file: 'invalid-js.rt', issue: new RTCodeError('Unexpected token ILLEGAL', 0, 32, 1, 1)}, + {file: 'invalid-single-root.rt', issue: new RTCodeError('Document should have no more than a single root element', 12, 23, 2, 1)}, + {file: 'invalid-repeat.rt', issue: new RTCodeError('rt-repeat invalid \'in\' expression \'a in b in c\'', 0, 35, 1, 1)}, + {file: 'invalid-rt-require.rt', issue: new RTCodeError("rt-require needs 'dependency' and 'as' attributes", 0, 14, 1, 1)}, + {file: 'invalid-brace.rt', issue: new RTCodeError('Unexpected end of input', 128, 163, 5, 11)}, + {file: 'invalid-style.rt', issue: new RTCodeError('Unexpected token ILLEGAL', 10, 39, 2, 5)} +]; - files.forEach(check); +test('invalid tests', function (t) { + t.plan(invalidFiles.length); + + invalidFiles.forEach(check); function check(testFile) { var filename = path.join(dataPath, testFile.file); @@ -56,10 +44,14 @@ test('invalid tests', function (t) { } catch (e) { error = e; } - t.deepEqual(errorEqual(error), errorEqual(testFile.issue), 'Expect convertTemplateToReact to throw an error'); + t.deepEqual(omitStack(error), omitStack(testFile.issue), 'Expect convertTemplateToReact to throw an error'); } }); +function omitStack(err) { + return _.omit(err, 'stack', 'toIssue'); +} + /** * @param {ERR} err * @return {ERR} @@ -72,19 +64,9 @@ function normalizeError(err) { test('invalid tests json', function (t) { var cli = require('../../src/cli'); var context = require('../../src/context'); - var files = [ - {file: 'invalid-scope.rt', issue: new reactTemplates.RTCodeError("invalid scope part 'a in a in a'", -1, -1)}, - {file: 'invalid-html.rt', issue: new reactTemplates.RTCodeError('Document should have a root element', -1, -1)}, - {file: 'invalid-exp.rt', issue: new reactTemplates.RTCodeError("Failed to parse text '\n {z\n'", 5, -1)}, - { - file: 'invalid-lambda.rt', - issue: new reactTemplates.RTCodeError("when using 'on' events, use lambda '(p1,p2)=>body' notation or use {} to return a callback function. error: [onClick='']", -1, -1) - }, - {file: 'invalid-js.rt', issue: new reactTemplates.RTCodeError('Line 7: Unexpected token ILLEGAL', 187, -1)} - ]; - t.plan(files.length); + t.plan(invalidFiles.length); - files.forEach(check); + invalidFiles.forEach(check); function check(testFile) { context.clear(); @@ -108,26 +90,15 @@ function errorEqualMessage(err, file) { return { index: err.index, line: err.line, - column: err.column || -1, + column: err.column, + startOffset: err.startOffset, + endOffset: err.endOffset, msg: err.message, level: 'ERROR', file: file }; } -/** - * @param {RTCodeError} err - * @return {{index: number, line: number, message: string, name: string}} - */ -function errorEqual(err) { - return { - index: err.index, - line: err.line, - message: err.message, - name: err.name - }; -} - test('conversion test', function (t) { var files = ['div.rt', 'test.rt', 'repeat.rt', 'inputs.rt', 'require.rt']; t.plan(files.length); @@ -218,8 +189,6 @@ test('html tests', function (t) { }); test('test context', function (t) { - t.plan(3); - var context = require('../../src/context'); context.clear(); t.equal(context.hasErrors(), false); @@ -227,39 +196,65 @@ test('test context', function (t) { t.equal(context.hasErrors(), true); context.clear(); t.equal(context.hasErrors(), false); + + t.end(); }); test('test shell', function (t) { - t.plan(3); - var shell = require('../../src/shell'); var context = require('../../src/context'); var newContext = _.cloneDeep(context); - var output; + var outputJSON; newContext.options.format = 'json'; newContext.report = function (text) { - output = text; + outputJSON = text; }; var r = shell.printResults(newContext); t.equal(r, 0); context.error('hi', '', 1, 1); r = shell.printResults(newContext); t.equal(r, 1); - t.equal(output, '[\n {\n "level": "ERROR",\n "msg": "hi",\n "file": null,\n "line": 1,\n "column": 1,\n "index": -1\n }\n]'); + var output = JSON.parse(outputJSON); + t.deepEqual(output, [{column: 1, endOffset: -1, file: null, index: -1, level: 'ERROR', line: 1, msg: 'hi', startOffset: -1}]); context.clear(); + t.end(); }); test('test shell', function (t) { - t.plan(1); - var filename = path.join(dataPath, 'div.rt'); var cli = require('../../src/cli'); var r = cli.execute(filename + ' -r --dry-run'); t.equal(r, 0); + t.end(); +}); + +test('test convertText', function (t) { + var texts = [ + {input: '{}', expected: '()'}, + {input: "a {'b'}", expected: '"a "+(\'b\')'} + ]; + t.plan(texts.length); + texts.forEach(check); + function check(testData) { + var r = reactTemplates._test.convertText({}, {}, testData.input); + t.equal(r, testData.expected); + } +}); + +test('test convertText errors', function (t) { + var texts = [ + {input: '{}', expected: '()'}, + {input: "a {'b'}", expected: '"a "+(\'b\')'} + ]; + t.plan(texts.length); + texts.forEach(check); + function check(testData) { + var r = reactTemplates._test.convertText({}, {}, testData.input); + t.equal(r, testData.expected); + } }); test('util.isStale', function (t) { - t.plan(2); var a = path.join(dataPath, 'a.tmp'); var b = path.join(dataPath, 'b.tmp'); @@ -280,4 +275,5 @@ test('util.isStale', function (t) { fs.unlinkSync(a); fs.unlinkSync(b); + t.end(); });