Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 338312

Summary:	[null] improve hierarchy of Scope - document design assumptions
Product:	[Eclipse Project] JDT	Reporter:	Stephan Herrmann <stephan.herrmann>
Component:	Core	Assignee:	Stephan Herrmann <stephan.herrmann>
Status:	CLOSED WONTFIX	QA Contact:
Severity:	minor
Priority:	P3	CC:	Olivier_Thomann, srikanth_sankaran
Version:	3.7
Target Milestone:	---
Hardware:	All
OS:	All
Whiteboard:	stalebug

Description Stephan Herrmann

2011-02-26 14:24:51 EST

While playing with null annotations (bug 186342) using the lookup package
as my test candidate I found that several methods in class Scope will
inevitably raise some warnings/errors because these methods don't well 
reflect the original design intent.

E.g., method classScope() is defined in the super class Scope, but it
makes sense only on ClassScope and BlockScope (incl. descendants).
A similar example is enclosingReceiverType().

In order to give a universal implementation those methods contain a branch
where null is returned, saying this method wasn't actually applicable
on the current scope instance.

OTOH many clients of these methods strictly assume that the result is non-null.

In the case of classScope() this could easily be fixed by inserting another
class in the hierarchy, say ClassScopeOrInside plus pushing down the method
to the newly created type. ClassScope and BlockScope would change their super
to the new type and the method could stop returning null.

For enclosingReceiverType() the situation is a bit trickier, because clients
exist that just use a scope of unknown kind and invoke this method. But still
small changes in the hierarchy (perhaps we need one or two additional 
interfaces?) plus moving some methods up/down the hierarchy would help to
make the design more explicit. Several signatures actually use Scope just
to say ClassScope or BlockScope, so excluding CompilationUnitScope would be
a merely syntactic change.

These changes would improve the maintainability right away, and if one day
JDT/Core will be able to use null annotations it will be much easier to
define the null contracts in this area.

Comment 1 Stephan Herrmann

2011-02-28 12:17:51 EST

Srikanth, this is one of the issues we talked about.
If my proposal makes any sense to you, I'd be happy to prepare a patch.
I think a common super class of ClassScope and BlockScope could resolve
most of this already. 
What do you think?

Comment 2 Srikanth Sankaran

2011-03-10 23:56:41 EST

(In reply to comment #1)
> Srikanth, this is one of the issues we talked about.
> If my proposal makes any sense to you, I'd be happy to prepare a patch.
> I think a common super class of ClassScope and BlockScope could resolve
> most of this already. 
> What do you think?

Would making classScope() an abstract method and pushing the current
code with appropriate changes to the different branches of the hierarchy
solve the issue ?

In principle, the proposed change sounds good, though I would wait to see
how large the changes are going to be. Even if the changes are straightforward
if they are pervasive, it is may be better to wait until after 3.7 as we are
already in RC0 phase. Particularly since this is a reactive cleanup to null
analysis work as opposed to an enabler for that work. If there are two many
of these warnings that pollute the overall quality of the null diagnostics,
then the proposed change could be done on a developer branch/workspace basis
to weed out these.

In summary, if the volume of change is small and well contained, let us go
for it now. otherwise we can do it just post 3.7, just so we can avoid the
risk however small of having to remind ourselves of the proverb about the
road to hell being paved with good intentions :)

Comment 3 Eclipse Genie

2019-12-29 16:34:00 EST

This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.