Fix REPL bug with previous line carryover

[TheUnlocked]

Fixes #1363.

Multiline inputs aren't an issue since multiline inputs are passed into evalCodeInternal in their entirety.

[codecov]

Codecov Report

Merging #1480 (63385d6) into main (6e6bf63) will increase coverage by 0.08%.
The diff coverage is 100.00%.

Impacted Files	Coverage Δ
src/repl.ts	82.41% <100.00%> (ø)
src/bin.ts	90.69% <0.00%> (ø)
src/util.ts	93.93% <0.00%> (+3.31%)	⬆️

[cspotcode]

A detailed explanation within the code! Love it; thanks for this.

[cspotcode]

Thanks for sending this.

I haven't looked at this code in a bit, but I noticed coverage of one line being removed by this change.

https://app.codecov.io/gh/TypeStrong/ts-node/compare/1480/changes#D1L605

Is it possible that the line in question is trying to do the same thing as this PR, but not doing it well enough? Again, I haven't looked at this code in a bit, so I don't know off-hand.

[cspotcode]

Also, we should add a test for this, since none of our tests currently are catching the bug being fixed here.

I recently made an effort to make our tests more approachable by splitting them into several files. Not perfect, still a work-in-progress. But REPL tests are here:
https://github.com/TypeStrong/ts-node/blob/main/src/test/repl/repl.spec.ts

[TheUnlocked]

Looks like that lost line of coverage was from the code attempting to fix this same sort of issue, but it seems like the motivation was probably slightly different at the time and so it had an overly restrictive condition. Since the new fix appears to subsume whatever that line was intended to do, I've removed that code.

[cspotcode]

Ok sounds good, thanks. My only remaining concern:

The old code was inside appendToEvalState, which is called in 3 different places. Your code is only used in 1, and I'm wondering if those 2 other places also need this semicolon insertion.

For example, I think the .type command might benefit from this as well.

If what I'm saying makes sense, can we move your code to where the old code was?

[TheUnlocked]

Moving the code to appendToEvalState causes regressions in other tests, particularly surrounding things like

[
    1,
    2,
    3
]

Looking more closely at the code, it seems like it was trying to do this fix for things like () and [] (to prevent function invocation and indexing) and actually covered subtraction and division, but it didn't capture addition and multiplication for some reason (possibly because * x isn't valid syntax, and maybe whoever wrote it just forgot that + x is).

Adding + and * to the regular expression without any of the added code fixes the issue (sort of, there was also a bug with that code where the control flow was slightly wrong, but it works when I fix that), but it also means that if any new operators are added to Javascript in the future, they would need to be explicitly opted in as well. Automatically inserting a semicolon at the end of every completed input results in plenty of not-strictly-necessary semicolons at the end of inputs, but those don't hurt anything, and it's less prone to failure later on.

The other two situations that use appendToEvalState are:

• .type, where it could previously insert a semicolon on the previous line before checking the type. Since a semicolon will always be inserted on the previous line in the new system anyways, and since all input changes from a .type command are reverted after it finishes running (so there's no need to add a semicolon after it), having semicolon insertion for that function call is unnecessary with the new code.
• As a shortcut to get an undo function to reset the REPL state to the beginning. Inserting a semicolon there is unnecessary (even if there is data in the state, the start function adds some code itself which seems to avoid the issue... I couldn't figure out any way to break it even with manually setting the state before starting the REPL).

There was an issue where evalCode didn't call evalCodeInternal and so wouldn't hit the fix, so I moved the fix into appendCompileAndEvalInput itself, and it seems to be working everywhere now as far as I can tell.

[cspotcode]

Thanks for the detailed analysis. Sounds good to me.

The only thing I changed:
I want to be paranoid about avoiding unnecessary semicolons, so I tweaked the logic so it only adds a semicolon if one isn't already there. Seems double-semicolons compile as an extra blank line which can affect stack trace line numbers. Probably not an issue for anyone, but I was able to make a test case for it, and it works.

[cspotcode]

@TheUnlocked this has been published in 10.3.0. Thanks again!

https://github.com/TypeStrong/ts-node/releases/tag/v10.3.0