From f328a6933af7aca221b08f694bb14b03701eca68 Mon Sep 17 00:00:00 2001
From: John Simoni <jsimoni03@gmail.com>
Date: Wed, 9 Sep 2020 00:29:06 -0700
Subject: [PATCH] Experimentally disable null expectations for throws
 assertions

---
 docs/03-assertions.md |  4 ++--
 lib/assert.js         | 14 +++++++++++---
 lib/load-config.js    |  2 +-
 test-tap/assert.js    | 34 ++++++++++++++++++++++++++++++++--
 4 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/docs/03-assertions.md b/docs/03-assertions.md
index 40e1777c8..4ed650dc1 100644
--- a/docs/03-assertions.md
+++ b/docs/03-assertions.md
@@ -244,7 +244,7 @@ Assert that an error is thrown. `fn` must be a function which should throw. The
 * `name`: the expected `.name` value of the thrown error
 * `code`: the expected `.code` value of the thrown error
 
-`expectation` does not need to be specified. If you don't need it but do want to set an assertion message you have to specify `null`.
+`expectation` does not need to be specified. If you don't need it but do want to set an assertion message you have to specify `undefined`. (AVA 3 also allows you to specify `null`. This will be removed in AVA 4. You can opt into this change early by enabling the `disableNullExpectations` experiment.)
 
 Example:
 
@@ -276,7 +276,7 @@ The thrown value *must* be an error. It is returned so you can run more assertio
 * `name`: the expected `.name` value of the thrown error
 * `code`: the expected `.code` value of the thrown error
 
-`expectation` does not need to be specified. If you don't need it but do want to set an assertion message you have to specify `null`.
+`expectation` does not need to be specified. If you don't need it but do want to set an assertion message you have to specify `undefined`. (AVA 3 also allows you to specify `null`. This will be removed in AVA 4. You can opt into this change early by enabling the `disableNullExpectations` experiment.)
 
 Example:
 
diff --git a/lib/assert.js b/lib/assert.js
index fe8f1bfa3..8201ea9fb 100644
--- a/lib/assert.js
+++ b/lib/assert.js
@@ -87,8 +87,16 @@ function getErrorWithLongStackTrace() {
 	return err;
 }
 
-function validateExpectations(assertion, expectations, numberArgs) { // eslint-disable-line complexity
+function validateExpectations(assertion, expectations, numberArgs, experiments) { // eslint-disable-line complexity
 	if (numberArgs === 1 || expectations === null || expectations === undefined) {
+		if (experiments.disableNullExpectations && expectations === null) {
+			throw new AssertionError({
+				assertion,
+				message: `The second argument to \`t.${assertion}()\` must be an expectation object or \`undefined\``,
+				values: [formatWithLabel('Called with:', expectations)]
+			});
+		}
+
 		expectations = {};
 	} else if (
 		typeof expectations === 'function' ||
@@ -465,7 +473,7 @@ class Assertions {
 			}
 
 			try {
-				expectations = validateExpectations('throws', expectations, args.length);
+				expectations = validateExpectations('throws', expectations, args.length, experiments);
 			} catch (error) {
 				fail(error);
 				return;
@@ -531,7 +539,7 @@ class Assertions {
 			}
 
 			try {
-				expectations = validateExpectations('throwsAsync', expectations, args.length);
+				expectations = validateExpectations('throwsAsync', expectations, args.length, experiments);
 			} catch (error) {
 				fail(error);
 				return Promise.resolve();
diff --git a/lib/load-config.js b/lib/load-config.js
index 7d349344a..ff6029737 100644
--- a/lib/load-config.js
+++ b/lib/load-config.js
@@ -7,7 +7,7 @@ const pkgConf = require('pkg-conf');
 
 const NO_SUCH_FILE = Symbol('no ava.config.js file');
 const MISSING_DEFAULT_EXPORT = Symbol('missing default export');
-const EXPERIMENTS = new Set(['configurableModuleFormat', 'disableSnapshotsInHooks', 'reverseTeardowns']);
+const EXPERIMENTS = new Set(['configurableModuleFormat', 'disableNullExpectations', 'disableSnapshotsInHooks', 'reverseTeardowns']);
 
 // *Very* rudimentary support for loading ava.config.js files containing an `export default` statement.
 const evaluateJsConfig = configFile => {
diff --git a/test-tap/assert.js b/test-tap/assert.js
index f5b38cbdf..be0f29eb5 100644
--- a/test-tap/assert.js
+++ b/test-tap/assert.js
@@ -14,7 +14,7 @@ const HelloMessage = require('./fixture/hello-message');
 let lastFailure = null;
 let lastPassed = false;
 
-const assertions = new class extends assert.Assertions {
+const AssertionsBase = class extends assert.Assertions {
 	constructor(overwrites = {}) {
 		super({
 			pass: () => {
@@ -35,7 +35,9 @@ const assertions = new class extends assert.Assertions {
 			...overwrites
 		});
 	}
-}();
+};
+
+const assertions = new AssertionsBase();
 
 function assertFailure(t, subset) {
 	if (!lastFailure) {
@@ -1474,6 +1476,34 @@ test('.throwsAsync() fails if passed a bad expectation', t => {
 	t.end();
 });
 
+test('.throws() fails if passed null expectation with disableNullExpectations', t => {
+	const asserter = new AssertionsBase({experiments: {disableNullExpectations: true}});
+
+	failsWith(t, () => {
+		asserter.throws(() => {}, null);
+	}, {
+		assertion: 'throws',
+		message: 'The second argument to `t.throws()` must be an expectation object or `undefined`',
+		values: [{label: 'Called with:', formatted: /null/}]
+	});
+
+	t.end();
+});
+
+test('.throwsAsync() fails if passed null expectation with disableNullExpectations', t => {
+	const asserter = new AssertionsBase({experiments: {disableNullExpectations: true}});
+
+	failsWith(t, () => {
+		asserter.throwsAsync(() => {}, null);
+	}, {
+		assertion: 'throwsAsync',
+		message: 'The second argument to `t.throwsAsync()` must be an expectation object or `undefined`',
+		values: [{label: 'Called with:', formatted: /null/}]
+	});
+
+	t.end();
+});
+
 test('.notThrows()', gather(t => {
 	// Passes because the function doesn't throw
 	passes(t, () => {