closure-compiler icon indicating copy to clipboard operation
closure-compiler copied to clipboard

fix potential null pointers returned by function toMaybeFunctionType()

Open TJJack opened this issue 3 years ago • 3 comments

TJJack avatar Mar 25 '21 13:03 TJJack

Looks like the this does not build and there do not appear to be any associated issues or unit tests. Were you hoping to contribute this or was this just a proof of concept?

concavelenz avatar Mar 25 '21 17:03 concavelenz

@concavelenz Thanks for you reply.

In fact, I noticed these potential bugs according to an existing fix, which denotes the return value of method toMaybeFunctionType() is nullable. I have further checked the implementation of this method, and the same result was reached.

BTW, I have fixed the compilation error caused by an incompatible return type. Please kindly review it.

TJJack avatar Mar 26 '21 01:03 TJJack

While toMaybeFunctionType() can return null, the call sites in this change are places where we reasonably believe it should always return a non-null FunctionType.

I don't think it makes sense to silently return when this invariant is violated, since that might be hiding another error.

lauraharker avatar Apr 29 '22 17:04 lauraharker

感谢您的来信,邮件已收到!我会尽快查看!                                                       ----姜佳君

TJJack avatar Oct 11 '22 06:10 TJJack

English translation of @TJJack comment above according to translate.google.com:

Thank you for your letter, the mail has been received! I will check it out soon! ---- Jiang Jiajun

brad4d avatar Oct 11 '22 18:10 brad4d

I don't think we want to take this pull request.

  1. There are no test cases to show real instances where it fixes something.
  2. Silently returning null in all of these locations is likely not the best solution. If anything, we should likely be adding checkNotNull(...), but even that seems unnecessary unless we are fixing an known failure.

brad4d avatar Oct 11 '22 18:10 brad4d