Mathjs: Config option to disable implicit multiplication

[HaraldStiwa]

Version: mathjs-v5.2.3

Implicit multiplication is hiding syntax errors, when using scope variables

Steps to reproduce:

let scope = {
  value: 3,
  target: 4
};
math.eval('value target ? 25.3, -1', scope));

Expected behaviour:
Some error is thrown.

Actual behaviour:
If-then-else value is returned.

I guess that implicit multiplication multiplies "value target" as "value * target". And then the result is implictely converted to a boolean expression.

Is there a way to disable implicit multiplication through configuration?

[HaraldStiwa]

Okay, I have found a work-around, so far:

const node = math.parse('value target ? 25.3, -1');
node.traverse((node, path, parent) => {
      if ((node.type === 'OperatorNode')) {
        if ((node.op === '*') && (node['implicit'])) {
          throw new Error('Invalid syntax: Implicit multiplication found');
        }
      }
    });
const code = node.compile();
code.eval(scope);

[josdejong]

Thanks for your input @HaraldStiwa. I like your idea of having an option to disable implicit multiplication. It's a convenient feature but also very tricky. And thanks for sharing your workaround :+1: .

I will rename this issue to reflect that we would like to create a config option to disable implicit multiplication.

Anyone interested in implementing this new option?

[nicholasnbg]

Hey @josdejong I'd be keen to have a crack at this one.

Am I correct that the default option would be to have implicit multiplication enabled?

[josdejong]

Thanks Nick!

I'm open to discuss both default options. We can say that implicit multiplication is tricky and turn it off by default. Or we could say let's keep everything backward compatible and leave it on by default (I guess that would be the easy way). Do you have any thoughts or preferences in this regard?

FYI: I've just merged the modular_architecture branch into develop, which contains a huge change in the architecture. Just release v6.0.0-beta.1 with this. There are still a few rough edges here and there, please don't hesitate if you have questions or encounter issues.

[nicholasnbg]

I think keeping the change backwards compatible is best, but this is my first contribution to this so I'd leave decisions up to those more experiences such as yourself.

[josdejong]

Ok let's keep it backward compatible then! We can always reconsider the default value in the future if we want.

[nicholasnbg]

Hey Jos,

I've implemented the config option, just to clarify, is implementing the logic for performing (or not) the implicit multiplication in scope here? Or is that a separate issue.

Also I have not been able to reproduce the behaviour in the original comment, I get the expected syntax error.

[josdejong]

Awesome! I think there is no need to change the implicit multiplication functionality itself (in parse.js), only to enable/disable it via a config option.

The original issue is about the precedence of implicit multiplication not matching the expectations in all cases, but that will remain an the issue no matter what choices we make. I propose not to make changes in that regard.

[HaraldStiwa]

Cool!
For me it would also be okay if it can be enabled/disabled via a config option. If there is a config option for it I do not care about the defaults ;-).

[josdejong]

Thanks for your feedback Harald!