Skip to content

Commit

Permalink
fix: #3253 cannot use identifiers containing special characters in fu…
Browse files Browse the repository at this point in the history
…nction `derivative`
  • Loading branch information
josdejong committed Sep 4, 2024
1 parent 4c64018 commit d1ecf44
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 8 deletions.
1 change: 1 addition & 0 deletions src/core/function/typed.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ export const createTyped = /* #__PURE__ */ factory('typed', dependencies, functi
// string starting with an alphabetic character. It is used (at least)
// in the definition of the derivative() function, as the argument telling
// what to differentiate over must (currently) be a variable.
// TODO: deprecate the identifier type (it's not used anymore, see https://github.com/josdejong/mathjs/issues/3253)
{
name: 'identifier',
test: s => isString && /^\p{L}[\p{L}\d]*$/u.test(s)
Expand Down
17 changes: 11 additions & 6 deletions src/function/algebra/derivative.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,20 @@ export const createDerivative = /* #__PURE__ */ factory(name, dependencies, ({
return options.simplify ? simplify(res) : res
}

typed.addConversion(
{ from: 'identifier', to: 'SymbolNode', convert: parse })
function parseIdentifier (string) {
const symbol = parse(string)
if (!symbol.isSymbolNode) {
throw new TypeError('Invalid variable. ' +
`Cannot parse ${JSON.stringify(string)} into a variable in function derivative`)
}
return symbol
}

const derivative = typed(name, {
'Node, SymbolNode': plainDerivative,
'Node, SymbolNode, Object': plainDerivative
'Node, SymbolNode, Object': plainDerivative,
'Node, string': (node, symbol) => plainDerivative(node, parseIdentifier(symbol)),
'Node, string, Object': (node, symbol, options) => plainDerivative(node, parseIdentifier(symbol), options)

/* TODO: implement and test syntax with order of derivatives -> implement as an option {order: number}
'Node, SymbolNode, ConstantNode': function (expr, variable, {order}) {
Expand All @@ -97,9 +105,6 @@ export const createDerivative = /* #__PURE__ */ factory(name, dependencies, ({
*/
})

typed.removeConversion(
{ from: 'identifier', to: 'SymbolNode', convert: parse })

derivative._simplify = true

derivative.toTex = function (deriv) {
Expand Down
10 changes: 8 additions & 2 deletions test/unit-tests/function/algebra/derivative.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,12 @@ describe('derivative', function () {
compareString(derivative(math.parse('x^2'), math.parse('x')), '2 * x')
})

it('should accept string input containing special characters', function () {
// NOTE: we use `parse` here on purpose to see whether derivative accepts it
compareString(derivative('1 + x', 'x'), '1')
compareString(derivative('1 + $x', '$x'), '1')
})

describe('expression parser', function () {
it('should evaluate a derivative containing string value', function () {
const res = math.evaluate('derivative("x^2", "x")')
Expand Down Expand Up @@ -260,7 +266,7 @@ describe('derivative', function () {
it('should throw error for incorrect argument types', function () {
assert.throws(function () {
derivative('42', '42')
}, /TypeError: Unexpected type of argument in function derivative \(expected: SymbolNode or identifier, actual: string, index: 1\)/)
}, /TypeError: Invalid variable. Cannot parse "42" into a variable in function derivative/)

assert.throws(function () {
derivative('[1, 2; 3, 4]', 'x')
Expand All @@ -274,7 +280,7 @@ describe('derivative', function () {
it('should throw error if incorrect number of arguments', function () {
assert.throws(function () {
derivative('x + 2')
}, /TypeError: Too few arguments in function derivative \(expected: SymbolNode or identifier, index: 1\)/)
}, /TypeError: Too few arguments in function derivative \(expected: string or SymbolNode or boolean, index: 1\)/)

assert.throws(function () {
derivative('x + 2', 'x', {}, true, 42)
Expand Down

0 comments on commit d1ecf44

Please sign in to comment.