check scope in rt-if and fix eslint warnings

.gitignore

@@ -21,5 +21,5 @@ npm-debug.log
 /coverage
 
 ### Test Output ###
-test/data/*.rt.actual.js
+test/data/**/*.rt.actual.js
-test/data/*.code.js
+test/data/**/*.code.js

playground/playground.js

@@ -203,17 +203,11 @@ define(['react', 'jquery', 'lodash', './playground-fiddle.rt', './playground.rt'
             var editor = this.refs.editorRT;
             var name = window.reactTemplates.normalizeName(state.name) + 'RT';
             var code = null;
-            var annot = null;
             try {
                 code = window.reactTemplates.convertTemplateToReact(html.trim().replace(/\r/g, ''), {modules: 'none', name: name});
                 clearMessage(editor);
             } catch (e) {
-                if (e.name === 'RTCodeError') {
+                var annot = e.name === 'RTCodeError' ? {line: e.line, message: e.message, index: e.index} : {line: 1, message: e.message};
-                    //index: -1 line: -1 message: "Document should have a root element" name: "RTCodeError"
-                    annot = {line: e.line, message: e.message, index: e.index};
-                } else {
-                    annot = {line: 1, message: e.message};
-                }
                 this.showErrorAnnotation(annot, editor);
                 //showMessage(editor, msg);
                 console.log(e);

src/RTCodeError.js

@@ -114,7 +114,7 @@ function getNodeLoc(context, node) {
     var end;
     if (node.data) {
         end = node.startIndex + node.data.length;
-    } else if (node.next) {
+    } else if (node.next) { // eslint-disable-line
         end = node.next.startIndex;
     } else {
         end = context.html.length;

src/api.js

@@ -41,12 +41,7 @@ function convertFile(source, target, options, context) {
     if (shouldAddName) {
         options.name = reactTemplates.normalizeName(path.basename(source, path.extname(source))) + 'RT';
     }
-    var js;
+    var js = options.modules === 'jsrt' ? convertJSRTToJS(html, context, options) : convertRT(html, context, options);
-    if (options.modules === 'jsrt') {
-        js = convertJSRTToJS(html, context, options);
-    } else {
-        js = convertRT(html, context, options);
-    }
     if (!options.dryRun) {
         fs.writeFileSync(target, js);
     }

src/cli.js

@@ -26,11 +26,11 @@ function executeOptions(currentOptions) {
         }
     } else if (currentOptions.listTargetVersion) {
         printVersions(currentOptions);
-    } else if (!files.length) {
+    } else if (files.length) {
-        console.log(options.generateHelp());
-    } else {
         _.forEach(files, handleSingleFile.bind(this, currentOptions));
         ret = shell.printResults(context);
+    } else {
+        console.log(options.generateHelp());
     }
     return ret;
 }

src/reactTemplates.js

@@ -34,10 +34,15 @@ var propsMergeFunction = [
 ].join('\n');
 
 var classSetTemplate = _.template('_.keys(_.pick(<%= classSet %>, _.identity)).join(" ")');
-var simpleTagTemplate = _.template('<%= name %>(<%= props %><%= children %>)');
+
-var tagTemplate = _.template('<%= name %>.apply(this, [<%= props %><%= children %>])');
+function getTagTemplateString(simpleTagTemplate, shouldCreateElement) {
-var simpleTagTemplateCreateElement = _.template('React.createElement(<%= name %>,<%= props %><%= children %>)');
+    if (simpleTagTemplate) {
-var tagTemplateCreateElement = _.template('React.createElement.apply(this, [<%= name %>,<%= props %><%= children %>])');
+        return shouldCreateElement ? 'React.createElement(<%= name %>,<%= props %><%= children %>)' : '<%= name %>(<%= props %><%= children %>)';
+    }
+    return shouldCreateElement ? 'React.createElement.apply(this, [<%= name %>,<%= props %><%= children %>])' : '<%= name %>.apply(this, [<%= props %><%= children %>])';
+}
+
+
 var commentTemplate = _.template(' /* <%= data %> */ ');
 
 var repeatAttr = 'rt-repeat';
@@ -344,32 +349,12 @@ function convertHtmlToReact(node, context) {
         }
 
         if (node.attribs[scopeAttr]) {
-            data.innerScope = {
+            handleScopeAttribute(node, context, data);
-                scopeName: '',
+        }
-                innerMapping: {},
-                outerMapping: {}
-            };
 
-            data.innerScope.outerMapping = _.zipObject(context.boundParams, context.boundParams);
+        if (node.attribs[ifAttr]) {
-
+            validateIfAttribute(node, context, data);
-            _(node.attribs[scopeAttr]).split(';').invoke('trim').compact().forEach( function (scopePart) {
+            data.condition = node.attribs[ifAttr].trim();
-                var scopeSubParts = _(scopePart).split(' as ').invoke('trim').value();
-                if (scopeSubParts.length < 2) {
-                    throw RTCodeError.buildFormat(context, node, "invalid scope part '%s'", scopePart);
-                }
-                var alias = scopeSubParts[1];
-                var value = scopeSubParts[0];
-                validateJS(alias, node, context);
-
-                // this adds both parameters to the list of parameters passed further down
-                // the scope chain, as well as variables that are locally bound before any
-                // function call, as with the ones we generate for rt-scope.
-                stringUtils.addIfMissing(context.boundParams, alias);
-
-                data.innerScope.scopeName += stringUtils.capitalize(alias);
-                data.innerScope.innerMapping[alias] = 'var ' + alias + ' = ' + value + ';';
-                validateJS(data.innerScope.innerMapping[alias], node, context);
-            }).value();
         }
 
         data.props = generateProps(node, context);
@@ -385,20 +370,14 @@ function convertHtmlToReact(node, context) {
                 }
             }
         }
-        if (node.attribs[ifAttr]) {
+
-            data.condition = node.attribs[ifAttr].trim();
-        }
         data.children = utils.concatChildren(_.map(node.children, function (child) {
             var code = convertHtmlToReact(child, context);
             validateJS(code, child, context);
             return code;
         }));
 
-        if (hasNonSimpleChildren(node)) {
+        data.body = _.template(getTagTemplateString(!hasNonSimpleChildren(node), reactSupport.shouldUseCreateElement(context)))(data);
-            data.body = reactSupport.shouldUseCreateElement(context) ? tagTemplateCreateElement(data) : tagTemplate(data);
-        } else {
-            data.body = reactSupport.shouldUseCreateElement(context) ? simpleTagTemplateCreateElement(data) : simpleTagTemplate(data);
-        }
 
         if (node.attribs[scopeAttr]) {
             var functionBody = _.values(data.innerScope.innerMapping).join('\n') + 'return ' + data.body;
@@ -429,6 +408,54 @@ function convertHtmlToReact(node, context) {
     }
 }
 
+function handleScopeAttribute(node, context, data) {
+    data.innerScope = {
+        scopeName: '',
+        innerMapping: {},
+        outerMapping: {}
+    };
+
+    data.innerScope.outerMapping = _.zipObject(context.boundParams, context.boundParams);
+
+    _(node.attribs[scopeAttr]).split(';').invoke('trim').compact().forEach( function (scopePart) {
+        var scopeSubParts = _(scopePart).split(' as ').invoke('trim').value();
+        if (scopeSubParts.length < 2) {
+            throw RTCodeError.buildFormat(context, node, "invalid scope part '%s'", scopePart);
+        }
+        var alias = scopeSubParts[1];
+        var value = scopeSubParts[0];
+        validateJS(alias, node, context);
+
+        // this adds both parameters to the list of parameters passed further down
+        // the scope chain, as well as variables that are locally bound before any
+        // function call, as with the ones we generate for rt-scope.
+        stringUtils.addIfMissing(context.boundParams, alias);
+
+        data.innerScope.scopeName += stringUtils.capitalize(alias);
+        data.innerScope.innerMapping[alias] = 'var ' + alias + ' = ' + value + ';';
+        validateJS(data.innerScope.innerMapping[alias], node, context);
+    }).value();
+}
+
+function validateIfAttribute(node, context, data) {
+    var innerMappingKeys = _.keys(data.innerScope && data.innerScope.innerMapping || {});
+    var ifAttributeTree = null;
+    try {
+        ifAttributeTree = esprima.parse(node.attribs[ifAttr]);
+    } catch (e) {
+        throw new RTCodeError(e.message, e.index, -1);
+    }
+    if (ifAttributeTree && ifAttributeTree.body && ifAttributeTree.body.length === 1 &&
+        ifAttributeTree.body[0].type === 'ExpressionStatement') {
+        // make sure that rt-if does not use an inner mapping
+        if (ifAttributeTree.body[0].expression && utils.usesScopeName(innerMappingKeys, ifAttributeTree.body[0].expression)) {
+            throw RTCodeError.buildFormat(context, node, "invalid scope mapping used in if part '%s'", node.attribs[ifAttr]);
+        }
+    } else {
+        throw RTCodeError.buildFormat(context, node, "invalid if part '%s'", node.attribs[ifAttr]);
+    }
+}
+
 /**
  * @param node
  * @return {boolean}
@@ -507,21 +534,17 @@ function convertRT(html, reportContext, options) {
         .map(function (reqName) { return '"' + reqName + '"'; })
         .join(',');
     var requireVars = _.values(defines).join(',');
-    var buildImport;
+    var buildImportString;
     if (options.modules === 'typescript') {
-        buildImport = function (reqVar, reqPath) {
+        buildImportString = "import %s = require('%s');";
-            return util.format("import %s = require('%s');", reqVar, reqPath);
+    } else if (options.modules === 'es6') { // eslint-disable-line
-        };
+        buildImportString = "import %s from '%s';";
-    } else if (options.modules === 'es6') {
-        buildImport = function (reqVar, reqPath) {
-            return util.format("import %s from '%s';", reqVar, reqPath);
-        };
     } else {
-        buildImport = function (reqVar, reqPath) {
+        buildImportString = "var %s = require('%s');";
-            return util.format("var %s = require('%s');", reqVar, reqPath);
-        };
     }
-    var vars = _(defines).map(buildImport).join('\n');
+    var vars = _(defines).map(function (reqVar, reqPath) {
+        return util.format(buildImportString, reqVar, reqPath);
+    }).join('\n');
 
     if (options.flow) {
         vars = '/* @flow */\n' + vars;

src/utils.js

@@ -67,7 +67,55 @@ function validate(options, context, reportContext, node) {
     }
 }
 
+/**
+ * return true if any node in the given tree uses a scope name from the given set, false - otherwise.
+ * @param scopeNames a set of scope names to find
+ * @param node root of a syntax tree generated from an ExpressionStatement or one of its children.
+ */
+function usesScopeName(scopeNames, node) {
+    function usesScope(root) {
+        return usesScopeName(scopeNames, root);
+    }
+    if (_.isEmpty(scopeNames)) {
+        return false;
+    }
+    // rt-if="x"
+    if (node.type === 'Identifier') {
+        return _.includes(scopeNames, node.name);
+    }
+    // rt-if="e({key1: value1})"
+    if (node.type === 'Property') {
+        return usesScope(node.value);
+    }
+    // rt-if="e.x" or rt-if="e1[e2]"
+    if (node.type === 'MemberExpression') {
+        return node.computed ? usesScope(node.object) || usesScope(node.property) : usesScope(node.object);
+    }
+    // rt-if="!e"
+    if (node.type === 'UnaryExpression') {
+        return usesScope(node.argument);
+    }
+    // rt-if="e1 || e2" or rt-if="e1 | e2"
+    if (node.type === 'LogicalExpression' || node.type === 'BinaryExpression') {
+        return usesScope(node.left) || usesScope(node.right);
+    }
+    // rt-if="e1(e2, ... eN)"
+    if (node.type === 'CallExpression') {
+        return usesScope(node.callee) || _.some(node.arguments, usesScope);
+    }
+    // rt-if="f({e1: e2})"
+    if (node.type === 'ObjectExpression') {
+        return _.some(node.properties, usesScope);
+    }
+    // rt-if="e1[e2]"
+    if (node.type === 'ArrayExpression') {
+        return _.some(node.elements, usesScope);
+    }
+    return false;
+}
+
 module.exports = {
+    usesScopeName: usesScopeName,
     normalizeName: normalizeName,
     validateJS: validateJS,
     isStringOnlyCode: isStringOnlyCode,

test/data/if-with-scope/invalid-if-scope-1.rt

@@ -0,0 +1,5 @@
+<div key="active-users"
+     rt-scope="this.getCurrentActiveUsers() as activeUsers"
+     rt-if="this.bar(activeUsers.length)">
+    <span>some text</span>
+</div>

test/data/if-with-scope/invalid-if-scope-2.rt

@@ -0,0 +1,5 @@
+<div key="active-users"
+     rt-scope="this.getCurrentActiveUsers() as activeUsers"
+     rt-if="this.bar[activeUsers || 0]">
+    <span>some text</span>
+</div>

test/data/if-with-scope/invalid-if-scope-3.rt

@@ -0,0 +1,5 @@
+<div key="active-users"
+     rt-scope="this.getCurrentActiveUsers() as activeUsers"
+     rt-if="this.foo + activeUsers.length > this.bar">
+    <span>some text</span>
+</div>

test/data/if-with-scope/invalid-if-scope-4.rt

@@ -0,0 +1,5 @@
+<div key="active-users"
+     rt-scope="this.getCurrentActiveUsers as getCurrentActiveUsers"
+     rt-if="getCurrentActiveUsers().length">
+    <span>some text</span>
+</div>

test/data/if-with-scope/invalid-if-scope-5.rt

@@ -0,0 +1,5 @@
+<div key="active-users"
+     rt-scope="this.getCurrentActiveUsers() as activeUsers"
+     rt-if="this.bar({activeUsers})">
+    <span>some text</span>
+</div>

test/data/if-with-scope/valid-if-scope.rt

@@ -0,0 +1,5 @@
+<div key="active-users"
+     rt-scope="this.activeUsers as activeUsers"
+     rt-if="this.activeUsers">
+    <span>some text</span>
+</div>

test/data/if-with-scope/valid-if-scope.rt.js

@@ -0,0 +1,13 @@
+define([
+    'react/addons',
+    'lodash'
+], function (React, _) {
+    'use strict';
+    function scopeActiveUsers1() {
+        var activeUsers = this.activeUsers;
+        return React.createElement('div', { 'key': 'active-users' }, React.createElement('span', {}, 'some text'));
+    }
+    return function () {
+        return this.activeUsers ? scopeActiveUsers1.apply(this, []) : null;
+    };
+});

test/src/test.js

@@ -12,6 +12,11 @@ var RTCodeError = reactTemplates.RTCodeError;
 var dataPath = path.resolve(__dirname, '..', 'data');
 
 var invalidFiles = [
+    {file: 'if-with-scope/invalid-if-scope-1.rt', issue: new RTCodeError("invalid scope mapping used in if part 'this.bar(activeUsers.length)'", 0, 160, 1, 1)},
+    {file: 'if-with-scope/invalid-if-scope-2.rt', issue: new RTCodeError("invalid scope mapping used in if part 'this.bar[activeUsers || 0]'", 0, 158, 1, 1)},
+    {file: 'if-with-scope/invalid-if-scope-3.rt', issue: new RTCodeError("invalid scope mapping used in if part 'this.foo + activeUsers.length > this.bar'", 0, 172, 1, 1)},
+    {file: 'if-with-scope/invalid-if-scope-4.rt', issue: new RTCodeError("invalid scope mapping used in if part 'getCurrentActiveUsers().length'", 0, 170, 1, 1)},
+    {file: 'if-with-scope/invalid-if-scope-5.rt', issue: new RTCodeError("invalid scope mapping used in if part 'this.bar({activeUsers})'", 0, 155, 1, 1)},
     {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)},
@@ -92,6 +97,11 @@ function errorEqualMessage(err, file) {
     };
 }
 
+test('rt-if with rt-scope test', function (t) {
+    var files = ['if-with-scope/valid-if-scope.rt'];
+    testFiles(t, files);
+});
+
 test('conversion test', function (t) {
     var files = ['div.rt', 'test.rt', 'repeat.rt', 'inputs.rt', 'require.rt'];
     testFiles(t, files);