Backport rules from elm-review-cognitive-complexity

tests/CognitiveComplexity.elm

@@ -11,6 +11,7 @@ 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 Json.Encode as Encode
 import Review.Rule as Rule exposing (Rule)
 import Set exposing (Set)
 
@@ -56,7 +57,26 @@ I would for now recommend to use it with a very high threshold to find places in
 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!
+Please let me know how enabling this rule works out for you! If enforcing doesn't work for you, then you can use this as
+an insight rule instead.
+
+
+## Use as an insight rule
+
+If instead of enforcing a threshold, you wish to have an overview of the complexity for each function, you can run the
+rule using as an insight rule (using `elm-review --report=json --extract`), which would yield an output like the following:
+
+```json
+{
+  "Some.Module": {
+    "someFunction": 16,
+    "someOtherFunction": 0
+  },
+  "Some.Other.Module": {
+    "awesomeFunction": 2
+  }
+}
+```
 
 
 ## Complexity breakdown
@@ -171,6 +191,13 @@ elm-review --template jfmengels/elm-review-cognitive-complexity/example --rules
 
 The cognitive complexity is set to 15 in the configuration used by the example.
 
+If instead of enforcing a threshold, you wish to have an overview of the complexity for each function, you can run the
+rule like this (requires [`jq`](https://stedolan.github.io/jq/)):
+
+```bash
+elm-review --template jfmengels/elm-review-cognitive-complexity/example --extract --report=json --rules CognitiveComplexity | jq -r '.extracts.CognitiveComplexity'
+```
+
 
 ## Thanks
 
@@ -180,15 +207,35 @@ Thanks to G. Ann Campbell for the different talks she made on the subject.
 -}
 rule : Int -> Rule
 rule threshold =
-    Rule.newModuleRuleSchema "CognitiveComplexity" initialContext
+    Rule.newProjectRuleSchema "CognitiveComplexity" initialContext
+        |> Rule.withModuleVisitor (moduleVisitor threshold)
+        |> Rule.withModuleContextUsingContextCreator
+            { fromProjectToModule = fromProjectToModule
+            , fromModuleToProject = fromModuleToProject
+            , foldProjectContexts = foldProjectContexts
+            }
+        |> Rule.withDataExtractor dataExtractor
+        |> Rule.fromProjectRuleSchema
+
+
+moduleVisitor : Int -> Rule.ModuleRuleSchema schemaState ModuleContext -> Rule.ModuleRuleSchema { schemaState | hasAtLeastOneVisitor : () } ModuleContext
+moduleVisitor threshold schema =
+    schema
         |> Rule.withDeclarationExitVisitor declarationExitVisitor
         |> Rule.withExpressionEnterVisitor expressionEnterVisitor
         |> Rule.withExpressionExitVisitor expressionExitVisitor
-        |> Rule.withFinalModuleEvaluation (finalEvaluation threshold)
+        |> Rule.withFinalModuleEvaluation (finalModuleEvaluation threshold)
-        |> Rule.fromModuleRuleSchema
 
 
-type alias Context =
+type alias ProjectContext =
+    Dict String ComplexityDict
+
+
+type alias ComplexityDict =
+    Dict String Int
+
+
+type alias ModuleContext =
     { nesting : Int
     , operandsToIgnore : List Range
     , elseIfToIgnore : List Range
@@ -223,19 +270,61 @@ type IncreaseKind
     | IndirectRecursiveCall String
 
 
-initialContext : Context
+initialContext : ProjectContext
 initialContext =
-    { nesting = 0
+    Dict.empty
-    , operandsToIgnore = []
-    , elseIfToIgnore = []
-    , rangesWhereNestingIncreases = []
-    , references = Dict.empty
-    , increases = []
-    , functionsToReport = []
-    }
 
 
-expressionEnterVisitor : Node Expression -> Context -> ( List nothing, Context )
+fromProjectToModule : Rule.ContextCreator ProjectContext ModuleContext
+fromProjectToModule =
+    Rule.initContextCreator
+        (always
+            { nesting = 0
+            , operandsToIgnore = []
+            , elseIfToIgnore = []
+            , rangesWhereNestingIncreases = []
+            , references = Dict.empty
+            , increases = []
+            , functionsToReport = []
+            }
+        )
+
+
+fromModuleToProject : Rule.ContextCreator ModuleContext ProjectContext
+fromModuleToProject =
+    Rule.initContextCreator
+        (\moduleName moduleContext ->
+            let
+                recursiveCalls : RecursiveCalls
+                recursiveCalls =
+                    computeRecursiveCalls moduleContext
+            in
+            List.foldl
+                (\functionToReport acc ->
+                    let
+                        allIncreases : List Increase
+                        allIncreases =
+                            computeIncreases recursiveCalls functionToReport
+
+                        finalComplexity : Int
+                        finalComplexity =
+                            List.foldl (\{ increase } complexity -> increase + complexity) 0 allIncreases
+                    in
+                    Dict.insert (Node.value functionToReport.functionName) finalComplexity acc
+                )
+                Dict.empty
+                moduleContext.functionsToReport
+                |> Dict.singleton (String.join "." moduleName)
+        )
+        |> Rule.withModuleName
+
+
+foldProjectContexts : ProjectContext -> ProjectContext -> ProjectContext
+foldProjectContexts =
+    Dict.union
+
+
+expressionEnterVisitor : Node Expression -> ModuleContext -> ( List nothing, ModuleContext )
 expressionEnterVisitor node context =
     if List.member (Node.range node) context.rangesWhereNestingIncreases then
         ( [], expressionEnterVisitorHelp node { context | nesting = context.nesting + 1 } )
@@ -244,7 +333,7 @@ expressionEnterVisitor node context =
         ( [], expressionEnterVisitorHelp node context )
 
 
-expressionEnterVisitorHelp : Node Expression -> Context -> Context
+expressionEnterVisitorHelp : Node Expression -> ModuleContext -> ModuleContext
 expressionEnterVisitorHelp node context =
     case Node.value node of
         Expression.IfBlock _ _ else_ ->
@@ -398,7 +487,7 @@ incrementAndIgnore parentOperator node =
             ( [], [] )
 
 
-expressionExitVisitor : Node Expression -> Context -> ( List nothing, Context )
+expressionExitVisitor : Node Expression -> ModuleContext -> ( List nothing, ModuleContext )
 expressionExitVisitor node context =
     if List.member (Node.range node) context.rangesWhereNestingIncreases then
         ( [], expressionExitVisitorHelp node { context | nesting = context.nesting - 1 } )
@@ -407,7 +496,7 @@ expressionExitVisitor node context =
         ( [], expressionExitVisitorHelp node context )
 
 
-expressionExitVisitorHelp : Node Expression -> Context -> Context
+expressionExitVisitorHelp : Node Expression -> ModuleContext -> ModuleContext
 expressionExitVisitorHelp node context =
     case Node.value node of
         Expression.IfBlock _ _ _ ->
@@ -427,7 +516,7 @@ expressionExitVisitorHelp node context =
             context
 
 
-declarationExitVisitor : Node Declaration -> Context -> ( List (Rule.Error {}), Context )
+declarationExitVisitor : Node Declaration -> ModuleContext -> ( List (Rule.Error {}), ModuleContext )
 declarationExitVisitor node context =
     let
         functionsToReport : List FunctionToReport
@@ -455,66 +544,28 @@ declarationExitVisitor node context =
     )
 
 
-finalEvaluation : Int -> Context -> List (Rule.Error {})
+finalModuleEvaluation : Int -> ModuleContext -> List (Rule.Error {})
-finalEvaluation threshold context =
+finalModuleEvaluation threshold context =
     let
-        potentialRecursiveFunctions : Set String
-        potentialRecursiveFunctions =
-            List.map (.functionName >> Node.value) context.functionsToReport
-                |> Set.fromList
-
         recursiveCalls : RecursiveCalls
         recursiveCalls =
-            context.functionsToReport
+            computeRecursiveCalls context
-                |> List.map
-                    (\{ functionName, references } ->
-                        ( Node.value functionName, Dict.filter (\name _ -> Set.member name potentialRecursiveFunctions) references )
-                    )
-                |> Dict.fromList
-                |> findRecursiveCalls
     in
     List.filterMap
-        (\{ functionName, increases, references } ->
+        (\functionToReport ->
             let
-                recursiveCallsForFunctionName : List String
-                recursiveCallsForFunctionName =
-                    Dict.get (Node.value functionName) recursiveCalls
-                        |> Maybe.withDefault Set.empty
-                        |> Set.toList
-
                 allIncreases : List Increase
                 allIncreases =
-                    List.concat
+                    computeIncreases recursiveCalls functionToReport
-                        [ 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)
+                    List.foldl (\{ increase } acc -> increase + acc) 0 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
+                        { message = Node.value functionToReport.functionName ++ " has a cognitive complexity of " ++ String.fromInt finalComplexity ++ ", higher than the allowed " ++ String.fromInt threshold
                         , details =
                             if List.isEmpty allIncreases then
                                 explanation
@@ -527,7 +578,7 @@ finalEvaluation threshold context =
                                             |> String.join "\n"
                                        ]
                         }
-                        (Node.range functionName)
+                        (Node.range functionToReport.functionName)
                     )
 
             else
@@ -536,6 +587,58 @@ finalEvaluation threshold context =
         context.functionsToReport
 
 
+computeIncreases : RecursiveCalls -> FunctionToReport -> List Increase
+computeIncreases allRecursiveCalls { functionName, increases, references } =
+    case Dict.get (Node.value functionName) allRecursiveCalls of
+        Just recursiveCalls ->
+            Set.foldl
+                (\reference acc ->
+                    case Dict.get reference references of
+                        Just location ->
+                            { line = location
+                            , increase = 1
+                            , nesting = 0
+                            , kind =
+                                if Node.value functionName == reference then
+                                    RecursiveCall
+
+                                else
+                                    IndirectRecursiveCall reference
+                            }
+                                :: acc
+
+                        Nothing ->
+                            acc
+                )
+                increases
+                recursiveCalls
+
+        Nothing ->
+            increases
+
+
+computeRecursiveCalls : ModuleContext -> RecursiveCalls
+computeRecursiveCalls context =
+    let
+        potentialRecursiveFunctions : Set String
+        potentialRecursiveFunctions =
+            List.foldl
+                (\fn acc -> Set.insert (Node.value fn.functionName) acc)
+                Set.empty
+                context.functionsToReport
+    in
+    context.functionsToReport
+        |> List.foldl
+            (\{ functionName, references } acc ->
+                Dict.insert
+                    (Node.value functionName)
+                    (Dict.filter (\name _ -> Set.member name potentialRecursiveFunctions) references)
+                    acc
+            )
+            Dict.empty
+        |> findRecursiveCalls
+
+
 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."
@@ -600,9 +703,8 @@ type VisitState
 findRecursiveCalls : Dict String (Dict String a) -> RecursiveCalls
 findRecursiveCalls graph =
     graph
-        |> Dict.keys
+        |> Dict.foldl
-        |> List.foldl
+            (\vertice _ ( recursiveCalls, visited ) ->
-            (\vertice ( recursiveCalls, visited ) ->
                 let
                     res : { recursiveCalls : RecursiveCalls, visited : Visited, stack : List String }
                     res =
@@ -671,6 +773,20 @@ processDFSTree graph stack visited =
            )
 
 
+dataExtractor : ProjectContext -> Encode.Value
+dataExtractor projectContext =
+    Encode.dict identity encodeComplexityDict projectContext
+
+
+encodeComplexityDict : ComplexityDict -> Encode.Value
+encodeComplexityDict dict =
+    dict
+        |> Dict.toList
+        |> List.sortBy (Tuple.second >> negate)
+        |> List.map (\( name, complexity ) -> ( name, Encode.int complexity ))
+        |> Encode.object
+
+
 insertCycle : List String -> String -> RecursiveCalls -> RecursiveCalls
 insertCycle stack vertice recursiveCalls =
     case stack of

tests/CognitiveComplexityTest.elm

@@ -16,7 +16,13 @@ all =
 a = 1
 """
                     |> Review.Test.run (rule 1)
-                    |> Review.Test.expectNoErrors
+                    |> Review.Test.expectDataExtract
+                        """
+{
+  "A": {
+    "a": 0
+  }
+}"""
         , test "should report an error when the complexity is higher than the threshold" <|
             \() ->
                 """module A exposing (..)
@@ -46,6 +52,12 @@ Line 6: +4 for the if expression (including 3 for nesting)
 """ ]
                           }
                         ]
+                        """
+{
+  "A": {
+    "fun": 10
+  }
+}"""
         , test "should count a simple value or operation as 0" <|
             \() ->
                 """module A exposing (..)
@@ -53,6 +65,12 @@ fun n =
     n + 1
 """
                     |> expect [ { name = "fun", complexity = 0, details = [] } ]
+                        """
+{
+  "A": {
+    "fun": 0
+  }
+}"""
         , test "should count if expression as 1" <|
             \() ->
                 """module A exposing (..)
@@ -68,6 +86,12 @@ fun n =
                           , details = [ "Line 3: +1 for the if expression" ]
                           }
                         ]
