Repository: josdejong/mathjs
Pull request #922 (2017-08-13 19:06:50)

Auto implicit fixes
Continuing #921 


Patch:
diff --git a/lib/expression/node/OperatorNode.js b/lib/expression/node/OperatorNode.js
index 6dc3eff3c..88ce9b7fc 100644
--- a/lib/expression/node/OperatorNode.js
+++ b/lib/expression/node/OperatorNode.js
@@ -137,7 +137,7 @@ function factory (type, config, load, typed) {
    * @return {boolean[]}
    * @private
    */
-  function calculateNecessaryParentheses(root, parenthesis, args, latex) {
+  function calculateNecessaryParentheses(root, parenthesis, implicit, args, latex) {
     //precedence of the root OperatorNode
     var precedence = operators.getPrecedence(root, parenthesis);
     var associativity = operators.getAssociativity(root, parenthesis);
@@ -158,154 +158,184 @@ function factory (type, config, load, typed) {
       return parens;
     }
 
-    if (args.length === 0) {
-      return [];
-    } else if (args.length === 1) { //unary operators
-      //precedence of the operand
-      var operandPrecedence = operators.getPrecedence(args[0], parenthesis);
-
-      //handle special cases for LaTeX, where some of the parentheses aren't needed
-      if (latex && (operandPrecedence !== null)) {
-        var operandIdentifier;
-        var rootIdentifier;
-        if (parenthesis === 'keep') {
-          operandIdentifier = args[0].getIdentifier();
-          rootIdentifier = root.getIdentifier();
-        }
-        else {
-          //Ignore Parenthesis Nodes when not in 'keep' mode
-          operandIdentifier = args[0].getContent().getIdentifier();
-          rootIdentifier = root.getContent().getIdentifier();
-        }
-        if (operators.properties[precedence][rootIdentifier].latexLeftParens === false) {
-          return [false];
-        }
+    var result = undefined;
+    switch (args.length) {
+      case 0:
+        result = [];
+        break;
+
+      case 1: //unary operators
+        //precedence of the operand
+        var operandPrecedence = operators.getPrecedence(args[0], parenthesis);
+
+        //handle special cases for LaTeX, where some of the parentheses aren't needed
+        if (latex && (operandPrecedence !== null)) {
+          var operandIdentifier;
+          var rootIdentifier;
+          if (parenthesis === 'keep') {
+            operandIdentifier = args[0].getIdentifier();
+            rootIdentifier = root.getIdentifier();
+          }
+          else {
+            //Ignore Parenthesis Nodes when not in 'keep' mode
+            operandIdentifier = args[0].getContent().getIdentifier();
+            rootIdentifier = root.getContent().getIdentifier();
+          }
+          if (operators.properties[precedence][rootIdentifier].latexLeftParens === false) {
+            result = [false];
+            break;
+          }
 
-        if (operators.properties[operandPrecedence][operandIdentifier].latexParens === false) {
-          return [false];
+          if (operators.properties[operandPrecedence][operandIdentifier].latexParens === false) {
+            result = [false];
+            break;
+          }
         }
-      }
 
-      if (operandPrecedence === null) {
-        //if the operand has no defined precedence, no parens are needed
-        return [false];
-      }
-
-      if (operandPrecedence <= precedence) {
-        //if the operands precedence is lower, parens are needed
-        return [true];
-      }
+        if (operandPrecedence === null) {
+          //if the operand has no defined precedence, no parens are needed
+          result = [false];
+          break;
+        }
 
-      //otherwise, no parens needed
-      return [false];
-    } else if (args.length === 2) { //binary operators
-      var lhsParens; //left hand side needs parenthesis?
-      //precedence of the left hand side
-      var lhsPrecedence = operators.getPrecedence(args[0], parenthesis);
-      //is the root node associative with the left hand side
-      var assocWithLhs = operators.isAssociativeWith(root, args[0], parenthesis);
-
-      if (lhsPrecedence === null) {
-        //if the left hand side has no defined precedence, no parens are needed
-        //FunctionNode for example
-        lhsParens = false;
-      }
-      else if ((lhsPrecedence === precedence) && (associativity === 'right') && !assocWithLhs) {
-        //In case of equal precedence, if the root node is left associative
-        // parens are **never** necessary for the left hand side.
-        //If it is right associative however, parens are necessary
-        //if the root node isn't associative with the left hand side
-        lhsParens = true;
-      }
-      else if (lhsPrecedence < precedence) {
-        lhsParens = true;
-      }
-      else {
-        lhsParens = false;
-      }
+        if (operandPrecedence <= precedence) {
+          //if the operands precedence is lower, parens are needed
+          result = [true];
+          break;
+        }
 
-      var rhsParens; //right hand side needs parenthesis?
-      //precedence of the right hand side
-      var rhsPrecedence = operators.getPrecedence(args[1], parenthesis);
-      //is the root node associative with the right hand side?
-      var assocWithRhs = operators.isAssociativeWith(root, args[1], parenthesis);
+        //otherwise, no parens needed
+        result = [false];
+        break;
+
+      case 2: //binary operators
+        var lhsParens; //left hand side needs parenthesis?
+        //precedence of the left hand side
+        var lhsPrecedence = operators.getPrecedence(args[0], parenthesis);
+        //is the root node associative with the left hand side
+        var assocWithLhs = operators.isAssociativeWith(root, args[0], parenthesis);
+
+        if (lhsPrecedence === null) {
+          //if the left hand side has no defined precedence, no parens are needed
+          //FunctionNode for example
+          lhsParens = false;
+        }
+        else if ((lhsPrecedence === precedence) && (associativity === 'right') && !assocWithLhs) {
+          //In case of equal precedence, if the root node is left associative
+          // parens are **never** necessary for the left hand side.
+          //If it is right associative however, parens are necessary
+          //if the root node isn't associative with the left hand side
+          lhsParens = true;
+        }
+        else if (lhsPrecedence < precedence) {
+          lhsParens = true;
+        }
+        else {
+          lhsParens = false;
+        }
 
-      if (rhsPrecedence === null) {
-        //if the right hand side has no defined precedence, no parens are needed
-        //FunctionNode for example
-        rhsParens = false;
-      }
-      else if ((rhsPrecedence === precedence) && (associativity === 'left') && !assocWithRhs) {
-        //In case of equal precedence, if the root node is right associative
-        // parens are **never** necessary for the right hand side.
-        //If it is left associative however, parens are necessary
-        //if the root node isn't associative with the right hand side
-        rhsParens = true;
-      }
-      else if (rhsPrecedence < precedence) {
-        rhsParens = true;
-      }
-      else {
-        rhsParens = false;
-      }
+        var rhsParens; //right hand side needs parenthesis?
+        //precedence of the right hand side
+        var rhsPrecedence = operators.getPrecedence(args[1], parenthesis);
+        //is the root node associative with the right hand side?
+        var assocWithRhs = operators.isAssociativeWith(root, args[1], parenthesis);
 
-      //handle special cases for LaTeX, where some of the parentheses aren't needed
-      if (latex) {
-        var rootIdentifier;
-        var lhsIdentifier;
-        var rhsIdentifier;
-        if (parenthesis === 'keep') {
-          rootIdentifier = root.getIdentifier();
-          lhsIdentifier = root.args[0].getIdentifier();
-          rhsIdentifier = root.args[1].getIdentifier();
+        if (rhsPrecedence === null) {
+          //if the right hand side has no defined precedence, no parens are needed
+          //FunctionNode for example
+          rhsParens = false;
+        }
+        else if ((rhsPrecedence === precedence) && (associativity === 'left') && !assocWithRhs) {
+          //In case of equal precedence, if the root node is right associative
+          // parens are **never** necessary for the right hand side.
+          //If it is left associative however, parens are necessary
+          //if the root node isn't associative with the right hand side
+          rhsParens = true;
+        }
+        else if (rhsPrecedence < precedence) {
+          rhsParens = true;
         }
         else {
-          //Ignore ParenthesisNodes when not in 'keep' mode
-          rootIdentifier = root.getContent().getIdentifier();
-          lhsIdentifier = root.args[0].getContent().getIdentifier();
-          rhsIdentifier = root.args[1].getContent().getIdentifier();
+          rhsParens = false;
         }
 
-        if (lhsPrecedence !== null) {
-          if (operators.properties[precedence][rootIdentifier].latexLeftParens === false) {
-            lhsParens = false;
+        //handle special cases for LaTeX, where some of the parentheses aren't needed
+        if (latex) {
+          var rootIdentifier;
+          var lhsIdentifier;
+          var rhsIdentifier;
+          if (parenthesis === 'keep') {
+            rootIdentifier = root.getIdentifier();
+            lhsIdentifier = root.args[0].getIdentifier();
+            rhsIdentifier = root.args[1].getIdentifier();
           }
-
-          if (operators.properties[lhsPrecedence][lhsIdentifier].latexParens === false) {
-            lhsParens = false;
+          else {
+            //Ignore ParenthesisNodes when not in 'keep' mode
+            rootIdentifier = root.getContent().getIdentifier();
+            lhsIdentifier = root.args[0].getContent().getIdentifier();
+            rhsIdentifier = root.args[1].getContent().getIdentifier();
           }
-        }
 
-        if (rhsPrecedence !== null) {
-          if (operators.properties[precedence][rootIdentifier].latexRightParens === false) {
-            rhsParens = false;
+          if (lhsPrecedence !== null) {
+            if (operators.properties[precedence][rootIdentifier].latexLeftParens === false) {
+              lhsParens = false;
+            }
+
+            if (operators.properties[lhsPrecedence][lhsIdentifier].latexParens === false) {
+              lhsParens = false;
+            }
           }
 
-          if (operators.properties[rhsPrecedence][rhsIdentifier].latexParens === false) {
-            rhsParens = false;
+          if (rhsPrecedence !== null) {
+            if (operators.properties[precedence][rootIdentifier].latexRightParens === false) {
+              rhsParens = false;
+            }
+
+            if (operators.properties[rhsPrecedence][rhsIdentifier].latexParens === false) {
+              rhsParens = false;
+            }
           }
         }
-      }
 
-      return [lhsParens, rhsParens];
-    } else if ((args.length > 2) && ((root.getIdentifier() === 'OperatorNode:add') || (root.getIdentifier() === 'OperatorNode:multiply'))) {
-      var parensArray = args.map(function (arg) {
-        var argPrecedence = operators.getPrecedence(arg, parenthesis);
-        var assocWithArg = operators.isAssociativeWith(root, arg, parenthesis);
-        var argAssociativity = operators.getAssociativity(arg, parenthesis);
-        if (argPrecedence === null) {
-          //if the argument has no defined precedence, no parens are needed
-          return false;
-        } else if ((precedence === argPrecedence) && (associativity === argAssociativity) && !assocWithArg) {
-          return true;
-        } else if (argPrecedence < precedence) {
+        result = [lhsParens, rhsParens];
+        break;
+
+      default:
+        if ((root.getIdentifier() === 'OperatorNode:add') || (root.getIdentifier() === 'OperatorNode:multiply')) {
+          var result = args.map(function (arg) {
+            var argPrecedence = operators.getPrecedence(arg, parenthesis);
+            var assocWithArg = operators.isAssociativeWith(root, arg, parenthesis);
+            var argAssociativity = operators.getAssociativity(arg, parenthesis);
+            if (argPrecedence === null) {
+              //if the argument has no defined precedence, no parens are needed
+              return false;
+            } else if ((precedence === argPrecedence) && (associativity === argAssociativity) && !assocWithArg) {
+              return true;
+            } else if (argPrecedence < precedence) {
+              return true;
+            }
+
+            return false;
+          });
+        }
+        break;
+    }
+
+    //handles an edge case of 'auto' parentheses with implicit multiplication of ConstantNode
+    //In that case print parentheses for ParenthesisNodes even though they normally wouldn't be
+    //printed.
+    if ((args.length >= 2) && (root.getIdentifier() === 'OperatorNode:multiply') && root.implicit && (parenthesis === 'auto') && (implicit === 'hide')) {
+      result = args.map(function (arg, index) {
+        var isParenthesisNode = (arg.getIdentifier() === 'ParenthesisNode');
+        if (result[index] || isParenthesisNode) { //put in parenthesis?
           return true;
         }
 
         return false;
       });
-      return parensArray;
     }
+
+    return result;
   }
 
   /**
@@ -317,7 +347,7 @@ function factory (type, config, load, typed) {
     var parenthesis = (options && options.parenthesis) ? options.parenthesis : 'keep';
     var implicit = (options && options.implicit) ? options.implicit : 'hide';
     var args = this.args;
-    var parens = calculateNecessaryParentheses(this, parenthesis, args, false);
+    var parens = calculateNecessaryParentheses(this, parenthesis, implicit, args, false);
 
     if (args.length === 1) { //unary operators
       var assoc = operators.getAssociativity(this, parenthesis);
@@ -381,7 +411,7 @@ function factory (type, config, load, typed) {
     var parenthesis = (options && options.parenthesis) ? options.parenthesis : 'keep';
     var implicit = (options && options.implicit) ? options.implicit : 'hide';
     var args = this.args;
-    var parens = calculateNecessaryParentheses(this, parenthesis, args, false);
+    var parens = calculateNecessaryParentheses(this, parenthesis, implicit, args, false);
 
     if (args.length === 1) { //unary operators
       var assoc = operators.getAssociativity(this, parenthesis);
@@ -447,7 +477,7 @@ function factory (type, config, load, typed) {
     var parenthesis = (options && options.parenthesis) ? options.parenthesis : 'keep';
     var implicit = (options && options.implicit) ? options.implicit : 'hide';
     var args = this.args;
-    var parens = calculateNecessaryParentheses(this, parenthesis, args, true);
+    var parens = calculateNecessaryParentheses(this, parenthesis, implicit, args, true);
     var op = latex.operators[this.fn];
     op = typeof op === 'undefined' ? this.op : op; //fall back to using this.op
 


Test patch:
diff --git a/test/expression/node/OperatorNode.test.js b/test/expression/node/OperatorNode.test.js
index ba187dfe8..91a4678c4 100644
--- a/test/expression/node/OperatorNode.test.js
+++ b/test/expression/node/OperatorNode.test.js
@@ -724,4 +724,34 @@ describe('OperatorNode', function() {
     assert.equal(h.toTex({implicit: 'hide'}), '2~\\left(3+4\\right)');
     assert.equal(h.toTex({implicit: 'show'}), '2\\cdot\\left(3+4\\right)');
   });
+
+  it ('should stringify implicit multiplications between ConstantNodes with parentheses', function () {
+    var a = math.parse('(4)(4)(4)(4)');
+    var b = math.parse('4b*4(4)');
+    var c = math.parse('(4(4(4)))');
+
+    assert.equal(a.toString({implicit: 'hide', parenthesis: 'auto'}), '(4) (4) (4) (4)');
+    assert.equal(b.toString({implicit: 'hide', parenthesis: 'auto'}), '4 b * 4 (4)');
+    assert.equal(c.toString({implicit: 'hide', parenthesis: 'auto'}), '4 (4 (4))');
+  });
+
+  it ('should LaTeX implicit multiplications between ConstantNodes with parentheses', function () {
+    var a = math.parse('(4)(4)(4)(4)');
+    var b = math.parse('4b*4(4)');
+    var c = math.parse('(4(4(4)))');
+
+    assert.equal(a.toTex({implicit: 'hide', parenthesis: 'auto'}), '\\left(4\\right)~\\left(4\\right)~\\left(4\\right)~\\left(4\\right)');
+    assert.equal(b.toTex({implicit: 'hide', parenthesis: 'auto'}), '4~\\mathrm{b}\\cdot4~\\left(4\\right)');
+    assert.equal(c.toTex({implicit: 'hide', parenthesis: 'auto'}), '4~\\left(4~\\left(4\\right)\\right)');
+  });
+
+  it ('should HTML implicit multiplications between ConstantNodes with parentheses', function () {
+    var a = math.parse('(4)(4)(4)(4)');
+    var b = math.parse('4b*4(4)');
+    var c = math.parse('(4(4(4)))');
+
+    assert.equal(a.toHTML({implicit: 'hide', parenthesis: 'auto'}), '<span class="math-parenthesis math-round-parenthesis">(</span><span class="math-number">4</span><span class="math-parenthesis math-round-parenthesis">)</span><span class="math-operator math-binary-operator math-implicit-binary-operator"></span><span class="math-parenthesis math-round-parenthesis">(</span><span class="math-number">4</span><span class="math-parenthesis math-round-parenthesis">)</span><span class="math-operator math-binary-operator math-implicit-binary-operator"></span><span class="math-parenthesis math-round-parenthesis">(</span><span class="math-number">4</span><span class="math-parenthesis math-round-parenthesis">)</span><span class="math-operator math-binary-operator math-implicit-binary-operator"></span><span class="math-parenthesis math-round-parenthesis">(</span><span class="math-number">4</span><span class="math-parenthesis math-round-parenthesis">)</span>');
+    assert.equal(b.toHTML({implicit: 'hide', parenthesis: 'auto'}), '<span class="math-number">4</span><span class="math-operator math-binary-operator math-implicit-binary-operator"></span><span class="math-symbol">b</span><span class="math-operator math-binary-operator math-explicit-binary-operator">*</span><span class="math-number">4</span><span class="math-operator math-binary-operator math-implicit-binary-operator"></span><span class="math-parenthesis math-round-parenthesis">(</span><span class="math-number">4</span><span class="math-parenthesis math-round-parenthesis">)</span>');
+    assert.equal(c.toHTML({implicit: 'hide', parenthesis: 'auto'}), '<span class="math-number">4</span><span class="math-operator math-binary-operator math-implicit-binary-operator"></span><span class="math-parenthesis math-round-parenthesis">(</span><span class="math-number">4</span><span class="math-operator math-binary-operator math-implicit-binary-operator"></span><span class="math-parenthesis math-round-parenthesis">(</span><span class="math-number">4</span><span class="math-parenthesis math-round-parenthesis">)</span><span class="math-parenthesis math-round-parenthesis">)</span>');
+  });
 });