diff --git a/CHANGELOG.md b/CHANGELOG.md index a56bc39ad..68ff32f9c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -85,6 +85,9 @@ - `truncate 1` to `1` - `round (toFloat n)` to `n` (and same for ceiling, floor, truncate) - `(Record first second).first` to `first` +- `List.drop -1 list` to `list` +- `List.drop 3 [ a, b ]` to `[]` (same for lists with determined size like `List.singleton`) +- `List.drop 2 [ a, b, c ]` to `[ c ]` Bug fixes: - Fixed an issue where `Dict.intersect Dict.empty` would be fixed to `Dict.empty` diff --git a/src/Simplify.elm b/src/Simplify.elm index 43b7d44a8..dfd73f11e 100644 --- a/src/Simplify.elm +++ b/src/Simplify.elm @@ -786,6 +786,12 @@ Destructuring using case expressions List.drop 0 list --> list + List.drop 3 [ a, b ] + --> [] + + List.drop 2 [ a, b, c ] + --> [ c ] + List.reverse [] --> [] @@ -4912,6 +4918,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 @@ -4923,10 +4945,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 @@ -7138,19 +7157,102 @@ 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 - ) + Evaluate.getInt checkInfo checkInfo.firstArg + |> Maybe.andThen + (\count -> + firstThatConstructsJust + [ \() -> + callWithNonPositiveIntCheckErrorSituation + { int = count, intDescription = "count", fn = checkInfo.fn } + |> Maybe.map + (\situation -> alwaysReturnsLastArgError situation listCollection checkInfo) + , \() -> dropOnSmallerCollectionCheck { dropCount = count } listCollection checkInfo + , \() -> + dropOnLargerConstructionFromListLiteralWillRemoveTheseElementsCheck { dropCount = count } + listCollection + checkInfo + ] + () + ) + , unnecessaryCallOnEmptyCheck listCollection + ] + + +{-| The drop check + + drop n (collection with size <= n) --> empty + +So for example + + Array.drop 3 (Array.repeat 2 a) --> [] + +-} +dropOnSmallerCollectionCheck : { dropCount : Int } -> TypeProperties (CollectionProperties (EmptiableProperties ConstantProperties otherProperties)) -> CheckInfo -> Maybe (Error {}) +dropOnSmallerCollectionCheck config collection checkInfo = + case fullyAppliedLastArg checkInfo of + Just listArg -> + case listDetermineLength checkInfo listArg of + Just (Exactly length) -> + if config.dropCount >= length then + Just + (alwaysResultsInUnparenthesizedConstantError + (qualifiedToString checkInfo.fn ++ " with a count greater than or equal to the given " ++ collection.represents ++ "'s length") + { replacement = collection.empty.asString } + checkInfo + ) + + else + Nothing _ -> Nothing - , unnecessaryCallOnEmptyCheck listCollection - ] + + Nothing -> + Nothing + + +{-| The drop check + + drop n (fromList on list with size > n) + --> (fromList on list with the first n elements removed) + +So for example + + Array.drop 2 (Array.fromList [ a, b, c ]) + --> Array.fromList [ c ] + +-} +dropOnLargerConstructionFromListLiteralWillRemoveTheseElementsCheck : { dropCount : Int } -> TypeProperties (ConstructibleFromListProperties otherProperties) -> CheckInfo -> Maybe (Error {}) +dropOnLargerConstructionFromListLiteralWillRemoveTheseElementsCheck config constructibleFromList checkInfo = + case fullyAppliedLastArg checkInfo of + Just lastArg -> + case fromListGetLiteral constructibleFromList checkInfo.lookupTable lastArg of + Just fromListLiteral -> + case List.drop config.dropCount fromListLiteral.elements of + (Node elementAfterDroppedRange _) :: _ -> + Just + (Rule.errorWithFix + { message = qualifiedToString checkInfo.fn ++ " with a count less than the given " ++ constructibleFromList.represents ++ "'s length will remove these elements" + , details = [ "You can remove the first " ++ String.fromInt config.dropCount ++ " elements from the " ++ constructionFromListOnLiteralDescription constructibleFromList.fromList ++ "." ] + } + checkInfo.fnRange + (keepOnlyFix { parentRange = checkInfo.parentRange, keep = Node.range lastArg } + ++ [ Fix.removeRange + { start = startWithoutBoundary fromListLiteral.range + , end = elementAfterDroppedRange.start + } + ] + ) + ) + + [] -> + Nothing + + Nothing -> + Nothing + + Nothing -> + Nothing emptiableMapNChecks : TypeProperties (EmptiableProperties ConstantProperties otherProperties) -> CheckInfo -> Maybe (Error {}) @@ -10805,6 +10907,16 @@ collectionUnionChecks config collection = ] +constructionFromListOnLiteralDescription : ConstructionFromList -> String +constructionFromListOnLiteralDescription fromListConstruction = + case fromListConstruction of + ConstructionAsList -> + "list literal" + + ConstructionFromListCall fn -> + qualifiedToString fn ++ " call" + + collectionUnionWithLiteralsChecks : { leftElementsStayOnTheLeft : Bool } -> @@ -10829,12 +10941,7 @@ collectionUnionWithLiteralsChecks config operationInfo collection checkInfo = let fromListLiteralDescription : String fromListLiteralDescription = - case collection.fromList of - ConstructionAsList -> - "list literal" - - ConstructionFromListCall fn -> - qualifiedToString fn ++ " call" + constructionFromListOnLiteralDescription collection.fromList in Just (Rule.errorWithFix diff --git a/tests/Simplify/ListTest.elm b/tests/Simplify/ListTest.elm index b47cf3168..afc100385 100644 --- a/tests/Simplify/ListTest.elm +++ b/tests/Simplify/ListTest.elm @@ -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" , details = [ "You can replace this call by the list itself." ] , under = "List.drop" } @@ -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" } @@ -7150,12 +7150,92 @@ 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" + } + |> Review.Test.whenFixed """module A exposing (..) +a = identity +""" + ] + , test "should replace [a, b, c] |> List.drop 2 by [c]" <| + \() -> + """module A exposing (..) +a = [b, c, d] |> List.drop 2 +""" + |> Review.Test.run ruleWithDefaults + |> Review.Test.expectErrors + [ Review.Test.error + { message = "List.drop with a count less than the given list's length will remove these elements" + , details = [ "You can remove the first 2 elements from the list literal." ] + , under = "List.drop" + } + |> Review.Test.whenFixed """module A exposing (..) +a = [d] +""" + ] + , test "should replace List.singleton a |> List.drop 1 by []" <| + \() -> + """module A exposing (..) +a = List.singleton b |> List.drop 1 +""" + |> Review.Test.run ruleWithDefaults + |> Review.Test.expectErrors + [ Review.Test.error + { message = "List.drop with a count greater than or equal to the given list's length will always result in []" + , details = [ "You can replace this call by []." ] + , under = "List.drop" + } + |> Review.Test.whenFixed """module A exposing (..) +a = [] +""" + ] + , test "should replace [ a, b ] |> List.drop 2 by []" <| + \() -> + """module A exposing (..) +a = [ b, c ] |> List.drop 2 +""" + |> Review.Test.run ruleWithDefaults + |> Review.Test.expectErrors + [ Review.Test.error + { message = "List.drop with a count greater than or equal to the given list's length will always result in []" + , details = [ "You can replace this call by []." ] + , under = "List.drop" + } + |> Review.Test.whenFixed """module A exposing (..) +a = [] """ ] ]