+                        """
+{
+  "A": {
+    "fun": 1
+  }
+}"""
         , test "should count if else expressions" <|
             \() ->
                 """module A exposing (..)
@@ -88,6 +112,12 @@ Line 5: +1 for the else if expression
 """ ]
                           }
                         ]
+                        """
+{
+  "A": {
+    "fun": 2
+  }
+}"""
         , test "should properly decrement when exiting else expression" <|
             \() ->
                 """module A exposing (..)
@@ -116,6 +146,12 @@ Line 12: +1 for the if expression
 """ ]
                           }
                         ]
+                        """
+{
+  "A": {
+    "fun": 3
+  }
+}"""
         , test "should increment nesting when inside a let function" <|
             \() ->
                 """module A exposing (..)
@@ -137,6 +173,12 @@ Line 5: +2 for the if expression (including 1 for nesting)
 """ ]
                           }
                         ]
+                        """
+{
+  "A": {
+    "fun": 2
+  }
+}"""
         , test "should properly decrement nesting when exiting a let function" <|
             \() ->
                 """module A exposing (..)
@@ -161,6 +203,12 @@ Line 8: +1 for the if expression
 """ ]
                           }
                         ]
+                        """
+{
+  "A": {
+    "fun": 1
+  }
+}"""
         , test "should count case expression as 1" <|
             \() ->
                 """module A exposing (..)
@@ -178,6 +226,12 @@ fun n =
                           , details = [ "Line 3: +1 for the case expression" ]
                           }
                         ]
+                        """
+{
+  "A": {
+    "fun": 1
+  }
+}"""
         , test "should count nesting of case expressions" <|
             \() ->
                 """module A exposing (..)
