LoginSignup
14
1

More than 3 years have passed since last update.

@mado-m(naoto higuchi)

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

Last updated at Posted at 2019-12-01

この記事はただの集団 Advent Calendar 2019の1日目の記事です。

はじめに

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

WartRemover

  • 2019/12/01時点で最新のバージョン2.4.3を用いる。組み込みWarts数は37個。

ArrayEquals

  • エラーとなるコード
Array(1) == Array(1)

[wartremover:ArrayEquals] == is disabled, use sameElements instead

  • 修正コード
Array(1) sameElements Array(1)

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

Any

  • エラーとなるコード 
val any = List(1, true, "three")

[wartremover:Any] Inferred type containing Any: List[Any]

  • 修正コード
val any = List(1, 2, 3)

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

AnyVal

  • エラーとなるコード 
val xs = List(1, true)

[wartremover:AnyVal] Inferred type containing AnyVal: List[AnyVal]

  • 修正コード
val xs = List(1, 2)

Anyと同じ意味合い。

AsInstanceOf

  • エラーとなるコード
x.asInstanceOf[String]

[wartremover:AsInstanceOf] asInstanceOf is disabled

修正案

  • asInstanceOfを使わない。

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

DefaultArguments

エラーとなるコード 

def x(y: Int = 0) = {}

[wartremover:DefaultArguments] Function has default arguments

修正コード

def x(y: Int) = {}

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

EitherProjectionPartial

エラーとなるコード 

Left(1).left.get

[wartremover:EitherProjectionPartial] LeftProjection#get is disabled - use LeftProjection#toOption instead

修正コード

Left(1).left.toOption

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

Enumeration

  • エラーとなるコード 
class x extends Enumeration {}

[wartremover:Enumeration] Enumeration is disabled - use case objects instead

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

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

Equals

  • エラーとなるコード 
5 == "5"

[wartremover:Equals] == is disabled - use === or equivalent instead

  • 修正コード
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

  • エラーとなるコード 
implicit val foo = 5

[wartremover:ExplicitImplicitTypes] implicit definitions must have an explicit type ascription

  • 修正コード
implicit val foo: Int = 5

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

FinalCaseClass

  • エラーとなるコード 
case class Foo()

[wartremover:FinalCaseClass] case classes must be final

  • 修正コード
final case class Foo()

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

FinalVal

  • エラーとなるコード 
final val v = 1

[wartremover:FinalVal] final val is disabled - use non-final val or final def or add type ascription

  • 修正コード
val v = 1

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

ImplicitConversion

  • エラーとなるコード 
implicit def int2Array(i: Int): Array[String] = Array.fill(i)("?")

[wartremover:ImplicitConversion] Implicit conversion is disabled

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

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

ImplicitParameter

  • エラーとなるコード 
def f()(implicit s: String) = ()

[wartremover:ImplicitParameter] Implicit parameters are disabled

  • 修正コード
def f()(s: String) = ()

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

IsInstanceOf

  • エラーとなるコード 
x.isInstanceOf[String]

[wartremover:IsInstanceOf] isInstanceOf is disabled

  • 修正案
    • isInstanceOfを使わない。

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

JavaConversions

  • エラーとなるコード 
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

  • 修正コード
import scala.collection.JavaConverters._
val scalaMap: Map[String, String] = Map()
val javaMap: java.util.Map[String, String] = scalaMap.asJava

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

JavaSerializable

  • エラーとなるコード 
object O extends Serializable
val v = List("", O)

[wartremover:JavaSerializable] Inferred type containing Serializable: java.io.Serializable

  • 修正コード
object O extends Serializable
val v = List(O)

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

LeakingSealed

  • エラーとなるコード 
sealed trait t
class c extends t

[wartremover:LeakingSealed] Descendants of a sealed type must be final or sealed

  • 修正コード
sealed trait t
sealed class c extends t

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

MutableDataStructures

  • エラーとなるコード 
import scala.collection.mutable.ListBuffer
val list = ListBuffer()

[wartremover:MutableDataStructures] scala.collection.mutable package is disabled

  • 修正コード
import scala.collection.immutable.List
val list = List()

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

NonUnitStatements

  • エラーとなるコード 
10

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

  • 修正コード
()

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

Nothing

  • エラーとなるコード 
val x = ???

[wartremover:Nothing] Inferred type containing Nothing: Nothing

  • 修正コード
val x = 10

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

Null

  • エラーとなるコード 
val s: String = null

[wartremover:Null] null is disabled

  • 修正コード
val s: String = "n"

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

Option2Iterable

  • エラーとなるコード 
Iterable(1, 2, 3).flatMap(Some(_))

[wartremover:Option2Iterable] Implicit conversion from Option to Iterable is disabled - use Option#toList instead

  • 修正コード
Iterable(1, 2, 3).flatMap(Some(_).toList)

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

OptionPartial

  • エラーとなるコード 
Some(1).get

[wartremover:OptionPartial] Option#get is disabled - use Option#fold instead

  • 修正コード
Some(1).fold(0)(i => i)

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

Overloading

  • エラーとなるコード 
class c {
  def equals(x: Int) = {}
}

