In general Option.get is a bad practice:
https://alvinalexander.com/scala/best-practice-option-some-none-pattern-scala-idioms#don-t-use-the-get-method-with-option
It's basically the same as a normal value that just throws a null pointer exception.
Practically, using Option.get causes the auto generated code to fail linters like WartRemover
It would be nice to change the code generation to, as a first step, simply call Option.getOrElse(throw new SensibleException()), or ideally use the pattern
Option match {
case Some(x) => x;
case None => handleFailureCase()
}
Doing the first simple case at least solves the problem with linting
In general, auto-generated code doesn't have to completely satisfy all style guides or linters recommendations that apply to normal user code. For example, we're not trying to target any specific line width but we're trying to make the generator keep it reasonable.
For this specific case we have something like this: if (opt.isDefined) { __size ++ = something(opt.get) }. The most idiomatic thing to use would be to use foreach, but that would mean an extra function call and probably and a function object for each optional field. Pattern matching comes second, but I find it actually less desirable than the current implementation for the case when there's just one case. We'll have to add case None => // for avoiding a warning on the match not being exhaustive.
My preference here would be to check if tools such as wart remover can skip linting the generated code. Happy to add any comment to the top of the generated code so they won't try to lint those files.
You can use wartremoverExcluded: TaskKey[Seq[File]]
I'll look in to options for my nix/pants environment at the wartremover layer
Most helpful comment
In general, auto-generated code doesn't have to completely satisfy all style guides or linters recommendations that apply to normal user code. For example, we're not trying to target any specific line width but we're trying to make the generator keep it reasonable.
For this specific case we have something like this:
if (opt.isDefined) { __size ++ = something(opt.get) }. The most idiomatic thing to use would be to useforeach, but that would mean an extra function call and probably and a function object for each optional field. Pattern matching comes second, but I find it actually less desirable than the current implementation for the case when there's just one case. We'll have to addcase None => //for avoiding a warning on the match not being exhaustive.My preference here would be to check if tools such as wart remover can skip linting the generated code. Happy to add any comment to the top of the generated code so they won't try to lint those files.