fix: numeric literals can be set as object keys and used to access objects
- Parser extended to accept numeric literals as object keys in object expressions.
- Parser extended to accept numeric literals as object accessors including negative numbers.
- Parser extended to accept operations and expressions resulting in numbers as object accessors.
- If a number is parsed as an object key or accessor, it is converted to a string before being used as an object key / accessor.
- If an operation or expression resulting in a number is parsed as an object accessor, the result of the operation / expression is evaluated before being converted to a string and used as an object accessor
- Numeric literals, operations and expressions resulting in numbers are now accepted in square brackets for object assigments. The resulting number is first converted to a string before being used as an object key.
- Care was taken to manage numbers with leading zeros either as object keys or accessors. eg 05. They're run through a Number constructor before being converted to strings to ensure there are no preceeding zeros
- Dot notation for object assignment and accessing remains unchanged
Closes #3352
@josdejong ready for review
This is just a preliminary review; I have not had a chance to go over the core code changes that implement the new functionality. However, there are no new test cases to demonstrate the new functionality (and help prevent future regressions on this feature), and a few irrelevant changes that do not belong in this PR, which I have commented. Please remove the irrelevant changes and add a thorough suite of unit tests for the new feature, and we can move on to a more thorough code review. Thanks!
Understood...I'll remove the unnecessary eslint, prettier suggestions and whitespace improvement and add tests for the parser adjustments I've made
@gwhitney unnecessary changes removed and unit test suites added....ready for review
@josdejong the contributor codegiyu has quite reasonably reflected the idiosyncrasy from JavaScript into the MathJS Expressions language that {2: 3} is a valid object literal with property '2' but {-2: 3} is a syntax error. Is that what MathJS would prefer? Or would you rather that {-2: 3}, for example, be allowed, as a literal for a plain object with property '-2'? I personally am a bit unclear on the motivation for JavaScript to disallow negative numeric literal keys in an object.
OK, now that @josdejong has reviewed the code in more detail, I will leave feedback on your changes to him. But @codegiyu you may want to push your changes so far so that Jos can see them. If you don't think they're ready for merging yet, you should click the "convert to draft" link, and then you can mark it "ready for review" when you think it's back in shape. Thanks!
I'll reply here in the main thread since I get a bit lost in the inline conversations 😅.
Let me try to explain in a bit more detail where I think we need to go. It may be the case that this resolves some of the current questions/issues already.
-
The method
accessneeds to change from:access(object, index)To:
access(object, evalIndex, evalProperty, scope, args)Because
accessnow needs to determine whether to evaluate as index or as property. Depending on whether dealing with an Array, Matrix, or Object, the functionaccesswill first callevalIndex(scope, args, context)orevalProperty(scope, args)and then apply it to the array/object. -
The
AccessorNode._compilemethod will then look something like this:_compile (math, argNames) { const evalObject = this.object._compile(math, argNames) const evalIndex = this.index._compile(math, argNames) const evalProperty = this.index._compileProperty(math, argNames) // DOES NOT YET EXIST return function evalAccessorNode (scope, args, context) { const object = evalObject(scope, args, context) return access(object, evalIndex, evalProperty, scope, args) } } -
The
AssignmentNode._compilemethod needs a similar change asAccessorNode._compile -
We need a new method
IndexNode._compileProperty, which replaces the need for our oldisObjectPropertyandgetObjectProperty. This needs to return an evaluation function which tries to evaluate the index as a property, which can be a string or number. If that is not successful, the function throws an exception.
Does this help?
@codegiyu did you see my last feedback on this PR? Please let me know if you need more pointers or help.
@codegiyu did you see my last feedback on this PR? Please let me know if you need more pointers or help.
I'm so sorry, I didn't notice your feedback earlier. I'm checking it out now
Thank you for the guide on a direction on modifying a few of the methods....I'll get it sorted as much as I can before midnight tonight
Thanks for the update! There is no need to hurry, I just wanted to know if it's still on your radar. Take your time.