Introduce elm-review

package.json

@@ -6,8 +6,12 @@
     "test": "bash tests/run-tests.sh",
     "test:random": "RANDOMIZED=1 bash tests/run-tests.sh",
     "format": "elm-format --validate src tests/src benchmarks",
+    "review": "elm-review src/ && cd tests && yarn review",
     "install-local": "rm -rf ~/.elm/0.19.1/packages/elm-explorations/test/1.2.2/ && mkdir -p ~/.elm/0.19.1/packages/elm-explorations/test/1.2.2/ && cp -r ./{src,elm.json,LICENSE,README.md} ~/.elm/0.19.1/packages/elm-explorations/test/1.2.2/"
   },
   "author": "Richard Feldman",
-  "license": "BSD-3-Clause"
+  "license": "BSD-3-Clause",
+  "devDependencies": {
+    "elm-review": "^2.7.5"
+  }
 }

review/elm.json

@@ -0,0 +1,40 @@
+{
+    "type": "application",
+    "source-directories": [
+        "src"
+    ],
+    "elm-version": "0.19.1",
+    "dependencies": {
+        "direct": {
+            "elm/core": "1.0.5",
+            "elm/json": "1.1.3",
+            "elm/project-metadata-utils": "1.0.2",
+            "jfmengels/elm-review": "2.8.1",
+            "jfmengels/elm-review-code-style": "1.0.0",
+            "jfmengels/elm-review-common": "1.2.1",
+            "jfmengels/elm-review-debug": "1.0.6",
+            "jfmengels/elm-review-documentation": "2.0.1",
+            "jfmengels/elm-review-simplify": "2.0.18",
+            "jfmengels/elm-review-unused": "1.1.22",
+            "stil4m/elm-syntax": "7.2.9"
+        },
+        "indirect": {
+            "elm/html": "1.0.0",
+            "elm/parser": "1.1.0",
+            "elm/random": "1.0.0",
+            "elm/regex": "1.0.0",
+            "elm/time": "1.0.0",
+            "elm/virtual-dom": "1.0.3",
+            "elm-community/list-extra": "8.6.0",
+            "elm-explorations/test": "1.2.2",
+            "miniBill/elm-unicode": "1.0.2",
+            "pzp1997/assoc-list": "1.0.0",
+            "rtfeldman/elm-hex": "1.0.0",
+            "stil4m/structured-writer": "1.0.3"
+        }
+    },
+    "test-dependencies": {
+        "direct": {},
+        "indirect": {}
+    }
+}

review/src/ReviewConfig.elm

