repl: fix tab completion not working with computer string properties

[dario-piotrowicz]

I noticed that the repl tab completion doesn't correctly handle computed properties targeted by strings.

For example given the following context:

constobj={one: 1};

The current repl would provide valid (number) completions for lines such as:

obj.one.to

But would not provide correct completions for lines such as:

obj['one'].to

In fact it would provide incorrect (string) completions.

See:

This PR addresses the above issue

Note

This PR is not making everything work perfectly, for example it is not addressing lines such as
obj['o' + 'ne'].to (but it's not making them any worse either).
I think that a generally great solution would require more substantial code changes (potentially with some AST analysis).
I am planning on keeping improving things here but I figured I could do it incrementally and that I'd open this PR to
start moving things in the right direction anyways.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.13%. Comparing base (d6dade5) to head (da0d780).
Report is 4 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #58709 +/- ## ======================================= Coverage 90.12% 90.13% ======================================= Files 637 637 Lines 188121 188121 Branches 36892 36892 ======================================= + Hits 169552 169568 +16 - Misses 11313 11316 +3 + Partials 7256 7237 -19 

Files with missing lines	Coverage Δ
lib/repl.js	94.87% <100.00%> (-0.26%)	⬇️

... and 23 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/67481/

[BridgeAR]

That is definitely a nice improvement!

We'd better use acorn to detect these things in the future though. We are for example still missing support for whitespace in-between. We also miss support for partial matches inside of a bracket. That would also be great.

[nodejs-github-bot]

Landed in 0722023

[dario-piotrowicz]

We'd better use acorn to detect these things in the future though. We are for example still missing support for whitespace in-between.

I 100% agree, as I mentioned in the PR description this was a first fix I figured I could already land (it also already includes some new tests which will help assuring that changes later work as intended)

I'll most likely be using acron on my next iteration here, also I really really want to get rid of this regex:

node/lib/repl.js

Lines 1228 to 1229 in da0d780

	constsimpleExpressionRE=
	/(?:[\w$'"`[{(](?:(\w||\t)*?['"`]|\$|['"`\]})])*\??(?:\.|])?)*?(?:[a-zA-Z_$])?(?:\w|\$)*\??\.?$/;

😁

(since it's very complex and not even fully correct)

---

We also miss support for partial matches inside of a bracket. That would also be great.

Can you elaborate? 🙂