Skip to content

Commit

Permalink
Add more review rules 446 (#466)
Browse files Browse the repository at this point in the history
Enables the remaining Unused review rules as discussed in #446. Things to note:
- a couple of files have been excluded because they're reported, but are essential for the executable to work properly (false positives, see discussion for #446)
- some minor changes to the copied external modules were necessary to make elm-review happy; if we decide to re-import them in the future, we'll need to re-apply these changes. This is not the optimal solution; we should probably either try to get the upstream modules to also use elm-review and clean up unused imports etc. or exclude them from elm-review.

Suggestions / feedback welcome :-)
  • Loading branch information
frankschmitt authored Oct 31, 2020
1 parent 4771b8c commit 728c13a
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 128 deletions.
36 changes: 20 additions & 16 deletions elm/review/src/ReviewConfig.elm
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,24 @@ import Review.Rule exposing (Rule)

config : List Rule
config =
[ NoUnused.Variables.rule
[ NoUnused.CustomTypeConstructors.rule []
|> Review.Rule.ignoreErrorsForFiles
[ "src/Test/Reporter/Reporter.elm" --ConsoleReport, JsonReport, JunitReport are used externally
, "src/Console/Text.elm" -- Monochrome, UseColor are used externally
]
, NoUnused.Exports.rule
|> Review.Rule.ignoreErrorsForFiles
[ "x" --"src/Test/Runner/Node/Vendor/Diff.elm"
, "src/Test/Runner/Node.elm" -- run, TestProgram are used externally
]
, NoUnused.Modules.rule
, NoUnused.CustomTypeConstructorArgs.rule
|> Review.Rule.ignoreErrorsForFiles
[ "src/Test/Runner/Node/Vendor/Diff.elm" -- UnexpectedPath is used for reporting errors
, "src/Test/Runner/JsMessage.elm" -- Test is used for JSON decoding
]
, NoUnused.Dependencies.rule
, NoUnused.Parameters.rule
, NoUnused.Patterns.rule
, NoUnused.Variables.rule
]



{-
config =
[ NoUnused.CustomTypeConstructors.rule []
, NoUnused.CustomTypeConstructorArgs.rule
, NoUnused.Dependencies.rule
, NoUnused.Exports.rule
, NoUnused.Modules.rule
, NoUnused.Parameters.rule
, NoUnused.Patterns.rule
, NoUnused.Variables.rule
]
-}
58 changes: 0 additions & 58 deletions elm/src/Console/Text.elm
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,13 @@ module Console.Text exposing
, Style
, Text
, UseColor(..)
, black
, blue
, bold
, concat
, cyan
, dark
, default
, green
, inverted
, magenta
, plain
, red
, render
, underline
, white
, yellow
)

Expand Down Expand Up @@ -92,11 +84,6 @@ plain =
-- FOREGROUND COLORS --


default : String -> Text
default =
Text { foreground = Default, background = Default, style = Normal, modifiers = [] }


red : String -> Text
red =
Text { foreground = Red, background = Default, style = Normal, modifiers = [] }
Expand All @@ -112,41 +99,6 @@ yellow =
Text { foreground = Yellow, background = Default, style = Normal, modifiers = [] }


black : String -> Text
black =
Text { foreground = Black, background = Default, style = Normal, modifiers = [] }


blue : String -> Text
blue =
Text { foreground = Blue, background = Default, style = Normal, modifiers = [] }


magenta : String -> Text
magenta =
Text { foreground = Magenta, background = Default, style = Normal, modifiers = [] }


cyan : String -> Text
cyan =
Text { foreground = Cyan, background = Default, style = Normal, modifiers = [] }


white : String -> Text
white =
Text { foreground = White, background = Default, style = Normal, modifiers = [] }


inverted : Text -> Text
inverted txt =
case txt of
Text styles str ->
Text { styles | modifiers = Inverted :: styles.modifiers } str

Texts texts ->
Texts (List.map inverted texts)