@@ -0,0 +1,69 @@
+module ReviewConfig exposing (config)
+
+{-| Do not rename the ReviewConfig module or the config function, because
+`elm-review` will look for these.
+
+To add packages that contain rules, add them to this review project using
+
+    `elm install author/packagename`
+
+when inside the directory containing this file.
+
+-}
+
+import Docs.NoMissing exposing (exposedModules, onlyExposed)
+import Docs.ReviewAtDocs
+import Docs.ReviewLinksAndSections
+import Docs.UpToDateReadmeLinks
+import NoDebug.Log
+import NoDebug.TodoOrToString
+import NoExposingEverything
+import NoImportingEverything
+import NoMissingTypeAnnotation
+import NoMissingTypeAnnotationInLetIn
+import NoMissingTypeExpose
+import NoPrematureLetComputation
+import NoSimpleLetBody
+import NoUnused.CustomTypeConstructorArgs
+import NoUnused.CustomTypeConstructors
+import NoUnused.Exports
+import NoUnused.Modules
+import NoUnused.Parameters
+import NoUnused.Patterns
+import NoUnused.Variables
+import Review.Rule as Rule exposing (Rule)
+import Simplify
+
+
+config : List Rule
+config =
+    [ NoExposingEverything.rule
+    , NoImportingEverything.rule []
+        |> Rule.ignoreErrorsForFiles
+            [ "src/Test/Html/Internal/ElmHtml/ToString.elm"
+            , "src/Test/Html/Internal/ElmHtml/Query.elm"
+            ]
+    , NoMissingTypeAnnotation.rule
+        |> Rule.ignoreErrorsForFiles [ "src/Test/Internal/KernelConstants.elm" ]
+    , NoMissingTypeExpose.rule
+    , NoSimpleLetBody.rule
+        |> Rule.ignoreErrorsForFiles [ "src/Simplify.elm" ]
+    , NoPrematureLetComputation.rule
+    , NoUnused.CustomTypeConstructors.rule []
+        |> Rule.ignoreErrorsForFiles [ "src/Test/Html/Internal/ElmHtml/InternalTypes.elm" ]
+    , NoUnused.CustomTypeConstructorArgs.rule
+        |> Rule.ignoreErrorsForFiles [ "src/Test/Html/Internal/ElmHtml/InternalTypes.elm" ]
+    , NoUnused.Exports.rule
+        |> Rule.ignoreErrorsForFiles
+            [ "src/Test/Html/Internal/ElmHtml/ToString.elm"
+            , "src/Test/Html/Internal/ElmHtml/Query.elm"
+            , "src/Test/Html/Internal/ElmHtml/InternalTypes.elm"
+            ]
+    , NoUnused.Modules.rule
+    , NoUnused.Parameters.rule
+    , NoUnused.Patterns.rule
+    , NoUnused.Variables.rule
+        |> Rule.ignoreErrorsForFiles [ "src/Simplify.elm" ]
+    , Simplify.rule Simplify.defaults
+        |> Rule.ignoreErrorsForFiles [ "src/Fuzz.elm" ]
+    ]

src/Expect.elm

@@ -731,16 +731,14 @@ equateWith reason comparison b a =
 
         usesFloats =
             isFloat (Internal.toString a) || isFloat (Internal.toString b)
-
-        floatError =
+    in
+    if usesFloats then
+        fail <|
             if String.contains reason "not" then
                 "Do not use Expect.notEqual with floats. Use Expect.notWithin instead."
 
             else
                 "Do not use Expect.equal with floats. Use Expect.within instead."
-    in
-    if usesFloats then
-        fail floatError
 
     else
         testWith Equality reason comparison b a

src/MicroBitwiseExtra.elm

@@ -28,17 +28,16 @@ isBitSet index num =
 
 int52FromTuple : ( Int, Int ) -> Int
 int52FromTuple ( highBits, lowBits ) =
-    (+)
-        (highBits
-            |> keepBits 20
-            |> signedToUnsigned
-            -- Bitwise.shiftLeftBy 32 would be buggy, so we do:
-            |> (*) 0x0000000100000000
-        )
-        (lowBits
+    (highBits
+        |> keepBits 20
+        |> signedToUnsigned
+        -- Bitwise.shiftLeftBy 32 would be buggy, so we do:
+        |> (*) 0x0000000100000000
+    )
+        + (lowBits
             |> signedToUnsigned
             |> keepBits 32
-        )
+          )
 
 
 int52ToTuple : Int -> ( Int, Int )

src/RandomRun.elm

@@ -128,18 +128,15 @@ deleteChunk chunk run =
         let
             list =
                 Queue.toList run.data
-
-            result =
-                { run
-                    | length = run.length - chunk.size
-                    , data =
-                        (List.take chunk.startIndex list
-                            ++ List.drop (chunk.startIndex + chunk.size) list
-                        )
-                            |> Queue.fromList
-                }
         in
-        result
+        { run
+            | length = run.length - chunk.size
+            , data =
+                (List.take chunk.startIndex list
+                    ++ List.drop (chunk.startIndex + chunk.size) list
+                )
+                    |> Queue.fromList
+        }
 
     else
         run
@@ -219,10 +216,6 @@ swapChunks :
     -> RandomRun
     -> Maybe RandomRun
 swapChunks { leftChunk, rightChunk } run =
