自作scalafix ruleの説明その3

scalafix

以下の続き。

前回書いてから、かなり増えて、もう少しで100個超えそう。

xuwei-k.hatenablog.com xuwei-k.hatenablog.com

以前書いた時から、これ書いている2024年3月時点のversion 0.4.1までに増えたものを全部簡単に説明書く。

あと、少なくとも今回説明は書かないですが、以前までに作ったruleに関しても細かい改善などもしてます。

https://github.com/xuwei-k/scalafix-rules/tree/98f97c6e01723a671a8b722c7fe5f88cfc9623f2

CompareSameValue

a1.b == a1.b というように、見た目明らかに同じTree同士を比較していたら警告する。 a1.b == a2.b みたいに書きたかったのにコピペミス、みたいなことが稀に発生したので。

DiscardValue

configで指定したもの、あるいは事前に用意されたいくつかの非同期系の型を捨てていたら警告を出す。 これはSemanticRule。 Scala 2標準では、確かUnit以外を捨ててたら一括で警告を出すものがあるが、それだと強過ぎるので、もっと細かく個別に検出したい型を指定して、より捨てていたらバグっぽいものだけを警告したかったので。 これ書いた時点では、以下がデフォルトで用意してある (用意してないものもconfigに数行追加で可能)

  • scala.concurrent.Future
  • monix.eval.Task
  • cats.effect.IO
  • org.atnos.eff.Eff

EitherFold

Eitherをmatch式で処理しているものをfoldに書き換える。作れるからなんとなく作ったけど、あまり個人的に実用する予定はない。 末尾再帰のためあえてmatch式、といったパターンもあり得るし(それの考慮はせずに書き換えてしまう)

EitherGetOrElse

EitherFoldと同様

EitherMap

EitherFoldやEitherGetOrElseと同様だが、それらと比較すれば、この書き換えパターンは多少は実用的かも？

EtaExpand

.f(x => a.b(x)) を .f(a.b) に書き換えたりする。 SyntacticRuleだし、そもそも仮にSemanticRuleで書いたとしても、結構書き換え不可能なパターンはあり(推論のされ方などが変わって壊れる)、それも書き換えてしまうので、あまり正確ではないので、それを踏まえて使う必要あり。

ExtendsProductWithSerializable

主に

  • 自身がsealed
  • sub classやobjectが全てcase classやcase object

という条件の場合に、親に extends Product with Serializable を付与する。 Scala 3では改善されたので普通必要ないが、Scala 2では昔から行われている面倒な？テクニックというかノウハウを、多少自動化した。

継承した方がいい理由は、ここでは説明しないので以下を参照

FilterNot

a.filter(b => !f(b)) を a.filterNot(b => f(b)) に書き換えるだけ。あるいはfilterNot内部で ! 使ってたら、その逆をする。

FilterSize

xs.filter(f).size を xs.count(f) に書き換え。型を見ておらず、標準のcollectionと同じシグネチャという暗黙の過程で書き換えるので、そうでない場合は壊れる

FinalObjectWarn

すごく大昔には override object 的な機能があって意味がある場合があった？が、最新のScala 3ではobjectにfinal付与しても警告が出るだけなので、それをscalafixでも警告するだけのもの。

FlatMapCollect

x.flatMap {
  case 1 => Some(10)
  case 2 => Some(20)
  case _ => None
}

のように

  • 最後のmatchしなかったパターンだけNone
  • それ以外はSomeで包む

という場合は

x.collect {
  case 1 => 10
  case 2 => 20
}

に書き換え可能なはず？なので、書き換えるもの。

GroupMap

Scala 2.13からgroupMapというメソッドが増えたので、それに書き換え可能かもしれない場合に書き換えるもの。

https://github.com/scala/scala/blob/c2333de3fec88325bacb08d65264dbd290b54dcc/src/library/scala/collection/Iterable.scala#L595

例えば

  def f[A, B](xs: List[(A, B)]): Map[A, List[B]] =
    xs.groupBy(_._1).view.mapValues(_.map(x => x._2)).toMap

は

  def f[A, B](xs: List[(A, B)]): Map[A, List[B]] =
    xs.groupMap(_._1)(x => x._2)