@@ -199,6 +253,12 @@ Line 6: +2 for the case expression (including 1 for nesting)
 """ ]
                           }
                         ]
+                        """
+{
+  "A": {
+    "fun": 3
+  }
+}"""
         , test "should decrement the nesting when leaving a nested structure" <|
             \() ->
                 """module A exposing (..)
@@ -226,6 +286,12 @@ Line 12: +2 for the case expression (including 1 for nesting)
 """ ]
                           }
                         ]
+                        """
+{
+  "A": {
+    "fun": 8
+  }
+}"""
         , test "should increment once when using the && boolean operator" <|
             \() ->
                 """module A exposing (..)
@@ -246,6 +312,12 @@ Line 4: +1 for the use of `&&`
 """ ]
                           }
                         ]
+                        """
+{
+  "A": {
+    "fun": 2
+  }
+}"""
         , test "should increment once when using the || boolean operator" <|
             \() ->
                 """module A exposing (..)
@@ -266,6 +338,12 @@ Line 4: +1 for the use of `||`
 """ ]
                           }
                         ]
+                        """
+{
+  "A": {
+    "fun": 2
+  }
+}"""
         , test "should increment when mixing boolean operators" <|
             \() ->
                 """module A exposing (..)
@@ -290,6 +368,12 @@ Line 5: +1 for the use of `&&`
 """ ]
                           }
                         ]
+                        """
+{
+  "A": {
+    "fun": 4
+  }
+}"""
         , test "should not increment for anonymous functions" <|
             \() ->
                 """module A exposing (..)
