Re: [GSoC][PATCH v2] Add a diff driver for JavaScript languages.

[xing zhi jiang <a97410985new@gmail.com>]

Sorry for the late reply. Because I was busy last two weeks. And
grateful for your review.

On Tue, 15 Mar 2022 at 01:20, Glen Choo <chooglen@google.com> wrote:
>
> > diff --git a/t/t4018/javascript-function b/t/t4018/javascript-function
> > new file mode 100644
> > index 0000000000..0cc0bf54e7
> > --- /dev/null
> > +++ b/t/t4018/javascript-function
> > @@ -0,0 +1,4 @@
> > +function RIGHT(a, b) {
> > +
> > +  return a + b; // ChangeMe
> > +}
> > diff --git a/t/t4018/javascript-function-2 b/t/t4018/javascript-function-2
> > new file mode 100644
> > index 0000000000..06cfb779f0
> > --- /dev/null
> > +++ b/t/t4018/javascript-function-2
> > @@ -0,0 +1,10 @@
> > +function test(a, b) {
> > +  return {
> > +                     RIGHT: function () {
> > +                             currentUpdateRemovedChunks.forEach(function (chunkId) {
> > +                                     delete $installedChunks$[chunkId];
> > +                             });
> > +                             currentUpdateRemovedChunks = ChangeMe;
> > +   }
> > +  }
> > +}
>
> There is also the ES2015 'method shorthand' syntax [3], e.g. `bar` in:
>
>   const foo = {
>     bar() {
>       console.log('hi');
>     }
>   }
>
> [3] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Method_definitions
>
The ES2015 method shorthand is already matched by the regex for
function in class
"^[\t ]*((static[\t ]+)?((async|get|set)[\t
]+)?[$_[:alpha:]][$_[:alnum:]]*[\t ]*\\(.*)"
So I update the comment for this regex in the v3 patch. The comment is
"/* match function in class and ES5 method shorthand */"

> > diff --git a/t/t4018/javascript-function-belong-to-IIFE b/t/t4018/javascript-function-belong-to-IIFE
> > new file mode 100644
> > index 0000000000..6e5fe858c0
> > --- /dev/null
> > +++ b/t/t4018/javascript-function-belong-to-IIFE
> > @@ -0,0 +1,6 @@
> > +(function () {
> > +  this.$RIGHT = function (needle, modifier) {
> > +      let a = 5;
> > +      return ChangeMe;
> > +  };
> > +}).call(aaaa.prototype);
>
>  Does the IIFE matter in this case? This line:
>
>   this.$RIGHT = function (needle, modifier) {
>
> looks extremely similar to the previous test of `foo = function bar()`.
>
> Or perhaps this is meant to demonstrate the edge case of "matching in a
> complicated construct"? If so, perhaps we should test other edge cases
> like:
>
>    function WRONG() {
>      let RIGHT = function (needle, modifier) {
>          let a = 5;
>          return ChangeMe;
>      };
>    }
>
Currently, I realize this test case is redundant. Because I trust the
function keyword. So the regex "^(.*=[\t ]*function[\t
]*([$_[:alnum:]]+[\t ]*)?\\(.*)\n" is loose for LHS.
I will remove this case in the v3 patch.