lastIndex unused

[xogeny]

In my current work on my evaluator, I noticed this line. I'm pretty confident that lastIndex is completely unused. It is initialized to 0 and never changed. Furthermore, the comparisons between re.lastIndex and expr.value.lastindex are meaningless because those are the same variable. The check on the value of re.lastIndex vs. str.length is also unnecessary since the only possible value for re.lastIndex is zero. So the only way it can be true (and return undefined) is if str.length===0. But in that case, there will be no match (which means you'd never get to that check).

I've removed all references to lastIndex in my version and the test suite still passes. So I'm not sure what is intended here, but it doesn't seem like lastIndex accomplishes anything and the test suite certainly doesn't seem to require it.

[xogeny]

OK, I was mistaken here. The lastIndex property on the Regex represents some internal state. My comment about the comparison of expr.value.lastIndex and re.lastIndex is still true (there are still the same value). But the statements based on the assumption that lastIndex never changes are not correct.

However, given that lastIndex is stateful there is something else that makes me nervous here and that is that the Regex could be potentially reused somehow. So it seems like it would be better to make a local copy for the closure being constructed here.

There isn't really anything "wrong" with the current version (it still does what it is supposed to). In my version I rewrote it a little bit just to simplify it. With my changes applied to v1.5.x, the function evaluateRegex looks like this:

function evaluateRegex(expr: ast.RegexNode, input: any, environment: Environment) {
    let re = new RegExp(expr.value);
    var closure = function(str) {
        var result;
        var match = re.exec(str);
        if (match !== null) {
            result = {
                match: match[0],
                start: match.index,
                end: match.index + match[0].length,
                groups: [],
            };
            if (match.length > 1) {
                for (var i = 1; i < match.length; i++) {
                    result.groups.push(match[i]);
                }
            }
            result.next = function() {
                if (re.lastIndex >= str.length) {
                    return undefined;
                } else {
                    var next = closure(str);
                    if (next && next.match === "") {
                        // matches zero length string; this will never progress
                        throw {
                            code: "D1004",
                            stack: new Error().stack,
                            position: expr.position,
                            value: expr.value.source,
                        };
                    }
                    return next;
                }
            };
        }

        return result;
    };
    return closure;
}

[andrew-coleman]

Thanks for the comment. I'll revisit this aspect of the implementation.