Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More List.drop simplifications #270

Merged
merged 3 commits into from
Oct 10, 2023
Merged

More List.drop simplifications #270

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4624190
Add List.drop -literal check by introducing callWithNonPositiveIntChe…
lue-bird Oct 10, 2023
6c00cb4
Add List.drop tests for lists with known length
lue-bird Oct 10, 2023
23f386a
Add latest List.drop simplifications to changelog
lue-bird Oct 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 24 additions & 15 deletions src/Simplify.elm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4912,6 +4912,22 @@ callWithNonPositiveIntCanBeReplacedByCheck :
-> CheckInfo
-> Maybe (Error {})
callWithNonPositiveIntCanBeReplacedByCheck config checkInfo =
callWithNonPositiveIntCheckErrorSituation { fn = checkInfo.fn, int = config.int, intDescription = config.intDescription }
|> Maybe.map
(\situation ->
alwaysResultsInUnparenthesizedConstantError situation
{ replacement = config.replacement }
checkInfo
)


callWithNonPositiveIntCheckErrorSituation :
{ fn : ( ModuleName, String )
, int : number
, intDescription : String
}
-> Maybe String
callWithNonPositiveIntCheckErrorSituation config =
if config.int <= 0 then
let
lengthDescription : String
Expand All @@ -4923,10 +4939,7 @@ callWithNonPositiveIntCanBeReplacedByCheck config checkInfo =
config.intDescription ++ " 0"
in
Just
(alwaysResultsInUnparenthesizedConstantError (qualifiedToString checkInfo.fn ++ " with " ++ lengthDescription)
{ replacement = config.replacement }
checkInfo
)
(qualifiedToString config.fn ++ " with " ++ lengthDescription)

else
Nothing
Expand Down Expand Up @@ -7138,17 +7151,13 @@ listDropChecks : CheckInfo -> Maybe (Error {})
listDropChecks =
firstThatConstructsJust
[ \checkInfo ->
case Evaluate.getInt checkInfo checkInfo.firstArg of
Just 0 ->
Just
(alwaysReturnsLastArgError
(qualifiedToString checkInfo.fn ++ " 0")
{ represents = "list" }
checkInfo
)

_ ->
Nothing
Evaluate.getInt checkInfo checkInfo.firstArg
|> Maybe.andThen
(\int ->
callWithNonPositiveIntCheckErrorSituation { int = int, intDescription = "count", fn = checkInfo.fn }
)
|> Maybe.map
(\situation -> alwaysReturnsLastArgError situation listCollection checkInfo)
, unnecessaryCallOnEmptyCheck listCollection
]

Expand Down
38 changes: 35 additions & 3 deletions tests/Simplify/ListTest.elm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7118,7 +7118,7 @@ a = List.drop 0 list
|> Review.Test.run ruleWithDefaults
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "List.drop 0 will always return the same given list"
{ message = "List.drop with count 0 will always return the same given list"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like the previous error message was better 🤷‍♂️

, details = [ "You can replace this call by the list itself." ]
, under = "List.drop"
}
Expand All @@ -7134,7 +7134,7 @@ a = list |> List.drop 0
|> Review.Test.run ruleWithDefaults
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "List.drop 0 will always return the same given list"
{ message = "List.drop with count 0 will always return the same given list"
, details = [ "You can replace this call by the list itself." ]
, under = "List.drop"
}
Expand All @@ -7150,7 +7150,39 @@ a = List.drop 0
|> Review.Test.run ruleWithDefaults
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "List.drop 0 will always return the same given list"
{ message = "List.drop with count 0 will always return the same given list"
, details = [ "You can replace this call by identity." ]
, under = "List.drop"
}
|> Review.Test.whenFixed """module A exposing (..)
a = identity
"""
]
, test "should replace list |> List.drop -1 by list" <|
\() ->
"""module A exposing (..)
a = list |> List.drop -1
"""
|> Review.Test.run ruleWithDefaults
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "List.drop with negative count will always return the same given list"
, details = [ "You can replace this call by the list itself." ]
, under = "List.drop"
}
|> Review.Test.whenFixed """module A exposing (..)
a = list
"""
]
, test "should replace List.drop -1 by identity" <|
\() ->
"""module A exposing (..)
a = List.drop -1
"""
|> Review.Test.run ruleWithDefaults
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "List.drop with negative count will always return the same given list"
, details = [ "You can replace this call by identity." ]
, under = "List.drop"
}
Expand Down