Remove wrong assumption on opaque methods #2857
Conversation
allredj
requested review from
cesaro,
cristina-david,
mgudemann,
NathanJPhillips,
peterschrammel,
pkesseli,
smowton and
thk123
as code owners
| @@ -2213,7 +2208,7 @@ void java_bytecode_convert_methodt::convert_invoke( | |||
| symbol.pretty_name = id2string(arg0.get(ID_C_class)).substr(6) + "." + | |||
| id2string(symbol.base_name) + "()"; | |||
| symbol.type = arg0.type(); | |||
| symbol.type.set(ID_access, ID_public); | |||
| symbol.type.set(ID_access, ID_private); | |||
There was a problem hiding this comment.
Based on the above argument - can we assume it is in fact protected?
There was a problem hiding this comment.
It's not guaranteed. It can well be private. The previous assumption (ID_public) was based on the fact that we wouldn't call these functions in generated tests. But I don't think we can completely exclude finding calls to private methods in generated tests, e.g. when mocking private methods (if we do partial mocking in the future), so we need to know consider that the method can be private. Anyway it's too early to make assumptions on how we'll be calling these methods, so we need to stay accurate.
Should I make the comment clearer?
|
test-gen bump: https://github.com/diffblue/test-gen/pull/2205 |
allredj
force-pushed
the
fix-opaque-accessibility
branch
from
584344f to
3caa28b
Compare
When converting a method call, set the access identifier to `ID_private` instead of `ID_public`. The latter lead to directly accessing protected methods from generated tests that do not inherit from the class where that method is defined.
allredj
force-pushed
the
fix-opaque-accessibility
branch
from
3caa28b to
1d8c735
Compare
When converting a method call, set the access identifier to
ID_privateinstead ofID_public.The latter lead to directly accessing protected methods from generated tests that do not inherit from the class where that method is defined.
Assuming that the method is not accessible is the only safe guess at this point.
To avoid over-using PowerMock, the strategies that discard the classes (such as
--load-containing-class-only) need to be altered/amended so that we keep that information.This PR has no tests as changes are only visible in test-gen.