GhcLintでコードをチェックする

結果集計

$ grep "^src" doc/wk_lint_result.md | cut -d " " -f 4- | sort | uniq -c | sort -r
  18 error| Suggestion: Move brackets to avoid $
   8 error| Suggestion: Redundant $
   7 error| Suggestion: Redundant bracket
   4 error| Suggestion: Avoid lambda
   3 warning| Redundant return
   3 warning| Redundant lambda
   3 error| Suggestion: Use String
   2 error| Suggestion: Use infix
   1 warning| Use notElem
   1 warning| Redundant if
   1 warning| Redundant do
   1 warning| Evaluate
   1 warning| Eta reduce
   1 error| Suggestion: Use if

$の無駄使いが多いようです。

また、全体的にRedundantが多いですね。割合を計算してみると。

$ grep "^src" doc/wk_lint_result.md | cut -d " " -f 4- | sort | uniq -c | sort -nr | awk '{a += $1;} END {print a}'
54
$ grep "^src" doc/wk_lint_result.md | cut -d " " -f 4- | sort | uniq -c | sort -nr | grep Redundant
   8 error| Suggestion: Redundant $
   7 error| Suggestion: Redundant bracket
   3 warning| Redundant return
   3 warning| Redundant lambda
   1 warning| Redundant if
   1 warning| Redundant do
$ grep "^src" doc/wk_lint_result.md | cut -d " " -f 4- | sort | uniq -c | sort -nr | grep Redundant | awk '{a += $1;} END {print a}'
23
$ echo "23/54*100" | bc -l
42.59259259259259259200

42%が冗長でした。

結果内容

• 冗長なラムダ

src/Atoparsec05.hs|72 col 1 warning| Redundant lambda
|| Found:
||   incAndaddBindParserState
||     = \ bind ParserState{..} ->
||         ParserState (markerIdx + 1) (bind : bindList)
|| Why not:
||   incAndaddBindParserState bind ParserState{..}
||     = ParserState (markerIdx + 1) (bind : bindList)

• エータ簡約する

src/Atoparsec05.hs|110 col 5 warning| Eta reduce
|| Found:
||   mkBin ope x y = mkBinaryOperator (BC.unpack ope) x y
|| Why not:
||   mkBin ope = mkBinaryOperator (BC.unpack ope)

• [Char]ではなくStringを使う

src/Atoparsec05.hs|117 col 11 error| Suggestion: Use String
|| Found:
||   [Char] -> Parser Char
|| Why not:
||   String -> Parser Char

• ライブラリにある関数を使う

src/Atoparsec05.hs|118 col 28 warning| Use notElem
|| Found:
||   not (elem c xs)
|| Why not:
||   notElem c xs

notElemなんてあったかなとおもったら、確かにある。

*Atoparsec04> :t elem
elem :: (Foldable t, Eq a) => a -> t a -> Bool
*Atoparsec04> :t notElem
notElem :: (Foldable t, Eq a) => a -> t a -> Bool

• 中置演算子を使う

src/Atoparsec05.hs|118 col 33 error| Suggestion: Use infix
|| Found:
||   elem c xs
|| Why not:
||   c `elem` xs

• タプルから値を取り出して関数適用するのではなく、関数をuncurryして直接タプルを関数適用できるようにする。

src/Atoparsec05.hs|268 col 28 warning| Evaluate
|| Found:
||   ope_ lhs (fst $ P.head xs) (snd $ P.head xs)
|| Why not:
||   uncurry (ope_ lhs) (P.head xs)

• do不要

src/Atoparsec05.hs|331 col 22 warning| Redundant do
|| Found:
||   do case val of
||          Left sch -> do lift $ char '.'
||                         x <- _idenStr
||                         params <- parenExprList
||                         return $ expr $ mkFunctionCall' x sch params
||          Right idn -> do params <- parenExprList
||                          return $ expr $ mkFunctionCall' idn "" params
|| Why not:
||   case val of
||       Left sch -> do lift $ char '.'
||                      x <- _idenStr
||                      params <- parenExprList
||                      return $ expr $ mkFunctionCall' x sch params
||       Right idn -> do params <- parenExprList
||                       return $ expr $ mkFunctionCall' idn "" params

• 評価結果を返すならif不要

src/Atoparsec05.hs|415 col 11 warning| Redundant if
|| Found:
||   if x == '+' then True else False
|| Why not:
||   x == '+'

• 不要なかっこ

src/Atoparsec05.hs|477 col 14 error| Suggestion: Redundant bracket
|| Found:
||   (lift $ string "json") <|>
||     (lift (string "datetime" <|> "date" <|> "time"))
|| Why not:
||   (lift $ string "json") <|>
||     lift (string "datetime" <|> "date" <|> "time")

• 不要なラムダ

src/Atoparsec05.hs|570 col 37 error| Suggestion: Avoid lambda
|| Found:
||   \ x -> x == c
|| Why not:
||   (== c)

• 不要な$

src/Atoparsec05.hs|588 col 8 error| Suggestion: Redundant $
|| Found:
||   lift $ peekChar
|| Why not:
||   lift peekChar

• caseで評価をTrue/Falseでする場合ifをつかう

src/Atoparsec05.hs|594 col 8 error| Suggestion: Use if
|| Found:
||   case arw of
||       True -> do docPathArrow
||                  docPath <- (lift $ char '$') >> (lift $ char '.') >>
||                               atomDocumentPath
||                  return $
||                    exprColumnIdentifier $
||                      columnIdentifierNameDocumentPahtItem str docPath
||       False -> return $ exprIdentifierName str
|| Why not:
||   if arw then
||     (do docPathArrow
||         docPath <- (lift $ char '$') >> (lift $ char '.') >>
||                      atomDocumentPath
||         return $
||           exprColumnIdentifier $
||             columnIdentifierNameDocumentPahtItem str docPath)
||     else return $ exprIdentifierName str

• 不要なリターンは避ける。(多分コードを書いていて途中に検証用のロジックをいれいて消したのでこの形になっていたのだと思う。)

src/Atoparsec05.hs|666 col 20 warning| Redundant return
|| Found:
||   do x <- many1 atomPathItem
||      return x
|| Why not:
||   do many1 atomPathItem

• かっこの範囲は必要最低限に狭くする

src/Atoparsec05.hs|709 col 20 error| Suggestion: Move brackets to avoid $
|| Found:
||   (lift $ string "[*]") >> return '*'
|| Why not:
||   lift (string "[*]") >> return '*'

• 不要なラムダは避ける

src/Atoparsec05.hs|712 col 20 error| Suggestion: Avoid lambda
|| Found:
||   \ x -> (mkArrayIndex . fromIntegral) x
|| Why not:
||   (mkArrayIndex . fromIntegral)

• $を避ける

src/Atoparsec05.hs|725 col 18 error| Suggestion: Move brackets to avoid $
|| Found:
||   (lift $ stringCI "asc") <|> lift (stringCI "desc")
|| Why not:
||   lift (stringCI "asc") <|> lift (stringCI "desc")