@@ -302,6 +386,12 @@ fun n =
                           , details = []
                           }
                         ]
+                        """
+{
+  "A": {
+    "fun": 0
+  }
+}"""
         , test "should increment the nesting inside anonymous functions" <|
             \() ->
                 """module A exposing (..)
@@ -322,6 +412,12 @@ fun n =
                           , details = [ "Line 5: +2 for the if expression (including 1 for nesting)" ]
                           }
                         ]
+                        """
+{
+  "A": {
+    "fun": 2
+  }
+}"""
         , test "should properly decrement the nesting when exiting an anonymous function" <|
             \() ->
                 """module A exposing (..)
@@ -340,6 +436,12 @@ fun n =
                           , details = [ "Line 6: +1 for the if expression" ]
                           }
                         ]
+                        """
+{
+  "A": {
+    "fun": 1
+  }
+}"""
         , test "should increment when finding a recursive call" <|
             \() ->
                 """module A exposing (..)
@@ -359,6 +461,12 @@ Line 4: +1 for the recursive call
 """ ]
                           }
                         ]
+                        """
+{
+  "A": {
+    "fun": 2
+  }
+}"""
         , test "should only increment once, even if there are multiple recursive calls" <|
             \() ->
                 """module A exposing (..)
@@ -379,6 +487,12 @@ Line 4: +1 for the recursive call
 """ ]
                           }
                         ]
