eclipse-aspectj / aspectj

Other
303 stars 86 forks source link

Include ajc$ prefix in Around Advice/Shadow Method Name from NameMangeler #249

Closed warrenc5 closed 1 year ago

warrenc5 commented 1 year ago

This is a obscure and seemingly inconsequential request, so please allow me to explain.

I encountered this as a conflict with the JAIN SLEE 1.1 specification while working on a large legacy code base.

I have some fields whose natural camel-case name (based on their type) is sbbSomething

The weaver creates around advice/shadow method names starting with the field name.

It's illegal in JAIN SLEE for an Sbb component to have (non standard) method names starting with sbb (section 6.12 - Method Name Restrictions) - So I can't deploy those weaved components into the SLEE.

I can change the field name, but I noticed this discrepancy in the aspect weaver naming and patched the name mangler.

I wanted others advice before I fix the tests and raise a PR. Is the omission of PREFIX intentional or otherwise a design pattern/consideration of the aspectj weaver?

The patch would be to add PREFIX + getExtractableName into the two locations in NameMangeler.

Is there any deeper impact of change? Or is it just localized to the generated bytecodes. - The suffix seems to be a use site counter so I can't see any way anyone would rely on these method names for anything else.

kriegaex commented 1 year ago

Hello @warrenc5. I admit, I first had to find out what JAIN SLEE is at all. I found an outdated, fragmentary Wikipedia article from 2012, flagged for not citing sources. Then I found two ancient JSRs from 1999 (released 2004) and 2004 (released 2008), respectively. Next, I found https://github.com/RestComm/jain-slee, which leads me to think that JAIN SLEE is quite exotic and mostly dead. I might be wrong about this, but it is my first impression.

It's illegal in JAIN SLEE for an Sbb component to have (non standard) method names starting with sbb (section 6.12 - Method Name Restrictions) - So I can't deploy those weaved components into the SLEE.

AspectJ aims to be compatible with the Java Language Specification, not with any specific naming standards enforced by other specifications. But for example, a Java Bean is still a Java Bean, even after AspectJ weaving has taken place.

Anyway, section 6.12 from JAIN SLEE v1.1 says:

6.12 Method name restrictions

Non-private (such as public, protected, or package private) methods that are declared by the SBB Developer must not begin with sbb or ejb. A SLEE implementation can use method names that begin with sbb when needed without being concerned with possible method name conflicts with SBB Developer declared method names.

This restriction does not apply when the SBB Developer is implementing a required sbb<XXX> method declared by the SLEE, such as the life cycle methods declared in the javax.slee.Sbb interface. (Clarified in 1.1)

So that is where your problem originates. AspectJ generates method names starting with sbb, because your field names start with sbb, and some kind of classfile scanner stumbles across this fact. If there is no way to exclude certain method patterns from scanning, I suggest to simply rename the problematic fields to _sbb or something completely different, respecting the method prefix also for fields, if you use byte code instrumentation creating methods for said fields, of which AspectJ is one in this case. Maybe using load-time instead of compile-time weaving could also help, if class scanning is part of the build rather than the runtime process.

I do not feel so inclined to change the AspectJ innards for this specific requirement. Sorry to disappoint you.

warrenc5 commented 1 year ago

Thank you @kriegaex for your insight and feedback.