mirror of
https://github.com/jfmengels/elm-review.git
synced 2024-11-30 13:32:03 +03:00
138 lines
5.2 KiB
Elm
138 lines
5.2 KiB
Elm
module NoRedundantConcat exposing (rule)
|
|
|
|
{-|
|
|
|
|
@docs rule
|
|
|
|
-}
|
|
|
|
import Elm.Syntax.Expression as Expression exposing (Expression)
|
|
import Elm.Syntax.Infix as Infix
|
|
import Elm.Syntax.Node as Node exposing (Node)
|
|
import Elm.Syntax.Range exposing (Range)
|
|
import Review.Fix as Fix
|
|
import Review.Rule as Rule exposing (Error, Rule)
|
|
import Util
|
|
|
|
|
|
{-| Forbids using concatenation (`++` and `List.concat`) when it's not needed.
|
|
|
|
Expressions like `[ a, b ] ++ c` will be flagged, with a fix proposing to
|
|
rewrite that to somethine like `a :: b :: c`. This is more performant and makes
|
|
it clear that we're talking about consing items to the head of a list. Easy!
|
|
|
|
Expressions like `[ a, b ] ++ [ c, d ]` could be rewritten to a single literal
|
|
list: no need to perform the concatenation at runtime when we can just write it
|
|
ourselves! So, the fix will propose writing that as `[ a, b, c, d ]`.
|
|
|
|
Expression like `List.concat [ [ a ], [ b ], [ c ] ]` could become `[ a, b c ]`.
|
|
|
|
Finally, expressions like `"foo" ++ "bar"` can also be `"foobar"`.
|
|
|
|
To use this rule, add it to your `elm-review` config like so:
|
|
|
|
module ReviewConfig exposing (config)
|
|
|
|
import NoRedundantConcat
|
|
import Review.Rule exposing (Rule)
|
|
|
|
config : List Rule
|
|
config =
|
|
[ NoRedundantConcat.rule
|
|
]
|
|
|
|
-}
|
|
rule : Rule
|
|
rule =
|
|
Rule.newModuleRuleSchema "NoRedundantConcat" ()
|
|
|> Rule.withSimpleExpressionVisitor expressionVisitor
|
|
|> Rule.fromModuleRuleSchema
|
|
|
|
|
|
expressionVisitor : Node Expression -> List (Error {})
|
|
expressionVisitor (Node.Node range expression) =
|
|
case expression of
|
|
Expression.OperatorApplication "++" _ (Node.Node _ (Expression.ListExpr leftItems)) (Node.Node _ (Expression.ListExpr rightItems)) ->
|
|
[ Rule.errorWithFix
|
|
{ message = "Concatenating a literal list with another literal list can be written as a single list literal"
|
|
, details =
|
|
[ "Expressions like `[ foo ] ++ [ bar ]` can be written as `[ foo, bar ]`."
|
|
, "Using 'complex' expressions when not necessary can make code look a lot more complex than it really is. When you need to put two literal lists together, you can just put them together! No need to have that happen at runtime."
|
|
]
|
|
}
|
|
range
|
|
[ combineLists range leftItems rightItems ]
|
|
]
|
|
|
|
Expression.OperatorApplication "++" _ (Node.Node _ (Expression.Literal left)) (Node.Node _ (Expression.Literal right)) ->
|
|
[ Rule.errorWithFix
|
|
{ message = "Concatenating a literal string with another literal string is redundant"
|
|
, details =
|
|
[ "Expressions like `\"foo\" ++ \"bar\"` are harder to read than `\"foobar\"`. Consider simplifying this expression."
|
|
]
|
|
}
|
|
range
|
|
[ Fix.replaceRangeBy range (Util.expressionToString range (Expression.Literal (left ++ right))) ]
|
|
]
|
|
|
|
Expression.OperatorApplication "++" _ (Node.Node _ (Expression.ListExpr items)) right ->
|
|
[ Rule.errorWithFix
|
|
{ message = "Concatenating a literal list with something else can be written using cons operators"
|
|
, details =
|
|
[ "Expressions like `[ foo ] ++ b` can be written as `foo :: b`."
|
|
, "This preserves the mental model that `List` is a linked list, with the performance considerations associated with those."
|
|
]
|
|
}
|
|
range
|
|
[ concatItems range (List.reverse items) right ]
|
|
]
|
|
|
|
Expression.Application [ Node.Node _ (Expression.FunctionOrValue [ "List" ] "concat"), Node.Node _ (Expression.ListExpr items) ] ->
|
|
attemptToCombineItems range items []
|
|
|
|
_ ->
|
|
[]
|
|
|
|
|
|
attemptToCombineItems : Range -> List (Node Expression) -> List (Node Expression) -> List (Error {})
|
|
attemptToCombineItems range expressions acc =
|
|
case expressions of
|
|
[] ->
|
|
[ Rule.errorWithFix
|
|
{ message = "Using List.concat to concatenate list literals is redundant"
|
|
, details = [ "Rather than using `List.concat`, consider putting the elements into a single list literal" ]
|
|
}
|
|
range
|
|
[ Fix.replaceRangeBy range
|
|
(Util.expressionToString range (Expression.ListExpr acc))
|
|
]
|
|
]
|
|
|
|
(Node.Node _ (Expression.ListExpr items)) :: rest ->
|
|
attemptToCombineItems range rest (acc ++ items)
|
|
|
|
_ ->
|
|
[]
|
|
|
|
|
|
combineLists : Range -> List (Node Expression) -> List (Node Expression) -> Fix.Fix
|
|
combineLists range left right =
|
|
(left ++ right)
|
|
|> Expression.ListExpr
|
|
|> Util.expressionToString range
|
|
|> Fix.replaceRangeBy range
|
|
|
|
|
|
concatItems : Range -> List (Node Expression) -> Node Expression -> Fix.Fix
|
|
concatItems range items ((Node.Node ontoRange ontoExpr) as onto) =
|
|
case items of
|
|
[] ->
|
|
Fix.replaceRangeBy range (Util.expressionToString ontoRange ontoExpr)
|
|
|
|
item :: rest ->
|
|
concatItems range
|
|
rest
|
|
(Node.Node ontoRange
|
|
(Expression.OperatorApplication "::" Infix.Non item onto)
|
|
)
|