1873157 - Preview popup is not displayed on function token when function param is a function that starts on same line call but spans multiple lines

Status: NEW

(DevTools :: Debugger, defect, P2)

Description

[Nicolas Chevobbe [:nchevobbe]]

On a page with the following

[document].forEach((item) =>
console.log(item));
debugger;

hovering document does not show the preview popup

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Attachment: STR

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Looks like isScopeInLine returns false (https://searchfox.org/mozilla-central/rev/9c509b8feb28c1e76ad41e65bf9fd87ef672b00f/devtools/client/debugger/src/selectors/isLineInScope.js#16,21), because the line is flagged out of scope location in from https://searchfox.org/mozilla-central/rev/9c509b8feb28c1e76ad41e65bf9fd87ef672b00f/devtools/client/debugger/src/workers/parser/findOutOfScopeLocations.js#109-130

not sure why yet

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Attachment: js file

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

Attachment: STR

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

function myCbFunction(cb) {}

// no preview
myCbFunction(() => 
  42)
// preview
myCbFunction(() => 42)
myCbFunction(
  () => 42)

so it really seems to be an issue if the function param starts on the same line as where the function call and spans multiple lines.
It also look like this only happens for function parameters, so it might be an issue with how we retrieve functions here https://searchfox.org/mozilla-central/rev/9c509b8feb28c1e76ad41e65bf9fd87ef672b00f/devtools/client/debugger/src/workers/parser/findOutOfScopeLocations.js#114

Comment 6

[Nicolas Chevobbe [:nchevobbe]]

So, I'm a bit slow today and just realized that in preview, we check if the line of the hovered token is in scope (https://searchfox.org/mozilla-central/rev/9c509b8feb28c1e76ad41e65bf9fd87ef672b00f/devtools/client/debugger/src/actions/preview.js#44), although the parser worker indicates which locations are out of scope, where a location is an object with a start and an end position, and position are line + columns.
So in the case of:

myCbFunction(() => 
  42)
debugger;

We want to hover myCbFunction, so we may have a cursor position of (1,2) for example.
The parser worker indicates the (1,19) -> (2,5) is out of scope.
So here, the line 1 from the (1,2) position is flagged as out of scope

So now, why is this not the case with

myCbFunction(() => 42)
debugger;

the parser worker would give us an out of scope location of (1,19) -> (1,27), so line 1 should be considered out of scope.
But it's not, because in https://searchfox.org/mozilla-central/rev/9c509b8feb28c1e76ad41e65bf9fd87ef672b00f/devtools/client/debugger/src/actions/ast/setInScopeLines.js#15,20-25

function getOutOfScopeLines(outOfScopeLocations) {
...
  const uniqueLines = new Set();
...
    for (let i = location.start.line; i < location.end.line; i++) {
      uniqueLines.add(i);
...
  }

we have a strict < between start and en line. So when a location is on a single line, it's not considered an out-of-scope line.

What matters in the end is that getPreview should not check if the line is out of scope, but if the location is.
At the moment, we're storing in scope lines in redux (https://searchfox.org/mozilla-central/rev/9c509b8feb28c1e76ad41e65bf9fd87ef672b00f/devtools/client/debugger/src/reducers/ast.js#58-66), and so we'd need to change things a bit