csense-oss / idea-kotlin-checked-exceptions

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

Support declaring a function that can accept a throwing lambda #33

Open daniel-jasinski opened 6 months ago

daniel-jasinski commented 6 months ago

It would be great if we could declare functions like callSomething as safe for throwing lambdas. Then we could make the Throws declaration on the caller to also apply to the lambda body.

image

Tvede-dk commented 2 months ago

Hi, so this was actually implemented as a POC, based on this small annotations lib that I have as well https://github.com/csense-oss/csense-kotlin-annotations In there there is an annotation named "RethrowsExceptions" and "CatchesExceptions". As the name implies if these are put on a lambda, then that would either be the same as "callThoughts" (rethrows exceptions) or whenever it catches the exception. Eg image So far I would recommend to create the annotation in your own project, as I'm working on getting the various libraries into Maven central, which means the package names will have to change ... :( The package name(s) that the plugin currently recognizes are

//for rethrows / callthough
csense.kotlin.annotations.exceptions.RethrowsExceptions
org.csenseoss.kotlin.annotations.exceptions.RethrowsExceptions
//for catching
csense.kotlin.annotations.exceptions.CatchesExceptions
org.csenseoss.kotlin.annotations.exceptions.CatchesExceptions

I'm at this point not aware of any regular annotation libraries that have these types of annotations. (and or any that is kotlin MPP as well).

There might also be some cases that I have not yet covered in the plugin, but it should work just fine (just as fine as the custom files).

GeorgeVoloshin commented 2 months ago

Hello,

Sorry, but is there any way to do the same thing with library functions, such as async or launch? Or is the only way to write my own wrapper with the @RethrowsExceptions or @CatchesExceptions annotation?

daniel-jasinski commented 2 months ago

Thanks @Tvede-dk !

These annotations are exactly what I was looking for! Would these annotations also work with SOURCE retention instead of BINARY?

Tvede-dk commented 2 months ago

Hi @GeorgeVoloshin The simple answer is that any function with either annotations or contracts (calls in place) should be parsed. Any other type of function is either hardcoded in the plugin (so I try to hardcode most of the standard libraries), or it comes from the files. I could also add the possibility of doing a comment on the function, like a "middle" ground between annotations and the other types of declaring this. Do you have an example of what you want?

Tvede-dk commented 2 months ago

@daniel-jasinski hehe, that depends. If you have directly access to the code via say a submodule it would work with source retention. BUT as soon as its turned into any binary files (jar, war, apk .... etc), then binary retention is needed. Thus if it is a published package at a maven repo, it would need to be binary. (otherwise the plugin usually have no way of getting the annotation(s)). I have seen examples where the plugin gets the source code instead of binary code, however, that is super flaky to rely on. (I have no idea how IDEA / IntelliJ decides what "code" to present to a plugin, its a hit and miss thing). To be on the safe side I would do binary retention.

GeorgeVoloshin commented 2 months ago

@Tvede-dk Hmm... Maybe I'm doing something wrong because your example (https://github.com/csense-oss/idea-kotlin-checked-exceptions/issues/33#issuecomment-2316308337) isn't working for me either. image

I followed everything mentioned here (https://github.com/csense-oss/csense-kotlin-annotations?tab=readme-ov-file#installation) (just changed the version to 0.0.63), but it still doesn't work for me.

Tvede-dk commented 2 months ago

@GeorgeVoloshin Ah, so that is because I have not suppoted type annotations in the plugin. the "working" example is where the annotation is on the parameter itself. So if you move the annotation it should work :)

image

GeorgeVoloshin commented 2 months ago

Oh, sorry, my mistake. I apologize for my carelessness. If I move the annotation, everything starts working in this example. Thank you very much.

But I would like to return to my original question. Could you please tell me if there's a way to apply an annotation so that the exceptions thrown in launch, async, and coroutineScope blocks can also be specified in @Throws, and they stop being highlighted as warnings? Because I use launch, async, and coroutineScope a lot in my code to run asynchronous code, and it's inconvenient when a third of the project is highlighted in yellow because of this. For example: image

I specified @Throws(TestException::class), but the error in launch is still highlighted as warning.

Tvede-dk commented 1 month ago

@GeorgeVoloshin No need to apologize. Not only was I expecting your example to work, but its also not easy to understand the difference (and the real challenge is to update the plugin, which is on me :) )

Now with regards to launch / async ect. The real problem is the flow of exception is NOT linear. that means, for that to work, the plugin would have to track all scopes, effectively doing a very deep analysis of coroutine scopes. which not to mention, would still require special care for say SupervisorJob (which consumes uncaught exceptions and does not terminate the parent job ...) This means, that its very hard / unlikely to just be supported. In your example, its only because of the function "coroutineScope" that it "appears linear". Take this expanded example image

it prints image (since we do not wait on the launch etc, thus the exception is not throw outright, if there is a "z.join"; then it does throw...) which is due to the "way" jobs / coroutines handles exceptions.

Now this does highlight a potential consideration: when dealing with coroutines there might be a few constructs that would be easy to recognize (as seen from this talk from kotlin conf https://www.youtube.com/watch?v=a3agLJQ6vt8) the end result is very often (also in my experience) a launch / async on a parent scope, usually on a SupervisorJob. That however does not stop the plugin from potentially highlighting "join / await" locations. (although, you cannot annotate a job / Deferred with throws ... ) thus hypothetically this would be possible to do something like this image

and yes, coroutineScope would then also need to be handled specially. And it might have to be a bit "eager", and not verify specifically that you are calling say "launch" on the right scope, as that again would be a tracking nightmare for the plugin. Thus detecting that Globalscope (or any other scope) is not in the "containing" scope is very likely to much for now, eg: image

It will also be quite difficult dealing with custom functions etc, since await & join would be special "functions" that the plugin will look for. although, with more annotations, it would be a simple "tracking" like all the other ones...

I will try and come up with a rough sketch / POC of this, as it is a good idea, and definitely a thing that the plugin should know about :)