repl: make own properties shadow prototype properties

Previously, the code displayed properties backwards (e.g., showing prototype properties before own properties). It also did uniqueness checks during this processing, so these checks were done backwards. After this change, the properties continue to be displayed backwards, but the uniqueness checks are done in the proper order. See also: #21586 which was discovered during the testing of this fix. Fixes: #15199 PR-URL: #21588 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
nodejs · Jul 14, 2018 · 4b613d3 · 4b613d3
1 parent bba500d
commit 4b613d3
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 7 deletions.
diff --git a/lib/repl.js b/lib/repl.js
@@ -1229,20 +1229,20 @@ function complete(line, callback) {
       // Completion group 0 is the "closest"
       // (least far up the inheritance chain)
       // so we put its completions last: to be closest in the REPL.
-      for (i = completionGroups.length - 1; i >= 0; i--) {
+      for (i = 0; i < completionGroups.length; i++) {
         group = completionGroups[i];
         group.sort();
-        for (var j = 0; j < group.length; j++) {
+        for (var j = group.length - 1; j >= 0; j--) {
           c = group[j];
           if (!hasOwnProperty(uniq, c)) {
-            completions.push(c);
+            completions.unshift(c);
             uniq[c] = true;
           }
         }
-        completions.push(''); // Separator btwn groups
+        completions.unshift(''); // Separator btwn groups
       }
-      while (completions.length && completions[completions.length - 1] === '') {
-        completions.pop();
+      while (completions.length && completions[0] === '') {
+        completions.shift();
       }
     }
 

diff --git a/test/parallel/test-repl-tab-complete.js b/test/parallel/test-repl-tab-complete.js
@@ -149,7 +149,10 @@ putIn.run([
   ' one:1',
   '};'
 ]);
-testMe.complete('inner.o', getNoResultsFunction());
+// See: https://github.com/nodejs/node/issues/21586
+// testMe.complete('inner.o', getNoResultsFunction());
+testMe.complete('inner.o', common.mustCall(function(error, data) {
+}));
 
 putIn.run(['.clear']);
 
@@ -206,6 +209,20 @@ testMe.complete('toSt', common.mustCall(function(error, data) {
   assert.deepStrictEqual(data, [['toString'], 'toSt']);
 }));
 
+// own properties should shadow properties on the prototype
+putIn.run(['.clear']);
+putIn.run([
+  'var x = Object.create(null);',
+  'x.a = 1;',
+  'x.b = 2;',
+  'var y = Object.create(x);',
+  'y.a = 3;',
+  'y.c = 4;'
+]);
+testMe.complete('y.', common.mustCall(function(error, data) {
+  assert.deepStrictEqual(data, [['y.b', '', 'y.a', 'y.c'], 'y.']);
+}));
+
 // Tab complete provides built in libs for require()
 putIn.run(['.clear']);