lishunli / projectlombok

Automatically exported from code.google.com/p/projectlombok
0 stars 0 forks source link

@NonNull at methods. #526

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Hi.

Currenly u implemented #514. It for parameters. I think it need at to methods 
to. Like it:

@NonNull
public String test() 
{
  return null;
}

ill generate code

public String test()
{
 String a = null;
 if(a == null) 
 {
   throws new NullPointerException("test cant return null");
 }
 return a;
}

I need this future - to remove Jetbrains NotNullInsrumental 

Original issue reported on code.google.com by vistall.valeriy on 3 Jun 2013 at 6:52

GoogleCodeExporter commented 9 years ago
Or make - method

public static <T> throwIfNull(T value)
{  
  if(value == null) 
  {
    throws new NullPointerException()
  }
  return value;
}

and replace tree by 

@NonNull
public String test() 
{
  return throwIfNull(null);
}

Original comment by vistall.valeriy on 4 Jun 2013 at 4:39

GoogleCodeExporter commented 9 years ago
I don't understand entirely what you mean, so I'm going to make a guess here. 
Do you mean, that you want to wrap the return statement of a method, in order 
to check that it will not return null?

If so, the next thing I don't understand is why. You will always put this 
annotation on a method in your own code base, so if you decide that it may not 
return null, "just don't do it"? And if, in your method, you depend on the 
return value of another method which you cannot influence, assert on its return 
value and throw NPE if applicable?

Original comment by askon...@gmail.com on 13 Jun 2013 at 7:34

GoogleCodeExporter commented 9 years ago
Yeah, if it our code - it can be fixed. But)

if we have

public A getSomeA() {
  return null;
}

@NonNull
public A getA() {
  return getSomeA();
}

getSomeA() is not annotated - and it can return null. 

U can say - that fix it, but sub method can be in Library/JDK, that u cant 
control. And with large codebase - controlling is hard(if u not single 
commiter).

next code like

void test()
{ 
  val a = getA();

  println(a.toString));

  call(a); 
}

void call(@NonNull A test) {}

U can see - that it ill throw NPE when entered line:

println(a.toString));

but if, getAill wrapped return statement - it ill throw that return value is 
null.

For me - i make fork(https://github.com/consulo/lombok/commits/master), i 
implemented it.

It wrapped return statement to:

@NonNull
public A getA() {
  return NonNullException.throwIfNull(getSomeA());
}

Regards

Original comment by vistall.valeriy on 13 Jun 2013 at 8:07

GoogleCodeExporter commented 9 years ago
This is kinda hard to do; we'd have to insert a null-check just 'above' every 
return statement, but that is not possible:

* We cannot duplicate the expression but we'd have to do that to inject this 
null check straight into your method. We can _probably_ create a local variable 
with the same type as your return type, but we'd have to do some research to 
make sure that's _ALWAYS_ legal, and it may definitely cause problems due to 
'style' issues (where users have turned on certain warnings/errors on certain 
usages, such as implicit autoboxing. Normally we suppress these, but we can't 
do that for code injected into a method).

* We'd have to block up return statements that aren't in blocks. We can't do 
anything to: "if (x) return 5;", we'd have to first turn that into "if (x) { 
return 5; }". If we change too much in your own code, eclipse starts losing 
track of it and error underlines and the like will start ending up in the wrong 
places.

* We need to jump through some hoops for autoboxing; it makes sense to add 
@NonNull to a method that returns Integer, but, you can also write 'return 5;' 
in one of those, and null-checking '5' is a compile time error. We can't detect 
this, as you can also write 'return foo.bar.baz.x', where that is a field ref 
to an int, but we don't have resolution to figure that out.

* Eclipse's null analysis is too smart: If the thing we are null-checking 
couldn't possibly be null where we check it, eclipse will emit a warning saying 
so. We can't suppress this warning.

All in all what we really need here is a null-checking generified helper method:

public <T> T nullCheck(T arg, String methodName) {
    if (arg == null) throw new NullPointerException(methodName " cannot return null.");
}

but where would we place this helper method? So far lombok isn't a runtime 
dependency, and this feature is not important enough to try and change that.

The obvious answer is to use java7's Objects.requireNonNull, but then lombok 
would require java7, and we're not quite ready to demand this yet. Especially 
for a feature that is easily 'accidentally' triggered. Also, at some point 
eclipse may wise up and take calls to Objects.rNN into account for its 
unnecessary null-check warnings.

So, decent enough idea but too many issues associated with it for not enough 
gain to spend time on this right now. Unfortunate.

Original comment by reini...@gmail.com on 8 Jul 2013 at 8:30