elm-review/tests/NoPrematureLetComputationTest.elm
2021-10-17 10:00:47 +02:00

1037 lines
26 KiB
Elm

module NoPrematureLetComputationTest exposing (all)
import NoPrematureLetComputation exposing (rule)
import Review.Test
import Test exposing (Test, describe, test)
-- TODO Handle destructuring lets with multiple variables
-- TODO Incorporate https://github.com/jfmengels/elm-review/discussions/93 ? As an option?
-- Not needed because this rule currently only targets is only for functions
message : String
message =
"Let value was declared prematurely"
details : Int -> List String
details letInsertLine =
[ "This value is only used in some code paths, and it can therefore be computed unnecessarily."
, "Try moving it closer to where it is needed, I recommend to move it to line " ++ String.fromInt letInsertLine ++ "."
]
all : Test
all =
describe "NoPrematureLetComputation"
[ baseTests
, ignoreFixTests
, letDestructuringTests
, novingIntoFunctionTests
]
baseTests : Test
baseTests =
describe "Base tests"
[ test "should report a let declaration that could be computed in a if branch" <|
\() ->
"""module A exposing (..)
a b c d =
let
z : Int
z = 1
in
if b then
z
else
1
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details 8
, under = "z"
}
|> Review.Test.atExactly { start = { row = 5, column = 5 }, end = { row = 5, column = 6 } }
|> Review.Test.whenFixed """module A exposing (..)
a b c d =
if b then
let
z : Int
z = 1
in
z
else
1
"""
]
, test "should report a let declaration that could be computed in a if branch (referenced by record update expression)" <|
\() ->
"""module A exposing (..)
a b c d =
let
z = {a = 1}
in
if b then
{z | a = 2}
else
{a = 3}
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details 7
, under = "z"
}
|> Review.Test.atExactly { start = { row = 4, column = 5 }, end = { row = 4, column = 6 } }
|> Review.Test.whenFixed """module A exposing (..)
a b c d =
if b then
let
z = {a = 1}
in
{z | a = 2}
else
{a = 3}
"""
]
, test "should add to an existing let block instead of inserting a new let block" <|
\() ->
"""module A exposing (..)
a b c d =
let
z = {a = 1}
in
if b then
let
y = 1
in
{z | a = 2}
else
{a = 3}
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details 8
, under = "z"
}
|> Review.Test.atExactly { start = { row = 4, column = 5 }, end = { row = 4, column = 6 } }
|> Review.Test.whenFixed """module A exposing (..)
a b c d =
if b then
let
z = {a = 1}
y = 1
in
{z | a = 2}
else
{a = 3}
"""
]
, test "should not report let functions" <|
\() ->
"""module A exposing (..)
a b c d =
let
z n = 1
in
if b then
z 1
else
1
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
, test "should not report a let declaration is used in multiple if branches" <|
\() ->
"""module A exposing (..)
a b c d =
let
z = 1
in
if b then
z
else
z + 1
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
, test "should not report a let declaration if it's unused" <|
\() ->
"""module A exposing (..)
a b c d =
let
z = 1
in
if b then
1
else
2
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
, test "should not report a let declaration is used next to where it was declared" <|
\() ->
"""module A exposing (..)
a b c d =
let
z = 1
y = z * 2
in
if b then
y
else
y + 1
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
, test "should not report a let declaration without branches" <|
\() ->
"""module A exposing (..)
a b c d =
let
z = 1
in
z
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
, test "should report a let declaration that could be computed in a case branch" <|
\() ->
"""module A exposing (..)
a b c d =
let
z = 1
in
case b of
A ->
1
B ->
z
C ->
1
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details 10
, under = "z"
}
|> Review.Test.atExactly { start = { row = 4, column = 5 }, end = { row = 4, column = 6 } }
|> Review.Test.whenFixed """module A exposing (..)
a b c d =
case b of
A ->
1
B ->
let
z = 1
in
z
C ->
1
"""
]
, test "should not report a let declaration is used in multiple case branches" <|
\() ->
"""module A exposing (..)
a b c d =
let
z = 1
in
case b of
A ->
z
B ->
z
C ->
1
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
, test "should only remove the moved let declaration when there are other let declarations there" <|
\() ->
"""module A exposing (..)
a b c d =
let
y = 1
z = {a = 1}
in
if b then
{z | a = 2}
else
{a = 3}
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details 8
, under = "z"
}
|> Review.Test.atExactly { start = { row = 5, column = 5 }, end = { row = 5, column = 6 } }
|> Review.Test.whenFixed """module A exposing (..)
a b c d =
let
y = 1
in
if b then
let
z = {a = 1}
in
{z | a = 2}
else
{a = 3}
"""
]
, test "should not be confused by what happens in other declarations" <|
\() ->
"""module A exposing (..)
first =
case A of
A -> z
second =
let
z = let x = 1 in x
y = z
in
y
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
, test "should not report when a declaration is used in multiple sibling's declarations" <|
\() ->
"""module A exposing (..)
fun =
let
z = 1
y = z
x = z
in
1
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
, test "should not report when a declaration is used in multiple sibling's branches" <|
\() ->
"""module A exposing (..)
fun =
let
z = 1
y = if True then z else 1
x = if True then z else 1
in
1
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
, test "should not report when a declaration is used in a branch of a sibling's declaration" <|
\() ->
"""module A exposing (..)
a =
let
z = 1
viewInput =
let
submitOnEnter =
if True then
z
else
1
in
submitOnEnter
viewButtonRow =
z
in
1
a = 1
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
, test "should move a let declaration from one let to the other" <|
\() ->
"""module A exposing (..)
a =
let
z = 1
in
let
y = 1
x = 1
in
y + z
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details 7
, under = "z"
}
|> Review.Test.atExactly { start = { row = 4, column = 5 }, end = { row = 4, column = 6 } }
|> Review.Test.whenFixed """module A exposing (..)
a =
let
z = 1
y = 1
x = 1
in
y + z
"""
]
, test "should properly indent moved let declaration defined on multiple lines" <|
\() ->
"""module A exposing (..)
a o =
let
z =
foo
bar
baz
in
thing
(if o.expDetail then
let
y =
foo2
bar2
baz2
in
[ y
, z
]
else
[]
)
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details 12
, under = "z"
}
|> Review.Test.atExactly { start = { row = 4, column = 9 }, end = { row = 4, column = 10 } }
|> Review.Test.whenFixed """module A exposing (..)
a o =
thing
(if o.expDetail then
let
z =
foo
bar
baz
y =
foo2
bar2
baz2
in
[ y
, z
]
else
[]
)
"""
]
]
ignoreFixTests : Test
ignoreFixTests =
describe "Ignoring automatic fixes"
[ test "should not suggest a fix for let declarations that introduce variables in their implementation (lambda)" <|
\() ->
"""module A exposing (..)
a b c d =
let
z : Int
z = \\y -> y + 1
in
case b of
A y ->
if b then
z
else
1
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details 10
, under = "z"
}
|> Review.Test.atExactly { start = { row = 5, column = 5 }, end = { row = 5, column = 6 } }
]
, test "should suggest a fix for lambda that does not introduce variables" <|
\() ->
"""module A exposing (..)
a b c d =
let
z = \\() _ -> 1
in
case b of
A y ->
if b then
z
else
1
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details 9
, under = "z"
}
|> Review.Test.atExactly { start = { row = 4, column = 5 }, end = { row = 4, column = 6 } }
|> Review.Test.whenFixed
"""module A exposing (..)
a b c d =
case b of
A y ->
if b then
let
z = \\() _ -> 1
in
z
else
1
"""
]
, test "should not suggest a fix for let declarations that introduce variables in their implementation (let block)" <|
\() ->
"""module A exposing (..)
a b c d =
let
z : Int
z = let y = 1
in y
in
case b of
A y ->
if b then
z
else
1
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details 11
, under = "z"
}
|> Review.Test.atExactly { start = { row = 5, column = 5 }, end = { row = 5, column = 6 } }
]
, test "should not suggest a fix for let declarations that introduce variables in their implementation but still suggest fixes for others" <|
\() ->
"""module A exposing (..)
a b c d =
let
z : Int
z = \\y -> y + 1
x = 1
in
case b of
A y ->
if b then
z
else
x
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details 11
, under = "z"
}
|> Review.Test.atExactly { start = { row = 5, column = 5 }, end = { row = 5, column = 6 } }
, Review.Test.error
{ message = message
, details = details 13
, under = "x"
}
|> Review.Test.atExactly { start = { row = 6, column = 5 }, end = { row = 6, column = 6 } }
|> Review.Test.whenFixed """module A exposing (..)
a b c d =
let
z : Int
z = \\y -> y + 1
in
case b of
A y ->
if b then
z
else
let
x = 1
in
x
"""
]
, test "should not suggest a fix for let declarations that introduce variables in their implementation (case expression)" <|
\() ->
"""module A exposing (..)
a b c d =
let
z : Int
z = case c of
B y -> y + 1
in
case b of
A y ->
if b then
z
else
1
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details 11
, under = "z"
}
|> Review.Test.atExactly { start = { row = 5, column = 5 }, end = { row = 5, column = 6 } }
]
, test "should suggest a fix for let declarations that do not introduce variables in their implementation (case expression)" <|
\() ->
"""module A exposing (..)
a b c d =
let
z = case c of
B -> 1
in
case b of
A y ->
if b then
z
else
1
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details 10
, under = "z"
}
|> Review.Test.atExactly { start = { row = 4, column = 5 }, end = { row = 4, column = 6 } }
|> Review.Test.whenFixed """module A exposing (..)
a b c d =
case b of
A y ->
if b then
let
z = case c of
B -> 1
in
z
else
1
"""
]
]
letDestructuringTests : Test
letDestructuringTests =
describe "Let destructuring"
[ test "should report a let destructuring with a single value (record destructuring)" <|
\() ->
"""module A exposing (..)
a b c d =
let
{z} = {z = 1}
in
if b then
z
else
1
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details 7
, under = "z"
}
|> Review.Test.atExactly { start = { row = 4, column = 6 }, end = { row = 4, column = 7 } }
|> Review.Test.whenFixed """module A exposing (..)
a b c d =
if b then
let
{z} = {z = 1}
in
z
else
1
"""
]
, test "should report a let destructuring with a single value (named pattern destructuring)" <|
\() ->
"""module A exposing (..)
a =
let
(Foo z) =
point
in
if condition then
let
b =
1
in
z
else
[]
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details 9
, under = "z"
}
|> Review.Test.atExactly { start = { row = 4, column = 14 }, end = { row = 4, column = 15 } }
|> Review.Test.whenFixed """module A exposing (..)
a =
if condition then
let
(Foo z) =
point
b =
1
in
z
else
[]
"""
]
]
novingIntoFunctionTests : Test
novingIntoFunctionTests =
describe "Moving into functions"
[ test "should not report let declaration that would be moved to inside a lambda" <|
\() ->
"""module A exposing (..)
a =
let
z = 1
in
(\\b ->
if b then
z
else
1
)
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
, test "should try to move as close as possible to a lambda but not inside" <|
\() ->
"""module A exposing (..)
a =
let
z = 1
in
if c then
(\\b ->
if b then
z
else
1
)
else
identity
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details 7
, under = "z"
}
|> Review.Test.atExactly { start = { row = 4, column = 5 }, end = { row = 4, column = 6 } }
|> Review.Test.whenFixed """module A exposing (..)
a =
if c then
let
z = 1
in
(\\b ->
if b then
z
else
1
)
else
identity
"""
]
, test "should not move declaration inside of a let function" <|
\() ->
"""module A exposing (..)
a b c d =
let
z = 1
y () =
if cond then
z
else
1
in
y ()
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
, test "should report let declaration that could be moved to inside a lambda passed to Maybe.map" <|
\() ->
"""module A exposing (..)
a =
let
z = 1
in
Maybe.map
(\\b ->
z
)
x
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details 8
, under = "z"
}
|> Review.Test.atExactly { start = { row = 4, column = 5 }, end = { row = 4, column = 6 } }
|> Review.Test.whenFixed """module A exposing (..)
a =
Maybe.map
(\\b ->
let
z = 1
in
z
)
x
"""
]
, test "should report let declaration that could be moved to inside a lambda passed to Maybe.map2" <|
\() ->
"""module A exposing (..)
a =
let
z = 1
in
Maybe.map2
(\\b c ->
z
)
x
y
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details 8
, under = "z"
}
|> Review.Test.atExactly { start = { row = 4, column = 5 }, end = { row = 4, column = 6 } }
|> Review.Test.whenFixed """module A exposing (..)
a =
Maybe.map2
(\\b c ->
let
z = 1
in
z
)
x
y
"""
]
, test "should not report reference to inside a lambda passed to Maybe.map if there are missing arguments to Maybe.map" <|
\() ->
"""module A exposing (..)
a =
let
z = 1
maybeZ =
Maybe.map
(\\b ->
z
)
in
Maybe.map2
Tuple.pair
(maybeZ (Just x))
(maybeZ (Just y))
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
, test "should not report reference to inside a lambda passed to Maybe.map2 if there are missing arguments to Maybe.map2" <|
\() ->
"""module A exposing (..)
a =
let
z = 1
maybeZ =
Maybe.map2
(\\b c ->
z
)
x
in
Maybe.map2
Tuple.pair
(maybeZ (Just x))
(maybeZ (Just y))
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
, test "should not report reference to inside a lambda passed to List.map" <|
\() ->
"""module A exposing (..)
a =
let
z = 1
in
List.map
(\\b ->
z
)
x
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
, test "should report let declaration that could be moved to inside a lambda passed to Maybe.map using |>" <|
\() ->
"""module A exposing (..)
a =
let
z = 1
in
x
|> Maybe.map
(\\b ->
z
)
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details 9
, under = "z"
}
|> Review.Test.atExactly { start = { row = 4, column = 5 }, end = { row = 4, column = 6 } }
|> Review.Test.whenFixed """module A exposing (..)
a =
x
|> Maybe.map
(\\b ->
let
z = 1
in
z
)
"""
]
, test "should not report reference to inside a lambda passed to Maybe.map2 if there are missing arguments to Maybe.map (using |>)" <|
\() ->
"""module A exposing (..)
a =
let
z = 1
maybeZ =
x
|> Maybe.map2
(\\b ->
z
)
in
Maybe.map2
Tuple.pair
(maybeZ (Just x))
(maybeZ (Just y))
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
, test "should report let declaration that could be moved to inside a lambda passed to Maybe.map using <|" <|
\() ->
"""module A exposing (..)
a =
let
z = 1
in
Maybe.map
(\\b ->
z
) <| x
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details 8
, under = "z"
}
|> Review.Test.atExactly { start = { row = 4, column = 5 }, end = { row = 4, column = 6 } }
|> Review.Test.whenFixed """module A exposing (..)
a =
Maybe.map
(\\b ->
let
z = 1
in
z
) <| x
"""
]
, test "should not report reference to inside a lambda passed to Maybe.map2 if there are missing arguments to Maybe.map (using <|)" <|
\() ->
"""module A exposing (..)
a =
let
z = 1
maybeZ =
Maybe.map2
(\\b ->
z
) <| x
in
Maybe.map2
Tuple.pair
(maybeZ (Just x))
(maybeZ (Just y))
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
]