From 17eb20c77098841d45f0444f5f047c4d44fc614f Mon Sep 17 00:00:00 2001
From: Charles Lyding <19598772+clydin@users.noreply.github.com>
Date: Wed, 5 Oct 2022 09:56:29 -0400
Subject: [PATCH] fix(@angular-devkit/schematics): throw more relevant error
 when Rule returns invalid null value

A `null` value is not considered a valid return value for a schematics Rule. While the Rule
type should prevent this, casting could allow this to potentially occur. Previously, this
would accidentally be treated the same as a void return due to incomplete result checking.
However, recent refactoring caused the `null` case to fail with a non-obvious error message
when it should have failed with the existing `InvalidRuleResultException`. Non-tree result
objects including `null` will now fail with `InvalidRuleResultException`.

(cherry picked from commit 8eb58bdbe034e2cbae9cdae058ad4633c8f2a761)
---
 .../schematics/src/rules/call.ts              |  2 +-
 .../schematics/src/rules/call_spec.ts         | 36 ++++++-------------
 2 files changed, 12 insertions(+), 26 deletions(-)

diff --git a/packages/angular_devkit/schematics/src/rules/call.ts b/packages/angular_devkit/schematics/src/rules/call.ts
index 9f8655a88f5d..346517f98cef 100644
--- a/packages/angular_devkit/schematics/src/rules/call.ts
+++ b/packages/angular_devkit/schematics/src/rules/call.ts
@@ -91,7 +91,7 @@ async function callRuleAsync(rule: Rule, tree: Tree, context: SchematicContext):
     result = await result.pipe(defaultIfEmpty(tree)).toPromise();
   }
 
-  if (TreeSymbol in result) {
+  if (result && TreeSymbol in result) {
     return result as Tree;
   }
 
diff --git a/packages/angular_devkit/schematics/src/rules/call_spec.ts b/packages/angular_devkit/schematics/src/rules/call_spec.ts
index 283dcd68aea0..a8c8ea5896af 100644
--- a/packages/angular_devkit/schematics/src/rules/call_spec.ts
+++ b/packages/angular_devkit/schematics/src/rules/call_spec.ts
@@ -20,11 +20,11 @@ import {
   callSource,
 } from './call';
 
-const context: SchematicContext = ({
+const context: SchematicContext = {
   engine: null,
   debug: false,
   strategy: MergeStrategy.Default,
-} as {}) as SchematicContext;
+} as {} as SchematicContext;
 
 describe('callSource', () => {
   it('errors if undefined source', (done) => {
@@ -95,34 +95,20 @@ describe('callSource', () => {
 });
 
 describe('callRule', () => {
-  it('errors if invalid source object', (done) => {
-    const tree0 = observableOf(empty());
+  it('should throw InvalidRuleResultException when rule result is non-Tree object', async () => {
     const rule0: Rule = () => ({} as Tree);
 
-    callRule(rule0, tree0, context)
-      .toPromise()
-      .then(
-        () => done.fail(),
-        (err) => {
-          expect(err).toEqual(new InvalidRuleResultException({}));
-        },
-      )
-      .then(done, done.fail);
+    await expectAsync(callRule(rule0, empty(), context).toPromise()).toBeRejectedWithError(
+      InvalidRuleResultException,
+    );
   });
 
-  it('errors if Observable of invalid source object', (done) => {
-    const tree0 = observableOf(empty());
-    const rule0: Rule = () => observableOf({} as Tree);
+  it('should throw InvalidRuleResultException when rule result is null', async () => {
+    const rule0: Rule = () => null as unknown as Tree;
 
-    callRule(rule0, tree0, context)
-      .toPromise()
-      .then(
-        () => done.fail(),
-        (err) => {
-          expect(err).toEqual(new InvalidRuleResultException({}));
-        },
-      )
-      .then(done, done.fail);
+    await expectAsync(callRule(rule0, empty(), context).toPromise()).toBeRejectedWithError(
+      InvalidRuleResultException,
+    );
   });
 
   it('works with undefined result', (done) => {