-    let
-        list =
-            Queue.toList run.data
-    in
     Maybe.map2
         (\lefts rights ->
             replaceInList
@@ -232,7 +225,7 @@ swapChunks { leftChunk, rightChunk } run =
                     ]
                 )
                 run.length
-                list
+                (Queue.toList run.data)
         )
         {- TODO PERF: both of these are doing the Queue.toList etc. operations
            while we already have that factored out in the `list` var.

src/Simplify.elm

@@ -137,15 +137,14 @@ logRun label run =
 
 logState : String -> State a -> State a
 logState label state =
-    let
-        runString =
-            Debug.toString (RandomRun.toList state.randomRun)
-    in
     let
         _ =
             case Fuzz.Internal.generate (PRNG.hardcoded state.randomRun) state.fuzzer of
                 Generated { value } ->
                     let
+                        runString =
+                            Debug.toString (RandomRun.toList state.randomRun)
+
                         _ =
                             Debug.log (label ++ " - " ++ runString ++ " --->") value
                     in

src/Simplify/Cmd.elm

@@ -90,7 +90,7 @@ cmdsForRun run =
     List.fastConcat
         [ deletionCmds length
         , zeroCmds length
-        , minimizeChoiceCmds run length
+        , minimizeChoiceCmds run
         , minimizeFloatCmds run length
         , sortCmds length
         , redistributeCmds length
@@ -126,8 +126,8 @@ sortCmds length =
         }
 
 
-minimizeChoiceCmds : RandomRun -> Int -> List SimplifyCmd
-minimizeChoiceCmds run length =
+minimizeChoiceCmds : RandomRun -> List SimplifyCmd
+minimizeChoiceCmds run =
     run
         |> RandomRun.toList
         |> List.indexedMap Tuple.pair

src/Test/Fuzz.elm

@@ -13,7 +13,6 @@ import Test.Coverage exposing (CoverageReport(..))
 import Test.Coverage.Internal exposing (Coverage(..), ExpectedCoverage(..))
 import Test.Expectation exposing (Expectation(..))
 import Test.Internal exposing (Test(..), blankDescriptionFailure)
-import Test.Runner.Coverage
 import Test.Runner.Failure exposing (InvalidReason(..), Reason(..))
 
 

src/Test/Html/Internal/ElmHtml/InternalTypes.elm

@@ -19,9 +19,9 @@ module Test.Html.Internal.ElmHtml.InternalTypes exposing
 
 import Dict exposing (Dict)
 import Json.Decode exposing (field)
-import Test.Html.Internal.ElmHtml.Constants as Constants exposing (..)
-import Test.Html.Internal.ElmHtml.Helpers exposing (..)
-import Test.Html.Internal.ElmHtml.Markdown exposing (..)
+import Test.Html.Internal.ElmHtml.Constants as Constants
+import Test.Html.Internal.ElmHtml.Helpers exposing (filterKnownKeys)
+import Test.Html.Internal.ElmHtml.Markdown exposing (MarkdownModel, decodeMarkdownModel)
 import Test.Internal.KernelConstants exposing (kernelConstants)
 import VirtualDom
 
@@ -312,7 +312,7 @@ decodeMarkdownNodeRecord context =
 decodeStyles : Json.Decode.Decoder (Dict String String)
 decodeStyles =
     Json.Decode.oneOf
-        [ field styleKey (Json.Decode.dict Json.Decode.string)
+        [ field Constants.styleKey (Json.Decode.dict Json.Decode.string)
         , Json.Decode.succeed Dict.empty
         ]
 
@@ -353,15 +353,15 @@ decodeDictFilterMap decoder =
 decodeAttributes : Json.Decode.Decoder a -> Json.Decode.Decoder (Dict String a)
 decodeAttributes decoder =
     Json.Decode.oneOf
-        [ Json.Decode.field attributeKey (decodeDictFilterMap decoder)
+        [ Json.Decode.field Constants.attributeKey (decodeDictFilterMap decoder)
         , Json.Decode.succeed Dict.empty
         ]
 
 
 decodeEvents : (EventHandler -> VirtualDom.Handler msg) -> Json.Decode.Decoder (Dict String (VirtualDom.Handler msg))
 decodeEvents taggedEventDecoder =
     Json.Decode.oneOf
-        [ Json.Decode.field eventKey (Json.Decode.dict (Json.Decode.map taggedEventDecoder Json.Decode.value))
+        [ Json.Decode.field Constants.eventKey (Json.Decode.dict (Json.Decode.map taggedEventDecoder Json.Decode.value))
         , Json.Decode.succeed Dict.empty
         ]
 
@@ -373,7 +373,7 @@ decodeFacts (HtmlContext taggers eventDecoder) =
     Json.Decode.map5 Facts
         decodeStyles
         (decodeEvents (eventDecoder taggers))
-        (Json.Decode.maybe (Json.Decode.field attributeNamespaceKey Json.Decode.value))
+        (Json.Decode.maybe (Json.Decode.field Constants.attributeNamespaceKey Json.Decode.value))
         (decodeOthers Json.Decode.string)
         (decodeOthers Json.Decode.bool)
 

src/Test/Html/Internal/ElmHtml/Markdown.elm

@@ -1,11 +1,11 @@
 module Test.Html.Internal.ElmHtml.Markdown exposing
-    ( MarkdownOptions, MarkdownModel, baseMarkdownModel
+    ( MarkdownOptions, MarkdownModel
     , decodeMarkdownModel
     )
 
 {-| Markdown helpers
 
-@docs MarkdownOptions, MarkdownModel, baseMarkdownModel
+@docs MarkdownOptions, MarkdownModel
 
 @docs decodeMarkdownModel
 

src/Test/Html/Internal/ElmHtml/ToString.elm

@@ -95,14 +95,6 @@ nodeRecordToString options { tag, children, facts } =
             in
             "<" ++ tag ++ filling ++ ">"
 
-        closeTag =
-            "</" ++ tag ++ ">"
-
-        childrenStrings =
-            List.map (nodeToLines options) children
-                |> List.concat
-                |> List.map ((++) (String.repeat options.indent " "))
-
         styles =
             case Dict.toList facts.styles of
                 [] ->
@@ -111,7 +103,7 @@ nodeRecordToString options { tag, children, facts } =
                 styleValues ->
                     styleValues
                         |> List.map (\( key, value ) -> key ++ ":" ++ value ++ ";")
-                        |> String.join ""
+                        |> String.concat
                         |> (\styleString -> "style=\"" ++ styleString ++ "\"")
                         |> Just
 
@@ -127,12 +119,11 @@ nodeRecordToString options { tag, children, facts } =
                 |> Just
 
         boolToString b =
-            case b of
-                True ->
-                    "True"
+            if b then
+                "True"
 
-                False ->
-                    "False"
+            else
+                "False"
 
         boolAttributes =
             Dict.toList facts.boolAttributes
@@ -153,6 +144,15 @@ nodeRecordToString options { tag, children, facts } =
            element kinds.
         -}
         _ ->
-            [ openTag [ classes, styles, stringAttributes, boolAttributes ] ]
-                ++ childrenStrings
+            let
+                closeTag =
+                    "</" ++ tag ++ ">"
+
+                childrenStrings =
+                    children
+                        |> List.concatMap (nodeToLines options)
+                        |> List.map ((++) (String.repeat options.indent " "))
+            in
+            openTag [ classes, styles, stringAttributes, boolAttributes ]
+                :: childrenStrings
                 ++ [ closeTag ]

src/Test/Html/Query.elm

@@ -24,7 +24,8 @@ import Expect exposing (Expectation)
 import Html exposing (Html)
 import Test.Html.Internal.Inert as Inert
 import Test.Html.Query.Internal as Internal exposing (failWithQuery)
-import Test.Html.Selector.Internal exposing (Selector, selectorToString)
+import Test.Html.Selector exposing (Selector)
+import Test.Html.Selector.Internal exposing (selectorToString)