mirror of
https://github.com/jfmengels/elm-review.git
synced 2024-11-27 02:19:27 +03:00
Backport rules from other packages
This commit is contained in:
parent
775571b73b
commit
8ab6272a39
713
tests/CognitiveComplexity.elm
Normal file
713
tests/CognitiveComplexity.elm
Normal file
@ -0,0 +1,713 @@
|
||||
module CognitiveComplexity exposing (rule)
|
||||
|
||||
{-|
|
||||
|
||||
@docs rule
|
||||
|
||||
-}
|
||||
|
||||
import Dict exposing (Dict)
|
||||
import Elm.Syntax.Declaration as Declaration exposing (Declaration)
|
||||
import Elm.Syntax.Expression as Expression exposing (Expression)
|
||||
import Elm.Syntax.Node as Node exposing (Node)
|
||||
import Elm.Syntax.Range exposing (Location, Range)
|
||||
import Review.Rule as Rule exposing (Rule)
|
||||
import Set exposing (Set)
|
||||
|
||||
|
||||
{-| Reports functions that have a too high cognitive complexity.
|
||||
|
||||
You can configure the threshold above which a function will be reported (`15` in the example configuration below).
|
||||
|
||||
config =
|
||||
[ CognitiveComplexity.rule 15
|
||||
]
|
||||
|
||||
|
||||
## What is cognitive complexity?
|
||||
|
||||
Cognitive complexity is **not to be confused with "Cyclomatic Complexity"**, which has a different way of measuring the
|
||||
complexity.
|
||||
|
||||
Here's an explanation extracted from [free white paper](https://www.sonarsource.com/resources/white-papers/cognitive-complexity/)
|
||||
provided by SonarSource, the creators of the concept.
|
||||
|
||||
> Cognitive complexity tries to measure how hard it is to understand a function, primarily focusing on the control structures
|
||||
> that hinder the understanding of a function by reading it from top to bottom in one go, like you would for a novel.
|
||||
|
||||
> A Cognitive Complexity score is assessed according to three basic rules:
|
||||
|
||||
> 1. Ignore structures that allow multiple statements to be readably shorthanded into one
|
||||
> 2. Increment (add one) for each break in the linear flow of the code
|
||||
> 3. Increment when flow-breaking structures are nested
|
||||
|
||||
Some small differences may be found between the implementation detailed in the paper and this rule, as the idea was
|
||||
formulated more on imperative programming languages, and may not be applicable to a pure functional language like Elm.
|
||||
|
||||
You can read about how is works in the [complexity breakdown section](#complexity-breakdown) below.
|
||||
|
||||
|
||||
## When (not) to enable this rule
|
||||
|
||||
This rule is an experiment. I don't know if this will be more useful or detrimental, and I haven't yet figured out what
|
||||
the ideal complexity threshold for Elm projects is.
|
||||
|
||||
I would for now recommend to use it with a very high threshold to find places in your codebase that need refactoring,
|
||||
and eventually to enable it in your configuration to make sure no new extremely complex functions appear. As you refactor more
|
||||
and more of your codebase, you can gradually lower the threshold until you reach a level that you feel happy with.
|
||||
|
||||
Please let me know how enabling this rule works out for you!
|
||||
|
||||
|
||||
## Complexity breakdown
|
||||
|
||||
Following is a breakdown of how the complexity of a function is computed:
|
||||
|
||||
- If expression: Increases complexity by 1 + nesting, and increases nesting.
|
||||
`else if` also increases complexity by 1 + nesting, but doesn't further increase the nesting.
|
||||
`else` doesn't increase the complexity.
|
||||
|
||||
```js
|
||||
-- Total: 4
|
||||
a =
|
||||
if b then -- +1
|
||||
if c then -- +2, including 1 for nesting
|
||||
1
|
||||
else
|
||||
2
|
||||
else if d then -- +1
|
||||
3
|
||||
else -- +0
|
||||
4
|
||||
```
|
||||
|
||||
- Case expression: Increases complexity by 1 + nesting, regardless of how many cases are handled, and increases nesting.
|
||||
|
||||
```js
|
||||
-- Total: 3
|
||||
a =
|
||||
case b of -- +1
|
||||
A -> 1
|
||||
B -> 2
|
||||
C ->
|
||||
case c of -- +2, including 1 for nesting
|
||||
_ -> 3
|
||||
D -> 4
|
||||
```
|
||||
|
||||
- Let functions: Increases nesting.
|
||||
|
||||
```js
|
||||
-- Total: 2
|
||||
a =
|
||||
let
|
||||
fn b = -- increases nesting
|
||||
if b then -- +2, including 1 for nesting
|
||||
1
|
||||
else
|
||||
2
|
||||
|
||||
constant = -- Not a function, no increase
|
||||
True
|
||||
in
|
||||
fn constant
|
||||
```
|
||||
|
||||
- Anonymous functions: Increases nesting.
|
||||
|
||||
```js
|
||||
-- Total: 2
|
||||
a things =
|
||||
List.map
|
||||
(\thing -> -- increases nesting
|
||||
case thing of -- +2, including 1 for nesting
|
||||
Just _ -> 1
|
||||
Nothing -> 2
|
||||
)
|
||||
things
|
||||
```
|
||||
|
||||
- Logical operator suites: Increase complexity by 1 for every discontinuation of the suite
|
||||
|
||||
```elm
|
||||
a && b -- +1
|
||||
|
||||
-- This is still the same logical construction as
|
||||
-- above, and therefore about as hard to understand
|
||||
a && b && c && d && e -- +1
|
||||
|
||||
-- Total: 3
|
||||
a && b && c -- +1
|
||||
|| d -- +1 for breaking the chain of && with a ||
|
||||
|| not (e || f) -- +1 for breaking the chain
|
||||
-- with a `not` of a binary operation
|
||||
```
|
||||
|
||||
- Recursive function calls (direct or indirect): Increase complexity by 1 for each different call
|
||||
|
||||
```js
|
||||
-- Total: 2
|
||||
fun1 n =
|
||||
fun2 n -- +1
|
||||
+ fun2 n -- +0, already counted
|
||||
+ fun1 n -- +1
|
||||
|
||||
-- Total: 1
|
||||
fun2 n =
|
||||
fun1 n -- +1
|
||||
```
|
||||
|
||||
[The original metric](https://www.sonarsource.com/docs/CognitiveComplexity.pdf) increases the complexity for other
|
||||
structures that the Elm language doesn't have.
|
||||
|
||||
|
||||
## Try it out
|
||||
|
||||
You can try this rule out by running the following command:
|
||||
|
||||
```bash
|
||||
elm-review --template jfmengels/elm-review-cognitive-complexity/example --rules CognitiveComplexity
|
||||
```
|
||||
|
||||
The cognitive complexity is set to 15 in the configuration used by the example.
|
||||
|
||||
|
||||
## Thanks
|
||||
|
||||
Thanks to the team at SonarSource for designing the metric and for not restricting its use.
|
||||
Thanks to G. Ann Campbell for the different talks she made on the subject.
|
||||
|
||||
-}
|
||||
rule : Int -> Rule
|
||||
rule threshold =
|
||||
Rule.newModuleRuleSchema "CognitiveComplexity" initialContext
|
||||
|> Rule.withDeclarationExitVisitor declarationExitVisitor
|
||||
|> Rule.withExpressionEnterVisitor expressionEnterVisitor
|
||||
|> Rule.withExpressionExitVisitor expressionExitVisitor
|
||||
|> Rule.withFinalModuleEvaluation (finalEvaluation threshold)
|
||||
|> Rule.fromModuleRuleSchema
|
||||
|
||||
|
||||
type alias Context =
|
||||
{ nesting : Int
|
||||
, operandsToIgnore : List Range
|
||||
, elseIfToIgnore : List Range
|
||||
, rangesWhereNestingIncreases : List Range
|
||||
, increases : List Increase
|
||||
, references : Dict String Location
|
||||
, functionsToReport : List FunctionToReport
|
||||
}
|
||||
|
||||
|
||||
type alias FunctionToReport =
|
||||
{ functionName : Node String
|
||||
, increases : List Increase
|
||||
, references : Dict String Location
|
||||
}
|
||||
|
||||
|
||||
type alias Increase =
|
||||
{ line : Location
|
||||
, increase : Int
|
||||
, nesting : Int
|
||||
, kind : IncreaseKind
|
||||
}
|
||||
|
||||
|
||||
type IncreaseKind
|
||||
= If
|
||||
| ElseIf
|
||||
| Case
|
||||
| Operator String
|
||||
| RecursiveCall
|
||||
| IndirectRecursiveCall String
|
||||
|
||||
|
||||
initialContext : Context
|
||||
initialContext =
|
||||
{ nesting = 0
|
||||
, operandsToIgnore = []
|
||||
, elseIfToIgnore = []
|
||||
, rangesWhereNestingIncreases = []
|
||||
, references = Dict.empty
|
||||
, increases = []
|
||||
, functionsToReport = []
|
||||
}
|
||||
|
||||
|
||||
expressionEnterVisitor : Node Expression -> Context -> ( List nothing, Context )
|
||||
expressionEnterVisitor node context =
|
||||
if List.member (Node.range node) context.rangesWhereNestingIncreases then
|
||||
( [], expressionEnterVisitorHelp node { context | nesting = context.nesting + 1 } )
|
||||
|
||||
else
|
||||
( [], expressionEnterVisitorHelp node context )
|
||||
|
||||
|
||||
expressionEnterVisitorHelp : Node Expression -> Context -> Context
|
||||
expressionEnterVisitorHelp node context =
|
||||
case Node.value node of
|
||||
Expression.IfBlock _ _ else_ ->
|
||||
if not (List.member (Node.range node) context.elseIfToIgnore) then
|
||||
{ context
|
||||
| increases =
|
||||
{ line = (Node.range node).start
|
||||
, increase = context.nesting + 1
|
||||
, nesting = context.nesting
|
||||
, kind = If
|
||||
}
|
||||
:: context.increases
|
||||
, nesting = context.nesting + 1
|
||||
, elseIfToIgnore = Node.range else_ :: context.elseIfToIgnore
|
||||
}
|
||||
|
||||
else
|
||||
-- This if expression is an else if
|
||||
-- We want to increase the complexity but keep the same nesting as the parent if
|
||||
{ context
|
||||
| increases =
|
||||
{ line = (Node.range node).start
|
||||
, increase = context.nesting
|
||||
, nesting = context.nesting - 1
|
||||
, kind = ElseIf
|
||||
}
|
||||
:: context.increases
|
||||
, elseIfToIgnore = Node.range else_ :: context.elseIfToIgnore
|
||||
}
|
||||
|
||||
Expression.CaseExpression _ ->
|
||||
{ context
|
||||
| increases =
|
||||
{ line = (Node.range node).start
|
||||
, increase = context.nesting + 1
|
||||
, nesting = context.nesting
|
||||
, kind = Case
|
||||
}
|
||||
:: context.increases
|
||||
, nesting = context.nesting + 1
|
||||
}
|
||||
|
||||
Expression.LetExpression { declarations } ->
|
||||
{ context | rangesWhereNestingIncreases = computeRangesForLetDeclarations declarations ++ context.rangesWhereNestingIncreases }
|
||||
|
||||
Expression.OperatorApplication operator _ left right ->
|
||||
if (operator == "&&" || operator == "||") && not (List.member (Node.range node) context.operandsToIgnore) then
|
||||
let
|
||||
( increases, operandsToIgnore ) =
|
||||
incrementAndIgnoreForOperands
|
||||
operator
|
||||
[]
|
||||
left
|
||||
right
|
||||
in
|
||||
{ context
|
||||
| increases =
|
||||
{ line = (Node.range node).start
|
||||
, increase = 1
|
||||
, nesting = 0
|
||||
, kind = Operator operator
|
||||
}
|
||||
:: increases
|
||||
++ context.increases
|
||||
, operandsToIgnore = operandsToIgnore ++ context.operandsToIgnore
|
||||
}
|
||||
|
||||
else
|
||||
context
|
||||
|
||||
Expression.LambdaExpression _ ->
|
||||
{ context | nesting = context.nesting + 1 }
|
||||
|
||||
Expression.FunctionOrValue [] name ->
|
||||
{ context
|
||||
| references =
|
||||
if Dict.member name context.references then
|
||||
-- The reference already exists, and we want to keep the first reference
|
||||
-- for a better presentation
|
||||
context.references
|
||||
|
||||
else
|
||||
Dict.insert name (Node.range node).start context.references
|
||||
}
|
||||
|
||||
_ ->
|
||||
context
|
||||
|
||||
|
||||
computeRangesForLetDeclarations : List (Node Expression.LetDeclaration) -> List Range
|
||||
computeRangesForLetDeclarations declarations =
|
||||
List.filterMap
|
||||
(\letDecl ->
|
||||
case Node.value letDecl of
|
||||
Expression.LetFunction { declaration } ->
|
||||
if List.isEmpty (Node.value declaration).arguments then
|
||||
Nothing
|
||||
|
||||
else
|
||||
Just (declaration |> Node.value |> .expression |> Node.range)
|
||||
|
||||
Expression.LetDestructuring _ _ ->
|
||||
Nothing
|
||||
)
|
||||
declarations
|
||||
|
||||
|
||||
incrementAndIgnoreForOperands : String -> List Increase -> Node Expression -> Node Expression -> ( List Increase, List Range )
|
||||
incrementAndIgnoreForOperands operator increases left right =
|
||||
let
|
||||
( leftIncreases, leftIgnore ) =
|
||||
incrementAndIgnore operator left
|
||||
|
||||
( rightIncreases, rightIgnore ) =
|
||||
incrementAndIgnore operator right
|
||||
in
|
||||
( List.concat [ leftIncreases, rightIncreases, increases ]
|
||||
, Node.range left :: Node.range right :: leftIgnore ++ rightIgnore
|
||||
)
|
||||
|
||||
|
||||
incrementAndIgnore : String -> Node Expression -> ( List Increase, List Range )
|
||||
incrementAndIgnore parentOperator node =
|
||||
case Node.value node of
|
||||
Expression.OperatorApplication operator _ left right ->
|
||||
if operator == "&&" || operator == "||" then
|
||||
let
|
||||
newOperatorIncrease : List Increase
|
||||
newOperatorIncrease =
|
||||
if operator == parentOperator then
|
||||
[]
|
||||
|
||||
else
|
||||
[ { line = (Node.range node).start
|
||||
, increase = 1
|
||||
, nesting = 0
|
||||
, kind = Operator operator
|
||||
}
|
||||
]
|
||||
in
|
||||
incrementAndIgnoreForOperands
|
||||
operator
|
||||
newOperatorIncrease
|
||||
left
|
||||
right
|
||||
|
||||
else
|
||||
( [], [] )
|
||||
|
||||
_ ->
|
||||
( [], [] )
|
||||
|
||||
|
||||
expressionExitVisitor : Node Expression -> Context -> ( List nothing, Context )
|
||||
expressionExitVisitor node context =
|
||||
if List.member (Node.range node) context.rangesWhereNestingIncreases then
|
||||
( [], expressionExitVisitorHelp node { context | nesting = context.nesting - 1 } )
|
||||
|
||||
else
|
||||
( [], expressionExitVisitorHelp node context )
|
||||
|
||||
|
||||
expressionExitVisitorHelp : Node Expression -> Context -> Context
|
||||
expressionExitVisitorHelp node context =
|
||||
case Node.value node of
|
||||
Expression.IfBlock _ _ _ ->
|
||||
if not (List.member (Node.range node) context.elseIfToIgnore) then
|
||||
{ context | nesting = context.nesting - 1 }
|
||||
|
||||
else
|
||||
context
|
||||
|
||||
Expression.CaseExpression _ ->
|
||||
{ context | nesting = context.nesting - 1 }
|
||||
|
||||
Expression.LambdaExpression _ ->
|
||||
{ context | nesting = context.nesting - 1 }
|
||||
|
||||
_ ->
|
||||
context
|
||||
|
||||
|
||||
declarationExitVisitor : Node Declaration -> Context -> ( List (Rule.Error {}), Context )
|
||||
declarationExitVisitor node context =
|
||||
let
|
||||
functionsToReport : List FunctionToReport
|
||||
functionsToReport =
|
||||
case Node.value node of
|
||||
Declaration.FunctionDeclaration function ->
|
||||
{ functionName = function.declaration |> Node.value |> .name
|
||||
, increases = context.increases
|
||||
, references = context.references
|
||||
}
|
||||
:: context.functionsToReport
|
||||
|
||||
_ ->
|
||||
context.functionsToReport
|
||||
in
|
||||
( []
|
||||
, { nesting = 0
|
||||
, operandsToIgnore = []
|
||||
, elseIfToIgnore = []
|
||||
, rangesWhereNestingIncreases = []
|
||||
, references = Dict.empty
|
||||
, increases = []
|
||||
, functionsToReport = functionsToReport
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
finalEvaluation : Int -> Context -> List (Rule.Error {})
|
||||
finalEvaluation threshold context =
|
||||
let
|
||||
potentialRecursiveFunctions : Set String
|
||||
potentialRecursiveFunctions =
|
||||
List.map (.functionName >> Node.value) context.functionsToReport
|
||||
|> Set.fromList
|
||||
|
||||
recursiveCalls : RecursiveCalls
|
||||
recursiveCalls =
|
||||
context.functionsToReport
|
||||
|> List.map
|
||||
(\{ functionName, references } ->
|
||||
( Node.value functionName, Dict.filter (\name _ -> Set.member name potentialRecursiveFunctions) references )
|
||||
)
|
||||
|> Dict.fromList
|
||||
|> findRecursiveCalls
|
||||
in
|
||||
List.filterMap
|
||||
(\{ functionName, increases, references } ->
|
||||
let
|
||||
recursiveCallsForFunctionName : List String
|
||||
recursiveCallsForFunctionName =
|
||||
Dict.get (Node.value functionName) recursiveCalls
|
||||
|> Maybe.withDefault Set.empty
|
||||
|> Set.toList
|
||||
|
||||
allIncreases : List Increase
|
||||
allIncreases =
|
||||
List.concat
|
||||
[ increases
|
||||
, recursiveCallsForFunctionName
|
||||
|> List.filterMap
|
||||
(\referenceToRecursiveFunction ->
|
||||
Dict.get referenceToRecursiveFunction references
|
||||
|> Maybe.map (Tuple.pair referenceToRecursiveFunction)
|
||||
)
|
||||
|> List.map
|
||||
(\( reference, location ) ->
|
||||
{ line = location
|
||||
, increase = 1
|
||||
, nesting = 0
|
||||
, kind =
|
||||
if Node.value functionName == reference then
|
||||
RecursiveCall
|
||||
|
||||
else
|
||||
IndirectRecursiveCall reference
|
||||
}
|
||||
)
|
||||
]
|
||||
|
||||
finalComplexity : Int
|
||||
finalComplexity =
|
||||
List.sum (List.map .increase allIncreases)
|
||||
in
|
||||
if finalComplexity > threshold then
|
||||
Just
|
||||
(Rule.error
|
||||
{ message = Node.value functionName ++ " has a cognitive complexity of " ++ String.fromInt finalComplexity ++ ", higher than the allowed " ++ String.fromInt threshold
|
||||
, details =
|
||||
if List.isEmpty allIncreases then
|
||||
explanation
|
||||
|
||||
else
|
||||
explanation
|
||||
++ [ allIncreases
|
||||
|> List.sortBy (\{ line } -> ( line.row, line.column ))
|
||||
|> List.map explain
|
||||
|> String.join "\n"
|
||||
]
|
||||
}
|
||||
(Node.range functionName)
|
||||
)
|
||||
|
||||
else
|
||||
Nothing
|
||||
)
|
||||
context.functionsToReport
|
||||
|
||||
|
||||
explanation : List String
|
||||
explanation =
|
||||
[ "This metric is a heuristic to measure how easy to understand a piece of code is, primarily through increments for breaks in the linear flow and for nesting those breaks."
|
||||
, "The most common ways to reduce complexity is to extract sections into functions and to unnest control flow structures. Following is a breakdown of where complexity was found:"
|
||||
]
|
||||
|
||||
|
||||
explain : Increase -> String
|
||||
explain increase =
|
||||
"Line " ++ String.fromInt increase.line.row ++ ": +" ++ String.fromInt increase.increase ++ " for the " ++ kindToString increase.kind ++ mentionNesting increase.nesting
|
||||
|
||||
|
||||
mentionNesting : Int -> String
|
||||
mentionNesting nesting =
|
||||
if nesting == 0 then
|
||||
""
|
||||
|
||||
else
|
||||
" (including " ++ String.fromInt nesting ++ " for nesting)"
|
||||
|
||||
|
||||
kindToString : IncreaseKind -> String
|
||||
kindToString kind =
|
||||
case kind of
|
||||
If ->
|
||||
"if expression"
|
||||
|
||||
ElseIf ->
|
||||
"else if expression"
|
||||
|
||||
Case ->
|
||||
"case expression"
|
||||
|
||||
Operator operator ->
|
||||
"use of `" ++ operator ++ "`"
|
||||
|
||||
RecursiveCall ->
|
||||
"recursive call"
|
||||
|
||||
IndirectRecursiveCall fnName ->
|
||||
"indirect recursive call to " ++ fnName
|
||||
|
||||
|
||||
|
||||
-- FINDING RECURSIVE FUNCTIONS
|
||||
-- Inspired by the algorithm found at https://www.baeldung.com/cs/detecting-recursiveCalls-in-directed-graph
|
||||
|
||||
|
||||
type alias RecursiveCalls =
|
||||
Dict String (Set String)
|
||||
|
||||
|
||||
type alias Visited =
|
||||
Dict String VisitState
|
||||
|
||||
|
||||
type VisitState
|
||||
= InStack
|
||||
| Done
|
||||
|
||||
|
||||
findRecursiveCalls : Dict String (Dict String a) -> RecursiveCalls
|
||||
findRecursiveCalls graph =
|
||||
graph
|
||||
|> Dict.keys
|
||||
|> List.foldl
|
||||
(\vertice ( recursiveCalls, visited ) ->
|
||||
let
|
||||
res : { recursiveCalls : RecursiveCalls, visited : Visited, stack : List String }
|
||||
res =
|
||||
processDFSTree
|
||||
graph
|
||||
[ vertice ]
|
||||
(Dict.insert vertice InStack visited)
|
||||
in
|
||||
( mergeRecursiveCallsDict res.recursiveCalls recursiveCalls, res.visited )
|
||||
)
|
||||
( Dict.empty, Dict.empty )
|
||||
|> Tuple.first
|
||||
|
||||
|
||||
mergeRecursiveCallsDict : RecursiveCalls -> RecursiveCalls -> RecursiveCalls
|
||||
mergeRecursiveCallsDict left right =
|
||||
Dict.merge
|
||||
(\functionName calls dict -> Dict.insert functionName calls dict)
|
||||
(\functionName callsLeft callsRight dict -> Dict.insert functionName (Set.union callsLeft callsRight) dict)
|
||||
(\functionName calls dict -> Dict.insert functionName calls dict)
|
||||
left
|
||||
right
|
||||
Dict.empty
|
||||
|
||||
|
||||
processDFSTree : Dict String (Dict String a) -> List String -> Visited -> { recursiveCalls : RecursiveCalls, visited : Visited, stack : List String }
|
||||
processDFSTree graph stack visited =
|
||||
let
|
||||
vertices : List String
|
||||
vertices =
|
||||
List.head stack
|
||||
|> Maybe.andThen (\v -> Dict.get v graph)
|
||||
|> Maybe.withDefault Dict.empty
|
||||
|> Dict.keys
|
||||
in
|
||||
List.foldl
|
||||
(\vertice acc ->
|
||||
case Dict.get vertice visited of
|
||||
Just InStack ->
|
||||
{ acc | recursiveCalls = insertCycle stack vertice acc.recursiveCalls }
|
||||
|
||||
Just Done ->
|
||||
acc
|
||||
|
||||
Nothing ->
|
||||
let
|
||||
res : { recursiveCalls : RecursiveCalls, visited : Visited, stack : List String }
|
||||
res =
|
||||
processDFSTree
|
||||
graph
|
||||
(vertice :: stack)
|
||||
(Dict.insert vertice InStack visited)
|
||||
in
|
||||
{ recursiveCalls = mergeRecursiveCallsDict res.recursiveCalls acc.recursiveCalls, visited = res.visited }
|
||||
)
|
||||
{ recursiveCalls = Dict.empty, visited = visited }
|
||||
vertices
|
||||
|> (\res ->
|
||||
{ recursiveCalls = res.recursiveCalls
|
||||
, visited =
|
||||
List.head stack
|
||||
|> Maybe.map (\v -> Dict.insert v Done res.visited)
|
||||
|> Maybe.withDefault res.visited
|
||||
, stack = List.drop 1 stack
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
insertCycle : List String -> String -> RecursiveCalls -> RecursiveCalls
|
||||
insertCycle stack vertice recursiveCalls =
|
||||
case stack of
|
||||
x :: xs ->
|
||||
List.foldl
|
||||
(\( functionName, reference ) acc ->
|
||||
Dict.update
|
||||
functionName
|
||||
(Maybe.withDefault Set.empty >> Set.insert reference >> Just)
|
||||
acc
|
||||
)
|
||||
recursiveCalls
|
||||
(toTuples x (takeTop xs ( x, [] ) vertice) [])
|
||||
|
||||
[] ->
|
||||
recursiveCalls
|
||||
|
||||
|
||||
toTuples : String -> ( String, List String ) -> List ( String, String ) -> List ( String, String )
|
||||
toTuples x ( first, xs ) result =
|
||||
case xs of
|
||||
[] ->
|
||||
( x, first ) :: result
|
||||
|
||||
firstofXs :: restOfXs ->
|
||||
toTuples first ( firstofXs, restOfXs ) (( x, first ) :: result)
|
||||
|
||||
|
||||
takeTop : List String -> ( String, List String ) -> String -> ( String, List String )
|
||||
takeTop stack ( previousValue, previousValues ) stopValue =
|
||||
case stack of
|
||||
[] ->
|
||||
( previousValue, previousValues )
|
||||
|
||||
x :: xs ->
|
||||
if x /= stopValue then
|
||||
takeTop xs ( x, previousValue :: previousValues ) stopValue
|
||||
|
||||
else
|
||||
( x, previousValue :: previousValues )
|
557
tests/CognitiveComplexityTest.elm
Normal file
557
tests/CognitiveComplexityTest.elm
Normal file
@ -0,0 +1,557 @@
|
||||
module CognitiveComplexityTest exposing (all)
|
||||
|
||||
import CognitiveComplexity exposing (rule)
|
||||
import Elm.Syntax.Range exposing (Range)
|
||||
import Expect exposing (Expectation)
|
||||
import Review.Test
|
||||
import Test exposing (Test, describe, test)
|
||||
|
||||
|
||||
all : Test
|
||||
all =
|
||||
describe "CognitiveComplexity"
|
||||
[ test "should not report an error when the complexity is lower than the threshold" <|
|
||||
\() ->
|
||||
"""module A exposing (..)
|
||||
a = 1
|
||||
"""
|
||||
|> Review.Test.run (rule 1)
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should report an error when the complexity is higher than the threshold" <|
|
||||
\() ->
|
||||
"""module A exposing (..)
|
||||
fun n =
|
||||
if cond then -- +1
|
||||
if cond then -- +2
|
||||
if cond then -- +3
|
||||
if cond then -- +4
|
||||
1
|
||||
else
|
||||
2
|
||||
else
|
||||
2
|
||||
else
|
||||
2
|
||||
else
|
||||
2
|
||||
"""
|
||||
|> expect
|
||||
[ { name = "fun"
|
||||
, complexity = 10
|
||||
, details = [ String.trim """
|
||||
Line 3: +1 for the if expression
|
||||
Line 4: +2 for the if expression (including 1 for nesting)
|
||||
Line 5: +3 for the if expression (including 2 for nesting)
|
||||
Line 6: +4 for the if expression (including 3 for nesting)
|
||||
""" ]
|
||||
}
|
||||
]
|
||||
, test "should count a simple value or operation as 0" <|
|
||||
\() ->
|
||||
"""module A exposing (..)
|
||||
fun n =
|
||||
n + 1
|
||||
"""
|
||||
|> expect [ { name = "fun", complexity = 0, details = [] } ]
|
||||
, test "should count if expression as 1" <|
|
||||
\() ->
|
||||
"""module A exposing (..)
|
||||
fun n =
|
||||
if cond then
|
||||
1
|
||||
else
|
||||
2
|
||||
"""
|
||||
|> expect
|
||||
[ { name = "fun"
|
||||
, complexity = 1
|
||||
, details = [ "Line 3: +1 for the if expression" ]
|
||||
}
|
||||
]
|
||||
, test "should count if else expressions" <|
|
||||
\() ->
|
||||
"""module A exposing (..)
|
||||
fun n =
|
||||
if cond then -- +1
|
||||
1
|
||||
else if cond then -- +1
|
||||
2
|
||||
else
|
||||
2
|
||||
"""
|
||||
|> expect
|
||||
[ { name = "fun"
|
||||
, complexity = 2
|
||||
, details = [ String.trim """
|
||||
Line 3: +1 for the if expression
|
||||
Line 5: +1 for the else if expression
|
||||
""" ]
|
||||
}
|
||||
]
|
||||
, test "should properly decrement when exiting else expression" <|
|
||||
\() ->
|
||||
"""module A exposing (..)
|
||||
fun n =
|
||||
let
|
||||
_ =
|
||||
if cond then -- +1
|
||||
1
|
||||
else if cond then -- +1
|
||||
2
|
||||
else
|
||||
3
|
||||
in
|
||||
if cond then -- +1
|
||||
4
|
||||
else
|
||||
5
|
||||
"""
|
||||
|> expect
|
||||
[ { name = "fun"
|
||||
, complexity = 3
|
||||
, details = [ String.trim """
|
||||
Line 5: +1 for the if expression
|
||||
Line 7: +1 for the else if expression
|
||||
Line 12: +1 for the if expression
|
||||
""" ]
|
||||
}
|
||||
]
|
||||
, test "should increment nesting when inside a let function" <|
|
||||
\() ->
|
||||
"""module A exposing (..)
|
||||
fun n =
|
||||
let
|
||||
fn n =
|
||||
if cond then -- +2
|
||||
1
|
||||
else
|
||||
2
|
||||
in
|
||||
fn n
|
||||
"""
|
||||
|> expect
|
||||
[ { name = "fun"
|
||||
, complexity = 2
|
||||
, details = [ String.trim """
|
||||
Line 5: +2 for the if expression (including 1 for nesting)
|
||||
""" ]
|
||||
}
|
||||
]
|
||||
, test "should properly decrement nesting when exiting a let function" <|
|
||||
\() ->
|
||||
"""module A exposing (..)
|
||||
fun n =
|
||||
let
|
||||
fn1 n = -- nesting increment
|
||||
1
|
||||
|
||||
fn2 =
|
||||
if cond then -- +1
|
||||
1
|
||||
else
|
||||
2
|
||||
in
|
||||
1
|
||||
"""
|
||||
|> expect
|
||||
[ { name = "fun"
|
||||
, complexity = 1
|
||||
, details = [ String.trim """
|
||||
Line 8: +1 for the if expression
|
||||
""" ]
|
||||
}
|
||||
]
|
||||
, test "should count case expression as 1" <|
|
||||
\() ->
|
||||
"""module A exposing (..)
|
||||
fun n =
|
||||
case n of -- +1
|
||||
1 -> ()
|
||||
2 -> ()
|
||||
3 -> ()
|
||||
4 -> ()
|
||||
_ -> ()
|
||||
"""
|
||||
|> expect
|
||||
[ { name = "fun"
|
||||
, complexity = 1
|
||||
, details = [ "Line 3: +1 for the case expression" ]
|
||||
}
|
||||
]
|
||||
, test "should count nesting of case expressions" <|
|
||||
\() ->
|
||||
"""module A exposing (..)
|
||||
fun n =
|
||||
case n of -- +1
|
||||
1 -> ()
|
||||
2 -> ()
|
||||
3 -> case n of -- +2
|
||||
_ -> ()
|
||||
4 -> ()
|
||||
_ -> ()
|
||||
"""
|
||||
|> expect
|
||||
[ { name = "fun"
|
||||
, complexity = 3
|
||||
, details = [ String.trim """
|
||||
Line 3: +1 for the case expression
|
||||
Line 6: +2 for the case expression (including 1 for nesting)
|
||||
""" ]
|
||||
}
|
||||
]
|
||||
, test "should decrement the nesting when leaving a nested structure" <|
|
||||
\() ->
|
||||
"""module A exposing (..)
|
||||
fun n =
|
||||
if cond then -- +1
|
||||
if cond then -- +2
|
||||
if cond then -- +3
|
||||
1
|
||||
else
|
||||
2
|
||||
else
|
||||
2
|
||||
else
|
||||
case n of -- +2
|
||||
() -> ()
|
||||
"""
|
||||
|> expect
|
||||
[ { name = "fun"
|
||||
, complexity = 8
|
||||
, details = [ String.trim """
|
||||
Line 3: +1 for the if expression
|
||||
Line 4: +2 for the if expression (including 1 for nesting)
|
||||
Line 5: +3 for the if expression (including 2 for nesting)
|
||||
Line 12: +2 for the case expression (including 1 for nesting)
|
||||
""" ]
|
||||
}
|
||||
]
|
||||
, test "should increment once when using the && boolean operator" <|
|
||||
\() ->
|
||||
"""module A exposing (..)
|
||||
fun n =
|
||||
if -- +1
|
||||
a && b && c && d -- +1 for the usage of &&
|
||||
then
|
||||
1
|
||||
else
|
||||
2
|
||||
"""
|
||||
|> expect
|
||||
[ { name = "fun"
|
||||
, complexity = 2
|
||||
, details = [ String.trim """
|
||||
Line 3: +1 for the if expression
|
||||
Line 4: +1 for the use of `&&`
|
||||
""" ]
|
||||
}
|
||||
]
|
||||
, test "should increment once when using the || boolean operator" <|
|
||||
\() ->
|
||||
"""module A exposing (..)
|
||||
fun n =
|
||||
if -- +1
|
||||
a || b || c || d -- +1 for the usage of ||
|
||||
then
|
||||
1
|
||||
else
|
||||
2
|
||||
"""
|
||||
|> expect
|
||||
[ { name = "fun"
|
||||
, complexity = 2
|
||||
, details = [ String.trim """
|
||||
Line 3: +1 for the if expression
|
||||
Line 4: +1 for the use of `||`
|
||||
""" ]
|
||||
}
|
||||
]
|
||||
, test "should increment when mixing boolean operators" <|
|
||||
\() ->
|
||||
"""module A exposing (..)
|
||||
fun n =
|
||||
if -- +1
|
||||
a && b && c -- +1
|
||||
|| d || e -- +1
|
||||
&& f -- +1
|
||||
then
|
||||
1
|
||||
else
|
||||
2
|
||||
"""
|
||||
|> expect
|
||||
[ { name = "fun"
|
||||
, complexity = 4
|
||||
, details = [ String.trim """
|
||||
Line 3: +1 for the if expression
|
||||
Line 4: +1 for the use of `||`
|
||||
Line 4: +1 for the use of `&&`
|
||||
Line 5: +1 for the use of `&&`
|
||||
""" ]
|
||||
}
|
||||
]
|
||||
, test "should not increment for anonymous functions" <|
|
||||
\() ->
|
||||
"""module A exposing (..)
|
||||
fun n =
|
||||
List.map (\\m -> m + 1) n
|
||||
"""
|
||||
|> expect
|
||||
[ { name = "fun"
|
||||
, complexity = 0
|
||||
, details = []
|
||||
}
|
||||
]
|
||||
, test "should increment the nesting inside anonymous functions" <|
|
||||
\() ->
|
||||
"""module A exposing (..)
|
||||
fun n =
|
||||
List.map
|
||||
(\\m ->
|
||||
if cond then -- +2
|
||||
1
|
||||
|
||||
else
|
||||
2
|
||||
)
|
||||
n
|
||||
"""
|
||||
|> expect
|
||||
[ { name = "fun"
|
||||
, complexity = 2
|
||||
, details = [ "Line 5: +2 for the if expression (including 1 for nesting)" ]
|
||||
}
|
||||
]
|
||||
, test "should properly decrement the nesting when exiting an anonymous function" <|
|
||||
\() ->
|
||||
"""module A exposing (..)
|
||||
fun n =
|
||||
let
|
||||
_ = List.map (\\m -> m + 1) n
|
||||
in
|
||||
if cond then -- +1
|
||||
1
|
||||
else
|
||||
2
|
||||
"""
|
||||
|> expect
|
||||
[ { name = "fun"
|
||||
, complexity = 1
|
||||
, details = [ "Line 6: +1 for the if expression" ]
|
||||
}
|
||||
]
|
||||
, test "should increment when finding a recursive call" <|
|
||||
\() ->
|
||||
"""module A exposing (..)
|
||||
fun n =
|
||||
if n > 0 then -- +1
|
||||
1 + fun (n - 1) -- +1
|
||||
else
|
||||
1
|
||||
"""
|
||||
|> expectAtExactly
|
||||
[ { name = "fun"
|
||||
, complexity = 2
|
||||
, atExactly = { start = { row = 2, column = 1 }, end = { row = 2, column = 4 } }
|
||||
, details = [ String.trim """
|
||||
Line 3: +1 for the if expression
|
||||
Line 4: +1 for the recursive call
|
||||
""" ]
|
||||
}
|
||||
]
|
||||
, test "should only increment once, even if there are multiple recursive calls" <|
|
||||
\() ->
|
||||
"""module A exposing (..)
|
||||
fib n =
|
||||
if n > 0 then -- +1
|
||||
fib (n - 1) -- +1
|
||||
+ fib (n - 2) -- +0
|
||||
else
|
||||
0
|
||||
"""
|
||||
|> expectAtExactly
|
||||
[ { name = "fib"
|
||||
, complexity = 2
|
||||
, atExactly = { start = { row = 2, column = 1 }, end = { row = 2, column = 4 } }
|
||||
, details = [ String.trim """
|
||||
Line 3: +1 for the if expression
|
||||
Line 4: +1 for the recursive call
|
||||
""" ]
|
||||
}
|
||||
]
|
||||
, test "should increment the complexity for every recursive call in a chain" <|
|
||||
\() ->
|
||||
"""module A exposing (..)
|
||||
fun1 n = -- +1
|
||||
fun2 n
|
||||
|
||||
fun2 n = -- +1
|
||||
fun1 n
|
||||
"""
|
||||
|> expectAtExactly
|
||||
[ { name = "fun1"
|
||||
, complexity = 1
|
||||
, atExactly = { start = { row = 2, column = 1 }, end = { row = 2, column = 5 } }
|
||||
, details = [ String.trim """
|
||||
Line 3: +1 for the indirect recursive call to fun2
|
||||
""" ]
|
||||
}
|
||||
, { name = "fun2"
|
||||
, complexity = 1
|
||||
, atExactly = { start = { row = 5, column = 1 }, end = { row = 5, column = 5 } }
|
||||
, details = [ String.trim """
|
||||
Line 6: +1 for the indirect recursive call to fun1
|
||||
""" ]
|
||||
}
|
||||
]
|
||||
, test "should increment the complexity for every recursive call in a chain, for each different function call" <|
|
||||
\() ->
|
||||
"""module A exposing (..)
|
||||
fun1 n =
|
||||
fun2 n -- +1
|
||||
+ fun2 n -- +0, already counted
|
||||
+ fun1 n -- +1
|
||||
|
||||
fun2 n =
|
||||
fun1 n -- +1
|
||||
"""
|
||||
|> expectAtExactly
|
||||
[ { name = "fun1"
|
||||
, complexity = 2
|
||||
, atExactly = { start = { row = 2, column = 1 }, end = { row = 2, column = 5 } }
|
||||
, details =
|
||||
[ String.trim """
|
||||
Line 3: +1 for the indirect recursive call to fun2
|
||||
Line 5: +1 for the recursive call
|
||||
"""
|
||||
]
|
||||
}
|
||||
, { name = "fun2"
|
||||
, complexity = 1
|
||||
, atExactly = { start = { row = 7, column = 1 }, end = { row = 7, column = 5 } }
|
||||
, details = [ String.trim """
|
||||
Line 8: +1 for the indirect recursive call to fun1
|
||||
""" ]
|
||||
}
|
||||
]
|
||||
, test "should increment the complexity for every recursive call in a chain, for long chains" <|
|
||||
\() ->
|
||||
"""module A exposing (..)
|
||||
fun1 n =
|
||||
fun2 n -- +1
|
||||
fun2 n =
|
||||
fun3 n -- +1
|
||||
fun3 n =
|
||||
fun4 n -- +1
|
||||
fun4 n =
|
||||
fun5 n -- +1
|
||||
fun5 n =
|
||||
fun1 n -- +1
|
||||
"""
|
||||
|> expectAtExactly
|
||||
[ { name = "fun1"
|
||||
, complexity = 1
|
||||
, atExactly = { start = { row = 2, column = 1 }, end = { row = 2, column = 5 } }
|
||||
, details = [ "Line 3: +1 for the indirect recursive call to fun2" ]
|
||||
}
|
||||
, { name = "fun2"
|
||||
, complexity = 1
|
||||
, atExactly = { start = { row = 4, column = 1 }, end = { row = 4, column = 5 } }
|
||||
, details = [ "Line 5: +1 for the indirect recursive call to fun3" ]
|
||||
}
|
||||
, { name = "fun3"
|
||||
, complexity = 1
|
||||
, atExactly = { start = { row = 6, column = 1 }, end = { row = 6, column = 5 } }
|
||||
, details = [ "Line 7: +1 for the indirect recursive call to fun4" ]
|
||||
}
|
||||
, { name = "fun4"
|
||||
, complexity = 1
|
||||
, atExactly = { start = { row = 8, column = 1 }, end = { row = 8, column = 5 } }
|
||||
, details = [ "Line 9: +1 for the indirect recursive call to fun5" ]
|
||||
}
|
||||
, { name = "fun5"
|
||||
, complexity = 1
|
||||
, atExactly = { start = { row = 10, column = 1 }, end = { row = 10, column = 5 } }
|
||||
, details = [ "Line 11: +1 for the indirect recursive call to fun1" ]
|
||||
}
|
||||
]
|
||||
, test "the complexity of a function should not affect another function's computed complexity" <|
|
||||
\() ->
|
||||
"""module A exposing (..)
|
||||
simple n = 1
|
||||
|
||||
fun n =
|
||||
if cond then
|
||||
if cond then
|
||||
1
|
||||
else
|
||||
2
|
||||
else
|
||||
2
|
||||
|
||||
alsoSimple n =
|
||||
if cond then
|
||||
1
|
||||
else
|
||||
2
|
||||
"""
|
||||
|> expect
|
||||
[ { name = "simple"
|
||||
, complexity = 0
|
||||
, details = []
|
||||
}
|
||||
, { name = "fun"
|
||||
, complexity = 3
|
||||
, details = [ String.trim """
|
||||
Line 5: +1 for the if expression
|
||||
Line 6: +2 for the if expression (including 1 for nesting)
|
||||
""" ]
|
||||
}
|
||||
, { name = "alsoSimple"
|
||||
, complexity = 1
|
||||
, details = [ "Line 14: +1 for the if expression" ]
|
||||
}
|
||||
]
|
||||
]
|
||||
|
||||
|
||||
expect : List { name : String, complexity : Int, details : List String } -> String -> Expectation
|
||||
expect functionComplexities source =
|
||||
source
|
||||
|> Review.Test.run (rule -1)
|
||||
|> Review.Test.expectErrors
|
||||
(List.map
|
||||
(\{ name, complexity, details } ->
|
||||
Review.Test.error
|
||||
{ message = name ++ " has a cognitive complexity of " ++ String.fromInt complexity ++ ", higher than the allowed -1"
|
||||
, details = explanation ++ details
|
||||
, under = name
|
||||
}
|
||||
)
|
||||
functionComplexities
|
||||
)
|
||||
|
||||
|
||||
expectAtExactly : List { name : String, complexity : Int, details : List String, atExactly : Range } -> String -> Expectation
|
||||
expectAtExactly functionComplexities source =
|
||||
source
|
||||
|> Review.Test.run (rule -1)
|
||||
|> Review.Test.expectErrors
|
||||
(List.map
|
||||
(\{ name, complexity, details, atExactly } ->
|
||||
Review.Test.error
|
||||
{ message = name ++ " has a cognitive complexity of " ++ String.fromInt complexity ++ ", higher than the allowed -1"
|
||||
, details = explanation ++ details
|
||||
, under = name
|
||||
}
|
||||
|> Review.Test.atExactly atExactly
|
||||
)
|
||||
functionComplexities
|
||||
)
|
||||
|
||||
|
||||
explanation : List String
|
||||
explanation =
|
||||
[ "This metric is a heuristic to measure how easy to understand a piece of code is, primarily through increments for breaks in the linear flow and for nesting those breaks."
|
||||
, "The most common ways to reduce complexity is to extract sections into functions and to unnest control flow structures. Following is a breakdown of where complexity was found:"
|
||||
]
|
@ -1,4 +1,4 @@
|
||||
module List.Extra exposing (find)
|
||||
module List.Extra exposing (find, indexedFilterMap)
|
||||
|
||||
{-| Some utilities.
|
||||
-}
|
||||
@ -22,3 +22,18 @@ find predicate list =
|
||||
|
||||
else
|
||||
find predicate rest
|
||||
|
||||
|
||||
indexedFilterMap : (Int -> a -> Maybe b) -> Int -> List a -> List b -> List b
|
||||
indexedFilterMap predicate index list acc =
|
||||
case list of
|
||||
[] ->
|
||||
acc
|
||||
|
||||
x :: xs ->
|
||||
case predicate index x of
|
||||
Just b ->
|
||||
indexedFilterMap predicate (index + 1) xs (b :: acc)
|
||||
|
||||
Nothing ->
|
||||
indexedFilterMap predicate (index + 1) xs acc
|
||||
|
1007
tests/NoDeprecated.elm
Normal file
1007
tests/NoDeprecated.elm
Normal file
File diff suppressed because it is too large
Load Diff
1219
tests/NoDeprecatedTest.elm
Normal file
1219
tests/NoDeprecatedTest.elm
Normal file
File diff suppressed because it is too large
Load Diff
@ -170,33 +170,43 @@ expressionVisitor : Node Expression -> ModuleContext -> ( List (Error {}), Modul
|
||||
expressionVisitor node moduleContext =
|
||||
case Node.value node of
|
||||
Expression.FunctionOrValue _ "update" ->
|
||||
case ModuleNameLookupTable.moduleNameFor moduleContext.lookupTable node of
|
||||
Just moduleName ->
|
||||
if Set.member moduleName moduleContext.modulesThatExposeSubscriptionsAndUpdate then
|
||||
( [], { moduleContext | usesUpdateOfModule = Dict.insert moduleName (Node.range node) moduleContext.usesUpdateOfModule } )
|
||||
|
||||
else
|
||||
( [], moduleContext )
|
||||
|
||||
Nothing ->
|
||||
( [], moduleContext )
|
||||
( [], registerUpdateFunction node moduleContext )
|
||||
|
||||
Expression.FunctionOrValue _ "subscriptions" ->
|
||||
case ModuleNameLookupTable.moduleNameFor moduleContext.lookupTable node of
|
||||
Just moduleName ->
|
||||
if Set.member moduleName moduleContext.modulesThatExposeSubscriptionsAndUpdate then
|
||||
( [], { moduleContext | usesSubscriptionsOfModule = Set.insert moduleName moduleContext.usesSubscriptionsOfModule } )
|
||||
|
||||
else
|
||||
( [], moduleContext )
|
||||
|
||||
Nothing ->
|
||||
( [], moduleContext )
|
||||
( [], registerSubscriptionsFunction node moduleContext )
|
||||
|
||||
_ ->
|
||||
( [], moduleContext )
|
||||
|
||||
|
||||
registerUpdateFunction : Node a -> ModuleContext -> ModuleContext
|
||||
registerUpdateFunction node moduleContext =
|
||||
case ModuleNameLookupTable.moduleNameFor moduleContext.lookupTable node of
|
||||
Just moduleName ->
|
||||
if Set.member moduleName moduleContext.modulesThatExposeSubscriptionsAndUpdate then
|
||||
{ moduleContext | usesUpdateOfModule = Dict.insert moduleName (Node.range node) moduleContext.usesUpdateOfModule }
|
||||
|
||||
else
|
||||
moduleContext
|
||||
|
||||
Nothing ->
|
||||
moduleContext
|
||||
|
||||
|
||||
registerSubscriptionsFunction : Node a -> ModuleContext -> ModuleContext
|
||||
registerSubscriptionsFunction node moduleContext =
|
||||
case ModuleNameLookupTable.moduleNameFor moduleContext.lookupTable node of
|
||||
Just moduleName ->
|
||||
if Set.member moduleName moduleContext.modulesThatExposeSubscriptionsAndUpdate then
|
||||
{ moduleContext | usesSubscriptionsOfModule = Set.insert moduleName moduleContext.usesSubscriptionsOfModule }
|
||||
|
||||
else
|
||||
moduleContext
|
||||
|
||||
Nothing ->
|
||||
moduleContext
|
||||
|
||||
|
||||
finalEvaluation : ModuleContext -> List (Error {})
|
||||
finalEvaluation moduleContext =
|
||||
moduleContext.usesUpdateOfModule
|
||||
|
@ -100,7 +100,7 @@ expressionVisitor node context =
|
||||
Expression.FunctionOrValue [] "update" ->
|
||||
( [ Rule.error
|
||||
{ message = "`update` shouldn't call itself"
|
||||
, details = [ "If you wish to have the same behavior for different messages, move that behavior into a new function and call have it called in the handling of both messages." ]
|
||||
, details = [ "If you wish to have the same behavior for different messages, move that behavior into a new function and have it called in the handling of both messages." ]
|
||||
}
|
||||
(Node.range node)
|
||||
]
|
||||
|
@ -23,7 +23,7 @@ update msg model =
|
||||
|> Review.Test.expectErrors
|
||||
[ Review.Test.error
|
||||
{ message = "`update` shouldn't call itself"
|
||||
, details = [ "If you wish to have the same behavior for different messages, move that behavior into a new function and call have it called in the handling of both messages." ]
|
||||
, details = [ "If you wish to have the same behavior for different messages, move that behavior into a new function and have it called in the handling of both messages." ]
|
||||
, under = "update"
|
||||
}
|
||||
|> Review.Test.atExactly { start = { row = 8, column = 7 }, end = { row = 8, column = 13 } }
|
||||
|
@ -9,7 +9,7 @@ module NoSimpleLetBody exposing (rule)
|
||||
import Elm.Syntax.Expression as Expression exposing (Expression)
|
||||
import Elm.Syntax.Node as Node exposing (Node)
|
||||
import Elm.Syntax.Range exposing (Location, Range)
|
||||
import Review.Fix as Fix
|
||||
import Review.Fix as Fix exposing (Fix)
|
||||
import Review.Rule as Rule exposing (Rule)
|
||||
|
||||
|
||||
@ -87,98 +87,127 @@ rule =
|
||||
expressionVisitor : Node Expression -> List (Rule.Error {})
|
||||
expressionVisitor node =
|
||||
case Node.value node of
|
||||
Expression.LetExpression { declarations, expression } ->
|
||||
case Node.value expression of
|
||||
Expression.FunctionOrValue [] name ->
|
||||
let
|
||||
declarationData :
|
||||
{ previousEnd : Maybe Location
|
||||
, lastEnd : Maybe Location
|
||||
, last : Maybe { name : String, declarationRange : Range, expressionRange : Range }
|
||||
, foundDeclaredWithName : Bool
|
||||
}
|
||||
declarationData =
|
||||
List.foldl
|
||||
(\declaration { lastEnd, foundDeclaredWithName } ->
|
||||
case Node.value declaration of
|
||||
Expression.LetFunction function ->
|
||||
let
|
||||
functionDeclaration : Expression.FunctionImplementation
|
||||
functionDeclaration =
|
||||
Node.value function.declaration
|
||||
in
|
||||
{ previousEnd = lastEnd
|
||||
, lastEnd = Just (Node.range functionDeclaration.expression).end
|
||||
, last =
|
||||
if List.isEmpty functionDeclaration.arguments then
|
||||
Just
|
||||
{ name = Node.value functionDeclaration.name
|
||||
, declarationRange = Node.range declaration
|
||||
, expressionRange = Node.range functionDeclaration.expression
|
||||
}
|
||||
|
||||
else
|
||||
Nothing
|
||||
, foundDeclaredWithName = foundDeclaredWithName || Node.value functionDeclaration.name == name
|
||||
}
|
||||
|
||||
Expression.LetDestructuring _ _ ->
|
||||
{ previousEnd = lastEnd
|
||||
, lastEnd = Just (Node.range declaration).end
|
||||
, last = Nothing
|
||||
, foundDeclaredWithName = foundDeclaredWithName
|
||||
}
|
||||
)
|
||||
{ previousEnd = Nothing
|
||||
, lastEnd = Nothing
|
||||
, last = Nothing
|
||||
, foundDeclaredWithName = False
|
||||
}
|
||||
declarations
|
||||
in
|
||||
if declarationData.foundDeclaredWithName then
|
||||
[ Rule.errorWithFix
|
||||
{ message = "The referenced value should be inlined."
|
||||
, details =
|
||||
[ "The name of the value is redundant with the surrounding expression."
|
||||
, "If you believe that the expression needs a name because it is too complex, consider splitting the expression up more or extracting it to a new function."
|
||||
]
|
||||
}
|
||||
(Node.range expression)
|
||||
(case declarationData.last of
|
||||
Just last ->
|
||||
if last.name == name then
|
||||
case declarationData.previousEnd of
|
||||
Nothing ->
|
||||
-- It's the only element in the destructuring, we should remove move of the let expression
|
||||
[ Fix.removeRange { start = (Node.range node).start, end = last.expressionRange.start }
|
||||
, Fix.removeRange { start = last.expressionRange.end, end = (Node.range node).end }
|
||||
]
|
||||
|
||||
Just previousEnd ->
|
||||
-- There are other elements in the let body that we need to keep
|
||||
let
|
||||
indentation : String
|
||||
indentation =
|
||||
String.repeat ((Node.range node).start.column - 1) " "
|
||||
in
|
||||
[ Fix.replaceRangeBy { start = previousEnd, end = last.expressionRange.start } ("\n" ++ indentation ++ "in\n" ++ indentation)
|
||||
, Fix.removeRange { start = last.expressionRange.end, end = (Node.range node).end }
|
||||
]
|
||||
|
||||
else
|
||||
[]
|
||||
|
||||
Nothing ->
|
||||
[]
|
||||
)
|
||||
]
|
||||
|
||||
else
|
||||
[]
|
||||
|
||||
_ ->
|
||||
[]
|
||||
Expression.LetExpression letBlock ->
|
||||
visitLetExpression (Node.range node) letBlock
|
||||
|
||||
_ ->
|
||||
[]
|
||||
|
||||
|
||||
visitLetExpression : Range -> Expression.LetBlock -> List (Rule.Error {})
|
||||
visitLetExpression nodeRange { declarations, expression } =
|
||||
case Node.value expression of
|
||||
Expression.FunctionOrValue [] name ->
|
||||
let
|
||||
declarationData :
|
||||
{ previousEnd : Maybe Location
|
||||
, lastEnd : Maybe Location
|
||||
, last : Maybe { name : String, expressionRange : Range }
|
||||
, foundDeclaredWithName : Bool
|
||||
}
|
||||
declarationData =
|
||||
getDeclarationsData name declarations
|
||||
in
|
||||
if declarationData.foundDeclaredWithName then
|
||||
[ Rule.errorWithFix
|
||||
{ message = "The referenced value should be inlined."
|
||||
, details =
|
||||
[ "The name of the value is redundant with the surrounding expression."
|
||||
, "If you believe that the expression needs a name because it is too complex, consider splitting the expression up more or extracting it to a new function."
|
||||
]
|
||||
}
|
||||
(Node.range expression)
|
||||
(fix nodeRange name declarationData)
|
||||
]
|
||||
|
||||
else
|
||||
[]
|
||||
|
||||
_ ->
|
||||
[]
|
||||
|
||||
|
||||
getDeclarationsData :
|
||||
String
|
||||
-> List (Node Expression.LetDeclaration)
|
||||
->
|
||||
{ previousEnd : Maybe Location
|
||||
, lastEnd : Maybe Location
|
||||
, last : Maybe { name : String, expressionRange : Range }
|
||||
, foundDeclaredWithName : Bool
|
||||
}
|
||||
getDeclarationsData name declarations =
|
||||
List.foldl
|
||||
(\declaration { lastEnd, foundDeclaredWithName } ->
|
||||
case Node.value declaration of
|
||||
Expression.LetFunction function ->
|
||||
let
|
||||
functionDeclaration : Expression.FunctionImplementation
|
||||
functionDeclaration =
|
||||
Node.value function.declaration
|
||||
in
|
||||
{ previousEnd = lastEnd
|
||||
, lastEnd = Just (Node.range functionDeclaration.expression).end
|
||||
, last =
|
||||
if List.isEmpty functionDeclaration.arguments then
|
||||
Just
|
||||
{ name = Node.value functionDeclaration.name
|
||||
, expressionRange = Node.range functionDeclaration.expression
|
||||
}
|
||||
|
||||
else
|
||||
Nothing
|
||||
, foundDeclaredWithName = foundDeclaredWithName || Node.value functionDeclaration.name == name
|
||||
}
|
||||
|
||||
Expression.LetDestructuring _ _ ->
|
||||
{ previousEnd = lastEnd
|
||||
, lastEnd = Just (Node.range declaration).end
|
||||
, last = Nothing
|
||||
, foundDeclaredWithName = foundDeclaredWithName
|
||||
}
|
||||
)
|
||||
{ previousEnd = Nothing
|
||||
, lastEnd = Nothing
|
||||
, last = Nothing
|
||||
, foundDeclaredWithName = False
|
||||
}
|
||||
declarations
|
||||
|
||||
|
||||
fix :
|
||||
Range
|
||||
-> String
|
||||
->
|
||||
{ r
|
||||
| previousEnd : Maybe Location
|
||||
, last : Maybe { name : String, expressionRange : Range }
|
||||
}
|
||||
-> List Fix
|
||||
fix nodeRange name declarationData =
|
||||
case declarationData.last of
|
||||
Just last ->
|
||||
if last.name == name then
|
||||
case declarationData.previousEnd of
|
||||
Nothing ->
|
||||
-- It's the only element in the destructuring, we should remove move of the let expression
|
||||
[ Fix.removeRange { start = nodeRange.start, end = last.expressionRange.start }
|
||||
, Fix.removeRange { start = last.expressionRange.end, end = nodeRange.end }
|
||||
]
|
||||
|
||||
Just previousEnd ->
|
||||
-- There are other elements in the let body that we need to keep
|
||||
let
|
||||
indentation : String
|
||||
indentation =
|
||||
String.repeat (nodeRange.start.column - 1) " "
|
||||
in
|
||||
[ Fix.replaceRangeBy { start = previousEnd, end = last.expressionRange.start } ("\n" ++ indentation ++ "in\n" ++ indentation)
|
||||
, Fix.removeRange { start = last.expressionRange.end, end = nodeRange.end }
|
||||
]
|
||||
|
||||
else
|
||||
[]
|
||||
|
||||
Nothing ->
|
||||
[]
|
||||
|
@ -24,8 +24,13 @@ import Set exposing (Set)
|
||||
}
|
||||
]
|
||||
|
||||
If the license of a dependency is in the `allowed` list, the dependency will not be reported.
|
||||
If it's in the `forbidden`, the dependency will be reported as an error.
|
||||
If it's in neither, the dependency will be reported but with a different message asking you
|
||||
to add the license to either list.
|
||||
|
||||
-}
|
||||
rule : Configuration -> Rule
|
||||
rule : { allowed : List String, forbidden : List String } -> Rule
|
||||
rule configuration =
|
||||
Rule.newProjectRuleSchema "NoUnapprovedLicense" initialProjectContext
|
||||
|> Rule.withElmJsonProjectVisitor elmJsonVisitor
|
||||
|
@ -18,6 +18,7 @@ import Elm.Syntax.Node as Node exposing (Node(..))
|
||||
import Elm.Syntax.Pattern as Pattern exposing (Pattern)
|
||||
import Elm.Syntax.Range exposing (Range)
|
||||
import Elm.Syntax.TypeAnnotation as TypeAnnotation exposing (TypeAnnotation)
|
||||
import List.Extra
|
||||
import Review.ModuleNameLookupTable as ModuleNameLookupTable exposing (ModuleNameLookupTable)
|
||||
import Review.Rule as Rule exposing (Error, Rule)
|
||||
import Set exposing (Set)
|
||||
@ -106,7 +107,7 @@ type alias ModuleContext =
|
||||
{ lookupTable : ModuleNameLookupTable
|
||||
, isModuleExposed : Bool
|
||||
, exposed : Exposing
|
||||
, customTypeArgs : Dict String (Dict String (List Range))
|
||||
, customTypeArgs : List ( String, Dict String (List Range) )
|
||||
, usedArguments : Dict ( ModuleName, String ) (Set Int)
|
||||
, customTypesNotToReport : Set ( ModuleName, String )
|
||||
}
|
||||
@ -115,10 +116,9 @@ type alias ModuleContext =
|
||||
moduleVisitor : Rule.ModuleRuleSchema {} ModuleContext -> Rule.ModuleRuleSchema { hasAtLeastOneVisitor : () } ModuleContext
|
||||
moduleVisitor schema =
|
||||
schema
|
||||
|> Rule.withModuleDefinitionVisitor moduleDefinitionVisitor
|
||||
|> Rule.withDeclarationListVisitor declarationListVisitor
|
||||
|> Rule.withDeclarationEnterVisitor declarationVisitor
|
||||
|> Rule.withExpressionEnterVisitor expressionVisitor
|
||||
|> Rule.withModuleDefinitionVisitor (\node context -> ( [], moduleDefinitionVisitor node context ))
|
||||
|> Rule.withDeclarationEnterVisitor (\node context -> ( [], declarationVisitor node context ))
|
||||
|> Rule.withExpressionEnterVisitor (\node context -> ( [], expressionVisitor node context ))
|
||||
|
||||
|
||||
elmJsonVisitor : Maybe { a | project : Elm.Project.Project } -> ProjectContext -> ( List nothing, ProjectContext )
|
||||
@ -163,7 +163,7 @@ fromProjectToModule =
|
||||
{ lookupTable = lookupTable
|
||||
, isModuleExposed = Set.member (Rule.moduleNameFromMetadata metadata) projectContext.exposedModules
|
||||
, exposed = Exposing.Explicit []
|
||||
, customTypeArgs = Dict.empty
|
||||
, customTypeArgs = []
|
||||
, usedArguments = Dict.empty
|
||||
, customTypesNotToReport = Set.empty
|
||||
}
|
||||
@ -248,15 +248,22 @@ getNonExposedCustomTypes moduleContext =
|
||||
)
|
||||
|> Set.fromList
|
||||
in
|
||||
moduleContext.customTypeArgs
|
||||
|> Dict.filter (\typeName _ -> not <| Set.member typeName exposedCustomTypes)
|
||||
|> Dict.values
|
||||
|> List.foldl Dict.union Dict.empty
|
||||
List.foldl
|
||||
(\( typeName, args ) acc ->
|
||||
if Set.member typeName exposedCustomTypes then
|
||||
acc
|
||||
|
||||
else
|
||||
Dict.union args acc
|
||||
)
|
||||
Dict.empty
|
||||
moduleContext.customTypeArgs
|
||||
|
||||
else
|
||||
moduleContext.customTypeArgs
|
||||
|> Dict.values
|
||||
|> List.foldl Dict.union Dict.empty
|
||||
List.foldl
|
||||
(\( _, args ) acc -> Dict.union args acc)
|
||||
Dict.empty
|
||||
moduleContext.customTypeArgs
|
||||
|
||||
|
||||
foldProjectContexts : ProjectContext -> ProjectContext -> ProjectContext
|
||||
@ -282,105 +289,95 @@ foldProjectContexts newContext previousContext =
|
||||
-- MODULE DEFINITION VISITOR
|
||||
|
||||
|
||||
moduleDefinitionVisitor : Node Module -> ModuleContext -> ( List nothing, ModuleContext )
|
||||
moduleDefinitionVisitor : Node Module -> ModuleContext -> ModuleContext
|
||||
moduleDefinitionVisitor node moduleContext =
|
||||
( [], { moduleContext | exposed = Module.exposingList (Node.value node) } )
|
||||
{ moduleContext | exposed = Module.exposingList (Node.value node) }
|
||||
|
||||
|
||||
|
||||
-- DECLARATION LIST VISITOR
|
||||
|
||||
|
||||
declarationListVisitor : List (Node Declaration) -> ModuleContext -> ( List nothing, ModuleContext )
|
||||
declarationListVisitor nodes context =
|
||||
let
|
||||
customTypeArgs : List ( String, Dict String (List Range) )
|
||||
customTypeArgs =
|
||||
List.filterMap (collectCustomType context.lookupTable) nodes
|
||||
in
|
||||
( [], { context | customTypeArgs = Dict.fromList customTypeArgs } )
|
||||
|
||||
|
||||
collectCustomType : ModuleNameLookupTable -> Node Declaration -> Maybe ( String, Dict String (List Range) )
|
||||
collectCustomType lookupTable node =
|
||||
case Node.value node of
|
||||
Declaration.CustomTypeDeclaration typeDeclaration ->
|
||||
let
|
||||
customTypeConstructors : List ( String, List Range )
|
||||
customTypeConstructors =
|
||||
List.map
|
||||
(\(Node _ { name, arguments }) ->
|
||||
( Node.value name
|
||||
, arguments
|
||||
|> List.filter (isNever lookupTable >> not)
|
||||
|> List.map Node.range
|
||||
)
|
||||
)
|
||||
typeDeclaration.constructors
|
||||
in
|
||||
if List.isEmpty customTypeConstructors then
|
||||
Nothing
|
||||
|
||||
else
|
||||
Just ( Node.value typeDeclaration.name, Dict.fromList customTypeConstructors )
|
||||
|
||||
_ ->
|
||||
Nothing
|
||||
|
||||
|
||||
isNever : ModuleNameLookupTable -> Node TypeAnnotation -> Bool
|
||||
isNever lookupTable node =
|
||||
isNotNever : ModuleNameLookupTable -> Node TypeAnnotation -> Bool
|
||||
isNotNever lookupTable node =
|
||||
case Node.value node of
|
||||
TypeAnnotation.Typed (Node neverRange ( _, "Never" )) [] ->
|
||||
ModuleNameLookupTable.moduleNameAt lookupTable neverRange == Just [ "Basics" ]
|
||||
ModuleNameLookupTable.moduleNameAt lookupTable neverRange /= Just [ "Basics" ]
|
||||
|
||||
_ ->
|
||||
False
|
||||
True
|
||||
|
||||
|
||||
|
||||
-- DECLARATION VISITOR
|
||||
|
||||
|
||||
declarationVisitor : Node Declaration -> ModuleContext -> ( List nothing, ModuleContext )
|
||||
declarationVisitor : Node Declaration -> ModuleContext -> ModuleContext
|
||||
declarationVisitor node context =
|
||||
-- TODO Move to declaration list visitor, or the other way around
|
||||
case Node.value node of
|
||||
Declaration.FunctionDeclaration function ->
|
||||
( []
|
||||
, { context
|
||||
{ context
|
||||
| usedArguments =
|
||||
registerUsedPatterns
|
||||
(collectUsedPatternsFromFunctionDeclaration context function)
|
||||
context.usedArguments
|
||||
}
|
||||
)
|
||||
}
|
||||
|
||||
Declaration.CustomTypeDeclaration typeDeclaration ->
|
||||
if List.isEmpty typeDeclaration.constructors then
|
||||
context
|
||||
|
||||
else
|
||||
let
|
||||
customTypeConstructors : Dict String (List Range)
|
||||
customTypeConstructors =
|
||||
List.foldl
|
||||
(\(Node _ { name, arguments }) acc ->
|
||||
Dict.insert
|
||||
(Node.value name)
|
||||
(createArguments context.lookupTable arguments)
|
||||
acc
|
||||
)
|
||||
Dict.empty
|
||||
typeDeclaration.constructors
|
||||
in
|
||||
{ context
|
||||
| customTypeArgs = ( Node.value typeDeclaration.name, customTypeConstructors ) :: context.customTypeArgs
|
||||
}
|
||||
|
||||
_ ->
|
||||
( [], context )
|
||||
context
|
||||
|
||||
|
||||
createArguments : ModuleNameLookupTable -> List (Node TypeAnnotation) -> List Range
|
||||
createArguments lookupTable arguments =
|
||||
List.foldr
|
||||
(\argument acc ->
|
||||
if isNotNever lookupTable argument then
|
||||
Node.range argument :: acc
|
||||
|
||||
else
|
||||
acc
|
||||
)
|
||||
[]
|
||||
arguments
|
||||
|
||||
|
||||
collectUsedPatternsFromFunctionDeclaration : ModuleContext -> Expression.Function -> List ( ( ModuleName, String ), Set Int )
|
||||
collectUsedPatternsFromFunctionDeclaration context { declaration } =
|
||||
(Node.value declaration).arguments
|
||||
|> List.concatMap (collectUsedCustomTypeArgs context.lookupTable)
|
||||
collectUsedCustomTypeArgs context.lookupTable (Node.value declaration).arguments
|
||||
|
||||
|
||||
|
||||
-- EXPRESSION VISITOR
|
||||
|
||||
|
||||
expressionVisitor : Node Expression -> ModuleContext -> ( List nothing, ModuleContext )
|
||||
expressionVisitor : Node Expression -> ModuleContext -> ModuleContext
|
||||
expressionVisitor node context =
|
||||
case Node.value node of
|
||||
Expression.CaseExpression { cases } ->
|
||||
let
|
||||
usedArguments : List ( ( ModuleName, String ), Set Int )
|
||||
usedArguments =
|
||||
cases
|
||||
|> List.concatMap (Tuple.first >> collectUsedCustomTypeArgs context.lookupTable)
|
||||
collectUsedCustomTypeArgs context.lookupTable (List.map Tuple.first cases)
|
||||
in
|
||||
( [], { context | usedArguments = registerUsedPatterns usedArguments context.usedArguments } )
|
||||
{ context | usedArguments = registerUsedPatterns usedArguments context.usedArguments }
|
||||
|
||||
Expression.LetExpression { declarations } ->
|
||||
let
|
||||
@ -390,109 +387,107 @@ expressionVisitor node context =
|
||||
(\declaration ->
|
||||
case Node.value declaration of
|
||||
Expression.LetDestructuring pattern _ ->
|
||||
collectUsedCustomTypeArgs context.lookupTable pattern
|
||||
collectUsedCustomTypeArgs context.lookupTable [ pattern ]
|
||||
|
||||
Expression.LetFunction function ->
|
||||
collectUsedPatternsFromFunctionDeclaration context function
|
||||
)
|
||||
declarations
|
||||
in
|
||||
( [], { context | usedArguments = registerUsedPatterns usedArguments context.usedArguments } )
|
||||
{ context | usedArguments = registerUsedPatterns usedArguments context.usedArguments }
|
||||
|
||||
Expression.LambdaExpression { args } ->
|
||||
( []
|
||||
, { context
|
||||
{ context
|
||||
| usedArguments =
|
||||
registerUsedPatterns
|
||||
(List.concatMap (collectUsedCustomTypeArgs context.lookupTable) args)
|
||||
(collectUsedCustomTypeArgs context.lookupTable args)
|
||||
context.usedArguments
|
||||
}
|
||||
)
|
||||
}
|
||||
|
||||
Expression.OperatorApplication operator _ left right ->
|
||||
if operator == "==" || operator == "/=" then
|
||||
let
|
||||
customTypesNotToReport : Set ( ModuleName, String )
|
||||
customTypesNotToReport =
|
||||
Set.union
|
||||
(findCustomTypes context.lookupTable left)
|
||||
(findCustomTypes context.lookupTable right)
|
||||
findCustomTypes context.lookupTable [ left, right ]
|
||||
in
|
||||
( [], { context | customTypesNotToReport = Set.union customTypesNotToReport context.customTypesNotToReport } )
|
||||
{ context | customTypesNotToReport = Set.union customTypesNotToReport context.customTypesNotToReport }
|
||||
|
||||
else
|
||||
( [], context )
|
||||
context
|
||||
|
||||
Expression.Application ((Node _ (Expression.PrefixOperator operator)) :: restOfArgs) ->
|
||||
if operator == "==" || operator == "/=" then
|
||||
let
|
||||
customTypesNotToReport : Set ( ModuleName, String )
|
||||
customTypesNotToReport =
|
||||
List.foldl
|
||||
(findCustomTypes context.lookupTable >> Set.union)
|
||||
Set.empty
|
||||
restOfArgs
|
||||
findCustomTypes context.lookupTable restOfArgs
|
||||
in
|
||||
( [], { context | customTypesNotToReport = Set.union customTypesNotToReport context.customTypesNotToReport } )
|
||||
{ context | customTypesNotToReport = Set.union customTypesNotToReport context.customTypesNotToReport }
|
||||
|
||||
else
|
||||
( [], context )
|
||||
context
|
||||
|
||||
_ ->
|
||||
( [], context )
|
||||
context
|
||||
|
||||
|
||||
findCustomTypes : ModuleNameLookupTable -> Node Expression -> Set ( ModuleName, String )
|
||||
findCustomTypes lookupTable node =
|
||||
case Node.value node of
|
||||
Expression.FunctionOrValue rawModuleName functionName ->
|
||||
if isCustomTypeConstructor functionName then
|
||||
case ModuleNameLookupTable.moduleNameFor lookupTable node of
|
||||
Just moduleName ->
|
||||
Set.singleton ( moduleName, functionName )
|
||||
findCustomTypes : ModuleNameLookupTable -> List (Node Expression) -> Set ( ModuleName, String )
|
||||
findCustomTypes lookupTable nodes =
|
||||
findCustomTypesHelp lookupTable nodes []
|
||||
|> Set.fromList
|
||||
|
||||
Nothing ->
|
||||
Set.singleton ( rawModuleName, functionName )
|
||||
|
||||
else
|
||||
Set.empty
|
||||
findCustomTypesHelp : ModuleNameLookupTable -> List (Node Expression) -> List ( ModuleName, String ) -> List ( ModuleName, String )
|
||||
findCustomTypesHelp lookupTable nodes acc =
|
||||
case nodes of
|
||||
[] ->
|
||||
acc
|
||||
|
||||
Expression.TupledExpression expressions ->
|
||||
List.foldl (findCustomTypes lookupTable >> Set.union) Set.empty expressions
|
||||
node :: restOfNodes ->
|
||||
case Node.value node of
|
||||
Expression.FunctionOrValue rawModuleName functionName ->
|
||||
if isCustomTypeConstructor functionName then
|
||||
case ModuleNameLookupTable.moduleNameFor lookupTable node of
|
||||
Just moduleName ->
|
||||
findCustomTypesHelp lookupTable restOfNodes (( moduleName, functionName ) :: acc)
|
||||
|
||||
Expression.ParenthesizedExpression expression ->
|
||||
findCustomTypes lookupTable expression
|
||||
Nothing ->
|
||||
findCustomTypesHelp lookupTable restOfNodes (( rawModuleName, functionName ) :: acc)
|
||||
|
||||
Expression.Application [] ->
|
||||
Set.empty
|
||||
else
|
||||
findCustomTypesHelp lookupTable restOfNodes acc
|
||||
|
||||
Expression.Application (((Node _ (Expression.FunctionOrValue _ functionName)) as first) :: expressions) ->
|
||||
if isCustomTypeConstructor functionName then
|
||||
List.foldl (findCustomTypes lookupTable >> Set.union) Set.empty (first :: expressions)
|
||||
Expression.TupledExpression expressions ->
|
||||
findCustomTypesHelp lookupTable (expressions ++ restOfNodes) acc
|
||||
|
||||
else
|
||||
Set.empty
|
||||
Expression.ParenthesizedExpression expression ->
|
||||
findCustomTypesHelp lookupTable (expression :: restOfNodes) acc
|
||||
|
||||
Expression.OperatorApplication _ _ left right ->
|
||||
Set.union
|
||||
(findCustomTypes lookupTable left)
|
||||
(findCustomTypes lookupTable right)
|
||||
Expression.Application (((Node _ (Expression.FunctionOrValue _ functionName)) as first) :: expressions) ->
|
||||
if isCustomTypeConstructor functionName then
|
||||
findCustomTypesHelp lookupTable (first :: (expressions ++ restOfNodes)) acc
|
||||
|
||||
Expression.Negation expression ->
|
||||
findCustomTypes lookupTable expression
|
||||
else
|
||||
findCustomTypesHelp lookupTable restOfNodes acc
|
||||
|
||||
Expression.ListExpr expressions ->
|
||||
List.foldl (findCustomTypes lookupTable >> Set.union) Set.empty expressions
|
||||
Expression.OperatorApplication _ _ left right ->
|
||||
findCustomTypesHelp lookupTable (left :: right :: restOfNodes) acc
|
||||
|
||||
_ ->
|
||||
Set.empty
|
||||
Expression.Negation expression ->
|
||||
findCustomTypesHelp lookupTable (expression :: restOfNodes) acc
|
||||
|
||||
Expression.ListExpr expressions ->
|
||||
findCustomTypesHelp lookupTable (expressions ++ restOfNodes) acc
|
||||
|
||||
_ ->
|
||||
findCustomTypesHelp lookupTable restOfNodes acc
|
||||
|
||||
|
||||
isCustomTypeConstructor : String -> Bool
|
||||
isCustomTypeConstructor functionName =
|
||||
String.toList functionName
|
||||
|> List.take 1
|
||||
|> List.all Char.isUpper
|
||||
String.slice 0 1 functionName
|
||||
|> String.all Char.isUpper
|
||||
|
||||
|
||||
registerUsedPatterns : List ( ( ModuleName, String ), Set Int ) -> Dict ( ModuleName, String ) (Set Int) -> Dict ( ModuleName, String ) (Set Int)
|
||||
@ -511,48 +506,68 @@ registerUsedPatterns newUsedArguments previouslyUsedArguments =
|
||||
newUsedArguments
|
||||
|
||||
|
||||
collectUsedCustomTypeArgs : ModuleNameLookupTable -> Node Pattern -> List ( ( ModuleName, String ), Set Int )
|
||||
collectUsedCustomTypeArgs lookupTable (Node range pattern) =
|
||||
case pattern of
|
||||
Pattern.NamedPattern { name } args ->
|
||||
let
|
||||
subList : List ( ( ModuleName, String ), Set Int )
|
||||
subList =
|
||||
List.concatMap (collectUsedCustomTypeArgs lookupTable) args
|
||||
in
|
||||
case ModuleNameLookupTable.moduleNameAt lookupTable range of
|
||||
Just moduleName ->
|
||||
collectUsedCustomTypeArgs : ModuleNameLookupTable -> List (Node Pattern) -> List ( ( ModuleName, String ), Set Int )
|
||||
collectUsedCustomTypeArgs lookupTable nodes =
|
||||
collectUsedCustomTypeArgsHelp lookupTable nodes []
|
||||
|
||||
|
||||
collectUsedCustomTypeArgsHelp : ModuleNameLookupTable -> List (Node Pattern) -> List ( ( ModuleName, String ), Set Int ) -> List ( ( ModuleName, String ), Set Int )
|
||||
collectUsedCustomTypeArgsHelp lookupTable nodes acc =
|
||||
case nodes of
|
||||
[] ->
|
||||
acc
|
||||
|
||||
(Node range pattern) :: restOfNodes ->
|
||||
case pattern of
|
||||
Pattern.NamedPattern { name } args ->
|
||||
let
|
||||
usedPositions : Set Int
|
||||
usedPositions =
|
||||
args
|
||||
|> List.indexedMap Tuple.pair
|
||||
|> List.filter (\( _, subPattern ) -> not <| isWildcard subPattern)
|
||||
|> List.map Tuple.first
|
||||
|> Set.fromList
|
||||
newAcc : List ( ( ModuleName, String ), Set Int )
|
||||
newAcc =
|
||||
case ModuleNameLookupTable.moduleNameAt lookupTable range of
|
||||
Just moduleName ->
|
||||
( ( moduleName, name ), computeUsedPositions 0 args Set.empty ) :: acc
|
||||
|
||||
Nothing ->
|
||||
acc
|
||||
in
|
||||
( ( moduleName, name ), usedPositions ) :: subList
|
||||
collectUsedCustomTypeArgsHelp lookupTable (args ++ restOfNodes) newAcc
|
||||
|
||||
Nothing ->
|
||||
subList
|
||||
Pattern.TuplePattern patterns ->
|
||||
collectUsedCustomTypeArgsHelp lookupTable (patterns ++ restOfNodes) acc
|
||||
|
||||
Pattern.TuplePattern patterns ->
|
||||
List.concatMap (collectUsedCustomTypeArgs lookupTable) patterns
|
||||
Pattern.ListPattern patterns ->
|
||||
collectUsedCustomTypeArgsHelp lookupTable (patterns ++ restOfNodes) acc
|
||||
|
||||
Pattern.ListPattern patterns ->
|
||||
List.concatMap (collectUsedCustomTypeArgs lookupTable) patterns
|
||||
Pattern.UnConsPattern left right ->
|
||||
collectUsedCustomTypeArgsHelp lookupTable (left :: right :: restOfNodes) acc
|
||||
|
||||
Pattern.UnConsPattern left right ->
|
||||
List.concatMap (collectUsedCustomTypeArgs lookupTable) [ left, right ]
|
||||
Pattern.ParenthesizedPattern subPattern ->
|
||||
collectUsedCustomTypeArgsHelp lookupTable (subPattern :: restOfNodes) acc
|
||||
|
||||
Pattern.ParenthesizedPattern subPattern ->
|
||||
collectUsedCustomTypeArgs lookupTable subPattern
|
||||
Pattern.AsPattern subPattern _ ->
|
||||
collectUsedCustomTypeArgsHelp lookupTable (subPattern :: restOfNodes) acc
|
||||
|
||||
Pattern.AsPattern subPattern _ ->
|
||||
collectUsedCustomTypeArgs lookupTable subPattern
|
||||
_ ->
|
||||
collectUsedCustomTypeArgsHelp lookupTable restOfNodes acc
|
||||
|
||||
_ ->
|
||||
[]
|
||||
|
||||
computeUsedPositions : Int -> List (Node Pattern) -> Set Int -> Set Int
|
||||
computeUsedPositions index arguments acc =
|
||||
case arguments of
|
||||
[] ->
|
||||
acc
|
||||
|
||||
arg :: restOfArgs ->
|
||||
let
|
||||
newAcc : Set Int
|
||||
newAcc =
|
||||
if isWildcard arg then
|
||||
acc
|
||||
|
||||
else
|
||||
Set.insert index acc
|
||||
in
|
||||
computeUsedPositions (index + 1) restOfArgs newAcc
|
||||
|
||||
|
||||
isWildcard : Node Pattern -> Bool
|
||||
@ -574,35 +589,46 @@ isWildcard node =
|
||||
|
||||
finalEvaluation : ProjectContext -> List (Error { useErrorForModule : () })
|
||||
finalEvaluation context =
|
||||
context.customTypeArgs
|
||||
|> Dict.toList
|
||||
|> List.concatMap
|
||||
(\( moduleName, { moduleKey, args } ) ->
|
||||
args
|
||||
|> Dict.toList
|
||||
|> List.concatMap
|
||||
(\( name, ranges ) ->
|
||||
if Set.member ( moduleName, name ) context.customTypesNotToReport then
|
||||
[]
|
||||
Dict.foldl (finalEvaluationForSingleModule context) [] context.customTypeArgs
|
||||
|
||||
else
|
||||
case Dict.get ( moduleName, name ) context.usedArguments of
|
||||
Just usedArgumentPositions ->
|
||||
ranges
|
||||
|> List.indexedMap Tuple.pair
|
||||
|> List.filterMap
|
||||
(\( index, range ) ->
|
||||
if Set.member index usedArgumentPositions then
|
||||
Nothing
|
||||
|
||||
else
|
||||
Just (error moduleKey range)
|
||||
)
|
||||
finalEvaluationForSingleModule : ProjectContext -> ModuleName -> { moduleKey : Rule.ModuleKey, args : Dict String (List Range) } -> List (Error { useErrorForModule : () }) -> List (Error { useErrorForModule : () })
|
||||
finalEvaluationForSingleModule context moduleName { moduleKey, args } previousErrors =
|
||||
Dict.foldl
|
||||
(\name ranges acc ->
|
||||
let
|
||||
constructor : ( ModuleName, String )
|
||||
constructor =
|
||||
( moduleName, name )
|
||||
in
|
||||
if Set.member constructor context.customTypesNotToReport then
|
||||
acc
|
||||
|
||||
Nothing ->
|
||||
List.map (error moduleKey) ranges
|
||||
)
|
||||
)
|
||||
else
|
||||
errorsForUnusedArguments context.usedArguments moduleKey constructor ranges acc
|
||||
)
|
||||
previousErrors
|
||||
args
|
||||
|
||||
|
||||
errorsForUnusedArguments : Dict ( ModuleName, String ) (Set Int) -> Rule.ModuleKey -> ( ModuleName, String ) -> List Range -> List (Error anywhere) -> List (Error anywhere)
|
||||
errorsForUnusedArguments usedArguments moduleKey constructor ranges acc =
|
||||
case Dict.get constructor usedArguments of
|
||||
Just usedArgumentPositions ->
|
||||
List.Extra.indexedFilterMap
|
||||
(\index range ->
|
||||
if Set.member index usedArgumentPositions then
|
||||
Nothing
|
||||
|
||||
else
|
||||
Just (error moduleKey range)
|
||||
)
|
||||
0
|
||||
ranges
|
||||
acc
|
||||
|
||||
Nothing ->
|
||||
List.map (error moduleKey) ranges ++ acc
|
||||
|
||||
|
||||
error : Rule.ModuleKey -> Range -> Error anywhere
|
||||
|
File diff suppressed because it is too large
Load Diff
@ -808,6 +808,18 @@ type alias Thing = { id : Id ThingType }
|
||||
]
|
||||
)
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report a custom type constructor that uses extensible records on the type variable" <|
|
||||
\() ->
|
||||
"""
|
||||
module MyModule exposing (id)
|
||||
type alias User = { id : String, age : Int }
|
||||
type Id a = Id { a | id : String }
|
||||
|
||||
id : Id User
|
||||
id = Id { id = "ok", age = 10 }
|
||||
"""
|
||||
|> Review.Test.runWithProjectData project (rule [])
|
||||
|> Review.Test.expectNoErrors
|
||||
]
|
||||
|
||||
|
||||
|
@ -185,21 +185,13 @@ elmJsonVisitor maybeProject projectContext =
|
||||
( directProjectDependencies, directTestDependencies ) =
|
||||
case project of
|
||||
Elm.Project.Package { deps, testDeps } ->
|
||||
( deps
|
||||
|> List.map (Tuple.first >> Elm.Package.toString)
|
||||
|> Set.fromList
|
||||
, testDeps
|
||||
|> List.map (Tuple.first >> Elm.Package.toString)
|
||||
|> Set.fromList
|
||||
( listDependencies deps
|
||||
, listDependencies testDeps
|
||||
)
|
||||
|
||||
Elm.Project.Application { depsDirect, testDepsDirect } ->
|
||||
( depsDirect
|
||||
|> List.map (Tuple.first >> Elm.Package.toString)
|
||||
|> Set.fromList
|
||||
, testDepsDirect
|
||||
|> List.map (Tuple.first >> Elm.Package.toString)
|
||||
|> Set.fromList
|
||||
( listDependencies depsDirect
|
||||
, listDependencies testDepsDirect
|
||||
)
|
||||
in
|
||||
( []
|
||||
@ -214,6 +206,14 @@ elmJsonVisitor maybeProject projectContext =
|
||||
( [], projectContext )
|
||||
|
||||
|
||||
listDependencies : List ( Elm.Package.Name, a ) -> Set String
|
||||
listDependencies deps =
|
||||
List.foldl
|
||||
(\( name, _ ) acc -> Set.insert (Elm.Package.toString name) acc)
|
||||
Set.empty
|
||||
deps
|
||||
|
||||
|
||||
|
||||
-- IMPORT VISITOR
|
||||
|
||||
@ -329,7 +329,14 @@ unusedTestDependencyError elmJsonKey dependencies packageName =
|
||||
, range = findPackageNameInElmJson packageName elmJson
|
||||
}
|
||||
)
|
||||
(fromProject dependencies InTestDeps packageName >> Maybe.map (removeTestDependency >> toProject))
|
||||
(\project ->
|
||||
case fromProject dependencies InTestDeps packageName project of
|
||||
Just projectAndDependencyIdentifier ->
|
||||
Just (toProject (removeTestDependency projectAndDependencyIdentifier))
|
||||
|
||||
Nothing ->
|
||||
Nothing
|
||||
)
|
||||
|
||||
|
||||
findPackageNameInElmJson : String -> String -> Range
|
||||
@ -478,7 +485,7 @@ removeProjectDependency projectAndDependencyIdentifier =
|
||||
let
|
||||
directDependencies : List ( Elm.Package.Name, Elm.Version.Version )
|
||||
directDependencies =
|
||||
List.filter (isPackageWithName (Elm.Package.toString project.name) >> not) application.depsDirect
|
||||
List.filter (\pkg -> pkg |> isPackageWithName (Elm.Package.toString project.name) |> not) application.depsDirect
|
||||
|
||||
depsIndirect : Elm.Project.Deps Elm.Version.Version
|
||||
depsIndirect =
|
||||
@ -506,7 +513,7 @@ removeProjectDependency projectAndDependencyIdentifier =
|
||||
{ project
|
||||
| package =
|
||||
{ package
|
||||
| deps = List.filter (isPackageWithName (Elm.Package.toString project.name) >> not) package.deps
|
||||
| deps = List.filter (\pkg -> pkg |> isPackageWithName (Elm.Package.toString project.name) |> not) package.deps
|
||||
}
|
||||
}
|
||||
|
||||
@ -580,7 +587,7 @@ removeTestDependency projectAndDependencyIdentifier =
|
||||
let
|
||||
testDepsDirect : List ( Elm.Package.Name, Elm.Version.Version )
|
||||
testDepsDirect =
|
||||
List.filter (isPackageWithName (Elm.Package.toString project.name) >> not) application.testDepsDirect
|
||||
List.filter (\pkg -> pkg |> isPackageWithName (Elm.Package.toString project.name) |> not) application.testDepsDirect
|
||||
in
|
||||
ApplicationProject
|
||||
{ project
|
||||
@ -600,7 +607,7 @@ removeTestDependency projectAndDependencyIdentifier =
|
||||
{ project
|
||||
| package =
|
||||
{ package
|
||||
| testDeps = List.filter (isPackageWithName (Elm.Package.toString project.name) >> not) package.testDeps
|
||||
| testDeps = List.filter (\pkg -> pkg |> isPackageWithName (Elm.Package.toString project.name) |> not) package.testDeps
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -19,6 +19,7 @@ import Elm.Syntax.Import exposing (Import)
|
||||
import Elm.Syntax.Module as Module exposing (Module)
|
||||
import Elm.Syntax.ModuleName exposing (ModuleName)
|
||||
import Elm.Syntax.Node as Node exposing (Node(..))
|
||||
import Elm.Syntax.Pattern as Pattern exposing (Pattern)
|
||||
import Elm.Syntax.Range exposing (Range)
|
||||
import Elm.Syntax.TypeAnnotation as TypeAnnotation exposing (TypeAnnotation)
|
||||
import List.Extra
|
||||
@ -61,7 +62,7 @@ rule =
|
||||
, foldProjectContexts = foldProjectContexts
|
||||
}
|
||||
|> Rule.withContextFromImportedModules
|
||||
|> Rule.withElmJsonProjectVisitor elmJsonVisitor
|
||||
|> Rule.withElmJsonProjectVisitor (\project context -> ( [], elmJsonVisitor project context ))
|
||||
|> Rule.withFinalProjectEvaluation finalEvaluationForProject
|
||||
|> Rule.fromProjectRuleSchema
|
||||
|
||||
@ -69,11 +70,11 @@ rule =
|
||||
moduleVisitor : Rule.ModuleRuleSchema {} ModuleContext -> Rule.ModuleRuleSchema { hasAtLeastOneVisitor : () } ModuleContext
|
||||
moduleVisitor schema =
|
||||
schema
|
||||
|> Rule.withModuleDefinitionVisitor moduleDefinitionVisitor
|
||||
|> Rule.withCommentsVisitor commentsVisitor
|
||||
|> Rule.withImportVisitor importVisitor
|
||||
|> Rule.withDeclarationListVisitor declarationListVisitor
|
||||
|> Rule.withExpressionEnterVisitor expressionVisitor
|
||||
|> Rule.withModuleDefinitionVisitor (\project context -> ( [], moduleDefinitionVisitor project context ))
|
||||
|> Rule.withCommentsVisitor (\project context -> ( [], commentsVisitor project context ))
|
||||
|> Rule.withImportVisitor (\project context -> ( [], importVisitor project context ))
|
||||
|> Rule.withDeclarationListVisitor (\project context -> ( [], declarationListVisitor project context ))
|
||||
|> Rule.withExpressionEnterVisitor (\project context -> ( [], expressionVisitor project context ))
|
||||
|
||||
|
||||
|
||||
@ -89,6 +90,7 @@ type alias ProjectContext =
|
||||
, exposed : Dict String ExposedElement
|
||||
}
|
||||
, used : Set ( ModuleName, String )
|
||||
, constructors : Dict ( ModuleName, String ) String
|
||||
}
|
||||
|
||||
|
||||
@ -112,7 +114,7 @@ type ElmApplicationType
|
||||
type ExposedElementType
|
||||
= Function
|
||||
| TypeOrTypeAlias
|
||||
| ExposedType
|
||||
| ExposedType (List String)
|
||||
|
||||
|
||||
type alias ModuleContext =
|
||||
@ -130,6 +132,7 @@ initialProjectContext =
|
||||
{ projectType = IsApplication ElmApplication
|
||||
, modules = Dict.empty
|
||||
, used = Set.empty
|
||||
, constructors = Dict.empty
|
||||
}
|
||||
|
||||
|
||||
@ -146,17 +149,37 @@ fromProjectToModule lookupTable _ =
|
||||
|
||||
fromModuleToProject : Rule.ModuleKey -> Rule.Metadata -> ModuleContext -> ProjectContext
|
||||
fromModuleToProject moduleKey metadata moduleContext =
|
||||
let
|
||||
moduleName : ModuleName
|
||||
moduleName =
|
||||
Rule.moduleNameFromMetadata metadata
|
||||
in
|
||||
{ projectType = IsApplication ElmApplication
|
||||
, modules =
|
||||
Dict.singleton
|
||||
(Rule.moduleNameFromMetadata metadata)
|
||||
moduleName
|
||||
{ moduleKey = moduleKey
|
||||
, exposed = moduleContext.exposed
|
||||
}
|
||||
, used =
|
||||
moduleContext.elementsNotToReport
|
||||
|> Set.map (Tuple.pair <| Rule.moduleNameFromMetadata metadata)
|
||||
|> Set.map (Tuple.pair moduleName)
|
||||
|> Set.union moduleContext.used
|
||||
, constructors =
|
||||
Dict.foldl
|
||||
(\name element acc ->
|
||||
case element.elementType of
|
||||
ExposedType constructorNames ->
|
||||
List.foldl
|
||||
(\constructorName listAcc -> Dict.insert ( moduleName, constructorName ) name listAcc)
|
||||
acc
|
||||
constructorNames
|
||||
|
||||
_ ->
|
||||
acc
|
||||
)
|
||||
Dict.empty
|
||||
moduleContext.exposed
|
||||
}
|
||||
|
||||
|
||||
@ -165,6 +188,7 @@ foldProjectContexts newContext previousContext =
|
||||
{ projectType = previousContext.projectType
|
||||
, modules = Dict.union previousContext.modules newContext.modules
|
||||
, used = Set.union newContext.used previousContext.used
|
||||
, constructors = Dict.union previousContext.constructors newContext.constructors
|
||||
}
|
||||
|
||||
|
||||
@ -182,7 +206,7 @@ registerMultipleAsUsed usedElements moduleContext =
|
||||
-- ELM JSON VISITOR
|
||||
|
||||
|
||||
elmJsonVisitor : Maybe { a | project : Elm.Project.Project } -> ProjectContext -> ( List nothing, ProjectContext )
|
||||
elmJsonVisitor : Maybe { a | project : Elm.Project.Project } -> ProjectContext -> ProjectContext
|
||||
elmJsonVisitor maybeProject projectContext =
|
||||
case maybeProject |> Maybe.map .project of
|
||||
Just (Elm.Project.Package { exposed }) ->
|
||||
@ -196,15 +220,16 @@ elmJsonVisitor maybeProject projectContext =
|
||||
Elm.Project.ExposedDict fakeDict ->
|
||||
List.concatMap Tuple.second fakeDict
|
||||
in
|
||||
( []
|
||||
, { projectContext
|
||||
{ projectContext
|
||||
| projectType =
|
||||
exposedModuleNames
|
||||
|> List.map (Elm.Module.toString >> String.split ".")
|
||||
|> Set.fromList
|
||||
|> List.foldr
|
||||
(\moduleName acc ->
|
||||
Set.insert (Elm.Module.toString moduleName |> String.split ".") acc
|
||||
)
|
||||
Set.empty
|
||||
|> IsPackage
|
||||
}
|
||||
)
|
||||
}
|
||||
|
||||
Just (Elm.Project.Application { depsDirect }) ->
|
||||
let
|
||||
@ -216,10 +241,10 @@ elmJsonVisitor maybeProject projectContext =
|
||||
else
|
||||
ElmApplication
|
||||
in
|
||||
( [], { projectContext | projectType = IsApplication elmApplicationType } )
|
||||
{ projectContext | projectType = IsApplication elmApplicationType }
|
||||
|
||||
Nothing ->
|
||||
( [], { projectContext | projectType = IsApplication ElmApplication } )
|
||||
{ projectContext | projectType = IsApplication ElmApplication }
|
||||
|
||||
|
||||
|
||||
@ -228,18 +253,33 @@ elmJsonVisitor maybeProject projectContext =
|
||||
|
||||
finalEvaluationForProject : ProjectContext -> List (Error { useErrorForModule : () })
|
||||
finalEvaluationForProject projectContext =
|
||||
let
|
||||
used : Set ( ModuleName, String )
|
||||
used =
|
||||
Set.foldl
|
||||
(\(( moduleName, _ ) as key) acc ->
|
||||
case Dict.get key projectContext.constructors of
|
||||
Just typeName ->
|
||||
Set.insert ( moduleName, typeName ) acc
|
||||
|
||||
Nothing ->
|
||||
acc
|
||||
)
|
||||
projectContext.used
|
||||
projectContext.used
|
||||
in
|
||||
projectContext.modules
|
||||
|> removeExposedPackages projectContext
|
||||
|> Dict.toList
|
||||
|> List.concatMap (errorsForModule projectContext)
|
||||
|> List.concatMap (errorsForModule projectContext used)
|
||||
|
||||
|
||||
errorsForModule : ProjectContext -> ( ModuleName, { moduleKey : Rule.ModuleKey, exposed : Dict String ExposedElement } ) -> List (Error scope)
|
||||
errorsForModule projectContext ( moduleName, { moduleKey, exposed } ) =
|
||||
errorsForModule : ProjectContext -> Set ( ModuleName, String ) -> ( ModuleName, { moduleKey : Rule.ModuleKey, exposed : Dict String ExposedElement } ) -> List (Error scope)
|
||||
errorsForModule projectContext used ( moduleName, { moduleKey, exposed } ) =
|
||||
exposed
|
||||
|> removeApplicationExceptions projectContext
|
||||
|> removeReviewConfig moduleName
|
||||
|> Dict.filter (\name _ -> not <| Set.member ( moduleName, name ) projectContext.used)
|
||||
|> Dict.filter (\name _ -> not <| Set.member ( moduleName, name ) used)
|
||||
|> Dict.toList
|
||||
|> List.concatMap
|
||||
(\( name, element ) ->
|
||||
@ -253,7 +293,7 @@ errorsForModule projectContext ( moduleName, { moduleKey, exposed } ) =
|
||||
TypeOrTypeAlias ->
|
||||
"Exposed type or type alias"
|
||||
|
||||
ExposedType ->
|
||||
ExposedType _ ->
|
||||
"Exposed type"
|
||||
in
|
||||
[ Rule.errorForModuleWithFix moduleKey
|
||||
@ -304,42 +344,52 @@ removeReviewConfig moduleName dict =
|
||||
-- MODULE DEFINITION VISITOR
|
||||
|
||||
|
||||
moduleDefinitionVisitor : Node Module -> ModuleContext -> ( List nothing, ModuleContext )
|
||||
moduleDefinitionVisitor : Node Module -> ModuleContext -> ModuleContext
|
||||
moduleDefinitionVisitor moduleNode moduleContext =
|
||||
case Module.exposingList (Node.value moduleNode) of
|
||||
Exposing.All _ ->
|
||||
( [], { moduleContext | exposesEverything = True } )
|
||||
{ moduleContext | exposesEverything = True }
|
||||
|
||||
Exposing.Explicit list ->
|
||||
( [], { moduleContext | rawExposed = list } )
|
||||
{ moduleContext | rawExposed = list }
|
||||
|
||||
|
||||
commentsVisitor : List (Node String) -> ModuleContext -> ( List nothing, ModuleContext )
|
||||
commentsVisitor : List (Node String) -> ModuleContext -> ModuleContext
|
||||
commentsVisitor nodes moduleContext =
|
||||
if List.isEmpty moduleContext.rawExposed then
|
||||
( [], moduleContext )
|
||||
moduleContext
|
||||
|
||||
else
|
||||
let
|
||||
comments : List ( Int, String )
|
||||
comments =
|
||||
nodes
|
||||
|> List.Extra.find (Node.value >> String.startsWith "{-|")
|
||||
|> Maybe.map
|
||||
(\(Node range comment) ->
|
||||
comment
|
||||
|> String.lines
|
||||
|> List.drop 1
|
||||
|> List.indexedMap (\i line -> ( i + range.start.row + 1, line ))
|
||||
|> List.filter (Tuple.second >> String.startsWith "@docs ")
|
||||
)
|
||||
|> Maybe.withDefault []
|
||||
case List.Extra.find (\(Node _ comment) -> String.startsWith "{-|" comment) nodes of
|
||||
Just (Node range comment) ->
|
||||
let
|
||||
lines : List String
|
||||
lines =
|
||||
comment
|
||||
|> String.lines
|
||||
|> List.drop 1
|
||||
in
|
||||
List.Extra.indexedFilterMap
|
||||
(\lineNumber line ->
|
||||
if String.startsWith "@docs " line then
|
||||
Just ( lineNumber, line )
|
||||
|
||||
else
|
||||
Nothing
|
||||
)
|
||||
(range.start.row + 1)
|
||||
lines
|
||||
[]
|
||||
|
||||
Nothing ->
|
||||
[]
|
||||
in
|
||||
( []
|
||||
, { moduleContext
|
||||
{ moduleContext
|
||||
| exposed = collectExposedElements comments moduleContext.rawExposed
|
||||
}
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
collectExposedElements : List ( Int, String ) -> List (Node Exposing.TopLevelExpose) -> Dict String ExposedElement
|
||||
@ -349,8 +399,8 @@ collectExposedElements comments nodes =
|
||||
listWithPreviousRange =
|
||||
Nothing
|
||||
:: (nodes
|
||||
|> List.map (Node.range >> Just)
|
||||
|> List.take (List.length nodes - 1)
|
||||
|> List.map (\(Node range _) -> Just range)
|
||||
)
|
||||
|
||||
listWithNextRange : List Range
|
||||
@ -389,7 +439,7 @@ collectExposedElements comments nodes =
|
||||
( name
|
||||
, { range = untilEndOfVariable name range
|
||||
, rangesToRemove = []
|
||||
, elementType = ExposedType
|
||||
, elementType = ExposedType []
|
||||
}
|
||||
)
|
||||
|
||||
@ -495,7 +545,7 @@ untilEndOfVariable name range =
|
||||
-- IMPORT VISITOR
|
||||
|
||||
|
||||
importVisitor : Node Import -> ModuleContext -> ( List nothing, ModuleContext )
|
||||
importVisitor : Node Import -> ModuleContext -> ModuleContext
|
||||
importVisitor node moduleContext =
|
||||
case (Node.value node).exposingList |> Maybe.map Node.value of
|
||||
Just (Exposing.Explicit list) ->
|
||||
@ -506,50 +556,52 @@ importVisitor node moduleContext =
|
||||
|
||||
usedElements : List ( ModuleName, String )
|
||||
usedElements =
|
||||
list
|
||||
|> List.filterMap
|
||||
(Node.value
|
||||
>> (\element ->
|
||||
case element of
|
||||
Exposing.FunctionExpose name ->
|
||||
Just ( moduleName, name )
|
||||
List.filterMap
|
||||
(\(Node _ element) ->
|
||||
case element of
|
||||
Exposing.FunctionExpose name ->
|
||||
Just ( moduleName, name )
|
||||
|
||||
Exposing.TypeOrAliasExpose name ->
|
||||
Just ( moduleName, name )
|
||||
Exposing.TypeOrAliasExpose name ->
|
||||
Just ( moduleName, name )
|
||||
|
||||
Exposing.TypeExpose { name } ->
|
||||
Just ( moduleName, name )
|
||||
Exposing.TypeExpose { name } ->
|
||||
Just ( moduleName, name )
|
||||
|
||||
Exposing.InfixExpose _ ->
|
||||
Nothing
|
||||
)
|
||||
)
|
||||
Exposing.InfixExpose _ ->
|
||||
Nothing
|
||||
)
|
||||
list
|
||||
in
|
||||
( [], registerMultipleAsUsed usedElements moduleContext )
|
||||
registerMultipleAsUsed usedElements moduleContext
|
||||
|
||||
Just (Exposing.All _) ->
|
||||
( [], moduleContext )
|
||||
moduleContext
|
||||
|
||||
Nothing ->
|
||||
( [], moduleContext )
|
||||
moduleContext
|
||||
|
||||
|
||||
|
||||
-- DECLARATION LIST VISITOR
|
||||
|
||||
|
||||
declarationListVisitor : List (Node Declaration) -> ModuleContext -> ( List nothing, ModuleContext )
|
||||
declarationListVisitor : List (Node Declaration) -> ModuleContext -> ModuleContext
|
||||
declarationListVisitor declarations moduleContext =
|
||||
let
|
||||
moduleContextWithUpdatedConstructors : ModuleContext
|
||||
moduleContextWithUpdatedConstructors =
|
||||
{ moduleContext | exposed = List.foldl addConstructorsToExposedCustomTypes moduleContext.exposed declarations }
|
||||
|
||||
typesUsedInDeclaration_ : List ( List ( ModuleName, String ), Bool )
|
||||
typesUsedInDeclaration_ =
|
||||
declarations
|
||||
|> List.map (typesUsedInDeclaration moduleContext)
|
||||
|> List.map (typesUsedInDeclaration moduleContextWithUpdatedConstructors)
|
||||
|
||||
testFunctions : List String
|
||||
testFunctions =
|
||||
declarations
|
||||
|> List.filterMap (testFunctionName moduleContext)
|
||||
|> List.filterMap (testFunctionName moduleContextWithUpdatedConstructors)
|
||||
|
||||
allUsedTypes : List ( ModuleName, String )
|
||||
allUsedTypes =
|
||||
@ -558,25 +610,22 @@ declarationListVisitor declarations moduleContext =
|
||||
|
||||
contextWithUsedElements : ModuleContext
|
||||
contextWithUsedElements =
|
||||
registerMultipleAsUsed allUsedTypes moduleContext
|
||||
registerMultipleAsUsed allUsedTypes moduleContextWithUpdatedConstructors
|
||||
in
|
||||
( []
|
||||
, { contextWithUsedElements
|
||||
{ contextWithUsedElements
|
||||
| exposed =
|
||||
contextWithUsedElements.exposed
|
||||
|> (if moduleContext.exposesEverything then
|
||||
identity
|
||||
if moduleContextWithUpdatedConstructors.exposesEverything then
|
||||
contextWithUsedElements.exposed
|
||||
|
||||
else
|
||||
let
|
||||
declaredNames : Set String
|
||||
declaredNames =
|
||||
declarations
|
||||
|> List.filterMap (Node.value >> declarationName)
|
||||
|> Set.fromList
|
||||
in
|
||||
Dict.filter (\name _ -> Set.member name declaredNames)
|
||||
)
|
||||
else
|
||||
let
|
||||
declaredNames : Set String
|
||||
declaredNames =
|
||||
declarations
|
||||
|> List.filterMap (\(Node _ declaration) -> declarationName declaration)
|
||||
|> Set.fromList
|
||||
in
|
||||
Dict.filter (\name _ -> Set.member name declaredNames) contextWithUsedElements.exposed
|
||||
, elementsNotToReport =
|
||||
typesUsedInDeclaration_
|
||||
|> List.concatMap
|
||||
@ -590,8 +639,35 @@ declarationListVisitor declarations moduleContext =
|
||||
|> List.map Tuple.second
|
||||
|> List.append testFunctions
|
||||
|> Set.fromList
|
||||
}
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
addConstructorsToExposedCustomTypes : Node Declaration -> Dict String ExposedElement -> Dict String ExposedElement
|
||||
addConstructorsToExposedCustomTypes node exposed =
|
||||
case Node.value node of
|
||||
Declaration.CustomTypeDeclaration type_ ->
|
||||
case Dict.get (Node.value type_.name) exposed of
|
||||
Just exposedElement ->
|
||||
case exposedElement.elementType of
|
||||
ExposedType [] ->
|
||||
let
|
||||
constructors : List String
|
||||
constructors =
|
||||
List.map (\c -> c |> Node.value |> .name |> Node.value) type_.constructors
|
||||
in
|
||||
Dict.insert
|
||||
(Node.value type_.name)
|
||||
{ exposedElement | elementType = ExposedType constructors }
|
||||
exposed
|
||||
|
||||
_ ->
|
||||
exposed
|
||||
|
||||
Nothing ->
|
||||
exposed
|
||||
|
||||
_ ->
|
||||
exposed
|
||||
|
||||
|
||||
isType : String -> Bool
|
||||
@ -634,10 +710,7 @@ testFunctionName : ModuleContext -> Node Declaration -> Maybe String
|
||||
testFunctionName moduleContext node =
|
||||
case Node.value node of
|
||||
Declaration.FunctionDeclaration function ->
|
||||
case
|
||||
function.signature
|
||||
|> Maybe.map (Node.value >> .typeAnnotation >> Node.value)
|
||||
of
|
||||
case Maybe.map (\(Node _ value) -> Node.value value.typeAnnotation) function.signature of
|
||||
Just (TypeAnnotation.Typed typeNode _) ->
|
||||
if
|
||||
(Tuple.second (Node.value typeNode) == "Test")
|
||||
@ -663,30 +736,37 @@ typesUsedInDeclaration : ModuleContext -> Node Declaration -> ( List ( ModuleNam
|
||||
typesUsedInDeclaration moduleContext declaration =
|
||||
case Node.value declaration of
|
||||
Declaration.FunctionDeclaration function ->
|
||||
( function.signature
|
||||
|> Maybe.map (Node.value >> .typeAnnotation >> collectTypesFromTypeAnnotation moduleContext)
|
||||
|> Maybe.withDefault []
|
||||
( case function.signature of
|
||||
Just signature ->
|
||||
[]
|
||||
|> collectTypesFromTypeAnnotation moduleContext [ (Node.value signature).typeAnnotation ]
|
||||
|> findUsedConstructors moduleContext.lookupTable (Node.value function.declaration).arguments
|
||||
|
||||
Nothing ->
|
||||
findUsedConstructors moduleContext.lookupTable (Node.value function.declaration).arguments []
|
||||
, False
|
||||
)
|
||||
|
||||
Declaration.CustomTypeDeclaration type_ ->
|
||||
( type_.constructors
|
||||
|> List.concatMap (Node.value >> .arguments)
|
||||
|> List.concatMap (collectTypesFromTypeAnnotation moduleContext)
|
||||
, not <|
|
||||
case Dict.get (Node.value type_.name) moduleContext.exposed |> Maybe.map .elementType of
|
||||
Just ExposedType ->
|
||||
True
|
||||
let
|
||||
arguments : List (Node TypeAnnotation)
|
||||
arguments =
|
||||
List.concatMap (\constructor -> (Node.value constructor).arguments) type_.constructors
|
||||
in
|
||||
( collectTypesFromTypeAnnotation moduleContext arguments []
|
||||
, case Dict.get (Node.value type_.name) moduleContext.exposed |> Maybe.map .elementType of
|
||||
Just (ExposedType _) ->
|
||||
False
|
||||
|
||||
_ ->
|
||||
False
|
||||
_ ->
|
||||
True
|
||||
)
|
||||
|
||||
Declaration.AliasDeclaration alias_ ->
|
||||
( collectTypesFromTypeAnnotation moduleContext alias_.typeAnnotation, False )
|
||||
( collectTypesFromTypeAnnotation moduleContext [ alias_.typeAnnotation ] [], False )
|
||||
|
||||
Declaration.PortDeclaration signature ->
|
||||
( collectTypesFromTypeAnnotation moduleContext signature.typeAnnotation, False )
|
||||
( collectTypesFromTypeAnnotation moduleContext [ signature.typeAnnotation ] [], False )
|
||||
|
||||
Declaration.InfixDeclaration _ ->
|
||||
( [], False )
|
||||
@ -695,90 +775,150 @@ typesUsedInDeclaration moduleContext declaration =
|
||||
( [], False )
|
||||
|
||||
|
||||
collectTypesFromTypeAnnotation : ModuleContext -> Node TypeAnnotation -> List ( ModuleName, String )
|
||||
collectTypesFromTypeAnnotation moduleContext node =
|
||||
case Node.value node of
|
||||
TypeAnnotation.FunctionTypeAnnotation a b ->
|
||||
collectTypesFromTypeAnnotation moduleContext a ++ collectTypesFromTypeAnnotation moduleContext b
|
||||
collectTypesFromTypeAnnotation : ModuleContext -> List (Node TypeAnnotation) -> List ( ModuleName, String ) -> List ( ModuleName, String )
|
||||
collectTypesFromTypeAnnotation moduleContext nodes acc =
|
||||
case nodes of
|
||||
[] ->
|
||||
acc
|
||||
|
||||
TypeAnnotation.Typed (Node range ( _, name )) params ->
|
||||
case ModuleNameLookupTable.moduleNameAt moduleContext.lookupTable range of
|
||||
Just moduleName ->
|
||||
( moduleName, name ) :: List.concatMap (collectTypesFromTypeAnnotation moduleContext) params
|
||||
node :: restOfNodes ->
|
||||
case Node.value node of
|
||||
TypeAnnotation.FunctionTypeAnnotation left right ->
|
||||
collectTypesFromTypeAnnotation moduleContext (left :: right :: restOfNodes) acc
|
||||
|
||||
Nothing ->
|
||||
List.concatMap (collectTypesFromTypeAnnotation moduleContext) params
|
||||
TypeAnnotation.Typed (Node range ( _, name )) params ->
|
||||
case ModuleNameLookupTable.moduleNameAt moduleContext.lookupTable range of
|
||||
Just moduleName ->
|
||||
collectTypesFromTypeAnnotation moduleContext (params ++ restOfNodes) (( moduleName, name ) :: acc)
|
||||
|
||||
TypeAnnotation.Record list ->
|
||||
list
|
||||
|> List.map (Node.value >> Tuple.second)
|
||||
|> List.concatMap (collectTypesFromTypeAnnotation moduleContext)
|
||||
Nothing ->
|
||||
collectTypesFromTypeAnnotation moduleContext (params ++ restOfNodes) acc
|
||||
|
||||
TypeAnnotation.GenericRecord _ list ->
|
||||
list
|
||||
|> Node.value
|
||||
|> List.map (Node.value >> Tuple.second)
|
||||
|> List.concatMap (collectTypesFromTypeAnnotation moduleContext)
|
||||
TypeAnnotation.Record fields ->
|
||||
let
|
||||
subNodes : List (Node TypeAnnotation)
|
||||
subNodes =
|
||||
List.map (\(Node _ ( _, value )) -> value) fields
|
||||
in
|
||||
collectTypesFromTypeAnnotation moduleContext (subNodes ++ restOfNodes) acc
|
||||
|
||||
TypeAnnotation.Tupled list ->
|
||||
List.concatMap (collectTypesFromTypeAnnotation moduleContext) list
|
||||
TypeAnnotation.GenericRecord _ (Node _ fields) ->
|
||||
let
|
||||
subNodes : List (Node TypeAnnotation)
|
||||
subNodes =
|
||||
List.map (\(Node _ ( _, value )) -> value) fields
|
||||
in
|
||||
collectTypesFromTypeAnnotation moduleContext (subNodes ++ restOfNodes) acc
|
||||
|
||||
TypeAnnotation.GenericType _ ->
|
||||
[]
|
||||
TypeAnnotation.Tupled list ->
|
||||
collectTypesFromTypeAnnotation moduleContext (list ++ restOfNodes) acc
|
||||
|
||||
TypeAnnotation.Unit ->
|
||||
[]
|
||||
_ ->
|
||||
collectTypesFromTypeAnnotation moduleContext restOfNodes acc
|
||||
|
||||
|
||||
|
||||
-- EXPRESSION VISITOR
|
||||
|
||||
|
||||
expressionVisitor : Node Expression -> ModuleContext -> ( List nothing, ModuleContext )
|
||||
expressionVisitor : Node Expression -> ModuleContext -> ModuleContext
|
||||
expressionVisitor node moduleContext =
|
||||
case Node.value node of
|
||||
Expression.FunctionOrValue _ name ->
|
||||
case ModuleNameLookupTable.moduleNameFor moduleContext.lookupTable node of
|
||||
Just moduleName ->
|
||||
( []
|
||||
, registerAsUsed
|
||||
registerAsUsed
|
||||
( moduleName, name )
|
||||
moduleContext
|
||||
)
|
||||
|
||||
Nothing ->
|
||||
( [], moduleContext )
|
||||
moduleContext
|
||||
|
||||
Expression.RecordUpdateExpression (Node range name) _ ->
|
||||
case ModuleNameLookupTable.moduleNameAt moduleContext.lookupTable range of
|
||||
Just moduleName ->
|
||||
( []
|
||||
, registerAsUsed
|
||||
registerAsUsed
|
||||
( moduleName, name )
|
||||
moduleContext
|
||||
)
|
||||
|
||||
Nothing ->
|
||||
( [], moduleContext )
|
||||
moduleContext
|
||||
|
||||
Expression.LetExpression { declarations } ->
|
||||
let
|
||||
used : List ( ModuleName, String )
|
||||
used =
|
||||
List.concatMap
|
||||
(\declaration ->
|
||||
List.foldl
|
||||
(\declaration acc ->
|
||||
case Node.value declaration of
|
||||
Expression.LetFunction function ->
|
||||
function.signature
|
||||
|> Maybe.map (Node.value >> .typeAnnotation >> collectTypesFromTypeAnnotation moduleContext)
|
||||
|> Maybe.withDefault []
|
||||
case function.signature of
|
||||
Just signature ->
|
||||
acc
|
||||
|> collectTypesFromTypeAnnotation moduleContext [ (Node.value signature).typeAnnotation ]
|
||||
|> findUsedConstructors moduleContext.lookupTable (Node.value function.declaration).arguments
|
||||
|
||||
Expression.LetDestructuring _ _ ->
|
||||
[]
|
||||
Nothing ->
|
||||
findUsedConstructors moduleContext.lookupTable (Node.value function.declaration).arguments acc
|
||||
|
||||
Expression.LetDestructuring pattern _ ->
|
||||
findUsedConstructors moduleContext.lookupTable [ pattern ] acc
|
||||
)
|
||||
[]
|
||||
declarations
|
||||
in
|
||||
( [], registerMultipleAsUsed used moduleContext )
|
||||
registerMultipleAsUsed used moduleContext
|
||||
|
||||
Expression.CaseExpression { cases } ->
|
||||
let
|
||||
usedConstructors : List ( ModuleName, String )
|
||||
usedConstructors =
|
||||
findUsedConstructors
|
||||
moduleContext.lookupTable
|
||||
(List.map Tuple.first cases)
|
||||
[]
|
||||
in
|
||||
registerMultipleAsUsed usedConstructors moduleContext
|
||||
|
||||
_ ->
|
||||
( [], moduleContext )
|
||||
moduleContext
|
||||
|
||||
|
||||
findUsedConstructors : ModuleNameLookupTable -> List (Node Pattern) -> List ( ModuleName, String ) -> List ( ModuleName, String )
|
||||
findUsedConstructors lookupTable patterns acc =
|
||||
case patterns of
|
||||
[] ->
|
||||
acc
|
||||
|
||||
pattern :: restOfPatterns ->
|
||||
case Node.value pattern of
|
||||
Pattern.NamedPattern qualifiedNameRef newPatterns ->
|
||||
let
|
||||
newAcc : List ( ModuleName, String )
|
||||
newAcc =
|
||||
case ModuleNameLookupTable.moduleNameFor lookupTable pattern of
|
||||
Just moduleName ->
|
||||
( moduleName, qualifiedNameRef.name ) :: acc
|
||||
|
||||
Nothing ->
|
||||
acc
|
||||
in
|
||||
findUsedConstructors lookupTable (newPatterns ++ restOfPatterns) newAcc
|
||||
|
||||
Pattern.TuplePattern newPatterns ->
|
||||
findUsedConstructors lookupTable (newPatterns ++ restOfPatterns) acc
|
||||
|
||||
Pattern.UnConsPattern left right ->
|
||||
findUsedConstructors lookupTable (left :: right :: restOfPatterns) acc
|
||||
|
||||
Pattern.ListPattern newPatterns ->
|
||||
findUsedConstructors lookupTable (newPatterns ++ restOfPatterns) acc
|
||||
|
||||
Pattern.AsPattern node _ ->
|
||||
findUsedConstructors lookupTable (node :: restOfPatterns) acc
|
||||
|
||||
Pattern.ParenthesizedPattern node ->
|
||||
findUsedConstructors lookupTable (node :: restOfPatterns) acc
|
||||
|
||||
_ ->
|
||||
findUsedConstructors lookupTable restOfPatterns acc
|
||||
|
@ -283,7 +283,7 @@ a = Test.describe "thing" []
|
||||
"""
|
||||
|> Review.Test.runWithProjectData package rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not ReviewConfig.config" <|
|
||||
, test "should not report ReviewConfig.config" <|
|
||||
\() ->
|
||||
"""
|
||||
module ReviewConfig exposing (config)
|
||||
@ -328,13 +328,94 @@ type Exposed = VariantA | VariantB
|
||||
, test "should not report a used exposed custom type (type signature)" <|
|
||||
\() ->
|
||||
[ """
|
||||
module A exposing (Exposed)
|
||||
module A exposing (Exposed, variantA)
|
||||
type Exposed = VariantA | VariantB
|
||||
variantA = VariantA
|
||||
""", """
|
||||
module B exposing (main)
|
||||
import A
|
||||
main : A.Exposed
|
||||
main = VariantA
|
||||
main = A.variantA
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData application rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report a used exposed custom type (value usage)" <|
|
||||
\() ->
|
||||
[ """
|
||||
module A exposing (Exposed(..))
|
||||
type Exposed = VariantA | VariantB
|
||||
""", """
|
||||
module B exposing (main)
|
||||
import A
|
||||
main = A.VariantA
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData application rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report a used exposed custom type (case expression usage)" <|
|
||||
\() ->
|
||||
[ """
|
||||
module A exposing (main)
|
||||
import Variant
|
||||
import Html
|
||||
|
||||
main =
|
||||
case config of
|
||||
Variant.A -> Html.text "a"
|
||||
""", """
|
||||
module Variant exposing (Variant(..))
|
||||
|
||||
type Variant = A
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData application rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report a used exposed custom type (argument destructuring usage)" <|
|
||||
\() ->
|
||||
[ """
|
||||
module A exposing (main)
|
||||
import Variant
|
||||
import Html
|
||||
|
||||
main a =
|
||||
let
|
||||
fn (Variant.A str) = value
|
||||
in
|
||||
fn a
|
||||
""", """
|
||||
module Variant exposing (Variant(..))
|
||||
|
||||
type Variant = A String
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData application rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report a used exposed custom type (function declaration destructuring)" <|
|
||||
\() ->
|
||||
[ """
|
||||
module A exposing (fn)
|
||||
import Variant
|
||||
import Html
|
||||
|
||||
n (Variant.A str) = value
|
||||
""", """
|
||||
module Variant exposing (Variant(..))
|
||||
|
||||
type Variant = A String
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData application rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report a used exposed custom type (let destructuring usage)" <|
|
||||
\() ->
|
||||
[ """
|
||||
module A exposing (main)
|
||||
import Variant
|
||||
import Html
|
||||
|
||||
main =
|
||||
let (Variant.A str) = value
|
||||
in str
|
||||
""", """
|
||||
module Variant exposing (Variant(..))
|
||||
|
||||
type Variant = A String
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData application rule
|
||||
|> Review.Test.expectNoErrors
|
||||
@ -499,33 +580,12 @@ main =
|
||||
let
|
||||
type1 : B.Type1
|
||||
type1 =
|
||||
1
|
||||
B.type1
|
||||
in
|
||||
type1
|
||||
""", """module B exposing (Type1)
|
||||
""", """module B exposing (Type1, type1)
|
||||
type Type1 = Type1
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData application rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report an exposed type if it is used in a port (input)" <|
|
||||
\() ->
|
||||
[ """module Main exposing (main)
|
||||
import B
|
||||
main = somePort
|
||||
port somePort : (B.Type1 -> msg) -> Sub msg
|
||||
""", """module B exposing (Type1)
|
||||
type alias Type1 = { user : String }
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData application rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report an exposed type if it is used in a port (output)" <|
|
||||
\() ->
|
||||
[ """module Main exposing (main)
|
||||
import B
|
||||
main = somePort
|
||||
port somePort : B.Type1 -> Cmd msg
|
||||
""", """module B exposing (Type1)
|
||||
type alias Type1 = { user : String }
|
||||
type1 = Type1
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData application rule
|
||||
|> Review.Test.expectNoErrors
|
||||
@ -779,6 +839,28 @@ init = 1
|
||||
]
|
||||
)
|
||||
]
|
||||
, test "should not report an exposed type if it is used in a port (input)" <|
|
||||
\() ->
|
||||
[ """module Main exposing (main)
|
||||
import B
|
||||
main = somePort
|
||||
port somePort : (B.Type1 -> msg) -> Sub msg
|
||||
""", """module B exposing (Type1)
|
||||
type alias Type1 = { user : String }
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData application rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report an exposed type if it is used in a port (output)" <|
|
||||
\() ->
|
||||
[ """module Main exposing (main)
|
||||
import B
|
||||
main = somePort
|
||||
port somePort : B.Type1 -> Cmd msg
|
||||
""", """module B exposing (Type1)
|
||||
type alias Type1 = { user : String }
|
||||
""" ]
|
||||
|> Review.Test.runOnModulesWithProjectData application rule
|
||||
|> Review.Test.expectNoErrors
|
||||
]
|
||||
|
||||
|
||||
|
@ -231,9 +231,9 @@ declarationListVisitor list context =
|
||||
|
||||
Application elmApplicationType ->
|
||||
let
|
||||
mainName : String
|
||||
mainName =
|
||||
mainFunctionName elmApplicationType
|
||||
isMain : String -> Bool
|
||||
isMain =
|
||||
isMainFunction elmApplicationType
|
||||
|
||||
containsMainFunction : Bool
|
||||
containsMainFunction =
|
||||
@ -241,7 +241,7 @@ declarationListVisitor list context =
|
||||
(\declaration ->
|
||||
case Node.value declaration of
|
||||
Declaration.FunctionDeclaration function ->
|
||||
(function.declaration |> Node.value |> .name |> Node.value) == mainName
|
||||
isMain (function.declaration |> Node.value |> .name |> Node.value)
|
||||
|
||||
_ ->
|
||||
False
|
||||
@ -253,11 +253,11 @@ declarationListVisitor list context =
|
||||
)
|
||||
|
||||
|
||||
mainFunctionName : ElmApplicationType -> String
|
||||
mainFunctionName elmApplicationType =
|
||||
isMainFunction : ElmApplicationType -> String -> Bool
|
||||
isMainFunction elmApplicationType =
|
||||
case elmApplicationType of
|
||||
ElmApplication ->
|
||||
"main"
|
||||
\name -> name == "main"
|
||||
|
||||
LamderaApplication ->
|
||||
"app"
|
||||
\name -> name == "main" || name == "app"
|
||||
|
@ -200,11 +200,19 @@ app = text ""
|
||||
, under = "Reported"
|
||||
}
|
||||
]
|
||||
, test "should report modules that contain a top-level `app` function in Lamdera applications" <|
|
||||
, test "should not report modules that contain a top-level `app` function in Lamdera applications" <|
|
||||
\() ->
|
||||
"""
|
||||
module Reported exposing (app)
|
||||
app = text ""
|
||||
"""
|
||||
|> Review.Test.runWithProjectData lamderaApplication rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report modules that contain a top-level `main` function in Lamdera applications" <|
|
||||
\() ->
|
||||
"""
|
||||
module Reported exposing (main)
|
||||
main = text ""
|
||||
"""
|
||||
|> Review.Test.runWithProjectData lamderaApplication rule
|
||||
|> Review.Test.expectNoErrors
|
||||
|
@ -35,7 +35,7 @@ We're only offering fixes for lambdas here because we believe unused parameters
|
||||
|
||||
## Fail
|
||||
|
||||
Value `something` is not used:
|
||||
Value `number` is not used:
|
||||
|
||||
add1 number =
|
||||
1
|
||||
@ -187,18 +187,27 @@ declarationExitVisitor node context =
|
||||
|
||||
getArgNames : List (List Declared) -> FunctionArgs
|
||||
getArgNames declared =
|
||||
declared
|
||||
|> List.indexedMap
|
||||
(\index args ->
|
||||
case args of
|
||||
[ arg ] ->
|
||||
Just ( index, arg.name )
|
||||
getArgNamesHelp declared 0 Dict.empty
|
||||
|
||||
_ ->
|
||||
Nothing
|
||||
)
|
||||
|> List.filterMap identity
|
||||
|> Dict.fromList
|
||||
|
||||
getArgNamesHelp : List (List Declared) -> Int -> FunctionArgs -> FunctionArgs
|
||||
getArgNamesHelp declared index acc =
|
||||
case declared of
|
||||
[] ->
|
||||
acc
|
||||
|
||||
args :: restOfDeclared ->
|
||||
let
|
||||
newAcc : Dict Int String
|
||||
newAcc =
|
||||
case args of
|
||||
[ arg ] ->
|
||||
Dict.insert index arg.name acc
|
||||
|
||||
_ ->
|
||||
acc
|
||||
in
|
||||
getArgNamesHelp restOfDeclared (index + 1) newAcc
|
||||
|
||||
|
||||
getParametersFromPatterns : Source -> Node Pattern -> List Declared
|
||||
@ -462,7 +471,7 @@ registerFunctionCall fnName numberOfIgnoredArguments arguments context =
|
||||
| locationsToIgnoreForUsed =
|
||||
Dict.merge
|
||||
Dict.insert
|
||||
(\key new old -> Dict.insert key (List.append new old))
|
||||
(\key new old -> Dict.insert key (new ++ old))
|
||||
Dict.insert
|
||||
locationsToIgnore
|
||||
context.locationsToIgnoreForUsed
|
||||
|
File diff suppressed because it is too large
Load Diff
@ -8,6 +8,7 @@ import NoUnused.Variables exposing (rule)
|
||||
import Review.Project as Project exposing (Project)
|
||||
import Review.Project.Dependency as Dependency exposing (Dependency)
|
||||
import Review.Test
|
||||
import Review.Test.Dependencies
|
||||
import Test exposing (Test, describe, test)
|
||||
|
||||
|
||||
@ -382,7 +383,36 @@ a = let b = 1
|
||||
, under = "b"
|
||||
}
|
||||
|> Review.Test.atExactly { start = { row = 2, column = 9 }, end = { row = 2, column = 10 } }
|
||||
|> Review.Test.whenFixed "module SomeModule exposing (a, b)\na = let \n c = 2\n in c"
|
||||
|> Review.Test.whenFixed ("""module SomeModule exposing (a, b)
|
||||
a = let$
|
||||
c = 2
|
||||
in c""" |> String.replace "$" " ")
|
||||
]
|
||||
, test "should report unused variables from let even if they are exposed by name (multiple ones with type annotations)" <|
|
||||
\() ->
|
||||
"""module SomeModule exposing (a, b)
|
||||
a = let
|
||||
b : number
|
||||
b = 1
|
||||
|
||||
c : number
|
||||
c = 2
|
||||
in c"""
|
||||
|> Review.Test.run rule
|
||||
|> Review.Test.expectErrors
|
||||
[ Review.Test.error
|
||||
{ message = "`let in` variable `b` is not used"
|
||||
, details = details
|
||||
, under = "b"
|
||||
}
|
||||
|> Review.Test.atExactly { start = { row = 4, column = 9 }, end = { row = 4, column = 10 } }
|
||||
|> Review.Test.whenFixed ("""module SomeModule exposing (a, b)
|
||||
a = let
|
||||
$
|
||||
|
||||
c : number
|
||||
c = 2
|
||||
in c""" |> String.replace "$" " ")
|
||||
]
|
||||
, test "should report unused function from let even if they are exposed by name" <|
|
||||
\() ->
|
||||
@ -452,6 +482,61 @@ a = let _ = 1
|
||||
, under = "_"
|
||||
}
|
||||
|> Review.Test.whenFixed """module SomeModule exposing (a)
|
||||
a = 2"""
|
||||
]
|
||||
, test "should not report a wildcard assignment used for a Debug.log call with all arguments (simple call)" <|
|
||||
\() ->
|
||||
"""module SomeModule exposing (a)
|
||||
a = let _ = Debug.log "ok" ()
|
||||
in 2"""
|
||||
|> Review.Test.run rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report a wildcard assignment used for a Debug.log call with all arguments (using <|)" <|
|
||||
\() ->
|
||||
"""module SomeModule exposing (a)
|
||||
a = let _ = Debug.log "ok" <| ()
|
||||
in 2"""
|
||||
|> Review.Test.run rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report a wildcard assignment used for a Debug.log call with all arguments (using |>)" <|
|
||||
\() ->
|
||||
"""module SomeModule exposing (a)
|
||||
a = let _ = () |> Debug.log "ok"
|
||||
in 2"""
|
||||
|> Review.Test.run rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should report a wildcard assignment when used for a Debug.log call without all the arguments" <|
|
||||
\() ->
|
||||
"""module SomeModule exposing (a)
|
||||
a = let _ = Debug.log "ok"
|
||||
in 2"""
|
||||
|> Review.Test.run rule
|
||||
|> Review.Test.expectErrors
|
||||
[ Review.Test.error
|
||||
{ message = "Value assigned to `_` is unused"
|
||||
, details =
|
||||
[ "This value has been assigned to a wildcard, which makes the value unusable. You should remove it at the location I pointed at."
|
||||
]
|
||||
, under = "_"
|
||||
}
|
||||
|> Review.Test.whenFixed """module SomeModule exposing (a)
|
||||
a = 2"""
|
||||
]
|
||||
, test "should report an unused named declaration even if it uses Debug.log" <|
|
||||
\() ->
|
||||
"""module SomeModule exposing (a)
|
||||
a = let xyz = Debug.log "ok" ()
|
||||
in 2"""
|
||||
|> Review.Test.run rule
|
||||
|> Review.Test.expectErrors
|
||||
[ Review.Test.error
|
||||
{ message = "`let in` variable `xyz` is not used"
|
||||
, details =
|
||||
[ "You should either use this value somewhere, or remove it at the location I pointed at."
|
||||
]
|
||||
, under = "xyz"
|
||||
}
|
||||
|> Review.Test.whenFixed """module SomeModule exposing (a)
|
||||
a = 2"""
|
||||
]
|
||||
, test "should report () destructuring" <|
|
||||
@ -1429,6 +1514,49 @@ identity x =
|
||||
]
|
||||
)
|
||||
]
|
||||
, test "should not report used import even if a used lambda param is named in the same way elsewhere" <|
|
||||
\() ->
|
||||
"""module A exposing (list)
|
||||
import Html exposing (Html, label, text)
|
||||
|
||||
list : List (Html msg)
|
||||
list =
|
||||
[ label [] []
|
||||
, Maybe.map
|
||||
(\\label ->
|
||||
text label
|
||||
)
|
||||
(Just "string")
|
||||
|> Maybe.withDefault (text "")
|
||||
]
|
||||
"""
|
||||
|> Review.Test.runWithProjectData
|
||||
(Review.Test.Dependencies.projectWithElmCore
|
||||
|> Project.addDependency Review.Test.Dependencies.elmHtml
|
||||
)
|
||||
rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should not report used import even if a used let variable is named in the same way elsewhere" <|
|
||||
\() ->
|
||||
"""module A exposing (list)
|
||||
import Html exposing (Html, label, text)
|
||||
|
||||
list : List (Html msg)
|
||||
list =
|
||||
[ label [] []
|
||||
, let
|
||||
label =
|
||||
"string"
|
||||
in
|
||||
text label
|
||||
]
|
||||
"""
|
||||
|> Review.Test.runWithProjectData
|
||||
(Review.Test.Dependencies.projectWithElmCore
|
||||
|> Project.addDependency Review.Test.Dependencies.elmHtml
|
||||
)
|
||||
rule
|
||||
|> Review.Test.expectNoErrors
|
||||
, test "should report unused import even if a variant arg is named in the same way" <|
|
||||
\() ->
|
||||
[ """module A exposing (identity)
|
||||
|
1420
tests/Simplify.elm
1420
tests/Simplify.elm
File diff suppressed because it is too large
Load Diff
38
tests/Simplify/Match.elm
Normal file
38
tests/Simplify/Match.elm
Normal file
@ -0,0 +1,38 @@
|
||||
module Simplify.Match exposing
|
||||
( Match(..)
|
||||
, map
|
||||
, maybeAndThen
|
||||
)
|
||||
|
||||
{-|
|
||||
|
||||
@docs Match
|
||||
@docs map
|
||||
@docs maybeAndThen
|
||||
|
||||
-}
|
||||
|
||||
|
||||
type Match a
|
||||
= Determined a
|
||||
| Undetermined
|
||||
|
||||
|
||||
map : (a -> b) -> Match a -> Match b
|
||||
map mapper match =
|
||||
case match of
|
||||
Determined a ->
|
||||
Determined (mapper a)
|
||||
|
||||
Undetermined ->
|
||||
Undetermined
|
||||
|
||||
|
||||
maybeAndThen : (a -> Match b) -> Maybe a -> Match b
|
||||
maybeAndThen fn maybe =
|
||||
case maybe of
|
||||
Just a ->
|
||||
fn a
|
||||
|
||||
Nothing ->
|
||||
Undetermined
|
File diff suppressed because it is too large
Load Diff
Loading…
Reference in New Issue
Block a user