Remove duplicate function createTextRange

[ghost]

This is the same as createRange.

[rbuckton]

We treat -1 as a special value to indicate a synthetic position. Rather than adding a noAssert, we could just write:

Debug.assert(pos === -1 || end === -1 || end >= pos);

[ghost]

Had to put /*noAssert*/ back in a single case because this PR detected a bug #23370. @rbuckton Could you update your review?

[rbuckton]

We sometimes insert temp variables into the declaration list, and the first temp we insert caches the initializer on the RHS. Since all we care about is inserting a mapping at the beginning and end of the list, we could probably just change this to:

if (declarations.length) {
    const range = reduceLeft(declarations, (range, node) => {
        range.pos = range.pos === -1 ? node.pos : node.pos === -1 ? range.pos : Math.min(range.pos, node.pos);
        range.end = range.end === -1 ? node.end : node.end === -1 ? range.end : Math.max(range.end, node.end);
        return range;
    }, createRange(-1, -1));
    setSourceMapRange(declarationList, range);
}

Though we would need to verify the results in the debugger.

[ghost]

@rbuckton Tried that out and there were some baseline changes -- not sure if good or bad.

[rbuckton]

@Andy-MS Try stepping through an example in a debugger using source maps to see if the mappings make sense.

[ghost]

It looks correct. For a simple example:

var { a, b } = 0;

The output is:

var _a = 0, a = _a.a, b = _a.b;

The old source map is:

>>>var _a = 0, a = _a.a, b = _a.b;
1 >
2 >^^^^
3 >    ^^^^^^
4 >          ^^
5 >            ^^^^^^^^
6 >                    ^^
7 >                      ^^^^^^^^
8 >                              ^
9 >                               ^^^^^^^^->
1 >var 
2 >
3 >    { a, b } = 0
4 >          
5 >            a
6 >                    , 
7 >                      b
8 >                               } = 0;
1 >Emitted(1, 1) Source(1, 5) + SourceIndex(0)
2 >Emitted(1, 5) Source(1, 5) + SourceIndex(0)
3 >Emitted(1, 11) Source(1, 17) + SourceIndex(0)
4 >Emitted(1, 13) Source(1, 7) + SourceIndex(0)
5 >Emitted(1, 21) Source(1, 8) + SourceIndex(0)
6 >Emitted(1, 23) Source(1, 10) + SourceIndex(0)
7 >Emitted(1, 31) Source(1, 11) + SourceIndex(0)
8 >Emitted(1, 32) Source(1, 18) + SourceIndex(0)

The new source map is:

>>>var _a = 0, a = _a.a, b = _a.b;
1 >
2 >^^^^
3 >    ^^^^^^
4 >          ^^
5 >            ^^^^^^^^
6 >                    ^^
7 >                      ^^^^^^^^
8 >                              ^
9 >                               ^^^^^^^^->
1 >var 
2 >
3 >    { a, b } = 0
4 >          
5 >            a
6 >                    , 
7 >                      b } = 0
8 >                              ;
1 >Emitted(1, 1) Source(1, 5) + SourceIndex(0)
2 >Emitted(1, 5) Source(1, 5) + SourceIndex(0)
3 >Emitted(1, 11) Source(1, 17) + SourceIndex(0)
4 >Emitted(1, 13) Source(1, 7) + SourceIndex(0)
5 >Emitted(1, 21) Source(1, 8) + SourceIndex(0)
6 >Emitted(1, 23) Source(1, 10) + SourceIndex(0)
7 >Emitted(1, 31) Source(1, 17) + SourceIndex(0)
8 >Emitted(1, 32) Source(1, 18) + SourceIndex(0)

The difference being 7 >: Previously we only counted up to the end of b, now we count all the way to the end.

Also tested with debugging in vscode with an ES5 target, stepping "into" at every opportunity in the following example:

function f() {
    return {
        get a() {
            return 0;
        },
        get b() {
            return 0;
        }
    }
}

const { a, b } = f();

In both cases, the places the cursor moved to were the same:

• { after const
• return in f(), then after } at the end of return {...}
• a in { a, b }
• return in get a(), then the end of that line
• b in { a, b }
• return in get b(), then the end of that line
• Then it took me to the bottom of a.js for some reason, in both cases.

[ghost]

@rbuckton Good to go?

[ghost]

@rbuckton Please review

[RyanCavanaugh]

@Andy-MS please merge at your leisure