dark : Text -> Text
dark txt =
case txt of
Expand All @@ -161,16 +113,6 @@ dark txt =
-- STYLES --


bold : Text -> Text
bold txt =
case txt of
Text styles str ->
Text { styles | style = Bold } str

Texts texts ->
Texts (List.map dark texts)


underline : Text -> Text
underline txt =
case txt of
Expand Down
2 changes: 1 addition & 1 deletion elm/src/Test/Reporter/Console.elm
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ reportSummary useColor { todos, passed, failed, duration } autoFail =
( Just failure, 0, _ ) ->
Err ( yellow, "TEST RUN INCOMPLETE", " because " ++ failure )

( _, _, _ ) ->
_ ->
Err ( red, "TEST RUN FAILED", "" )

headline =
Expand Down
2 changes: 1 addition & 1 deletion elm/src/Test/Reporter/Highlightable.elm
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module Test.Reporter.Highlightable exposing (Highlightable, diffLists, fromDiff, map, resolve)
module Test.Reporter.Highlightable exposing (Highlightable, diffLists, map, resolve)

import Test.Runner.Node.Vendor.Diff as Diff exposing (Change(..))

Expand Down
11 changes: 0 additions & 11 deletions elm/src/Test/Reporter/TestResults.elm
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ module Test.Reporter.TestResults exposing
, SummaryInfo
, TestResult
, isFailure
, isTodo
, outcomesFromExpectations
)

Expand Down Expand Up @@ -42,16 +41,6 @@ type alias Failure =
}


isTodo : Outcome -> Bool
isTodo outcome =
case outcome of
Todo _ ->
True

_ ->
False


isFailure : Outcome -> Bool
isFailure outcome =
case outcome of
Expand Down
9 changes: 1 addition & 8 deletions elm/src/Test/Runner/Node/Vendor/Console.elm
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module Test.Runner.Node.Vendor.Console exposing (bgBlack, bgBlue, bgCyan, bgGreen, bgMagenta, bgRed, bgWhite, bgYellow, black, blue, bold, colorsInverted, cyan, dark, green, magenta, plain, red, underline, white, yellow)
module Test.Runner.Node.Vendor.Console exposing (bgBlack, bgBlue, bgCyan, bgGreen, bgMagenta, bgRed, bgWhite, bgYellow, black, blue, bold, colorsInverted, cyan, dark, green, magenta, red, underline, white, yellow)

{-| -}

Expand Down Expand Up @@ -43,13 +43,6 @@ module Test.Runner.Node.Vendor.Console exposing (bgBlack, bgBlue, bgCyan, bgGree
-}


{-| Display the text in the console's default style.
-}
plain : String -> String
plain str =
String.join "" [ "\u{001B}[0m", str, "\u{001B}[0m" ]


{-| Make the text darker.
This can be used with other text modifiers, such as color.
Expand Down
34 changes: 1 addition & 33 deletions elm/src/Test/Runner/Node/Vendor/Diff.elm
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module Test.Runner.Node.Vendor.Diff exposing
( Change(..)
, diff, diffLines
, diff
)

{-| Compares two list and returns how they have changed.
Expand Down Expand Up @@ -79,38 +79,6 @@ type BugReport
| UnexpectedPath ( Int, Int ) (List ( Int, Int ))


{-| Compares two text.
Giving the following text
a =
"""aaa
bbb
ddd"""
b =
"""zzz
aaa
ccc
ddd"""
results in
[ Added "zzz"
, NoChange "aaa"
, Removed "bbb"
, Added "ccc"
, NoChange "ddd"
]
.
-}
diffLines : String -> String -> List (Change String)
diffLines a b =
diff (String.lines a) (String.lines b)


{-| Compares general lists.
diff [ 1, 3 ] [ 2, 3 ] == [ Removed 1, Added 2, NoChange 3 ] -- True
Expand Down

0 comments on commit 728c13a

Please sign in to comment.