50 IG bugs found in 60 seconds...


with the new @Nullable checking. Real live, dump-an-NPE-stack-trace bugs. Worse, bugs that would only occur under obscure circumstances (usually in mid-edit when the parse tree is in an inconsistent state) that there was no way in hell I would find in testing. In short, that's fifty exception stack trace e-mails that won't be showing up in my inbox. And the PSI doesn't even look to be fully annotated yet!

Yow!

May I humbly suggest to JetBrains marketing that this is a major differentiating feature, and should be trumpetted as such.

--Dave Griffith

28 comments
Comment actions Permalink

May I humbly suggest to JetBrains marketing that this
is a major differentiating feature, and should be
trumpetted as such.


But really, this doesn't work unless the libraries you're working with (and your own code) is correctly annotated, and I don't see this really catching up before we have a JCP standard.

We can push things, thou. If you're an open source developer, do whatever you can to annotate your source code! The annotations.jar deployable is tiny, and I believe it's fully redistributable (JetBrains need to clear this up, just to be safe. An explicit BSD-like, or even public domain license on the jar file would be nice) -- your users won't suffer from your use of annotations.

Of course, you can still do the same if you're a commercial developer (specially if your product is a reasonably popular library) :)

0
Comment actions Permalink


Could anybody give a little code usage example/some guiding lines?
I tried placing @NotNull and @Nullable in many places, in my code, but
it's always red, with an error message in the status bar.

Alain

0
Comment actions Permalink

Dave Griffith wrote:

with the new @Nullable checking. Real live, dump-an-NPE-stack-trace bugs.


Great! Now I guess what we need is some more inspections for @Nullable
constraints :) Some obvious things that come to mind:

1) It would be great to have a warning for any occurrence of "return
null;" in a method that is not @Nullable, with a quickfix that adds the
@Nullable declaration to the method. Extending this for any value that
might be null would be even better -- at least, for any "return
nullableMethod();" or "return nullableField()".

2) Similarly, an error when you return null or something nullable in a
@NotNull method.

3) A warning for any method which is @Nullable but overrides a method
which is not @Nullable. I guess this introduces difficulties when you
override a library method that you can't make @Nullable, though.

4) A warning for any method which is not marked @NotNull but overrides a
method which is marked @NotNull.

I guess I should add feature requests for these things in JIRA, but
right now I'm too tired to go through that procedure. On Monday perhaps...

0
Comment actions Permalink

1. add annotations.jar to your classpath (found in IDEA's redist directory)

2.import the annotations you need
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

3. annotate your methods and things:
@Nullable public String doStuff() {
....
}

4. Turn on inspection Local Code Analysis -> Constant conditions &
exceptions


Vince.


0
Comment actions Permalink


Agreed. The great thing about the first one, of course, is that it would make it very easy for a library vendor to annotate their code. A similar inspection for adding @NotNull to methods which provably can't return null would help as well.

--Dave Griffith

0
Comment actions Permalink


Agreed. The great thing about the first one, of course, is that it would make it very easy for a library vendor to annotate their code. A similar inspection for adding @NotNull to methods which provably can't return null would help as well.

--Dave Griffith

0
Comment actions Permalink

Vincent

>...
>

Thanks.
Having never used annotations, I was completely lost.
It wouldn't hurt if the IDEA team would write more informative
announcements change logs.



>3. annotate your methods and things:

@Nullable public String doStuff() {
....
}

>4. Turn on inspection Local Code Analysis -> Constant conditions &
>exceptions
>

>
Works fine.


Is there another inspection that would detect the problem in the code below?

@NotNull <<--- no problem detected here.
private static String bar () {
return null;
}


Alain

0
Comment actions Permalink

I think that's what Jonas was suggesting in the same thread. It would
definitely be very helpful.

Vince.

@NotNull <<--- no problem detected here.
private static String bar () {
return null;
}



0
Comment actions Permalink

Is there another inspection that would detect the problem in the code
below?

@NotNull <<--- no problem detected here.
private static String bar () {
return null;
}


My plugin detects this. It should be released today or some time this weekend.


0
Comment actions Permalink

Isn't done yet. But will be done for sure.
-


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


0
Comment actions Permalink

Maxim

Isn't done yet. But will be done for sure.



An (inspection + quickfix) that checks for missing @Nullable (and adds
it, as a qfix) would be great too.
Do I need to post a request, or are you already working on it?

Alain

0
Comment actions Permalink

Request would be great anyways. Am I lazy filing them myself? :)

-


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

Maxim

>> Isn't done yet. But will be done for sure.
>>

An (inspection + quickfix) that checks for missing @Nullable (and adds
it, as a qfix) would be great too.
Do I need to post a request, or are you already working on it?
Alain



0
Comment actions Permalink

Spend your time implementing things, we can spend ours filing requests
for you :)

Vince.


0
Comment actions Permalink

The annotations.jar
deployable is tiny, and I believe it's fully redistributable
(JetBrains need to clear this up, just to be safe. An explicit
BSD-like, or even public domain license on the jar file would be nice)


It's in redist folder so it's indeed redistributable. As to redist license
itself it should be made more relaxed indeed. I'll ask management for that.

-


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


0
Comment actions Permalink

Request would be great anyways. Am I lazy filing them myself? :)