+                        """
+{
+  "A": {
+    "fib": 2
+  }
+}"""
         , test "should increment the complexity for every recursive call in a chain" <|
             \() ->
                 """module A exposing (..)
@@ -404,6 +518,12 @@ Line 6: +1 for the indirect recursive call to fun1
 """ ]
                           }
                         ]
+                        """{
+                         "A": {
+                           "fun1": 1,
+                           "fun2": 1
+                         }
+                       }"""
         , test "should increment the complexity for every recursive call in a chain, for each different function call" <|
             \() ->
                 """module A exposing (..)
@@ -434,6 +554,12 @@ Line 8: +1 for the indirect recursive call to fun1
 """ ]
                           }
                         ]
+                        """    {
+                                 "A": {
+                                   "fun1": 2,
+                                   "fun2": 1
+                                 }
+                               }"""
         , test "should increment the complexity for every recursive call in a chain, for long chains" <|
             \() ->
                 """module A exposing (..)
@@ -475,6 +601,16 @@ fun5 n =
                           , details = [ "Line 11: +1 for the indirect recursive call to fun1" ]
                           }
                         ]
+                        """
+                        {
+                          "A": {
+                            "fun1": 1,
+                            "fun2": 1,
+                            "fun3": 1,
+                            "fun4": 1,
+                            "fun5": 1
+                          }
+                       }"""
         , test "the complexity of a function should not affect another function's computed complexity" <|
             \() ->
                 """module A exposing (..)
@@ -512,42 +648,55 @@ Line 6: +2 for the if expression (including 1 for nesting)
                           , details = [ "Line 14: +1 for the if expression" ]
                           }
                         ]
+                        """{
+ "A": {
+   "fun": 3,
+   "alsoSimple": 1,
+   "simple": 0
+ }
+}"""
         ]
 
 
-expect : List { name : String, complexity : Int, details : List String } -> String -> Expectation
+expect : List { name : String, complexity : Int, details : List String } -> String -> String -> Expectation
-expect functionComplexities source =
+expect functionComplexities dataExtract source =
     source
         |> Review.Test.run (rule -1)
-        |> Review.Test.expectErrors
+        |> Review.Test.expect
-            (List.map
+            [ Review.Test.moduleErrors "A"
-                (\{ name, complexity, details } ->
+                (List.map
-                    Review.Test.error
+                    (\{ name, complexity, details } ->
-                        { message = name ++ " has a cognitive complexity of " ++ String.fromInt complexity ++ ", higher than the allowed -1"
+                        Review.Test.error
-                        , details = explanation ++ details
+                            { message = name ++ " has a cognitive complexity of " ++ String.fromInt complexity ++ ", higher than the allowed -1"
-                        , under = name
+                            , details = explanation ++ details
-                        }
+                            , under = name
+                            }
+                    )
+                    functionComplexities
                 )
-                functionComplexities
+            , Review.Test.dataExtract dataExtract
-            )
+            ]
 
 
-expectAtExactly : List { name : String, complexity : Int, details : List String, atExactly : Range } -> String -> Expectation
+expectAtExactly : List { name : String, complexity : Int, details : List String, atExactly : Range } -> String -> String -> Expectation
-expectAtExactly functionComplexities source =
+expectAtExactly functionComplexities dataExtract source =
     source
         |> Review.Test.run (rule -1)
-        |> Review.Test.expectErrors
+        |> Review.Test.expect
-            (List.map
+            [ Review.Test.moduleErrors "A"
-                (\{ name, complexity, details, atExactly } ->
+                (List.map
-                    Review.Test.error
+                    (\{ name, complexity, details, atExactly } ->
-                        { message = name ++ " has a cognitive complexity of " ++ String.fromInt complexity ++ ", higher than the allowed -1"
+                        Review.Test.error
-                        , details = explanation ++ details
+                            { message = name ++ " has a cognitive complexity of " ++ String.fromInt complexity ++ ", higher than the allowed -1"
-                        , under = name
+                            , details = explanation ++ details
-                        }
+                            , under = name
-                        |> Review.Test.atExactly atExactly
+                            }
+                            |> Review.Test.atExactly atExactly
+                    )
+                    functionComplexities
                 )
-                functionComplexities
+            , Review.Test.dataExtract dataExtract
-            )
+            ]
 
 
 explanation : List String