start implementing NULLISH-COALESCING, likely incomplete

[ArneBab]

I don’t really know what I’m doing, but maybe this can be a start.

For https://github.com/mooz/js2-mode/issues/559

[dgutov]

You should probably start with a parser in tests/parser.el.

Otherwise, this looks halfway complete, but you need to actually create the corresponding syntax node.

Take a look at js2-parse-or-expr.

The exact place in call hierarchy, though, should be determined by operator precedence. Looking at https://tc39.es/proposal-nullish-coalescing/#prod-ShortCircuitExpression, the new function should probably wrap the said js2-parse-or-expr.

[ArneBab]

Could you review again? I’m not sure whether my tests are sensible, but I think I’m creating the nodes now by putting ?? between cond and or.

[dgutov]

At the first glance, the code looks reasonable. Does it work?

Regarding the tests, please try using the js2-deftest-parse form.

[ArneBab]

It works insofar that I get no more errors.

But using parse I now found some tests that fail. The problem is that I don’t understand why. I must be missing something.

[dgutov]

What kind of failures do you see?

[dgutov]

You can post some output from the ert buffer.

[ArneBab]

That’s what I see:

Selector: js2-nullish-coalescing-operator-inexisting-field
Passed:  0
Failed:  1 (1 unexpected)
Skipped: 0
Total:   1/1

Started at:   2020-07-14 16:16:31+0200
Finished.
Finished at:  2020-07-14 16:16:31+0200

F

F js2-nullish-coalescing-operator-inexisting-field
    Buffer: *Test buffer (js2-nullish-coalescing-operator-inexisting-field): origin*<4>
    Buffer: *Test buffer (js2-nullish-coalescing-operator-inexisting-field): copy*<4>
    (ert-test-failed
     ((should
       (string=
        (or reference code-string)
        (buffer-substring-no-properties
         (point-min)
         (point))))
      :form
      (string= "var a = {}; a.nonexistant ?? 1;" "var a = {};
a.nonexistant ?? 1;")
      :value nil))

Selector: js2-nullish-coalescing-operator-null-value
Passed:  0
Failed:  1 (1 unexpected)
Skipped: 0
Total:   1/1

Started at:   2020-07-14 16:17:17+0200
Finished.
Finished at:  2020-07-14 16:17:17+0200

F

F js2-nullish-coalescing-operator-null-value
    Buffer: *Test buffer (js2-nullish-coalescing-operator-null-value): origin*<2>
    Buffer: *Test buffer (js2-nullish-coalescing-operator-null-value): copy*<2>
    (ert-test-failed
     ((should
       (string=
        (or reference code-string)
        (buffer-substring-no-properties
         (point-min)
         (point))))
      :form
      (string= "var b = 1; null ?? b;" "var b = 1;
null ?? b;")
      :value nil))

Selector: js2-nullish-coalescing-operator-null-variable
Passed:  0
Failed:  1 (1 unexpected)
Skipped: 0
Total:   1/1

Started at:   2020-07-14 16:18:00+0200
Finished.
Finished at:  2020-07-14 16:18:00+0200

F

F js2-nullish-coalescing-operator-null-variable
    Buffer: *Test buffer (js2-nullish-coalescing-operator-null-variable): origin*<10>
    Buffer: *Test buffer (js2-nullish-coalescing-operator-null-variable): copy*<10>
    (ert-test-failed
     ((should
       (string=
        (or reference code-string)
        (buffer-substring-no-properties
         (point-min)
         (point))))
      :form
      (string= "var a = null; a ?? 1;" "var a = null;
a ?? 1;")
      :value nil))

Selector: js2-nullish-coalescing-operator-in-if
Passed:  0
Failed:  1 (1 unexpected)
Skipped: 0
Total:   1/1

Started at:   2020-07-14 16:18:42+0200
Finished.
Finished at:  2020-07-14 16:18:43+0200

F

F js2-nullish-coalescing-operator-in-if
    Buffer: *Test buffer (js2-nullish-coalescing-operator-in-if): origin*<9>
    (ert-test-failed
     ((should
       (= 0
          (length
           (js2-ast-root-errors ast))))
      :form
      (= 0 1)
      :value nil))

[dgutov]

The first 3 can be fixed by adding a newline inside the original string.

The last one looks like a problem, though.

[jming422]

I believe I fixed the problem that was causing that test to fail - I opened a PR onto @ArneBab's fork here: https://github.com/ArneBab/js2-mode/pull/1

I'm still pretty new to the world of open-source contributions, so if I should have done something besides open a PR onto his fork, just let me know! I wasn't sure exactly what the best thing to do was.

[ArneBab]

@jming422 with your changes all tests pass — thank you very much!

Since I saw it quickly a pull request to my fork worked. Had I not seen this in time, you could also have opened another pull-request with the collected changes. Which route to take depends on the preferences in the project.

[dgutov]

@jming422 Thanks for your help, but was there a reason you changed the parsing structure, putting the new operator between & and ==?

According to this table: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Operator_Precedence, the nullish coaliescing op goes between ?: and ||, which fit my original suggestion.

Reverting your changes to js2-mode.el from db17df4670355c6b27dfed13959d8da442a154ed (except the paren, of course) kept the tests passing. The last test failed, BTW, because return is not allowed on the top-level (outside of any functions).

But thanks for fixing the tests.

[dgutov]

I'll merge this now, but please feel free to correct me later.

[dgutov]

@ArneBab Thanks for doing this.

From what I can see, you only have signed copyright assignment papers for Hurd contributions, and not for Emacs? Would you like me to send you the form for Emacs copyright assignment, so that you can continue contributing without limits?

[jming422]

@jming422 Thanks for your help, but was there a reason you changed the parsing structure, putting the new operator between & and ==?

According to this table: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Operator_Precedence, the nullish coaliescing op goes between ?: and ||, which fit my original suggestion.

There wasn't a reason besides my own inexperience - I didn't realize that would have affected anything, but looking at it now after reading the docs you linked I see my error.

But thanks for fixing the tests.

Glad I was still able to help somehow 😅

[ArneBab]

@jming422 you did not just help somehow: you got this over the block which kept it hanging!

@dgutov please send me the form, yes. arne_bab ät web dot de

Thank you both!