-
Notifications
You must be signed in to change notification settings - Fork 23
add ThrownExceptionNotInFunction rule to the Analyzer Plugin #585
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
add ThrownExceptionNotInFunction rule to the Analyzer Plugin #585
Conversation
Closes #500 |
It's a draft. We can consider wider solution to warn when |
analyzer/src/main/scala/com/avsystem/commons/analyzer/ThrownExceptionNotInMonixScope.scala
Outdated
Show resolved
Hide resolved
analyzer/src/main/scala/com/avsystem/commons/analyzer/ThrownExceptionNotInMonixScope.scala
Outdated
Show resolved
Hide resolved
…. The Scala compiler loves to infer Nothing as a generic type but that is almost always incorrect. Explicit type arguments should be used instead.
…her type. The Scala compiler loves to infer Nothing as a generic type but that is almost always incorrect. Explicit type arguments should be used instead." This reverts commit 9622a3f.
fffed2f
to
ae2b07a
Compare
32644de
to
0c3b54c
Compare
…side-the-monadic-context-of-task
…nadic-context-of-task
@ddworak Can we either proceed this MR or close it? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a new analyzer rule, ThrownExceptionNotInFunction, to detect cases where an exception is thrown directly as a function argument instead of wrapping it in a function literal. It includes:
- Implementation of the new analyzer rule scanning for thrown exceptions used in place of functions.
- Registration of the rule in the
AnalyzerPlugin
. - Comprehensive tests covering method calls, constructors, multiple argument lists, and indirect exception-throwing scenarios.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
analyzer/src/main/scala/com/avsystem/commons/analyzer/ThrownExceptionNotInFunction.scala | Added the new rule class that reports thrown exceptions as function arguments. |
analyzer/src/main/scala/com/avsystem/commons/analyzer/AnalyzerPlugin.scala | Registered the new ThrownExceptionNotInFunction rule in the plugin. |
analyzer/src/test/scala/com/avsystem/commons/analyzer/ThrownExceptionNotInFunctionTest.scala | Added unit tests for various call patterns to verify the rule’s behavior. |
Comments suppressed due to low confidence (1)
analyzer/src/main/scala/com/avsystem/commons/analyzer/ThrownExceptionNotInFunction.scala:6
- [nitpick] Consider adding a Scaladoc comment above this analyzer rule class to summarize its purpose and usage for future maintainers.
final class ThrownExceptionNotInFunction(g: Global) extends AnalyzerRule(g, "thrownExceptionNotInFunction") {
analyzer/src/main/scala/com/avsystem/commons/analyzer/AnalyzerPlugin.scala
Show resolved
Hide resolved
analyzer/src/main/scala/com/avsystem/commons/analyzer/ThrownExceptionNotInFunction.scala
Show resolved
Hide resolved
} | ||
|
||
test("Testing multiple arguments") { | ||
assertErrors(26, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debugging why an assertErrors(26, ...
failed is impossible without editing the tests. Let's work on the developer experience in this test by using sth like org.scalatest.Inspectors
in conjunction with assertErrors(1
and assertNoErrors
(example, I'm sure you can suggest the best solution available).
No description provided.