chore(shell-api,java-shell): returnsPromise implies async MONGOSH-655 (#749)

- Make sure that all methods marked with `@returnsPromise`
  actually return promises
- Update the java-shell code to account for that
- Remove special handling in the Promise-marking code
  that was previously necessary as a workaround to get
  the new async-rewriter going

Author: addaleax

packages/java-shell/src/main/kotlin/com/mongodb/mongosh/result/Cursor.kt

@@ -19,19 +19,19 @@ open class Cursor<out T> internal constructor(protected var cursor: Value?, priv
     }
 
     override fun hasNext(): Boolean {
-        val (cursor, _) = checkClosed()
-        return cursor.invokeMember("hasNext").asBoolean()
+        val (cursor, converter) = checkClosed()
+        return converter.unwrapPromise(cursor.invokeMember("hasNext")).asBoolean()
     }
 
     override fun next(): T {
         val (cursor, converter) = checkClosed()
         if (!hasNext()) throw NoSuchElementException()
-        return converter.toJava(cursor.invokeMember("next")).value as T
+        return converter.toJava(converter.unwrapPromise(cursor.invokeMember("next"))).value as T
     }
 
     fun tryNext(): T {
         val (cursor, converter) = checkClosed()
-        return converter.toJava(cursor.invokeMember("tryNext")).value as T
+        return converter.toJava(converter.unwrapPromise(cursor.invokeMember("tryNext"))).value as T
     }
 
     fun close() {

packages/java-shell/src/test/resources/collection/aggregateWithNegativeBatchSize.expected.1.txt

@@ -1 +1 @@
-org.graalvm.polyglot.PolyglotException: Command failed with error 51024 (Location51024): 'BSON field 'batchSize' value must be >= 0, actual value '-1'' on server %mongohostport%. The full response is {"ok": 0.0, "errmsg": "BSON field 'batchSize' value must be >= 0, actual value '-1'", "code": 51024, "codeName": "Location51024"}
+com.mongodb.MongoCommandException: Command failed with error 51024 (Location51024): 'BSON field 'batchSize' value must be >= 0, actual value '-1'' on server %mongohostport%. The full response is {"ok": 0.0, "errmsg": "BSON field 'batchSize]' value must be >= 0, actual value '-1'", "code": 51024, "codeName": "Location51024"}

packages/java-shell/src/test/resources/collection/aggregateWithNegativeBatchSize.expected.3.txt

@@ -0,0 +1 @@
+com.mongodb.MongoCommandException: Command failed with error 51024 (Location51024): 'BSON field 'batchSize' value must be >= 0, actual value '-1'' on server %mongohostport%. The full response is {"ok": 0.0, "errmsg": "BSON field 'batchSize' value must be >= 0, actual value '-1'", "code": 51024, "codeName": "Location51024"}

packages/java-shell/src/test/resources/collection/aggregateWithNegativeBatchSize.expected.txt

@@ -1 +1 @@
-org.graalvm.polyglot.PolyglotException: Command failed with error 2 (BadValue): 'cursor.batchSize must not be negative' on server %mongohostport%. The full response is {"ok": 0.0, "errmsg": "cursor.batchSize must not be negative", "code": 2, "codeName": "BadValue"}
+com.mongodb.MongoCommandException: Command failed with error 2 (BadValue): 'cursor.batchSize must not be negative' on server %mongohostport%. The full response is {"ok": 0.0, "errmsg": "cursor.batchSize must not be negative", "code": 2, "codeName": "BadValue"}

packages/java-shell/src/test/resources/cursor/tailable.expected.txt

@@ -1 +1 @@
-org.graalvm.polyglot.PolyglotException: Query failed with error code 2 and error message 'error processing query: ns=admin.collTree: $and
+com.mongodb.MongoQueryException: Query failed with error code 2 and error message 'error processing query: ns=admin.collTree: $and

packages/shell-api/src/aggregation-cursor.ts

@@ -50,17 +50,17 @@ export default class AggregationCursor extends ShellApiClass {
   }
 
   @returnsPromise
-  forEach(f: (doc: Document) => void): Promise<void> {
+  async forEach(f: (doc: Document) => void): Promise<void> {
     return this._cursor.forEach(f);
   }
 
   @returnsPromise
-  hasNext(): Promise<boolean> {
+  async hasNext(): Promise<boolean> {
     return this._cursor.hasNext();
   }
 
   @returnsPromise
-  tryNext(): Promise<Document | null> {
+  async tryNext(): Promise<Document | null> {
     return this._cursor.tryNext();
   }
 
@@ -99,12 +99,12 @@ export default class AggregationCursor extends ShellApiClass {
   }
 
   @returnsPromise
-  next(): Promise<Document | null> {
+  async next(): Promise<Document | null> {
     return this._cursor.next();
   }
 
   @returnsPromise
-  toArray(): Promise<Document[]> {
+  async toArray(): Promise<Document[]> {
     return this._cursor.toArray();
   }
 

packages/shell-api/src/change-stream-cursor.ts

@@ -88,8 +88,7 @@ export default class ChangeStreamCursor extends ShellApiClass {
     return this._cursor.closed;
   }
 
-  @returnsPromise
-  isExhausted(): Promise<boolean> {
+  isExhausted(): never {
     throw new MongoshInvalidInputError('isExhausted is not implemented for ChangeStreams because after closing a cursor, the remaining documents in the batch are no longer accessible. If you want to see if the cursor is closed use isClosed. If you want to see if there are documents left in the batch, use tryNext.');
   }
 

packages/shell-api/src/collection.ts

@@ -753,9 +753,8 @@ export default class Collection extends ShellApiClass {
     );
   }
 
-  @returnsPromise
   @deprecated
-  save(): Promise<void> {
+  save(): never {
     throw new MongoshInvalidInputError(
       'Collection.save() is deprecated. Use insertOne, insertMany, updateOne, or updateMany.'
     );

packages/shell-api/src/cursor.ts

@@ -128,7 +128,7 @@ export default class Cursor extends ShellApiClass {
 
   @serverVersions([ServerVersions.earliest, '4.0.0'])
   @returnsPromise
-  count(): Promise<number> {
+  async count(): Promise<number> {
     return this._cursor.count();
   }
 
@@ -164,12 +164,12 @@ export default class Cursor extends ShellApiClass {
   }
 
   @returnsPromise
-  forEach(f: (doc: Document) => void): Promise<void> {
+  async forEach(f: (doc: Document) => void): Promise<void> {
     return this._cursor.forEach(f);
   }
 
   @returnsPromise
-  hasNext(): Promise<boolean> {
+  async hasNext(): Promise<boolean> {
     if (this._tailable) {
       printWarning(
         'If this is a tailable cursor with awaitData, and there are no documents in the batch, this method ' +
@@ -181,7 +181,7 @@ export default class Cursor extends ShellApiClass {
   }
 
   @returnsPromise
-  tryNext(): Promise<Document | null> {
+  async tryNext(): Promise<Document | null> {
     return this._cursor.tryNext();
   }
 
@@ -253,7 +253,7 @@ export default class Cursor extends ShellApiClass {
   }
 
   @returnsPromise
-  next(): Promise<Document | null> {
+  async next(): Promise<Document | null> {
     if (this._tailable) {
       printWarning(
         'If this is a tailable cursor with awaitData, and there are no documents in the batch, this' +
@@ -308,7 +308,7 @@ export default class Cursor extends ShellApiClass {
   }
 
   @returnsPromise
-  size(): Promise<number> {
+  async size(): Promise<number> {
     return this._cursor.count();
   }
 
@@ -336,7 +336,7 @@ export default class Cursor extends ShellApiClass {
   }
 
   @returnsPromise
-  toArray(): Promise<Document[]> {
+  async toArray(): Promise<Document[]> {
     return this._cursor.toArray();
   }
 

packages/shell-api/src/decorators.ts

@@ -295,11 +295,7 @@ export function shellApiClassDefault(constructor: Function): void {
 function markImplicitlyAwaited<T extends(...args: any) => Promise<any>>(orig: T): ((...args: Parameters<T>) => Promise<any>) {
   function wrapper(this: any, ...args: any[]) {
     const origResult = orig.call(this, ...args);
-    return Object.prototype.toString.call(origResult) === '[object Promise]' ? addHiddenDataProperty(
-      origResult,
-      Symbol.for('@@mongosh.syntheticPromise'),
-      true
-    ) : origResult;
+    return addHiddenDataProperty(origResult, Symbol.for('@@mongosh.syntheticPromise'), true);
   }
   Object.setPrototypeOf(wrapper, Object.getPrototypeOf(orig));
   Object.defineProperties(wrapper, Object.getOwnPropertyDescriptors(orig));
@@ -328,10 +324,14 @@ export function topologies(topologiesArray: Topologies[]): Function {
     descriptor.value.topologies = topologiesArray;
   };
 }
+export const nonAsyncFunctionsReturningPromises: string[] = []; // For testing.
 export function returnsPromise(_target: any, _propertyKey: string, descriptor: PropertyDescriptor): void {
   const orig = descriptor.value;
   orig.returnsPromise = true;
   descriptor.value = markImplicitlyAwaited(descriptor.value);
+  if (orig.constructor.name !== 'AsyncFunction') {
+    nonAsyncFunctionsReturningPromises.push(orig.name);
+  }
 }
 export function directShellCommand(_target: any, _propertyKey: string, descriptor: PropertyDescriptor): void {
   descriptor.value.isDirectShellCommand = true;