Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

2.0b2 Method selection incorrect when accessibility enabled #325

Open
sourceforge-issue-exporter opened this issue Sep 13, 2004 · 5 comments
Open

2.0b2 Method selection incorrect when accessibility enabled #325

sourceforge-issue-exporter opened this issue Sep 13, 2004 · 5 comments
Labels
auto-migrated sourceforge bug checked General out of scope v3.0

Comments

@sourceforge-issue-exporter
Copy link

sourceforge-issue-exporter commented Sep 13, 2004 •
edited by nickl-
Loading

If there are two accessible matching methods, and one
is private, and one is public, the public method should be
chosen. At the moment it is random. Strictly speaking at
the moment the first Matching method is chosen.

Example: suppose we have class Foo, with two methods
public void foo(String x);
private void foo(Integer x);

If we do Foo f = new Foo(); f.foo(null); the public
method should be invoked, not the private method. This
is not guaranteed.

Consider the following test case. This test case fails
under Beanshell 2.0b2 on JDK1.3.1. The test case
passing does not mean the code is working, as the order
in the result of Class.getDeclaredMethods is not defined.

The code to fix is in bsh.Reflect.java in methods
findMostSpecificSignature(), and findMostSpecificMethod
(). The easiest fix would seem to be get
findMostSpecificMethod to sort the candidates by how
public they are, before passing to
findMostSpecificSignature, so as to take advantage of
the current behaviour of choosing the first match.

public class BSHMethodInvocationTest extends TestCase {
    public BSHMethodInvocationTest() {
    }

    public BSHMethodInvocationTest(String s) {
        super(s);
    }

    public void testFindPublicMethods() throws EvalError {
        Interpreter i = new Interpreter();
        Capabilities.setAccessibility(true);
        i.eval("a = new bsh.BSHMethodInvocationTest.A(); a.doSomething('arg1', null);");
    }

    public static class A {
        private void doSomething(String arg1, Object arg2) {
            throw new Error("shouldn't find this");
        }
        private void doSomething(String arg1, Object [] arg2) {
            throw new Error("shouldn't find this");
        }
        private void doSomething(String arg1, Double arg2) {
            throw new Error("shouldn't find this");
        }
        public void doSomething(String arg1, String arg2) {
        }
        private void doSomething(String arg1, Integer arg2) {
            throw new Error("shouldn't find this");
        }
        private void doSomething(String arg1, Float arg2) {
            throw new Error("shouldn't find this");
        }
        private void doSomething(String arg1, java.util.Map arg2) {
            throw new Error("shouldn't find this");
        }
    }
}

Reported by: nfortescue

Original Ticket: "beanshell/bugs/187":https://sourceforge.net/p/beanshell/bugs/187
@sourceforge-issue-exporter
Copy link
Author

Logged In: YES
user_id=251678

Here's a fix I've made to my local copy. I've changed
findMostSpecificMethod in Reflect.java to look like this:

boolean isPublic = Modifier.isPublic( methods[i].getModifiers
() );
if ( publicOnly && !isPublic )
continue;

// method matches name
if ( methods[i].getName().equals( name ) ) {
if(isPublic) {
meths.insertElementAt(methods[i], 0);
sigs.insertElementAt(methods[i].getParameterTypes(), 0);
} else {
meths.addElement( methods[i] );
sigs.addElement( methods[i].getParameterTypes() );
}
}

Original comment by: nfortescue

@sourceforge-issue-exporter
Copy link
Author

Logged In: YES
user_id=18885

Well, so the first thing to note is that this situation
doesn't apply in regular Java. It's really an ambigous
method selection and normally you'd be forced to resolve
that with a cast. That brings up the issue that BeanShell
doesn't currently influence method selection with casts, but
it should...

So I think the question is whether we want to define our own
rule which says that public methods are favored or whether
we want to follow a more Java-like rule, detect the
ambiguity, and allow the user to disambiguate with a cast.

Pat

Original comment by: patn

@sourceforge-issue-exporter
Copy link
Author

Logged In: YES
user_id=251678

That's partly true. It would be great if you coudl resolve it with
a cast. However, when being called from code outside the
class a cast is not necessary in normal Java, because the
private method is not accessible. This resolves the ambiguity.

Because Beanshell allows access to private methods through
accessibility (if it is turned on) then the extra resolution is
needed.

So I think casting would be great, but in the public vs. private
situation, public should be chosen for compatibility with Java.

It would be good to have a policy for resolution as well for
other scoping eg, package vs private, etc.

Original comment by: nfortescue

@nickl-
Copy link
Member

nickl- commented Dec 8, 2022

For this to be tested I have to first be able to setAccessibility(true);

but:

accessible: module java.base does not "opens java.lang" to unnamed module @6fa4fbe3

see #629

@nickl- nickl- added the checked label Dec 8, 2022
@nickl-
Copy link
Member

nickl- commented Dec 8, 2022

test script

methodselection3.bsh.txt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
auto-migrated sourceforge bug checked General out of scope v3.0
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@nickl- @sourceforge-issue-exporter