[wartremover:Overloading] Overloading is disabled

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

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

Product

  • エラーとなるコード 
val any = List((1, 2, 3), (1, 2))

[wartremover:Product] Inferred type containing Product: Product with Serializable

  • 修正コード
val any: List[(Int, Int)] = List((2, 3), (1, 2))

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

PublicInference

  • エラーとなるコード 
def f() = {}

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

  • 修正コード
def f(): Unit = {}

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

Recursion

  • エラーとなるコード 
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

  • 修正コード
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を付けるべき。

Return

  • エラーとなるコード 
def foo(n:Int): Int = return n + 1

[wartremover:Return] return is disabled

  • 修正コード
def foo(n:Int): Int = n + 1

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

Serializable

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

StringPlusAny

  • エラーとなるコード 
println("foo" + 1)

[wartremover:StringPlusAny] Implicit conversion to string is disabled

  • 修正コード
println("foo" + 1.toString)

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

Throw

  • エラーとなるコード 
def foo(n: Int): Int = {
  n match {
    case 1 => 1
    case _ => throw new IllegalArgumentException("bar")
  }
}

[wartremover:Throw] throw is disabled

  • 修正コード
def foo(n: Int):Either[String, Int] = {
  n match {
    case 1 => Right(1)
    case _ => Left("bar")
  }
}

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

ToString

  • エラーとなるコード 
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

  • 修正コード
class Foo(i: Int) {
    override val toString: String = s"Foo($i)"
}
val foo: Foo = new Foo(5)
println(foo.toString)

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

TraversableOps

  • エラーとなるコード 
val foo = List(1, 2, 3)
println(foo.head)

[wartremover:TraversableOps] head is disabled - use headOption instead

  • 修正コード
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

  • エラーとなるコード 
println(Success(1).get)

[wartremover:TryPartial] Try#get is disabled

  • 修正コード
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

  • エラーとなるコード 
var x = 100

[wartremover:Var] var is disabled

  • 修正コード
val x = 100

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

While

  • エラーとなるコード 
var i = 0
while (i < 10) {
    println(i.toString)
    i += 1
}

[wartremover:While] while is disabled

  • 修正コード
(0 until 10).foreach(println(_))

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

wartremover-contrib

  • WartRemover公式コミュニティが提供している追加Warts集でより実践的なものが多い。
  • 2019/12/01時点で最新のバージョン1.3.1を用いる。組み込みWarts数は11個。

Apply

  • エラーとなるコード 
class C {
    def apply() = 1
}

[wartremover:Apply] apply is disabled

  • 修正コード
object C {
    def apply() = 1
}

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

ExposedTuples

  • エラーとなるコード 
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.

  • 修正コード
final case class CustomerAccount(customerId: String, accountTotal: Long)

def goodFoo(customerTotal: CustomerAccount) = {
    // Code
}

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

MissingOverride

  • エラーとなるコード 
trait T {
    def f(): Unit
}
class C extends T {
    final def f() = {}
}

[wartremover:MissingOverride] Method must have override modifier

  • 修正コード
trait T {
    def f(): Unit
}
class C extends T {
    override final def f() = {}
}

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

NoNeedForMonad

  • エラーとなるコード 
Option(1).flatMap(i => Option(2).map(j => i + j))

[wartremover:NoNeedForMonad] No need for Monad here (Applicative should suffice).

  • 修正コード
Option(1).flatMap(i => Option(i + 1).map(j => i + j))

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

OldTime

  • エラーとなるコード 
import java.util.Date
val x = new Date()

[wartremover:OldTime] The old Java time API is disabled. Use Java 8 java.time._ API instead.

  • 修正コード
import java.time.LocalDateTime
val x = LocalDateTime.now()

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

RefinedClasstag

  • エラーとなるコード 
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

  • 修正コード
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を使用すると最初の型しかチェックされず意図しない挙動になるため避けるべき。

SealedCaseClass

  • エラーとなるコード 
sealed case class Foo(i: Int)

[wartremover:SealedCaseClass] case classes must not be sealed

  • 修正コード
final case class Foo(i: Int)

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

SomeApply

  • エラーとなるコード 
Some(10)

[wartremover:SomeApply] Some.apply is disabled - use Option.apply instead

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

SymbolicName

  • エラーとなるコード 
def :+:(): Unit = {}

[wartremover:SymbolicName] Symbolic name is disabled

  • 修正コード
def join(): Unit = {}

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

UnintendedLaziness

  • エラーとなるコード 
val map: Map[Int, Int] = Map(1 -> 10)
map.mapValues(_ + 1)

[wartremover:UnintendedLaziness] GenMapLike#mapValues is disabled because it implicitly creates lazily evaluated collections.

  • 修正コード
val map: Map[Int, Int] = Map(1 -> 10)
map.toStream.map(_._2 + 1)

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

UnsafeInheritance

  • エラーとなるコード 
trait T {
    def positive = 1
}
class C extends T {
    override def positive = -1
}

[wartremover:UnsafeInheritance] Method must be final or abstract

  • 修正コード
trait T {
    final def positive = 1
}

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

まとめ

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

14
1
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
Sign upLogin
14
1