に書き換え可能なはず

IncorrectScaladocParam

/**
 * @param b 説明
 */
def foo(a: Int): X

のように、実際の引数とscaladocの説明の変数名が食い違っている場合に警告する

InterpolationToString

    s" aaa ${bbb.toString} ccc"

と、 s の標準のstring interpolation内部でtoStringを呼び出していた場合、 null の扱いなどのすごく細かい点を除いて、基本的に暗黙的にtoStringは呼ばれるので、それに頼る方針でいいなら明示的な呼び出しは必要ないはずなので toString 呼び出しを消す、というもの

InterpolationToStringWarn

上記の、書き換えをせずに警告だけ出す版

IsEmptyNonEmpty

!x.isEmpty を x.nonEmpty に書き換えるだけのもの。あるいは !x.nonEmpty を x.isEmpty に書き換え。

isEmpty や nonEmpty が存在しているか？は暗黙的に仮定してるので、そういうメソッドがないものに対して書き換えて壊れる可能性はある。

KeySet

Mapっぽいものに対して

x.map(_._1).toSet

を

x.keySet

と書き換えする。 これはSemanticRuleで型を見ているので、それなりに正確というか、型を見ないと書き換え間違いが大量になるので、型を見る実装にした。

LazyZip

list1.zip(list2).map { case (x1, x2) =>

のようなパターンで、以下のメソッドの場合は

"map", "flatMap", "filter", "exists" , "forall", "foreach"

lazyZipにした方が効率がいいので、書き換える

MapFlattenFlatMap

ListやOptionなどに対して

x.map(x => 何か).flatten

としてたら、おそらくflatMap書き換え可能だろう、という多少雑な仮定で

x.flatMap(x => 何か)

に書き換える。

MapToForeach

def y = {
  x1.map(f1) // こっち捨ててますよね？

  x2.map(f2)
}

という、Block式の最後ではない場合などに map してたら、( foreach が存在してるという雑な仮定の元)、foreachに書き換える。 というもの

NamedParamOrder

def f(x: A, y: B) =

という定義があったときに

f(x = a, y = b)

と

f(y = b, x = a)

という呼び出しの違いは、厳密には意味が異なる。 これは少なくともScala 2の仕様にはしっかり書かれており、

https://x.com/xuwei_k/status/1634500216060772352

後者のように

「定義順と呼び出し順が異なる」

場合は、compilerが内部的に以下のようなTreeを生成する

val tmp1 = b
val tmp2 = a

f(x = tmp2, y = tmp1)

これは、JVM上ではメソッドの引数の評価順は決まっているが、Scalaコードとの見た目が合わなくなるので、評価順を見た目と合わせるために、compilerは内部的に一時変数を作る。

この挙動は確かに嬉しい場合もあるが、大抵は

「定義順と呼び出し順が違うのは意図してない」

「そもそも引数の評価の副作用がない、副作用があってもその順番は問題にならない」

というパターンが大半では？という仮定のもと、大きなデメリットでもないが、compilerに余計なTreeを生成させないようにするために、名前指定は維持したまま、呼び出し順を定義順に合わせるように書き換えるもの。

ObjectFinal

object A {
  final val x = 何か
}

と書いた場合、JVMのclassファイル上では確かにfinalになるかどうか？の違いがあるが、そもそもScala的にはvalは書き換え不可能でobjectは継承実質不可能なので、finalの有無は、あまり実質的な違いがない。 Scala 2ではIntやStringなどの一部の型は final val にするとインライン化される、という仕様？があるが、それらの型ですらないもの(例えばListやOptionなど)を final val で定義しても本当にほぼ意味がないので、それを警告するもの。

OptionContains

a match {
  case Some(c) => c == b
  case None => false
}

のようなmatch式のコードを

a.contains(b)

に書き換え

OptionFilter

a match {
  case Some(b) if b % 2 == 0 => Some(b)
  case _ => None
}

というようなmatch式を

a.filter(b => b % 2 == 0)

に書き換え

OptionForallExists

Optionに対して、forallやexistsで書けるのにmatch式で書いてるのを書き換え

OptionGetOrElse

他と同様、match式をgetOrElseに書き換え

OptionGetWarn

Optionのget使っていたら警告。SemanticRuleで型を見る。

wartremoverやその他色々で検知可能だが、scalafixでもconfigなしで1つのruleで検知出来た方が便利なので、あえて単体で作った。

OptionOrElse

他と同様、Optionに対するmatch式を、可能な場合orElseに書き換え

OptionWhenUnless

Scala 2.13から増えた、Optionのwhenやunlessに可能な場合書き換え

PartialFunctionCondOpt

a match {
  case "1" => Some(4)
  case "2" => Some(10)
  case "3" => Some(123)
  case _ => None
}

flatMapとcollectの書き換えと似ているが、match式で、最後だけNoneで、あとはSomeで包んでいるときに PartialFunction.condOpt に書き換える

PartialFunction.condOpt(a) {
  case "1" => 4
  case "2" => 10
  case "3" => 123
}

RedundantCaseClassVal

case classのfieldは暗黙的にpublicなvalになるのに、意味なく val 付与していたら警告する

case class A(
  val x: Int, // ここval書く必要ないですよね？？？
  y: String
)

RemoveIf

if (a1 == a2) {
  true
} else {
  false
}

という進次郎構文？を

a1 == a2

に書き換える

RemoveUselessParamComments

/**
 * @param userId userId
 */
def foo(userId: UserId) =

というような、変数名と全く同じ、実質意味がないscaladocコメントを削除する

RepeatedRewrite

scalafmtで強制可能だが、scalafmt使ってないパターンや、ピンポイントでこれだけ書き換えたい場合用のscalafix rule。

可変長の引数に対して展開して渡す記法が新しくなったが、その

f(xs: _*)

という古い方から

f(xs *)

という新しい方に書き換える。

ReplacePlaceholder

.map(a => a.b) などを .map(_.b) に書き換え。

これも結構壊れるパターンがあるので注意。

ReuseInstances

match式で

case Some(b) => Some(b)

のように、全く同じインスタンスを生成し直してる場合に、

case x @ Some(_) => x

というように、既存のものを再利用するように書き換える。

SameParamOverloading

    def x1[A: ClassTag](a: A): Int 
    def x1[A](b: A): String

というように、一応原理上定義は可能だが、implicitや型引数除いて引数が全く同じようなパターンの危険な？オーバーロードの定義を警告する。

SizeToLength

これはSemanticRuleで型を見る。

JVMの最適化が効けば変わらない可能性もあるが、原理上、StringやArrayの場合は、直接生えているlengthを呼んだ方が、sizeよりも効率がいいかもしれないので、そちらに書き換えるもの。

SlickFilter

Slickにおいて以下のようなものを書いても

    query.filter { x =>
      a1 match {
        case Some(value) =>
          x.a1 === value
        case _ =>
          true: Rep[Boolean]
      }
    }

正しく動くは動くが、実際に生成されるSQLの内部にもtrueが固定で出力されてしまうので filterOpt などのもっといい感じのものに書き換える。

上記のような単純なfilterOpt以外にも書き換えをしてくれるが、説明面倒なので省略。

StringFormatToInterpolation

" %s aaa %s bbb".format(x, y)

という %s だけで、それ以上細かくformatを指定してない場合は、string interpolationに書き換えた方が、見やすいというか、変数の数を原理上間違える余地がなくなって安心安全なので、書き換えるもの。

UnmooredDocComment

/**

と

*/

で囲む形式のコメントは、classやmethodの先頭に書いて、

ある意味scaladoc(ここでいうscaladocとはsbtのdocコマンドでhtmlが生成されるようなもの)、に反映するために存在しているが、 たまにそのあたりの理解が曖昧で、完全にメソッドの途中などに

def foo(何か色々引数) = {

  /** ここでは〇〇という処理をする */
  xxx.yyy(zzz)

  /**
   * aaaがbbbの場合だけcccという処理をする
   */
  if (ddd) {
    eee
  } 
}

と書く人がいるが、少なくともscala 2のscaladocコマンドでもコンパイルオプションによってはそれは警告が出て、間違った使い方なので、それを警告するもの。

UselessParamCommentsWarn

RemoveUselessParamCommentsと同様の検出をするが、書き換えずに警告するだけのもの