Repository: josdejong/mathjs (js)

Issue:
Derivative doesn't recognize constants like pi and e
The function `derivative` currently doesn't handle built-in constants like `pi` and `e`:

```js
math.derivative('e^x', 'x') // Uncaught Error: Invalid symbol in rule: e
```

and sometimes a weird indirect error is thrown: 

```js
math.derivative('log(x, 10)', 'x') // Uncaught Error: Invalid symbol in rule: e
```

What is a good way to deal with built-in constants?


Pull request description:
Fixed simplify to recognize named constants
This fixes #838.

Patch:
diff --git a/lib/function/algebra/simplify.js b/lib/function/algebra/simplify.js
index 1df6adc73..81ab08822 100644
--- a/lib/function/algebra/simplify.js
+++ b/lib/function/algebra/simplify.js
@@ -1,7 +1,7 @@
 'use strict';
 
 
-function factory (type, config, load, typed) {
+function factory (type, config, load, typed, math) {
   var parse = load(require('../../expression/parse'));
   var ConstantNode = load(require('../../expression/node/ConstantNode'));
   var FunctionNode = load(require('../../expression/node/FunctionNode'));
@@ -114,6 +114,28 @@ function factory (type, config, load, typed) {
     });
   }
 
+  // All constants that are allowed in rules
+  var SUPPORTED_CONSTANTS = {
+    true: true,
+    false: true,
+    e: true,
+    i: true,
+    Infinity: true,
+    LN2: true,
+    LN10: true,
+    LOG2E: true,
+    LOG10E: true,
+    NaN: true,
+    phi: true,
+    pi: true,
+    SQRT1_2: true,
+    SQRT2: true,
+    tau: true,
+    // null: false,
+    // uninitialized: false,
+    // version: false,
+  };
+
   // Array of strings, used to build the ruleSet.
   // Each l (left side) and r (right side) are parsed by
   // the expression parser into a node tree.
@@ -495,7 +517,18 @@ function factory (type, config, load, typed) {
       if (rule.name.length === 0) {
         throw new Error('Symbol in rule has 0 length...!?');
       }
-      if (rule.name[0] == 'n' || rule.name.substring(0,2) == '_p') {
+
+      if (math.hasOwnProperty(rule.name)) {
+        if (!SUPPORTED_CONSTANTS[rule.name]) {
+          throw new Error('Built in constant: ' + rule.name + ' is not supported by simplify.');
+        }
+
+        // built-in constant must match exactly
+        if(rule.name !== node.name) {
+          return [];
+        }
+      }
+      else if (rule.name[0] == 'n' || rule.name.substring(0,2) == '_p') {
         // rule matches _anything_, so assign this node to the rule.name placeholder
         // Assign node to the rule.name placeholder.
         // Our parent will check for matches among placeholders.
@@ -536,11 +569,8 @@ function factory (type, config, load, typed) {
       }
     }
     else if (rule instanceof ConstantNode) {
-      // Literal constant in our rule, so much match node exactly
-      if(rule.value === node.value) {
-        // The constants match
-      }
-      else {
+      // Literal constant must match exactly
+      if(rule.value !== node.value) {
         return [];
       }
     }
@@ -607,5 +637,6 @@ function factory (type, config, load, typed) {
   return simplify;
 }
 
+exports.math = true;
 exports.name = 'simplify';
 exports.factory = factory;


Test patch:
diff --git a/test/function/algebra/simplify.test.js b/test/function/algebra/simplify.test.js
index 94ea9d4a4..742cf185f 100644
--- a/test/function/algebra/simplify.test.js
+++ b/test/function/algebra/simplify.test.js
@@ -54,6 +54,30 @@ describe('simplify', function() {
     simplifyAndCompare('foo(x)', 'foo(x)');
   });
 
+  it('should handle valid built-in constant symbols in rules', function() {
+    assert.equal(math.simplify('true', ['true -> 1']).toString(), '1');
+    assert.equal(math.simplify('false', ['false -> 0']).toString(), '0');
+    assert.equal(math.simplify('log(e)', ['log(e) -> 1']).toString(), '1');
+    assert.equal(math.simplify('sin(pi * x)', ['sin(pi * n) -> 0']).toString(), '0');
+    assert.equal(math.simplify('i', ['i -> 1']).toString(), '1');
+    assert.equal(math.simplify('Infinity', ['Infinity -> 1']).toString(), '1');
+    assert.equal(math.simplify('LN2', ['LN2 -> 1']).toString(), '1');
+    assert.equal(math.simplify('LN10', ['LN10 -> 1']).toString(), '1');
+    assert.equal(math.simplify('LOG2E', ['LOG2E -> 1']).toString(), '1');
+    assert.equal(math.simplify('LOG10E', ['LOG10E -> 1']).toString(), '1');
+    assert.equal(math.simplify('NaN', ['NaN -> 1']).toString(), '1');
+    assert.equal(math.simplify('phi', ['phi -> 1']).toString(), '1');
+    assert.equal(math.simplify('SQRT1_2', ['SQRT1_2 -> 1']).toString(), '1');
+    assert.equal(math.simplify('SQRT2', ['SQRT2 -> 1']).toString(), '1');
+    assert.equal(math.simplify('tau', ['tau -> 1']).toString(), '1');
+  });
+
+  it('should throw an error for invalid built-in constant symbols in rules', function() {
+    assert.throws(function(){ math.simplify('null', ['null -> 1']).toString(); });
+    assert.throws(function(){ math.simplify('uninitialized', ['uninitialized -> 1']).toString(); });
+    assert.throws(function(){ math.simplify('version', ['version -> 1']).toString(); });
+  });
+
   it('should remove addition of 0', function() {
     simplifyAndCompare('x+0', 'x');
     simplifyAndCompare('x-0', 'x');