OT: Nullable/NotNull affect design.

A bit off-topic: interface annotations enforce better design?

A problem:
PsiElement.getProject() normally returns non-null project unless it isn't
valid (psiElement.isValid() == false) so it cannot be documented as @NotNull
as is.
On the other hand annotating it as @Nullable will produce enormous amount
of false positives since in most cases we've ensured somehow already psiElement
is indeed valid.

Solution:
throw InvalidPsiElementAccess exception instead of returning null.
First of all we can now document getProject() (and many others) as @NotNull,
plus we have better design with early real problem diagnostics instead of
getting NPE somewhere miles behind from the faulty code.


Homework:
getParent() always returns @NotNull except forementioned validity problem
AND it is top-level PsiPackage or PsiDirectory, where @Nullable is perfectly
legal. Any ideas?

-


Maxim Shafirov
http://www.jetbrains.com
"Develop with pleasure!"


6 comments
Comment actions Permalink

"Checklists make you smarter. Automatically enforced checklists with heads-up display and quickfixes make you brilliant" -- Griffith's first law.

Do PsiClasses have PsiPackages as parents? I've written a lot of PSI tree searching code, and never realized that. Cool. I can use that, particularly if you implement IDEABKL-2742.

Inevitably, anything that causes designs to more explicit and checkable will run into the areas where Java is less than perfect for the expression of design. When you find this, it isn't a problem, it's just a place where you had a problem before and didn't realize it (or at least didn't document it). There is no good solution in (Java@Nullable@NotNullable) for the issue you describe, other than scattering a bunch of "assert" statements around. Java is too weak and your annotations too strong. Don't think there's a better annotation set, though Fortunately, you've got a tool that will create those assert statements for you. Just be sure to check that you actually mean it before you hit the "assert parent !=null" intention.

It feels like a problem, but it's actually victory.

--Dave Griffith

0
Comment actions Permalink

Hello Maxim,

MS> Homework:
MS> getParent() always returns @NotNull except forementioned validity
MS> problem
MS> AND it is top-level PsiPackage or PsiDirectory, where @Nullable is
MS> perfectly
MS> legal. Any ideas?
MS> -



You could return a NullParent null object instead of null (not sure about
this changing the semantics of the @NotNull annotation, but at least it'd
solve the possible NPE problem).

HTH,
Andrei


0
Comment actions Permalink

That's exactly is the problem I was discussing in a "@Nullable/@Notnull
please advise" June 25, 2005 12:37 AM
It is a very common case a method returns Null only under very narrow
conditions (e.g a bean during initialization, instance while being added to
a datastructure - getParent() getType() being very typical example) and then
stays not null for the rest of its life. Very often initialization is
package private or done within factory or in some other way is very
localized abd when object is finally presented to API user it is fully
initialized and not null. While API developer might benefit from this method
bein marked as nullable in few cases API user will suffer tonns of false
positives. I wonder if it could be a good idea to do following
1. Mark getParent() in interface as NOT NULL and mark it in implementation
as NULLABLE I wonder what would be the effect

As for throwing exception instead of returning null I thought it is a common
practice. We always do something like this and often pair it with find
method (so we could test for existance if it makes sence) which by
convention MUST be checked for null

e.g

Employee getEmployee(EmployeeOID oid) {
could return null
}

