Community
Participate
Working Groups
When advice doesn't match in a 1.5.0 enabled project, there is an adviceDidNotMatch warning against the line number of the advice. If you insert a line before this advice and save then the warning stays associated with the original line and not the new one. Full building puts the warning against the new line. This is a regression and didn't happen in AJ 1.5.0.
Created attachment 37544 [details] failing testcase Apply this patch to the tests project.
I think part of the fix for bug 129163 caused this bug. Within CrosscuttingMembers.replaceWith(..) we only calculate the changes to the shadowmungers and typemungers if we careAboutShadowMungers. If we change to always calculate the changes to the shadowmungers and typemungers then this bug no longer occurs.
The problem is the equals method in Advice. At the moment its only using kind, pointcut and signature to see if two pieces of advice are the same. In the case of this bug they are the same, however, they have different sourceLocations. Because we're thinking they're equal we're not updating the list of shadowMungers and so the original one is used in the calculation of the xlint warning (which gives the original line number). The reason part of the fix for bug 129163 causes this problem is that if we always check shadowmunger equality the first time we compare we compare something with nothing which is a change and the second time we check nothing against something. This forces the update. This is a regression because for 1.5.0 we always thought there was a change so we always updated (due to various implementations of equals).
The fix is to add implementation of the equals and hashcode methods within Advice and EclipseSourceLocation. Within Advice I've added a check for the equality of the SourceLocation to the equals method and in EclipseSourceLocation I've said that two are equal if they have the same start position, same end position and if their sourceFiles are equal.
Created attachment 37720 [details] patch implementing equals and hashcode in EclipseSourceLocation Apply this patch to the org.aspectj.ajdt.core project.
Created attachment 37721 [details] patch improving equals and hashcode methods in Advice Apply this patch to the weaver project.
Created attachment 37722 [details] testcase patch synchronized with the latest version in HEAD Apply this patch to the tests project.
Created attachment 37727 [details] testcase patch which tests that adding whitespace doesn't force full build Apply this patch to the tests project.
Created attachment 37728 [details] patch which doesn't force full build if there's a whitespace change Apply this patch to the weaver project.
An explanation for the previous two patches (comment #8 and comment #9)... By changing the equals methods as described in comment #4 this does have the knock on effect of thinking there's been a change if the sourcelocation changes. Which means that if there's a whitespace change then we force a full build...not really something we want to be doing......An alternative solution is to go ahead and work out whether there's been a change as we have been doing previously, however, to copy over the new list of shadowmungers regardless of whether we think there's been a change (this is all in CrosscuttingMembers.replaceWith(..)). That way we pick up any changes in sourcelocation without forcing a full build. This does require us to reset the list of shadowmungers in CrosscuttingMembersSet if we care about the shadowmungers otherwise we wont pick up the new ones.
The fix for this has been checked in and is available in both the latest AJ and AJDT dev builds. Therefore, closing this as fixed.
