Re: [aspectj-users] Proceeding with all arguments in native syntax

[Andrew Clement <andrew.clement@xxxxxxxxx>]

I do take your point :) I don’t think any of these ideas are trivial to implement, there is unfortunately often more to it than looks from the surface. Yours is probably simpler - but as usual the trouble is that when there is a workaround it is really hard to find the cycles to get these changes in. Any spare cycles end up better used fixing problems with no workaround.

Cheers,
Andy

> On Sep 5, 2020, at 9:58 PM, Alexander Kriegisch <alexander@xxxxxxxxxxxxxx> wrote:
> 
> Hi Andy.
> 
> Your idea is more sophisticated and more powerful too, no doubt. Me
> being a Scrum/Agile guy, I would rather prefer to have a way to bind all
> arguments like I proposed in AJ soon than waiting for another couple of
> years for the "grand scheme" implementation. The ticket you pointed to
> is from 2008 and I know you don't have a lot of free cycles. So I guess
> if your idea was easy to implement, you might have done so already. IMO
> a small improvement now is better than a big one which never happens.
> But of course, if you are ready to tackle the feature you just
> suggested, it would be great too. :) Or maybe the two features are the
> same order of magnitude with regard to complexity, even though that
> would surprise me.
> 
> Cheers
> -- 
> Alexander Kriegisch
> https://scrum-master.de
> 
> 
> Andy Clement schrieb am 06.09.2020 01:07 (GMT +07:00):
>> 
>> 
>> I'm not totally surprised casting the joinpoint didn't work, I just hoped
>> :) I think we can probably get that to work. On the face of it your
>> proposal sounds reasonable for binding args. However, I still find it very
>> ugly that you can't just get the parameters that met the pointcut
>> constraints and get rid of all that code that retests whether they are
>> annotated, which is why I pointed to
>> https://bugs.eclipse.org/bugs/show_bug.cgi?id=233718
>> 
>> aspect X {
>> // Note this advice will match and run multiple times - once for each
>> param
>> void around(String paramToScrub): call(* *(..,@Scrubbed (String),..)) &&
>> args(paramToScrub) { return proceed(scrubString(paramToScrub));
>> }
>> // More optimal perhaps, less nesting, probably easier to implement
>> void around(org.aspectj.lang.MatchedArg[] args): call(* *(..@Scrubbed
>> (String),..)) && args(matchedArgs) {
>> for (MatchedArg arg:args)
>> 
>> arg.setValue(scrubString(arg.getValue());
>> return proceed(args);
>> }
>> }
>> 
>> Andy
>> 
>> 
>> On Fri, 4 Sep 2020 at 18:44, Alexander Kriegisch <alexander@xxxxxxxxxxxxxx
>> <mailto:alexander@xxxxxxxxxxxxxx> > wrote:
>> 
>>> Bug 233718 is not really what I was concerned about.
>>> 
>>> Casting thisJoinPoint to ProceedingJoinPoint was an idea I also had
>>> before, but the subsequent proceed(args) call returns null. If you run
>>> this aspect in native syntax against the sample classes in my SO
>>> answer...
>>> 
>>> package de.scrum_master.aspect;
>>> 
>>> import java.lang.annotation.Annotation;
>>> import java.lang.reflect.Method;
>>> 
>>> import org.aspectj.lang.ProceedingJoinPoint;
>>> import org.aspectj.lang.SoftException;
>>> import org.aspectj.lang.reflect.MethodSignature;
>>> 
>>> import de.scrum_master.app.Scrubbed;
>>> 
>>> public aspect StringScrubberAspect {
>>> Object around() : call(public * *(.., @de.scrum_master.app.Scrubbed
>>> (String), ..)) {
>>> Method method = ((MethodSignature)
>>> thisJoinPoint.getSignature()).getMethod();
>>> Object[] args = thisJoinPoint.getArgs();
>>> Annotation[][] parameterAnnotations = method.getParameterAnnotations();
>>> for (int argIndex = 0; argIndex < args.length; argIndex++) {
>>> for (Annotation paramAnnotation : parameterAnnotations[argIndex]) {
>>> if (paramAnnotation instanceof Scrubbed)
>>> args[argIndex] = scrubString((String) args[argIndex]);
>>> }
>>> }
>>> try {
>>> System.out.println(thisJoinPoint instanceof ProceedingJoinPoint);
>>> Object result = ((ProceedingJoinPoint) thisJoinPoint).proceed(args);
>>> System.out.println(result);
>>> return result;
>>> } catch (Throwable t) {
>>> throw new SoftException(t);
>>> }
>>> }
>>> 
>>> private String scrubString(String string) {
>>> return string.replaceAll("[Ee]", "#");
>>> }
>>> }
>>> 
>>> ... the console log will be:
>>> 
>>> true
>>> null
>>> true
>>> null
>>> 
>>> How about extending 'args()' functionality in order to allow binding to
>>> an Object[] parameter which does not occur anywhere in the pointcut
>>> literal, automatically binding the equivalent of JoinPoint.getArgs() to
>>> it and thus allowing to proceed with the same bound parameter in both
>>> native and @AspectJ syntax? That would be kind of elegant, probably
>>> manageable from the perspective of pointcut parsing because there is no
>>> really new syntax, and the functionality to proceed with an Object[] is
>>> already in the code base, which is why it works in @AspectJ syntax.
>>> 
>>> --
>>> Alexander Kriegisch
>>> https://scrum-master.de
>>> 
>>> 
>>> Andy Clement schrieb am 05.09.2020 02:05 (GMT +07:00):
>>> 
>>>> I feel like there are some bugs that talk around this topic, but I
>>>> can't quite find the one I'm thinking of. I found
>>>> https://bugs.eclipse.org/bugs/show_bug.cgi?id=233718 and
>>> it was
>>>> interesting to read what I was writing 12 years ago :) I feel like i
>>>> prototyped an improved arg matching system but I hit something
>>>> seriously complex before finishing, and wrote about it in bugzilla
>>>> somewhere...
>>>> 
>>>> I don't think you are missing anything. Mixing the syntaxes should be
>>>> ok I think. Can you cast thisJoinPoint to a ProceedingJoinPoint in
>>>> AspectJ syntax around advice and use it? I can't quite remember.
>>>> 
>>>> 
>>>> On Thu, 3 Sep 2020 at 22:41, Alexander Kriegisch wrote:
>>>> 
>>>>> Check this out:
>>>>> https://stackoverflow.com/a/63735393/1082681
>>>>> 
>>>>> Is there anything I have overlooked? If so, how would I do that in
>>>>> native syntax? If not, would it be hard to implement?
>>> _______________________________________________
>>> aspectj-users mailing list
>>> aspectj-users@xxxxxxxxxxx
>>> <mailto:aspectj-users@xxxxxxxxxxx> 
>>> To unsubscribe from this list, visit
>>> https://www.eclipse.org/mailman/listinfo/aspectj-users
>> 
> 
> _______________________________________________
> aspectj-users mailing list
> aspectj-users@xxxxxxxxxxx
> To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/aspectj-users