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:

const obj = { 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

	const simpleExpressionRE =
	/(?:[\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? 🙂