scapegoat
scapegoat copied to clipboard
"Error" v.s. "Warning": Traversable.head
What is the intended meaning of "Error" level and "Warning" level rules?
I would have expected that an "Error" rule was one which meant that the code was very likely to be bugged, for example ComparingUnrelatedTypes
I would have expected that a "Warning" level rule was one where the code was likely to be inefficient or dangerous, but could be correct and could be justified.
Is that roughly what you mean by these terms? Can I add these definitions to the README?
I have some specific questions about the categorisations:
-
TraversableHead
is an "Error" level warning, but it feels like a "Warn" to me
I agree that this can often mask bugs, but it can be justified. For example, is this:
def doSomething(nonEmptyList: List[T]) = {
val first = nonEmptyList match {
case x :: _ => x
case _ => throw new ArgumentException("`nonEmptyList` may not be empty")
}
...
}
really better than this:
def doSomething(nonEmptyList: List[T]) = {
val first = nonEmptyList.head
...
}
The first seems rather boilerplate-heavy in internal code. It seems a bit harsh to declare the second an "error".
I'm looking at some of our code which uses Traversible.head, and I'm struggling to find refactorings that aren't just more verbose error messages like the above example.
-
CatchNpe
is an "Error" rather than "Warn"- Does this always indicate a bug in the inspected code? Why?
etc.
Maybe the ability to configure the level of a rule would side-step this problem.
I think you're right - we need to clarify what error vs warn really means. I guess in my head I was thinking:
Error - should be almost always avoided, can very likely lead to bugs Warn - you shouldn't be doing this, it's either not idomatic scala or can possibly lead to bugs Info - there's a better way but its really up to you.
On 24 May 2015 at 13:06, Richard Bradley [email protected] wrote:
Maybe the ability to configure the level of a rule would side-step this problem.
— Reply to this email directly or view it on GitHub https://github.com/sksamuel/scalac-scapegoat-plugin/issues/110#issuecomment-105007346 .