-
Notifications
You must be signed in to change notification settings - Fork 888
Make wrapped Java object not a JS array/map when static type is not list/map #1897
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.
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
base: master
Are you sure you want to change the base?
Conversation
Hi @rPraml , I have a question related to SecurityControllerTest, and git commit history said you were the author For an interface and implementation like this: public interface SomeInterface {
String foo();
}
_
public class SomeClass extends ArrayList<String> implements SomeInterface {
@Override
public String foo() { return "FOO"; }
public String bar() { return "BAR"; }
} When the access to (Also SecurityControllerTest is the only failing test for changes in this PR, I want to make it pass) |
Hello @ZZZank
I hope that we will soon make the decision and remove all the SecurityController workarounds ;)
This is AFAIK common design in java. For example if you have a I understand, what you would achieve with your PR, but i fear, it will break lots of tests, especially where generic containers (that are not lists or maps) are in place. Take the following test: It passes on develop, but fails on your branch, because AtomicReference.get() has as typeHint @Test
public void accessMapInAtomicRef() {
AtomicReference<Map<String, String>> reference = new AtomicReference<>();
Map<String, String> map = new HashMap<>();
map.put("a", "a");
reference.set(map);
assertEquals("a", runScriptAsString("value.get()['a']", reference, true));
assertEquals("a", runScriptAsString("value.get().a", reference, true));
}
now we have two ;) - please check, if the above is a legal use case (I think so) Roland |
I thought about this change again. The problem here is, that js is "duck typed" and that concept is also adapted to the wrap-factory. Let us consider the following example class HybridListMap implements List, Map { // I know, not really possible, because interfaces clashes
} Javascript looks only on the type of the object:
In java you can have a POJO, that has a This PR would fix this. (But i fear it will break lot of existing code) and PR #1853 will also improve type handling What we need now, is an improved cast. See ZZZank#3 Refering to #1885, maybe an explicit cast If we merge this PR, we should hide this functionality behind a flag (e.g. |
I tried an other aproach about the cast issue in #1902 - but I think we need help from one of the maintainers here, what the next steps could be |
I guess I didn't explain my question clearly enough. What confuses me is actually not implementation-only methods being not accessible when static type does not expose the method. There're two implementation-only methods used in rhino/tests/src/test/java/org/mozilla/javascript/tests/SecurityControllerTest.java Lines 90 to 93 in 5dedd0f
The strange thing here is that the test assume that rhino/tests/src/test/java/org/mozilla/javascript/tests/SecurityControllerTest.java Lines 98 to 104 in 5dedd0f
It's expecting error |
Fixes #1885