JavaScript: don't extract local constants and variables defined in arrow functions

[masatake]

Fix #4194.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.93%. Comparing base (e7db33b) to head (298f695).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #4197   +/-   ##
=======================================
  Coverage   85.93%   85.93%           
=======================================
  Files         241      241           
  Lines       59171    59179    +8     
=======================================
+ Hits        50847    50855    +8     
  Misses       8324     8324           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[masatake]

Surprisingly, even with this change, ctags extracts x, y, and z in

const f = () => {
    let x = () => 1
    return x
}

const g = () => {
    let y = function () { return 1 }
    return y
}

function h() {
    let z = function () { return 1 }
    return z
}

Do we need to treat callables in any special way?

[b4n]

This breaks scope after the end of the arrow function's body. Suggestion:

diff --git a/parsers/jscript.c b/parsers/jscript.c
index 6c09af3e3..a6d8e9fde 100644
--- a/parsers/jscript.c
+++ b/parsers/jscript.c
@@ -2912,8 +2912,7 @@ static bool parseStatementRHS (tokenInfo *const name, tokenInfo *const token, st
 			readToken (token);
 			if (isType (token, TOKEN_OPEN_CURLY))
 				parseBlock (token, state->indexForName);
-
-			if (isType (token, TOKEN_CLOSE_CURLY))
+			else if (! isType (token, TOKEN_SEMICOLON))
 				state->isTerminated = false;
 		}
 		vStringDelete (sig);
@@ -2951,7 +2950,13 @@ static bool parseStatementRHS (tokenInfo *const name, tokenInfo *const token, st
 				sig = NULL;
 				readToken (token);
 				if (isType (token, TOKEN_OPEN_CURLY))
+				{
 					parseBlock (token, state->indexForName);
+					/* here we're at the close curly but it's part of the arrow
+					 * function body, so skip over not to confuse further code */
+					readTokenFull(token, true, NULL);
+					state->isTerminated = isType (token, TOKEN_SEMICOLON);
+				}
 			}
 		}
 		if (isType (token, TOKEN_CLOSE_CURLY))

And to test this:

$ cat /tmp/a.js 
function a() {
  function b() {
    let x = 42;
    return x;
  }
  let y = b();
  return y
}

function c() {
  let d = () => {
    return 42;
  }
  
  function d2() {
    return 1;
  }
}

let e = () => {
  function f() {
    
  }
}

let g = () => {
  let h = () => {
    return 42;
  }
  let i = x => {
    return x * 42;
  }
  let j = () => {
    return 42;
  }
}

function k() {}
$ ./ctags -o- /tmp/a.js 
a	/tmp/a.js	/^function a() {$/;"	f
b	/tmp/a.js	/^  function b() {$/;"	f	function:a
c	/tmp/a.js	/^function c() {$/;"	f
d	/tmp/a.js	/^  let d = () => {$/;"	f	function:c
d2	/tmp/a.js	/^  function d2() {$/;"	f	function:c
e	/tmp/a.js	/^let e = () => {$/;"	f
f	/tmp/a.js	/^  function f() {$/;"	f	function:e
g	/tmp/a.js	/^let g = () => {$/;"	f
h	/tmp/a.js	/^  let h = () => {$/;"	f	function:g
i	/tmp/a.js	/^  let i = x => {$/;"	f	function:g
j	/tmp/a.js	/^  let j = () => {$/;"	f	function:g
k	/tmp/a.js	/^function k() {}$/;"	f

[b4n]

This is incorrect: parseBlock() will leave token on the block's close curly brace, which has nothing to do with the current statement's end. See review

[b4n]

Same here, the (weird?) check for the close curly brace is gonna break nesting.

[b4n]

Suggested change

	if (isType (token, TOKEN_CLOSE_CURLY))
	state->isTerminated = false;
	else if (! isType (token, TOKEN_SEMICOLON))
	state->isTerminated = false;

[b4n]

Suggested change

	if (isType (token, TOKEN_OPEN_CURLY))
	parseBlock (token, state->indexForName);
	if (isType (token, TOKEN_OPEN_CURLY))
	{
	parseBlock (token, state->indexForName);
	/* here we're at the close curly but it's part of the arrow
	* function body, so skip over not to confuse further code */
	readTokenFull(token, true, NULL);
	state->isTerminated = isType (token, TOKEN_SEMICOLON);
	}

[masatake]

@b4n Thank you very much. I should be strict when writing a test case.
Will you make a pull request? Or should I include your change to my commit and put a 'Co-Author' mark on the commit?

[b4n]

@masatake as you want, but you can integrate these changes to yours so we get one single PR, I'm happy with that.

[…] Do we need to treat callables in any special way?

I don't think we need to. ATM this is the same with usual nested functions, and so long as the scope is correct I think it's fine, and even useful when using the tags as a TOC.