csense-oss / idea-kotlin-checked-exceptions

A plugin that adds errors / hints / quickfix related to checked exceptions for kotlin.
MIT License
24 stars 2 forks source link

Highlight not always present #15

Closed cap-dlisbona closed 2 weeks ago

cap-dlisbona commented 1 year ago

Hello and thanks a lot for your work,

The highlight seems to work for the "throw" key word but not for the use of the function itself.

Currently using Android studio Giraffe with 2.0.0 Csense plugin and don't know what I am missing : /

Regards,

David

Tvede-dk commented 1 year ago

Hi, Im just happy to help :) So if the method "exception thrower" is not marked with throws, then the plugin assumes that it does not throw. if you added the throws it would of cause highlight it (as the plugin is trying to help you do, by highligting "throw Exception()" .

Eg. image

Then adding the throws annotation "propergates" the exception handling. think of it just like java, where the responsible code is the code "seeing" the exception that is not "expected" to throw (eg the throws annotation marks which function is expected to throw exceptions and for which ones its unexpected).

image

If you wanted the plugin to "ignore" these boundries, and show ALL potential exceptions, it would have to do both an expensive analysis of the entire codebase (which would tank performance), but also (even worse) existing code bases would very likely ALL be highlighted if there is any use of exceptions (as it would backtrack thoughout the whole code). Eg, the use of kotlins "first()" extension on lists throws (if the list is empty), and I imagine most people do not look into all the various exception the kotlin std lib actually throws (quite a lot actually, but as they documented its "exceptions", not for "regular code flow")

cap-dlisbona commented 1 year ago

Thanks a lot for these really clear explanations and precisions. The first() is indeed one of the method I would like to be taken into account. I understand the cost and the limitation but maybe a N-1 or N-2 check would be enough to cover most of the cases without being too expensive ?

Tvede-dk commented 1 year ago

My plan as of now is to hard code the kotlin std lib (for very obvious reasons the kotlin team will not annotate each method with throws which from their design stand point makes total sense) functions that does throw, and potentially add a setting to highlight them / not (since that might be against the "design" of kotlin). (it may require a tool since the kotlin std lib is quite huge... and its ever changing) :)

So if I did that rather than starting to highlight calling methods which can have an exception at say N depth, would that be sufficient? (Its a "bit" more complicated than it might seem, e.g., when the plugin see the "first" method its sometimes complied code, and thus there is no chance that it can determine that it throws, unless its annotated, which may explain why this is so cumbersome ) Or is the request "just" to highlight every function that could "have" an exception happen? :) (if so, is it just as a "can any exception happen here" type of question, because then a special inspection might make more sense that can manually be invoked via code inspections say)?

cap-dlisbona commented 1 year ago

Hello and thanks again for the detailed answer,

Not sure if I understood correctly the third and last point but for the second point I get the tricky part of an already compiled called function that throws.

I did not think of this cumbersome case. I was "naively" expecting smt that inspects the callee and search for a "throw" keyword inside it.

For the first point , indeed that looks like colossal and temporary job.

Tvede-dk commented 1 year ago

The code for just searching for a throws is "easy", its all the various details that is hard. So in your original example, you expected (as I read it) that accessing / using any function that can throw (even if not annotated) should be marked (if not handled ofc). The issue with an "n depth" is performance. However Idea does have the abillity to make inspections that are not ran on the fly, but rather through the "inspect code" menu. eg. image then I could instead add another inspection that does this "full" depth analysis and showcases all potential uncaught exceptions. image like your example? :)