feat(core): warn if an aspect was added via another aspect (aws#8639)

### Updated PR

Since we introduced [`stages`](aws#8423) which had the unintended side effect of the CDK not supporting adding an Aspect via another Aspect, we have not seen any issues reported beside aws#8536, that was resolved without requiring this capability.
Given that, and the fact this features has many sharp edges we decided to leave it unsupported, and add a warning in the meantime. 

-----------

Following up on aws#8536

If an aspect is added via another aspect, the inner aspect  will  not be invoked. Take for example this code:

```typescript
const app = new cdk.App();

app.node.applyAspect({
  visit(construct: cdk.IConstruct) {
    construct.node.applyAspect({
      visit(construct: cdk.IConstruct) {
        console.info("Invoking aspect on construct: " + construct.node.id);  // This will not be called
      }
    })
  }
});
```

Since aspects are added only on the top level node,  if an aspect is added while `InvokeAspects` is called on that node,  it will be ignored since it will not be added to list of aspects to invoke (`allAspectsHere` in the bellow code):
```typescript
function invokeAspects(root: IConstruct) {
  recurse(root, []);

  function recurse(construct: IConstruct, inheritedAspects: constructs.IAspect[]) {
    // hackery to be able to access some private members with strong types (yack!)
    const node: NodeWithAspectPrivatesHangingOut = construct.node._actualNode as any;

    const allAspectsHere = [...inheritedAspects ?? [], ...node._aspects];

    for (const aspect of allAspectsHere) {
      if (node.invokedAspects.includes(aspect)) { continue; }

      aspect.visit(construct); <-- an aspect that was added here will not be added to `allAspectsHere` and will be ignored

      node.invokedAspects.push(aspect);
    }

    for (const child of construct.node.children) {
      if (!Stage.isStage(child)) {
        recurse(child, allAspectsHere);
      }
    }
  }
}
 
```

Assuming this is not something we want to support**, we can detect it by comparing the size of `node._aspects` before and after the call to `aspect.visit`, and emit a warning if there has been a change. Note that while the aspect will not be invoked it will be added to every child construct during the recursive visit. Emitting a warning for each child construct will result in a noisy console, to prevent this I have added a flag that will only allow adding **one warning per application**, given this limitation I'm not sure  there is a lot of value in adding the warning, thoughts?
 
If we decide to add it I will add tests.



 (** theoretically we could support it by adding the aspects to `allAspectsHere`  during the loop, but this will require a non trivial implementation in order to avoid infinite recursion) 



----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

Author: NetaNir

packages/@aws-cdk/core/lib/private/synthesis.ts

@@ -58,17 +58,25 @@ function synthNestedAssemblies(root: IConstruct, options: StageSynthesisOptions)
  * twice for the same construct.
  */
 function invokeAspects(root: IConstruct) {
+  let nestedAspectWarning = false;
   recurse(root, []);
 
   function recurse(construct: IConstruct, inheritedAspects: constructs.IAspect[]) {
     // hackery to be able to access some private members with strong types (yack!)
     const node: NodeWithAspectPrivatesHangingOut = construct.node._actualNode as any;
 
     const allAspectsHere = [...inheritedAspects ?? [], ...node._aspects];
-
+    const nodeAspectsCount = node._aspects.length;
     for (const aspect of allAspectsHere) {
       if (node.invokedAspects.includes(aspect)) { continue; }
+
       aspect.visit(construct);
+      // if an aspect was added to the node while invoking another aspect it will not be invoked, emit a warning
+      // the `nestedAspectWarning` flag is used to prevent the warning from being emitted for every child
+      if (!nestedAspectWarning && nodeAspectsCount !== node._aspects.length) {
+        construct.node.addWarning('We detected an Aspect was added via another Aspect, and will not be applied');
+        nestedAspectWarning = true;
+      }
       node.invokedAspects.push(aspect);
     }
 

packages/@aws-cdk/core/test/test.aspect.ts

@@ -1,3 +1,4 @@
+import * as cxschema from '@aws-cdk/cloud-assembly-schema';
 import { Test } from 'nodeunit';
 import { App } from '../lib';
 import { IAspect } from '../lib/aspect';
@@ -17,6 +18,13 @@ class VisitOnce implements IAspect {
     }
   }
 }
+
+class MyAspect implements IAspect {
+  public visit(node: IConstruct): void {
+    node.node.addMetadata('foo', 'bar');
+  }
+}
+
 export = {
   'Aspects are invoked only once'(test: Test) {
     const app = new App();
@@ -28,4 +36,42 @@ export = {
     test.deepEqual(root.visitCounter, 1);
     test.done();
   },
+
+  'Warn if an Aspect is added via another Aspect'(test: Test) {
+    const app = new App();
+    const root = new MyConstruct(app, 'MyConstruct');
+    const child = new MyConstruct(root, 'ChildConstruct');
+    root.node.applyAspect({
+      visit(construct: IConstruct) {
+        construct.node.applyAspect({
+          visit(inner: IConstruct) {
+            inner.node.addMetadata('test', 'would-be-ignored');
+          },
+        });
+      },
+    });
+    app.synth();
+    test.deepEqual(root.node.metadata[0].type, cxschema.ArtifactMetadataEntryType.WARN);
+    test.deepEqual(root.node.metadata[0].data, 'We detected an Aspect was added via another Aspect, and will not be applied');
+    // warning is not added to child construct
+    test.equal(child.node.metadata.length, 0);
+    test.done();
+  },
+
+  'Do not warn if an Aspect is added directly (not by another aspect)'(test: Test) {
+    const app = new App();
+    const root = new MyConstruct(app, 'Construct');
+    const child = new MyConstruct(root, 'ChildConstruct');
+    root.node.applyAspect(new MyAspect());
+    app.synth();
+    test.deepEqual(root.node.metadata[0].type, 'foo');
+    test.deepEqual(root.node.metadata[0].data, 'bar');
+    test.deepEqual(root.node.metadata[0].type, 'foo');
+    test.deepEqual(child.node.metadata[0].data, 'bar');
+    // no warning is added
+    test.equal(root.node.metadata.length, 1);
+    test.equal(child.node.metadata.length, 1);
+    test.done();
+  },
+
 };