WartRemoverとwartremover-contribのBuilt-inWartsを全て試す ~for better Scala code~

この記事は[ただの集団 Advent Calendar 2019](https://adventar.org/calendars/4054)の1日目の記事です。

# はじめに
Scalaは堅牢なコードを簡潔に書くことが可能だが、理解が浅かったり多人数で開発していると意図とは異なる挙動になることも少なくない。
そこで、静的解析ツールであるWartRemoverを導入することで一部の機能を抑制し、より可読性・保守性が高いコードにすることができる。
ここではWartRemoverとその追加Wartsとして提供されているwartremover-contribの全組み込みWartsを実際にコードを書いて試すことでScalaの理解を深めたい。

# [WartRemover](https://www.wartremover.org/)
- 2019/12/01時点で最新のバージョン`2.4.3`を用いる。組み込みWarts数は37個。

## ArrayEquals
- エラーとなるコード

```scala
Array(1) == Array(1)
```
`[wartremover:ArrayEquals] == is disabled, use sameElements instead`

- 修正コード

```scala
Array(1) sameElements Array(1)
```

他のcollectionsの`==`と異なりArrayの`==`は参照比較なので代わりに`sameElements`を使うべき。

## Any
- エラーとなるコード 

```scala
val any = List(1, true, "three")
```
`[wartremover:Any] Inferred type containing Any: List[Any]`

- 修正コード

```scala
val any = List(1, 2, 3)
```

Any型を使うと型安全のメリットが薄れてしまうのでAny型の使用は控えるべき。

## AnyVal
- エラーとなるコード 

```scala
val xs = List(1, true)
```
`[wartremover:AnyVal] Inferred type containing AnyVal: List[AnyVal]`

- 修正コード

```scala
val xs = List(1, 2)
```

[Any](#Any)と同じ意味合い。

## AsInstanceOf
- エラーとなるコード

```scala
x.asInstanceOf[String]
```
`[wartremover:AsInstanceOf] asInstanceOf is disabled`

修正案

- `asInstanceOf`を使わない。

型安全のために`asInstanceOf`でキャストせずに、型が静的に解決されるようにするべき。

## DefaultArguments
エラーとなるコード 

```scala
def x(y: Int = 0) = {}
```
`[wartremover:DefaultArguments] Function has default arguments`

修正コード

```scala
def x(y: Int) = {}
```

デフォルト引数を使いたくなるケースはしばしばあるが、使ってしまうとメソッドを関数として使用することが難しくなってしまう。

## EitherProjectionPartial
エラーとなるコード 

```scala
Left(1).left.get
```
`[wartremover:EitherProjectionPartial] LeftProjection#get is disabled - use LeftProjection#toOption instead`

修正コード

```scala
Left(1).left.toOption
```

`scala.util.Either.LeftProjection`と`scala.util.Either.RightProjection`には値が入ってない場合もあるので`get`ではなく`toOption`を使用するべき。

## Enumeration
- エラーとなるコード 

```scala
class x extends Enumeration {}
```
`[wartremover:Enumeration] Enumeration is disabled - use case objects instead`

- 修正案
  - Enumerationを使用せずに列挙型を実現する。

Scalaの`Enumeration`は評判が悪い（リフレクションを使用しているためパフォーマンスが悪い、実行時エラーが発生する可能性がある、値にメソッドを定義できない、`.Value`が必要でコードが不自然になる）ため代わりに`sealed abstract class`を継承した`case object`の使用が推奨されている。 

- 参考文献
  - [ScalaのEnumerationは使うな](http://xerial.org/scala-cookbook/recipes/2012/06/29/enumeration) 

## Equals
- エラーとなるコード 

```scala
5 == "5"
```
`[wartremover:Equals] == is disabled - use === or equivalent instead`

- 修正コード

```scala
5 === "5"

@SuppressWarnings(Array("org.wartremover.warts.Equals"))
implicit final class AnyOps[A](self: A) {
  def ===(other: A): Boolean = self == other
}
```

Any型は異なる型を比較できるコードをコンパイルできる`==`, `equals`, `eq`, `ne`などを提供しているが、コンパイル時に異なる型比較を禁止できるメソッドは簡単に定義できるのでそちらを使った方が良い。

## ExplicitImplicitTypes
- エラーとなるコード 

```scala
implicit val foo = 5
```
`[wartremover:ExplicitImplicitTypes] implicit definitions must have an explicit type ascription`

- 修正コード

```scala
implicit val foo: Int = 5
```

`implicit`を使う場合に明示的に型が指定されていないと正しく解決できないことがあるので、全ての`implicit`オブジェクトには明示的な型が必要。

## FinalCaseClass
- エラーとなるコード 

```scala
case class Foo()
```
`[wartremover:FinalCaseClass] case classes must be final`

- 修正コード

```scala
final case class Foo()
```

Caseクラスは便利なメソッドを提供するが、継承すると壊れてしまうので`final`にするべき。

## FinalVal
- エラーとなるコード 

```scala
final val v = 1
```
` [wartremover:FinalVal] final val is disabled - use non-final val or final def or add type ascription`

- 修正コード

```scala
val v = 1
```

`final val`はインライン化されてしまうので、sbtのインクリメンタルコンパイルで矛盾が起きてしまう。

- 参考文献
  - [Scalaにおける細かい最適化のプラクティス](https://xuwei-k.hatenablog.com/entry/20130709/1373330529)
  - [Name hashing: Members with constant types are not tracked](https://github.com/sbt/sbt/issues/1543)

## ImplicitConversion
- エラーとなるコード 

```scala
implicit def int2Array(i: Int): Array[String] = Array.fill(i)("?")
```
`[wartremover:ImplicitConversion] Implicit conversion is disabled`

- 改善案
  - 暗黙的な型変換を行わない

暗黙的な型変換は安全性を弱め、かつ常に明示的な変換に置き換えられるので行わないほうが良い。

## ImplicitParameter
- エラーとなるコード 

```scala
def f()(implicit s: String) = ()
```
`[wartremover:ImplicitParameter] Implicit parameters are disabled`

- 修正コード

```scala
def f()(s: String) = ()
```

暗黙的なパラメータはインタフェースを混乱させ驚くべき不整合をもたらす可能性があるため使用しない方が良い。

## IsInstanceOf
- エラーとなるコード 

```scala
x.isInstanceOf[String]
```
`[wartremover:IsInstanceOf] isInstanceOf is disabled`

- 修正案
  - `isInstanceOf`を使わない。

型安全を保つために型が静的に確立されるようにすべき。

## JavaConversions
- エラーとなるコード 

```scala
import scala.collection.JavaConversions._
val scalaMap: Map[String, String] = Map()
val javaMap: java.util.Map[String, String] = scalaMap
```
`[wartremover:JavaConversions] scala.collection.JavaConversions is disabled - use scala.collection.JavaConverters instead`

- 修正コード

```scala
import scala.collection.JavaConverters._
val scalaMap: Map[String, String] = Map()
val javaMap: java.util.Map[String, String] = scalaMap.asJava
```

`JavaConversions`でJavaの型と暗黙的に変換してしまうとコードが読みづらくなってしまうため、`JavaConverters`を使用して明示的に変換するべき。

## JavaSerializable
- エラーとなるコード 

```scala
object O extends Serializable
val v = List("", O)
```
`[wartremover:JavaSerializable] Inferred type containing Serializable: java.io.Serializable`

- 修正コード

```scala
object O extends Serializable
val v = List(O)
```

Scalaコンパイラは`java.io.Serializable`を共通サブタイプとして推測することが多いが、それらは多くの場合誤りなので明示的に型を指定すべき。
例えば`String`は`scala.Serializable`ではなく`java.io.Serializable`のサブタイプである。

## LeakingSealed
- エラーとなるコード 

```scala
sealed trait t
class c extends t
```
`[wartremover:LeakingSealed] Descendants of a sealed type must be final or sealed`

- 修正コード

```scala
sealed trait t
sealed class c extends t
```

`sealed`タイプの子クラスが`final`か`sealed`になっていないと子クラスを経由して別ファイルで継承できでしまう。

## MutableDataStructures
- エラーとなるコード 

```scala
import scala.collection.mutable.ListBuffer
val list = ListBuffer()
```
`[wartremover:MutableDataStructures] scala.collection.mutable package is disabled`

- 修正コード

```scala
import scala.collection.immutable.List
val list = List()
```

可変コレクションを用いるとコードが複雑になってしまうため避けるべき。

## NonUnitStatements
- エラーとなるコード 

```scala
10
```
`[wartremover:PublicInference] Public member must have an explicit type ascription`

- 修正コード

```scala
()
```

Scalaでは文が任意の型を返せるが意図した文にするには常にUnitのみを返すべき。

## Nothing
- エラーとなるコード 

```scala
val x = ???
```
`[wartremover:Nothing] Inferred type containing Nothing: Nothing`

- 修正コード

```scala
val x = 10
```

Scalaコンパイラは`Nothing`をジェネリック型として推論することが多いがそれはほとんどの場合間違っているので、代わりに明示的に型を指定する必要がある。

## Null
- エラーとなるコード 

```scala
val s: String = null
```
`[wartremover:Null] null is disabled`

- 修正コード

```scala
val s: String = "n"
```

`null`は型の安全性を壊すので使用してはいけない。

## Option2Iterable
- エラーとなるコード 

```scala
Iterable(1, 2, 3).flatMap(Some(_))
```
`[wartremover:Option2Iterable] Implicit conversion from Option to Iterable is disabled - use Option#toList instead`

- 修正コード

```scala
Iterable(1, 2, 3).flatMap(Some(_).toList)
```

`Option`から`Iterable`への暗黙的な変換は思わぬバグを生む可能性があるので使用するべきではない。

## OptionPartial
- エラーとなるコード 

```scala
Some(1).get
```
`[wartremover:OptionPartial] Option#get is disabled - use Option#fold instead`

- 修正コード

```scala
Some(1).fold(0)(i => i)
```

`scala.Option#get`は値が`None`の場合に例外を投げてしまうので、`Some`と`None`の両方を明示的に処理するようにしなければならない。

## Overloading
- エラーとなるコード 

```scala
class c {
  def equals(x: Int) = {}
}
```
`[wartremover:Overloading] Overloading is disabled`

- 改善案
  - オーバーロードを使用しない

メソッドのオーバーロードは混乱を招く可能性があり、通常は回避できるため使用しない方が良い。

## Product
- エラーとなるコード 

```scala
val any = List((1, 2, 3), (1, 2))
```
`[wartremover:Product] Inferred type containing Product: Product with Serializable`

- 修正コード

```scala
val any: List[(Int, Int)] = List((2, 3), (1, 2))
```

Scalaコンパイラはジェネリックス型として`scala.Product`を推測することが多いがそれはほとんどの場合誤りなので、代わりに明示的に型を指定するべき。

## PublicInference
- エラーとなるコード 

```scala
def f() = {}
```
`[wartremover:PublicInference] Public member must have an explicit type ascription`

- 修正コード

```scala
def f(): Unit = {}
```

publicメンバーの戻り値の型推論はカプセル化を壊す可能性があるので明示的に型を指定するべき。

## Recursion
- エラーとなるコード 

```scala
def fact(n: Int): BigInt = {
  def innerFact(n: Int, f: BigInt): BigInt =
    n match {
      case 0 => f
      case _ => innerFact(n - 1, n * f)
    }
  innerFact(n, 1)
}
```
`[wartremover:Recursion] Unmarked recursion`

- 修正コード

```scala
def fact(n: Int): BigInt = {
  @scala.annotation.tailrec
  def innerFact(n: Int, f: BigInt): BigInt =
    n match {
      case 0 => f
      case _ => innerFact(n - 1, n * f)
    }
  innerFact(n, 1)
}
```

一般的な再帰はスタックの問題を引き起こすことがあるため、末尾再帰を使って、かつコンパイル時に末尾再帰になっているか検知できるように`@scala.annotation.tailrec`を付けるべき。

- 参考文献
  - [Scala における末尾再帰](https://bussorenre.hatenablog.jp/entry/2018/10/15/134615)
  - [Scalaではじめる末尾再帰](https://qiita.com/dashiishida/items/c20d6e08a260e3ce0f64)
  - [Scala tailrecアノテーション](http://www.ne.jp/asahi/hishidama/home/tech/scala/annotation/tailrec.html)

## Return
- エラーとなるコード 

```scala
def foo(n:Int): Int = return n + 1
```
`[wartremover:Return] return is disabled`

- 修正コード

```scala
def foo(n:Int): Int = n + 1
```

`return`で値を返してしまうと戻り値の型推論が難しくなってしまう。

## Serializable
[Product](#Product)とほぼ同じ意味合い、Scalaのバージョンによりどちらが適用されるか異なる。

## StringPlusAny
- エラーとなるコード 

```scala
println("foo" + 1)
```
`[wartremover:StringPlusAny] Implicit conversion to string is disabled`

- 修正コード

```scala
println("foo" + 1.toString)
```

`+`は意図しない挙動になることが多いため暗黙的に`toString`が呼ばれることに期待せず、文字列連結を行いたい場合はString型に変換してから連結する。

- 参考文献
  - [Do not concatenate Strings with +](https://nrinaudo.github.io/scala-best-practices/tricky_behaviours/string_concatenation.html)

## Throw
- エラーとなるコード 

```scala
def foo(n: Int): Int = {
  n match {
    case 1 => 1
    case _ => throw new IllegalArgumentException("bar")
  }
}
```
`[wartremover:Throw] throw is disabled`

- 修正コード

```scala
def foo(n: Int):Either[String, Int] = {
  n match {
    case 1 => Right(1)
    case _ => Left("bar")
  }
}
```

例外は副作用なので関数型プログラミングでは好まれない、代わりにEitherを使うべき。

- 参考文献
  - [Scalaでの例外処理 - Either,Option,util.control.Exception](https://yuroyoro.hatenablog.com/entry/20100719/1279519961)
  - [頑張りすぎないScala](https://www.slideshare.net/takezoe/scala-84830510)

## ToString
- エラーとなるコード 

```scala
class Foo(i: Int)
val foo: Foo = new Foo(5)
println(foo.toString)
```
`[wartremover:ToString] class Foo does not override toString and automatic toString is disabled`

- 修正コード

```scala
class Foo(i: Int) {
    override val toString: String = s"Foo($i)"
}
val foo: Foo = new Foo(5)
println(foo.toString)
```

`toString`は全てのクラスに存在するメソッドだが、クラス名に基づいているため名前を変更するとバグが生まれてしまう。
使用する場合は明示的にオーバーライドして使用するべき。

## TraversableOps
- エラーとなるコード 

```scala
val foo = List(1, 2, 3)
println(foo.head)
```
`[wartremover:TraversableOps] head is disabled - use headOption instead`

- 修正コード

```scala
val foo = List(1, 2, 3)
println(foo.headOption)
```

`scala.collection.Traversable`は`head`, `tail`, `init`, `last`, `reduce`, `reduceLeft`, `reduceRight`, `max`, `maxBy`, `min and minBy methods`を持っているがコレクションが空の場合に例外を吐いてしまう。
代わりに`headOption`, `drop(1)`, `dropRight(1)`, `lastOption`, `reduceOption or fold`, `reduceLeftOption or foldLeft and`, `reduceRightOption or foldRight respectively`を使って空コレクションの場合に明示的に対応するべき。

## TryPartial
- エラーとなるコード 

```scala
println(Success(1).get)
```
`[wartremover:TryPartial] Try#get is disabled`

- 修正コード

```scala
println(Success(1).map(x => x))
```

`scala.util.Try#get`は値が`Failure`の場合に例外を投げてしまう、代わりに`map`や`getOrElse`を利用して`Success`と`Failure`の両方に対応できるようにするべき。

## Unsafe
- 下記Warts一式をまとめたWartで、型安全のチェックにのみ使いたい場合に使用する。
  - `Any`, `AsInstanceOf`, `EitherProjectionPartial`, `IsInstanceOf`, `NonUnitStatements`, `Null`, `OptionPartial`, `Product`, `Return`, `Serializable`, `StringPlusAny`, `Throw`, `TraversableOps`, `TryPartial`, `Var`

## Var
- エラーとなるコード 

```scala
var x = 100
```
`[wartremover:Var] var is disabled`

- 修正コード

```scala
val x = 100
```

変数を可変にするとコードが複雑になってしまうので`var`の代わりに`val`を使用するべき。

## While
- エラーとなるコード 

```scala
var i = 0
while (i < 10) {
    println(i.toString)
    i += 1
}
```
`[wartremover:While] while is disabled`

- 修正コード

```scala
(0 until 10).foreach(println(_))
```

`while`は通常は低レベルのコードで使用され、パフォーマンスに問題がないのであれば使用すべきでない。

# [wartremover-contrib](https://github.com/wartremover/wartremover-contrib)
- WartRemover公式コミュニティが提供している追加Warts集でより実践的なものが多い。
- 2019/12/01時点で最新のバージョン`1.3.1`を用いる。組み込みWarts数は11個。

## Apply
- エラーとなるコード 

```scala
class C {
    def apply() = 1
}
```
`[wartremover:Apply] apply is disabled`

- 修正コード

```scala
object C {
    def apply() = 1
}
```

classの`apply`はコードの量を僅かに減らすが可読性を低下させバグを引き起こすため使わない方が良い（objectの場合は問題ない）

## ExposedTuples
- エラーとなるコード 

```scala
def badFoo(customerTotal: (String, Long)) = {
    // Code
}
```
`[wartremover:ExposedTuples] Avoid using tuples in public interfaces, as they only supply type information. Consider using a custom case class to add semantic meaning.`

- 修正コード

```scala
final case class CustomerAccount(customerId: String, accountTotal: Long)
    
def goodFoo(customerTotal: CustomerAccount) = {
    // Code
}
```

Tupleはセマンティックな意味ではなく型のみで記述されるため公開APIで使用するべきではなく、代わりにケースクラスなどでセマンティックな意味を追加するべき。

## MissingOverride
- エラーとなるコード 

```scala
trait T {
    def f(): Unit
}
class C extends T {
    final def f() = {}
}
```
` [wartremover:MissingOverride] Method must have override modifier`

- 修正コード

```scala
trait T {
    def f(): Unit
}
class C extends T {
    override final def f() = {}
}
```

`override`はオプションだが、メソッド名の変更などで予期せぬバグが発生する可能性があるので必ず付ける方が安全。

## NoNeedForMonad
- エラーとなるコード 

```scala
Option(1).flatMap(i => Option(2).map(j => i + j))
```
`[wartremover:NoNeedForMonad] No need for Monad here (Applicative should suffice).`

- 修正コード

```scala
Option(1).flatMap(i => Option(i + 1).map(j => i + j))
```

`Monad`は強力すぎて複雑なことができてしまい可読性も落ちるので`Applicative`で十分な時は使用するべきでない。

- 参考文献
  - [箱で考えるFunctor、ApplicativeそしてMonad](https://qiita.com/suin/items/0255f0637921dcdfe83b)
  - [Applicativeのススメ](https://kazu-yamamoto.hatenablog.jp/entry/20101211/1292021817)

## OldTime
- エラーとなるコード 

```scala
import java.util.Date
val x = new Date()
```
`[wartremover:OldTime] The old Java time API is disabled. Use Java 8 java.time._ API instead.`

- 修正コード

```scala
import java.time.LocalDateTime
val x = LocalDateTime.now()
```

日時を扱う際は`java.util`パッケージのAPIではなく新しくて扱いやすい`java.time`パッケージのAPIを使うべき。

## RefinedClasstag
- エラーとなるコード 

```scala
import scala.reflect.ClassTag
def methodWithClassTag[T]()(implicit ct: ClassTag[T]): Unit = {}
object RefinedClassTag {
    trait A
    trait B
}

import RefinedClassTag._
methodWithClassTag[A with B]()
```
`[wartremover:RefinedClasstag] Refined types should not be used in Classtags since only the first type will be checked at runtime. Type found: Main.RefinedClassTag.A with Main.RefinedClassTag.B`

- 修正コード

```scala
import scala.reflect.ClassTag
def methodWithClassTag[T]()(implicit ct: ClassTag[T]): Unit = {}
object RefinedClassTag {
    trait A
    trait B
    type Ab = A with B
}

import RefinedClassTag._
methodWithClassTag[Ab]()
```

ClasstagsにRefined typesを使用すると最初の型しかチェックされず意図しない挙動になるため避けるべき。

- 参考文献
  - [Add RefinedClasstag wart](https://github.com/wartremover/wartremover-contrib/pull/25)

## SealedCaseClass
- エラーとなるコード 

```scala
sealed case class Foo(i: Int)
```
`[wartremover:SealedCaseClass] case classes must not be sealed`

- 修正コード

```scala
final case class Foo(i: Int)
```

[FinalCaseClass](#FinalCaseClass)によるとCaseClassは`final`でなければならないが、Scalaでは`final`と`sealed`を同時に使うことはできない。

## SomeApply
- エラーとなるコード 

```scala
Some(10)
```
`[wartremover:SomeApply] Some.apply is disabled - use Option.apply instead`

- 修正コード

```scala
Option(10)
```

- `Some.apply`ではなく`Option.apply`を使うべき。
  - `null`の時に`Some.apply`だと`Some(null)`になってしまうが、`Option.apply`なら`None`となる。
  - `Some.apply`だと`Some[T]`と型推論されてしまうことがあるが、実際にはほとんどの場合で`Option[T]`を期待している。

## SymbolicName
- エラーとなるコード 

```scala
def :+:(): Unit = {}
```
`[wartremover:SymbolicName] Symbolic name is disabled`

- 修正コード

```scala
def join(): Unit = {}
```

記号のメソッド名は推論を難しくしてバグを混入しやすくなるので避けるべき。

## UnintendedLaziness
- エラーとなるコード 

```scala
val map: Map[Int, Int] = Map(1 -> 10)
map.mapValues(_ + 1)
```
`[wartremover:UnintendedLaziness] GenMapLike#mapValues is disabled because it implicitly creates lazily evaluated collections.`

- 修正コード

```scala
val map: Map[Int, Int] = Map(1 -> 10)
map.toStream.map(_._2 + 1)
```

`mapValues`と`filterKeys`は意図せず暗黙的に遅延評価が行われしまうので、意図してやる場合には明示的に`view`や`toStream`を使用するべき。
ただし、Scala2.13でこれは修正されているため、それ以上のバージョンであれば問題ない。

## UnsafeInheritance
- エラーとなるコード 

```scala
trait T {
    def positive = 1
}
class C extends T {
    override def positive = -1
}
```
`[wartremover:UnsafeInheritance] Method must be final or abstract`

- 修正コード

```scala
trait T {
    final def positive = 1
}
```

メソッドの実装をオーバーライドすると親の制約が破られる可能性があるので避けるべき。

# まとめ
有識者達のノウハウが凝縮されたツールなのでできるだけ全Wartsに従うべきだと思うが、使用するライブラリとの兼ね合い等で特定のWartを除外したい時もある。
全体で除外したい場合は`build.sbt`で除外設定することで可能で、一部で除外したい場合は`@SuppressWarnings`にWart名を指定することで可能だが、それらの場合は除外理由をコメント等で残しておくとスムーズな多人数開発ができると思う。