mirror of
https://github.com/jfmengels/elm-review.git
synced 2024-11-23 23:05:35 +03:00
Add SimplifyPiping rule
This commit is contained in:
parent
6ef2ee0d94
commit
65ed24f942
@ -22,7 +22,8 @@ You can read the slides for my [presentation](http://slides.com/jeroenengels/elm
|
||||
- [NoDebug](rules/NoDebug.md) - Forbid the use of `Debug` before it goes into production.
|
||||
- [NoDuplicateImports](rules/NoDuplicateImports.md) - Forbid importing the same module several times in a file.
|
||||
- [NoUnannotatedFunction](rules/NoUnannotatedFunction.md) - Ensure every top-level function declaration has a type annotation.
|
||||
- [NoExposingEverything](rules/no-class.md) - Forbid exporting everything in your modules `module Main exposing (..)`, to make your module explicit in what it exposes.
|
||||
- [NoExposingEverything](rules/NoExposingEverything.md) - Forbid exporting everything in your modules `module Main exposing (..)`, to make your module explicit in what it exposes.
|
||||
- [SimplifyPiping](rules/SimplifyPiping.md) - Simplify piped functions like `List.map f >> List.map g` to `List.map (f >> g)`
|
||||
|
||||
More rule ideas in this [slide](http://slides.com/jeroenengels/elm-lint#/5/3) and the ones below it.
|
||||
Note that some rules were implemented but may not be good ideas. Think for yourself and ask the community whether you should enable them.
|
||||
|
@ -21,6 +21,7 @@ import NoExposingEverything
|
||||
import NoImportingEverything
|
||||
import NoUnannotatedFunction
|
||||
import NoUnusedVariables
|
||||
import SimplifyPiping
|
||||
|
||||
|
||||
type Msg
|
||||
@ -35,6 +36,7 @@ rules =
|
||||
, NoImportingEverything.rule
|
||||
, NoUnannotatedFunction.rule
|
||||
, NoUnusedVariables.rule
|
||||
, SimplifyPiping.rule
|
||||
]
|
||||
|
||||
|
||||
|
117
rules/SimplifyPiping.elm
Normal file
117
rules/SimplifyPiping.elm
Normal file
@ -0,0 +1,117 @@
|
||||
module SimplifyPiping exposing (rule)
|
||||
|
||||
import Lint exposing (lint, doNothing)
|
||||
import Types exposing (LintRule, Error, Direction(..))
|
||||
import Ast.Expression exposing (..)
|
||||
import Set exposing (Set)
|
||||
|
||||
|
||||
type alias Context =
|
||||
{}
|
||||
|
||||
|
||||
rule : String -> List Error
|
||||
rule input =
|
||||
lint input implementation
|
||||
|
||||
|
||||
implementation : LintRule Context
|
||||
implementation =
|
||||
{ statementFn = doNothing
|
||||
, typeFn = doNothing
|
||||
, expressionFn = expressionFn
|
||||
, moduleEndFn = (\ctx -> ( [], ctx ))
|
||||
, initialContext = Context
|
||||
}
|
||||
|
||||
|
||||
error : String -> String -> Error
|
||||
error op fn =
|
||||
Error
|
||||
"SimplifyPiping"
|
||||
("Instead of `" ++ fn ++ " f " ++ op ++ " List.map g`, try " ++ fn ++ " (f " ++ op ++ " g)")
|
||||
|
||||
|
||||
simplifiableFns : Set String
|
||||
simplifiableFns =
|
||||
Set.fromList
|
||||
[ "List.map"
|
||||
, "Set.map"
|
||||
, "Array.map"
|
||||
, "Array.indexedMap"
|
||||
]
|
||||
|
||||
|
||||
nameOfMethod : List (List String) -> String
|
||||
nameOfMethod members =
|
||||
members
|
||||
|> List.concatMap (\a -> a)
|
||||
|> String.join "."
|
||||
|
||||
|
||||
reportIfSimplifiableMethod : String -> Expression -> Expression -> List Error
|
||||
reportIfSimplifiableMethod op left right =
|
||||
case [ left, right ] of
|
||||
[ Application (Access (Variable names1) fns1) _, Application (Access (Variable names2) fns2) _ ] ->
|
||||
if [ names1, fns1 ] == [ names2, fns2 ] && Set.member (nameOfMethod [ names1, fns1 ]) simplifiableFns then
|
||||
[ error op <| nameOfMethod [ names1, fns1 ] ]
|
||||
else
|
||||
[]
|
||||
|
||||
_ ->
|
||||
[]
|
||||
|
||||
|
||||
expressionFn : Context -> Direction Expression -> ( List Error, Context )
|
||||
expressionFn ctx node =
|
||||
case node of
|
||||
Enter (BinOp (Variable [ "|>" ]) (Application left _) right) ->
|
||||
case right of
|
||||
-- X.y f data |> X.y g |> foo
|
||||
BinOp (Variable [ "|>" ]) subRight _ ->
|
||||
( reportIfSimplifiableMethod ">>" left subRight, ctx )
|
||||
|
||||
-- X.y f data |> X.y g
|
||||
_ ->
|
||||
( reportIfSimplifiableMethod ">>" left right, ctx )
|
||||
|
||||
-- X.y f <| X.y g data
|
||||
Enter (BinOp (Variable [ "<|" ]) left (Application right _)) ->
|
||||
case left of
|
||||
-- foo <| X.y f <| X.y g data
|
||||
BinOp (Variable [ "<|" ]) subLeft _ ->
|
||||
( reportIfSimplifiableMethod "<<" left subLeft, ctx )
|
||||
|
||||
_ ->
|
||||
( reportIfSimplifiableMethod "<<" left right, ctx )
|
||||
|
||||
-- a |> X.y f |> X.y g
|
||||
Enter (BinOp (Variable [ "|>" ]) _ (BinOp (Variable [ "|>" ]) left right)) ->
|
||||
( reportIfSimplifiableMethod ">>" left right, ctx )
|
||||
|
||||
-- X.y f <| X.y g <| a
|
||||
Enter (BinOp (Variable [ "<|" ]) left (BinOp (Variable [ "<|" ]) right _)) ->
|
||||
( reportIfSimplifiableMethod "<<" left right, ctx )
|
||||
|
||||
Enter (BinOp (Variable [ ">>" ]) left right) ->
|
||||
case left of
|
||||
-- foo >> X.y f >> X.y g
|
||||
BinOp (Variable [ ">>" ]) _ subLeft ->
|
||||
( reportIfSimplifiableMethod ">>" subLeft right, ctx )
|
||||
|
||||
-- X.y f >> X.y g
|
||||
_ ->
|
||||
( reportIfSimplifiableMethod ">>" left right, ctx )
|
||||
|
||||
Enter (BinOp (Variable [ "<<" ]) left right) ->
|
||||
case left of
|
||||
-- foo << X.y f << X.y g
|
||||
BinOp (Variable [ "<<" ]) _ subLeft ->
|
||||
( reportIfSimplifiableMethod "<<" subLeft right, ctx )
|
||||
|
||||
-- X.y f << X.y g
|
||||
_ ->
|
||||
( reportIfSimplifiableMethod "<<" left right, ctx )
|
||||
|
||||
_ ->
|
||||
( [], ctx )
|
@ -8,6 +8,7 @@ import NoDuplicateImportsTest
|
||||
import NoImportingEverythingTest
|
||||
import NoUnannotatedFunctionTest
|
||||
import NoUnusedVariablesTest
|
||||
import SimplifyPipingTest
|
||||
|
||||
|
||||
main : Test.Runner.Node.TestProgram
|
||||
@ -26,4 +27,5 @@ all =
|
||||
, NoImportingEverythingTest.all
|
||||
, NoUnannotatedFunctionTest.all
|
||||
, NoUnusedVariablesTest.all
|
||||
, SimplifyPipingTest.all
|
||||
]
|
||||
|
126
tests/SimplifyPipingTest.elm
Normal file
126
tests/SimplifyPipingTest.elm
Normal file
@ -0,0 +1,126 @@
|
||||
port module SimplifyPipingTest exposing (all)
|
||||
|
||||
import Expect
|
||||
import Test exposing (describe, test, Test)
|
||||
import SimplifyPiping exposing (rule)
|
||||
import Types exposing (Error)
|
||||
|
||||
|
||||
error : String -> String -> Error
|
||||
error op fn =
|
||||
Error "SimplifyPiping" ("Instead of `" ++ fn ++ " f " ++ op ++ " List.map g`, try " ++ fn ++ " (f " ++ op ++ " g)")
|
||||
|
||||
|
||||
tests : List Test
|
||||
tests =
|
||||
[ test "should not report piping of the result of different functions of the same module" <|
|
||||
\() ->
|
||||
rule "a = b |> List.map f |> List.filter g"
|
||||
|> Expect.equal []
|
||||
, test "should not report piping of the result of similarly named functions of different modules" <|
|
||||
\() ->
|
||||
rule "a = b |> List.map f |> Set.map g"
|
||||
|> Expect.equal []
|
||||
, test "should report piping of List.map" <|
|
||||
\() ->
|
||||
rule "a = b |> List.map f |> List.map g"
|
||||
|> Expect.equal [ error ">>" "List.map" ]
|
||||
, test "should report piping of Set.map" <|
|
||||
\() ->
|
||||
rule "a = b |> Set.map f |> Set.map g"
|
||||
|> Expect.equal [ error ">>" "Set.map" ]
|
||||
, test "should report piping of Array.map" <|
|
||||
\() ->
|
||||
rule "a = b |> Array.map f |> Array.map g"
|
||||
|> Expect.equal [ error ">>" "Array.map" ]
|
||||
, test "should report piping of the result of Array.indexedMap" <|
|
||||
\() ->
|
||||
rule "a = b |> Array.indexedMap f |> Array.indexedMap g"
|
||||
|> Expect.equal [ error ">>" "Array.indexedMap" ]
|
||||
, test "should not report piping of the incomplete simplifiable function call" <|
|
||||
\() ->
|
||||
rule "a = List.map f |> List.map g"
|
||||
|> Expect.equal []
|
||||
, test "should not report piping the function in itself" <|
|
||||
\() ->
|
||||
rule "a = List.map f |> List.map g"
|
||||
|> Expect.equal []
|
||||
, test "should report piping the complete call into the same function" <|
|
||||
\() ->
|
||||
rule "a = List.map f data |> List.map g"
|
||||
|> Expect.equal [ error ">>" "List.map" ]
|
||||
, test "should report piping the complete call into the same function with an additional pipe at the end" <|
|
||||
\() ->
|
||||
rule "a = List.map f data |> List.map g |> foo"
|
||||
|> Expect.equal [ error ">>" "List.map" ]
|
||||
, test "should not report the use of a single simplifiable function" <|
|
||||
\() ->
|
||||
rule "a = List.map f"
|
||||
|> Expect.equal []
|
||||
, test "should not report the use of a single simplifiable function with piped functions as the argument" <|
|
||||
\() ->
|
||||
rule "a = List.map (f >> g)"
|
||||
|> Expect.equal []
|
||||
, test "should not report any piping of similar methods" <|
|
||||
\() ->
|
||||
rule "a = List.foo fn |> List.foo fn2"
|
||||
|> Expect.equal []
|
||||
, test "should not report direct piping of List.map and friends" <|
|
||||
\() ->
|
||||
rule """
|
||||
a = List.map |> List.map
|
||||
b = List.map >> List.map
|
||||
c = Set.map |> Set.map
|
||||
d = Set.map >> Set.map
|
||||
"""
|
||||
|> Expect.equal []
|
||||
, test "should report piping right to left" <|
|
||||
\() ->
|
||||
rule "a = List.map f <| List.map g <| b"
|
||||
|> Expect.equal [ error "<<" "List.map" ]
|
||||
, test "should report piping right to left with complete call at the right side" <|
|
||||
\() ->
|
||||
rule "a = List.map f <| List.map g data"
|
||||
|> Expect.equal [ error "<<" "List.map" ]
|
||||
, test "should report piping right to left with complete call at the right side and additional pipe on the left" <|
|
||||
\() ->
|
||||
rule "a = a <| List.map f <| List.map g data"
|
||||
|> Expect.equal [ error "<<" "List.map" ]
|
||||
, test "should report piping the functions directly" <|
|
||||
\() ->
|
||||
rule "a = List.map f >> List.map g"
|
||||
|> Expect.equal [ error ">>" "List.map" ]
|
||||
, test "should report piping the functions directly with additional pipe on the left" <|
|
||||
\() ->
|
||||
rule "a = foo >> List.map f >> List.map g"
|
||||
|> Expect.equal [ error ">>" "List.map" ]
|
||||
, test "should report piping the functions directly with additional pipe on the right" <|
|
||||
\() ->
|
||||
rule "a = List.map f >> List.map g >> bar"
|
||||
|> Expect.equal [ error ">>" "List.map" ]
|
||||
, test "should report piping the functions directly with additional pipe on both sides" <|
|
||||
\() ->
|
||||
rule "a = foo >> List.map f >> List.map g >> bar"
|
||||
|> Expect.equal [ error ">>" "List.map" ]
|
||||
, test "should report piping the functions directly, right to left" <|
|
||||
\() ->
|
||||
rule "a = List.map f << List.map g"
|
||||
|> Expect.equal [ error "<<" "List.map" ]
|
||||
, test "should report piping the functions directly with additional pipe on the left, right to left" <|
|
||||
\() ->
|
||||
rule "a = foo << List.map f << List.map g"
|
||||
|> Expect.equal [ error "<<" "List.map" ]
|
||||
, test "should report piping the functions directly with additional pipe on the right, right to left" <|
|
||||
\() ->
|
||||
rule "a = List.map f << List.map g << bar"
|
||||
|> Expect.equal [ error "<<" "List.map" ]
|
||||
, test "should report piping the functions directly with additional pipe on both sides, right to left" <|
|
||||
\() ->
|
||||
rule "a = foo << List.map f << List.map g << bar"
|
||||
|> Expect.equal [ error "<<" "List.map" ]
|
||||
]
|
||||
|
||||
|
||||
all : Test
|
||||
all =
|
||||
describe "SimplifyPiping" tests
|
Loading…
Reference in New Issue
Block a user