Bug 71159 - pointcut call(MethodPattern) matches non-visible methods in parent class
Summary: pointcut call(MethodPattern) matches non-visible methods in parent class
Status: RESOLVED FIXED
Alias: None
Product: AspectJ
Classification: Tools
Component: Compiler (show other bugs)
Version: 1.2   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 1.5.0 M4   Edit
Assignee: Adrian Colyer CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-07-30 10:36 EDT by David Grainger CLA
Modified: 2005-08-31 11:46 EDT (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Grainger CLA 2004-07-30 10:36:22 EDT
aspectjtools.jar / ajc v1.2

We believe that the call(methodpattern) pointcut has matching behaviour that is
inconsistent with what we expect from Java in relation to invocations of methods
on subclasses and superclasses.

Background: 
Our goal is to use an aspect to declare ajc compiler warnings on any invocation
to a target method, where the space of targets is defined as any visible method
of any class in a specified package P or its subpackages.

As well as straight invocations from types in packages oustide P and its
subpackages, we wish to match method invocations on P where the methods invoked
are inherited from P -- that is , they are obtained in a type that directly
extends a type in P or its subpackages.  

The fragment of the aspect we are using is:
public aspect Aspect1
{
  pointcut methodCalls() : 
    !within(Aspect1) &&
    call(* P..*.*(..)) ;         // 'P' is the target package space
  declare warning : methodCalls() : "invoking";
  //...
}

Consider these cases:

Case 1 method inheritance: 
Superclass A in package P declares and implements a public method M.  A direct
subclass B (in a package outside P) directly extends A and inherits this method.
 Now, any calls inside B to M or this.M() are matched by the call() join point
above which seeks to match calls to P..*.*() This is as we would expect since
the implementation in package space P is actually being called.

Case 2 method overriding: 
Superclass A in package P declares and implements a public method M.  Direct
subclass B (in a package outside P) overrides A.M with its own implementation
M'. M' does not invoke M.  Now, calls inside B to M' or this.M'() are still
matched by the call() join point above which seeks to match calls to P..*.*()
even though M' does not invoke or depend on M.  We do not expect this result
since we do not think M is actually called.

Case 3 redeclaration of non-visible method with the same name:  
Superclass A in package P declares and implements a private method M.  Direct
subclass B (in a package outside P) introduces its own method M having the same
signature as A.M.  Now, calls in B to M or this.M() are still matched by the
call() join point above which seeks to match calls to P..*.*() even though A.M
is not visible to B and is never called by it. We do not expect this result
since we do not think A.M is ever called.

The only way we can explain this apparent behaviour is by reasoning that the
compiler is treating the subclass B "as a type of" its parent A and somehow
concluding that method calls on B can be equated with calls to methods of
identical signature on A.  However this seems at odds with the rules for Java
visibility and with our expectations for when the call(...) joinpoint should match.

We have experimented with execution(...) join points to perform this matching
but that has turned up a different set of problems which we are still analyzing.

Please can you shed any light on what the call joinpoint is doing here?

regards, Dave
Comment 1 Adrian Colyer CLA 2004-08-09 15:32:58 EDT
marked as target 1.2.1
Comment 2 Andrew Clement CLA 2004-08-10 10:51:09 EDT
Case(1) - we all agree its right :)

===

Case(2) is 'working as designed'.  According to the new eclipse AspectJ book:

For the call() pcd: 

"If a declared type pattern is specified, then the pointcut will only match
calls made to methods that are declared in a type matched by the pattern, *or
one of its super-types.*"

So here we are talking about where things are *declared*, not whether the
subclass is calling the superclass.  Your subclass logic calls B().m() and
although the m() in the subclass runs, it is declared in the superclass (the
full signature for the declaration in the superclass matches your pointcut).

===

Case (3) is similar to case (2) but we are still thinking about it.  It may be
working as designed but it is unhelpful.  Although you aren't overriding m() in
the subclass, from the JVM spec (8.4.2):

"The signature of a method consists of the name of the method and the number and
types of formal parameters."

It doesn't include the visibility modifier.  So, if we ignore the visibility
modifier then this is the same as case (2).

Our AspectJ language guru, Erik Hilsdale is off back packing this week thinking
through the semantic issues... more to follow ...
Comment 3 Adrian Colyer CLA 2005-08-26 11:08:21 EDT
we should take note of visibility in ResolvedMemberImpl.getJoinPointSignatures
Comment 4 Adrian Colyer CLA 2005-08-31 11:46:14 EDT
Fixed!!

Given the program below, the "should match" warning does match, and the "should
not match" warning does not.

class A {
	
	private void foo() {}
	
}

class B extends A {
	
	protected void foo() {}
	
}

class C extends B {}


class D extends C {
	
	public void foo() {}
	
}

aspect X {
	
	void bar() {
		D d = new D();
		d.foo();
	}
	
	declare warning : call(* B.foo()) : "should match";
	declare warning : call(* A.foo()) : "should not match";
	
}

Will be available in next published build on AspectJ download page.