fix(core): parameter pointcut being mixed with its methodpointcut cou…

…nterpart

packages/common/src/abstract/abstract.type.ts

@@ -20,7 +20,7 @@ export const _defuseAbstract = (receipe: () => {}) => {
     }
 
     const multipleAbstractCalls =
-      (_isAbstractToken(val) &&
+      (isAbstractToken(val) &&
         (val as _AbstractTokenImpl).counter - abstractCounter !== 0) ||
       (_abstractThrows && abstractCounter - _abstractCounter !== 1);
     if (multipleAbstractCalls) {
@@ -54,4 +54,4 @@ export const abstract = <T extends any>(template?: T): T => {
   return new _AbstractTokenImpl<T>(++abstractCounter, template) as any;
 };
 
-export const _isAbstractToken = (val: any) => val instanceof _AbstractTokenImpl;
+export const isAbstractToken = (val: any) => val instanceof _AbstractTokenImpl;

packages/common/src/annotation/context/registry/by-ref-selector.ts

@@ -69,10 +69,16 @@ export class AnnotationsByRefSelector<
     annotation: Annotation<AnnotationType, S2>,
   ): AnnotationsSelector<T, S2, X>;
   annotations(
-    ...annotations: (AnnotationRef | Annotation)[]
+    ...annotations: (
+      | AnnotationRef
+      | Annotation<AnnotationType, AnnotationStub>
+    )[]
   ): AnnotationsSelector<T, S, X>;
   annotations(
-    ...annotations: (Annotation | AnnotationRef)[]
+    ...annotations: (
+      | Annotation<AnnotationType, AnnotationStub>
+      | AnnotationRef
+    )[]
   ): AnnotationsSelector<T, S, X> {
     let annotationRefs = this.annotationsRefs;
     if (annotations.length) {

packages/common/src/annotation/factory/annotation.factory.ts

@@ -12,6 +12,7 @@ import { DecoratorProviderRegistry } from './decorator-provider.registry';
 /* eslint-disable @typescript-eslint/no-empty-function */
 
 import { _copyPropsAndMeta, assert } from '@aspectjs/common/utils';
+import { ReflectError } from '../../public_api';
 import { reflectContext } from '../../reflect/reflect.context.global';
 import type { AnnotationStub } from '../annotation.types';
 import { AnnotationRegistry } from '../registry/annotation.registry';
@@ -227,11 +228,14 @@ export class AnnotationFactory {
               }
               return decoree;
             } catch (e) {
-              console.error(
-                `Error applying annotation hook ${name}: ${
-                  (e as Error).message
-                }`,
-              );
+              if (!(e instanceof ReflectError)) {
+                console.error(
+                  `Error applying annotation hook ${name}: ${
+                    (e as Error).message
+                  }`,
+                );
+              }
+
               throw e;
             }
           },

packages/common/src/public_api.ts

@@ -21,3 +21,4 @@ export * from './reflect/module/reflect-module.type';
 export * from './reflect/reflect-provider.type';
 export * from './reflect/reflect.context';
 export * from './reflect/reflect.context.global';
+export * from './reflect/reflect.error';

packages/common/src/reflect/reflect.error.ts

@@ -0,0 +1 @@
+export class ReflectError extends Error {}

packages/common/utils/src/utils.ts

@@ -32,7 +32,7 @@ export function isFunction(
 }
 
 export function isObject(value: unknown): value is object {
-  return typeof value === 'object' && !(value instanceof Array);
+  return typeof value === 'object' && !Array.isArray(value);
 }
 
 export function isNumber(value: unknown): value is number {

packages/core/src/advice/mutable-advice.context.ts

@@ -18,7 +18,7 @@ import type { AfterThrowContext } from '../advices/after-throw/after-throw.conte
 import type { AroundContext } from '../advices/around/around.context';
 import type { BeforeContext } from '../advices/before/before.context';
 import type { CompileContext } from '../advices/compile/compile.context';
-import { _BindableAnnotationTarget } from '../utils/bindable-annotation-target';
+import { _BindableAnnotationTarget } from '../utils/annotation-mixin-target';
 import { AdviceContext } from './advice.context';
 
 export class MutableAdviceContext<

packages/core/src/advices/after-return/after-return.context.ts

@@ -19,6 +19,7 @@ export interface AfterReturnContext<
   readonly annotations: AnnotationsByRefSelector<
     ToAnnotationType<T>
   >['annotations'];
+
   /** The 'this' instance bound to the current execution context **/
   readonly instance: X;
   /** the arguments originally passed to the joinpoint **/

packages/core/src/aspect/aspect-metadata.type.ts

@@ -1,4 +1,6 @@
+export interface AspectOptions {
+  readonly id?: string;
+}
 export interface AspectMetadata {
-  id?: string;
-  // TODO: add unique?: boolean flag
+  readonly id: string;
 }

packages/core/src/aspect/aspect.annotation.ts

@@ -4,7 +4,7 @@ import type { Weaver } from './../weaver/weaver';
 
 import { _CORE_ANNOTATION_FACTORY } from '../utils';
 
-import type { AspectMetadata } from './aspect-metadata.type';
+import type { AspectOptions } from './aspect-metadata.type';
 /* eslint-disable @typescript-eslint/no-unused-vars */
 
 /**
@@ -24,5 +24,5 @@ export const Aspect = _CORE_ANNOTATION_FACTORY.create(
   AnnotationType.CLASS,
   // eslint-disable-next-line @typescript-eslint/ban-ts-comment,@typescript-eslint/no-unused-vars
   // @ts-ignore
-  function Aspect(id: string | AspectMetadata = {}) {},
+  function Aspect(id: string | AspectOptions = {}) {},
 );

packages/core/src/aspect/aspect.registry.ts

@@ -12,12 +12,7 @@ import { AdviceRegistry } from '../advice/registry/advice.registry';
 import { WeavingError } from '../errors/weaving.error';
 import { WeaverContext } from '../weaver/context/weaver.context';
 
-import {
-  ASPECT_ID_SYMBOL,
-  AspectType,
-  getAspectMetadata,
-  isAspect,
-} from './aspect.type';
+import { AspectType, getAspectMetadata, isAspect } from './aspect.type';
 let _globalRegId = 0;
 
 export class AspectRegistry {
@@ -76,19 +71,19 @@ export class AspectRegistry {
     }
     this.aspectsMap.set(id, aspect);
 
-    aspect[ASPECT_ID_SYMBOL] = id;
-
     this.adviceReg.register(aspect);
   }
 
-  getAspects<T = unknown>(
-    aspect?: string | ConstructorType<T>,
-  ): (T & AspectType)[] {
-    return (
-      aspect
-        ? this.aspectsMap.get(getAspectId(aspect)) ?? []
-        : [...this.aspectsMap.values()].flatMap((a) => a)
-    ) as (T & AspectType)[];
+  getAspect<T = unknown>(
+    aspect: string | ConstructorType<T>,
+  ): (T & AspectType) | undefined {
+    return this.aspectsMap.get(getAspectId(aspect)) as
+      | (T & AspectType)
+      | undefined;
+  }
+
+  getAspects(): AspectType[] {
+    return [...this.aspectsMap.values()].flatMap((a) => a);
   }
 
   private __isAspectRegistered(aspect: AspectType) {
@@ -100,7 +95,5 @@ export class AspectRegistry {
 }
 
 function getAspectId(aspect: AspectType | string): string {
-  return typeof aspect === 'string'
-    ? aspect
-    : getPrototype(aspect).constructor.name;
+  return typeof aspect === 'string' ? aspect : getAspectMetadata(aspect).id;
 }

packages/core/src/aspect/aspect.type.ts

@@ -6,20 +6,14 @@ import {
   AnnotationType,
   reflectContext,
 } from '@aspectjs/common';
-import { AspectMetadata } from './aspect-metadata.type';
+import { AspectMetadata, AspectOptions } from './aspect-metadata.type';
 import { Aspect } from './aspect.annotation';
 
-export const ASPECT_ID_SYMBOL = Symbol.for('aspectjs:aspectId');
-
-export type AspectType = object & {
-  [ASPECT_ID_SYMBOL]?: string;
-};
+export type AspectType = object & {};
 
 let _globalAspectId = 0;
 
-export function getAspectMetadata(
-  aspect: AspectType,
-): Required<AspectMetadata> {
+export function getAspectMetadata(aspect: AspectType): AspectMetadata {
   const target = reflectContext()
     .get(AnnotationTargetFactory)
     .of<AspectType>(aspect);
@@ -46,14 +40,15 @@ export function isAspect(aspect: AspectType) {
 function coerceAspectOptions(
   aspectTarget: AnnotationTarget<AnnotationType.CLASS, AspectType>,
   idOrOptions: unknown,
-): Required<AspectMetadata> {
-  const options: AspectMetadata =
+): AspectMetadata {
+  const options: AspectOptions =
     typeof idOrOptions === 'object' ? { ...idOrOptions } : {};
 
-  options.id =
-    typeof idOrOptions === 'string'
-      ? idOrOptions
-      : options.id ??
-        `${aspectTarget.proto.constructor.name}#${_globalAspectId++}`;
-  return options as Required<AspectMetadata>;
+  return {
+    id:
+      typeof idOrOptions === 'string'
+        ? idOrOptions
+        : options.id ??
+          `${aspectTarget.proto.constructor.name}#${_globalAspectId++}`,
+  } satisfies AspectMetadata;
 }

packages/core/src/errors/aspect.error.ts

@@ -1,8 +1,8 @@
-import { AspectType, ASPECT_ID_SYMBOL } from '../aspect/aspect.type';
+import { AspectType, getAspectMetadata } from '../aspect/aspect.type';
 import { WeavingError } from './weaving.error';
 
 export class AspectError extends WeavingError {
   constructor(aspect: AspectType, msg: string) {
-    super(`[${aspect[ASPECT_ID_SYMBOL]}]: ${msg}`);
+    super(`[${getAspectMetadata(aspect).id}]: ${msg}`);
   }
 }

packages/core/src/errors/weaving.error.ts

@@ -1,4 +1,6 @@
+import { ReflectError } from '@aspectjs/common';
+
 /**
  * Error thrown during the weaving process meaning the weaver has illegal state.
  */
-export class WeavingError extends Error {}
+export class WeavingError extends ReflectError {}

packages/core/src/jit/canvas/jit-abstract-method-canvas.strategy.ts

@@ -0,0 +1,130 @@
+import { PointcutType } from '../../pointcut/pointcut-target.type';
+import { JitWeaverCanvasStrategy } from './jit-canvas.strategy';
+
+import {
+  MethodPropertyDescriptor,
+  _copyPropsAndMeta,
+  _defuseAbstract,
+  assert,
+} from '@aspectjs/common/utils';
+import { AdviceType } from '../../advice/advice-type.type';
+import { JoinPoint } from '../../advice/joinpoint';
+import { MutableAdviceContext } from '../../advice/mutable-advice.context';
+import { AdviceEntry } from '../../advice/registry/advice-entry.model';
+import { AdviceError } from '../../errors/advice.error';
+import { CompiledSymbol } from '../../weaver/canvas/canvas-strategy.type';
+import { renameFunction } from './canvas.utils';
+
+/**
+ * Canvas to advise method and parameters
+ */
+export abstract class AbstractJitMethodCanvasStrategy<
+  T extends PointcutType.METHOD | PointcutType.PARAMETER,
+  X = unknown,
+> extends JitWeaverCanvasStrategy<T, X> {
+  protected abstract getAdviceEntries<P extends AdviceType>(
+    pointcutType: P,
+  ): AdviceEntry<T, X, P>[];
+
+  compile(ctxt: MutableAdviceContext<T, X>): CompiledSymbol<T, X> {
+    // if method already compiled, it might also be linked.
+    // Use the last known compiled symbol as a reference to avoid linking twice.
+    let methodDescriptor = ctxt.target.getMetadata(
+      '@ajs:compiledSymbol',
+      () => ctxt.target.descriptor as CompiledSymbol<T, X>,
+    )!;
+    assert(!!methodDescriptor);
+
+    //  if no method compile advices, return method as is
+    const adviceEntries = this.getAdviceEntries(AdviceType.COMPILE);
+    if (!adviceEntries.length) {
+      return methodDescriptor;
+    }
+
+    assert(!!ctxt.target.propertyKey);
+    // if (!adviceEntries.length) {
+    //   return Reflect.getOwnPropertyDescriptor(
+    //     ctxt.target.proto,
+    //     ctxt.target.propertyKey,
+    //   ) as CompiledSymbol<T, X>;
+    // }
+
+    adviceEntries
+      //  prevent calling them twice.
+      .filter((e) => !ctxt.target.getMetadata(`compiled_${e.id}`, () => false))
+      .forEach((entry) => {
+        assert(typeof entry.advice === 'function');
+        Object.defineProperty(
+          ctxt.target.proto,
+          ctxt.target.propertyKey,
+          methodDescriptor,
+        );
+
+        let newMethodDescriptor = entry.advice.call(
+          entry.aspect,
+          ctxt.asCompileContext(),
+        ) as CompiledSymbol<T, X>;
+
+        if (newMethodDescriptor) {
+          if (typeof newMethodDescriptor === 'function') {
+            const surrogate = {
+              fn: newMethodDescriptor,
+            };
+            newMethodDescriptor = Object.getOwnPropertyDescriptor(
+              surrogate,
+              'fn',
+            )! as CompiledSymbol<T, X>;
+          }
+          if (typeof newMethodDescriptor.value !== 'function') {
+            throw new AdviceError(
+              entry.aspect,
+              entry.advice,
+              ctxt.target,
+              'should return void, a function, or a Method property descriptor',
+            );
+          }
+
+          _copyPropsAndMeta(newMethodDescriptor.value, methodDescriptor.value); // copy static props
+          methodDescriptor = newMethodDescriptor;
+        }
+
+        ctxt.target.defineMetadata(`compiled_${entry.id}`, true);
+      });
+    ctxt.target.defineMetadata('@ajs:compiledSymbol', methodDescriptor);
+    return methodDescriptor;
+  }
+
+  override callJoinpoint(
+    ctxt: MutableAdviceContext<T, X>,
+    originalSymbol: MethodPropertyDescriptor,
+  ): unknown {
+    return (ctxt.value = _defuseAbstract(() =>
+      originalSymbol.value.call(ctxt.instance, ...ctxt.args!),
+    ));
+  }
+
+  link(
+    ctxt: MutableAdviceContext<T, X>,
+    compiledSymbol: MethodPropertyDescriptor,
+    joinpoint: (...args: any[]) => unknown,
+  ): CompiledSymbol<T, X> {
+    compiledSymbol = wrapMethodDescriptor(ctxt, compiledSymbol, joinpoint);
+
+    return compiledSymbol as CompiledSymbol<T, X>;
+  }
+}
+
+function wrapMethodDescriptor<X>(
+  ctxt: MutableAdviceContext<PointcutType.METHOD | PointcutType.PARAMETER, X>,
+  descriptor: MethodPropertyDescriptor,
+  joinpoint: JoinPoint,
+): CompiledSymbol<PointcutType.METHOD | PointcutType.PARAMETER, X> {
+  return {
+    ...descriptor,
+    value: renameFunction(
+      joinpoint,
+      ctxt.target.descriptor.value,
+      `function ${String(ctxt.target.propertyKey)}$$advised`,
+    ),
+  };
+}