Employee findEmployee(EmployeeOID oid) {
Employee e = findEmployee(oid);
if (e == null) {
throw illegalArgumentException("Employee oid=" + oid + " could not be
found");
}
}


"Maxim Shafirov (JetBrains)" <max@jetbrains.com> wrote in message
news:c8a8a1bf104ff8c753102ff3673e@news.jetbrains.com...
>A bit off-topic: interface annotations enforce better design?
>

A problem:
PsiElement.getProject() normally returns non-null project unless it isn't
valid (psiElement.isValid() == false) so it cannot be documented as
@NotNull as is.
On the other hand annotating it as @Nullable will produce enormous amount
of false positives since in most cases we've ensured somehow already
psiElement is indeed valid.

>

Solution:
throw InvalidPsiElementAccess exception instead of returning
null. First of all we can now document getProject() (and many others) as
@NotNull, plus we have better design with early real problem diagnostics
instead of getting NPE somewhere miles behind from the faulty code.

>
>

Homework:
getParent() always returns @NotNull except forementioned validity problem
AND it is top-level PsiPackage or PsiDirectory, where @Nullable is
perfectly legal. Any ideas?
-------------------
Maxim Shafirov
http://www.jetbrains.com
"Develop with pleasure!"

>



0
Comment actions Permalink

Maxim

Any ideas?



Code flow analysis?

I thought there was already some, but the test below proved me wrong. I
find this a dangerous limitation, as we - I - tend to trust @Nullable
blindly:


public String test ()
{
String s = bar ();
if(s==null)
s = null; // shouldn't remove the @Nullable warning, but
it does :: DANGER
return s;
}

@Nullable
private String bar () {
return null;
}


Alain

0
Comment actions Permalink


"Dave Griffith" <dave.griffith@cnn.com> wrote in message
news:30144187.1120953834612.JavaMail.itn@is.intellij.net...

"Checklists make you smarter. Automatically enforced checklists with

heads-up display and quickfixes make you brilliant" -- Griffith's first
law.
>

Do PsiClasses have PsiPackages as parents? I've written a lot of PSI tree

searching code, and never realized that. Cool. I can use that,
particularly if you implement IDEABKL-2742.
No, unfortunately in PSI they have PsiFiles as parents. Also using tree
methods like getParent() in PSI is highly discouraged, due to the type
imprecision problems we are discussing. The general IDEA is to write
semantics rich methods that will return appropriate inheritors of
PsiElement.

>

Inevitably, anything that causes designs to more explicit and checkable

will run into the areas where Java is less than perfect for the expression
of design. When you find this, it isn't a problem, it's just a place where
you had a problem before and didn't realize it (or at least didn't document
it). There is no good solution in (Java@Nullable@NotNullable) for the
issue you describe, other than scattering a bunch of "assert" statements
around. Java is too weak and your annotations too strong. Don't think
there's a better annotation set, though

I would still propose @NotNullImplemented annotation that says nothing about
current method implementation (there is none for interfaces), but makes
overriding/implementing methods marked with @NotNull by default, unless
there is @Nullable explicitly written. Just a convenience annotation for the
cases when @Nullable are really exceptional in the hierarchy.


Eugene.


0
Comment actions Permalink

Just understood my mistake:
Not implementing methods, of course there are none for getParent(), but for
the cases when the qualifier of the method call has a type of overrider of
PsiElement.

"Eugene Vigdorchik (JetBrains)" <ven@jetbrains.com> wrote in message
news:dats1i$kel$1@is.intellij.net...
>

"Dave Griffith" <dave.griffith@cnn.com> wrote in message
news:30144187.1120953834612.JavaMail.itn@is.intellij.net...

"Checklists make you smarter. Automatically enforced checklists with

heads-up display and quickfixes make you brilliant" -- Griffith's first
law.
>

Do PsiClasses have PsiPackages as parents? I've written a lot of PSI

tree

searching code, and never realized that. Cool. I can use that,
particularly if you implement IDEABKL-2742.
No, unfortunately in PSI they have PsiFiles as parents. Also using tree
methods like getParent() in PSI is highly discouraged, due to the type
imprecision problems we are discussing. The general IDEA is to write
semantics rich methods that will return appropriate inheritors of
PsiElement.

>

>

Inevitably, anything that causes designs to more explicit and checkable

will run into the areas where Java is less than perfect for the expression
of design. When you find this, it isn't a problem, it's just a place

where

you had a problem before and didn't realize it (or at least didn't

document

it). There is no good solution in (Java@Nullable@NotNullable) for the
issue you describe, other than scattering a bunch of "assert" statements
around. Java is too weak and your annotations too strong. Don't think
there's a better annotation set, though

>

I would still propose @NotNullImplemented annotation that says nothing

about

current method implementation (there is none for interfaces), but makes
overriding/implementing methods marked with @NotNull by default, unless
there is @Nullable explicitly written. Just a convenience annotation for

the

cases when @Nullable are really exceptional in the hierarchy.

>
>

Eugene.

>
>


0

Please sign in to leave a comment.