>
Done:
"Inspection for missing @Nullable"
http://www.jetbrains.net/jira/browse/IDEA-1823

0
Comment actions Permalink


Thinking aloud here, I'm not sure where this progress is going to lead me:
I searched for all the "return null" in one project, to tag them with
@Nullable when appropriate, and I started changing a lot of them to
throw new IllegalArgumentException ("Invalid xxx: " + xxx);

It's more correct, but I fear it weakens the inspectability of the
modified code.

Alain

0
Comment actions Permalink

Thinking aloud here, I'm not sure where this progress is going to lead
me:
I searched for all the "return null" in one project, to tag them with
@Nullable when appropriate, and I started changing a lot of them to
throw new IllegalArgumentException ("Invalid xxx: " + xxx);
It's more correct, but I fear it weakens the inspectability of the
modified code.


How do you think it weakens inspectability?

Anyway. This is one way to do it, however if you use my Nully plugin (due
out this weekend) it will automatically guarantee nonnullness at runtime
by inserting appropriate checks (like your illegalargumentexception checks)
during compilation.


0
Comment actions Permalink

Keith

>

How do you think it weakens inspectability?

>


If I use a method that's nullable, the @Nullable inspection will warn me
immediatly, in the editor, at the calling location.

If I replace the
"return null"
by
"throw new IllegalArgumentException..",
I don't get this warning anymore.

Alain

0
Comment actions Permalink

Isn't this what checked exceptions are for? Maybe we need a new modifier
for exceptions:

@ProbableError class SomeException extends RuntimeException { .. }

Keith

>> How do you think it weakens inspectability?
>>

If I use a method that's nullable, the @Nullable inspection will warn
me immediatly, in the editor, at the calling location.

If I replace the
"return null"
by
"throw new IllegalArgumentException..",
I don't get this warning anymore.
Alain




0
Comment actions Permalink

In theory, that would be redundant. Everything under "Runtime Exception" is supposed to represent coding errors, as opposed to 'expected' exceptional conditions. Unfortunately, standard usage is morphing away from that to some extent, due the the fact that containers like unchecked exceptions much better than checked ones.

--Dave Griffith

0
Comment actions Permalink

I don't think it would be redundant. The C# modification SPEC# has the following
tree of exceptions:

Client failure
Provider failure
Admissible failure
Observed program error

In SPEC#, only "admissible failures" must be declared in a throws clause.
So, I compare them to Java:

Client failure - RuntimeException
Observed program error - RuntimeException or Error
Admissible failure - Other Exception

I think a @ProbableError annotation would split "client failure" from "observed
program error." I think it would also split probable errors from improbable
errors (the kinds of things we OpenAPI users use Logger.assert for).

In theory, that would be redundant. Everything under "Runtime
Exception" is supposed to represent coding errors, as opposed to
'expected' exceptional conditions. Unfortunately, standard usage is
morphing away from that to some extent, due the the fact that
containers like unchecked exceptions much better than checked ones.

--Dave Griffith




0
Comment actions Permalink

Should we file bugs, or is this just experimental?
R

0
Comment actions Permalink

We've already fixed some cases found internally though additional cases won't
hurt. Please include full method content where you feel false positive had
happened. The context is really matter.

-


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

Should we file bugs, or is this just experimental? R



0
Comment actions Permalink

Dave Griffith wrote:

3) A warning for any method which is @Nullable but
overrides a method
which is not @Nullable. I guess this introduces
difficulties when you
override a library method that you can't make
@Nullable, though.


Adding onto this idea, make it an error if a @Nullable method overrides a @NotNull one.

IDEA should also complain about a method with both @NotNull and @Nullable annotations (is it possible to have more than one annotation on a method?).

0
Comment actions Permalink

Adding onto this idea, make it an error if a @Nullable method overrides a @NotNull one.


There also should be an error (or strong warning), when a method does not
define @Nullable/@NotNull, but the super method does.

Tom

0
Comment actions Permalink

How about making @Nullable and @NotNull annotations inherited?

Currently one must annotate both the interfaces and their implementing classes, this violates Dont Repeat Yourself and introduces possibilities for inconsistencies.

If the method in the interface / superclass is marked as @Nullable or @NotNull, then logically the inheriting / overriding class should also be, to implement the contract.

Is there some fundamental reason why the @NotNull and @Nullable annotations are not annotated as @Inherited?

0
Comment actions Permalink

� wrote:

How about making @Nullable and @NotNull annotations inherited?


For return types, I guess @NotNull could be inherited, but not @Nullable
because an overriding method might want to provide a stronger guarantee:
this particular implementation can never return null. This would be
similar to the use of covariant return types where a return type can be
strengthened in an overriding method.

For argument types, the opposite would be true (if @Inherited applies to
such types -- I have no idea).

0
Comment actions Permalink

According to the Javadoc, it "has no effect if the annotated type is used
to annotate anything other than a class."

How about making @Nullable and @NotNull annotations inherited?

Currently one must annotate both the interfaces and their implementing
classes, this violates Dont Repeat Yourself and introduces
possibilities for inconsistencies.

If the method in the interface / superclass is marked as @Nullable or
@NotNull, then logically the inheriting / overriding class should also
be, to implement the contract.

Is there some fundamental reason why the @NotNull and @Nullable
annotations are not annotated as @Inherited?




0

Please sign in to leave a comment.