mirror of
https://github.com/jfmengels/elm-review.git
synced 2024-12-23 17:53:35 +03:00
Remove rule DefaultPatternPosition
The compiler now forbids redundant patterns 👍
This commit is contained in:
parent
7a4aaaa8b8
commit
6f26e13714
@ -5,7 +5,6 @@ module LintConfig exposing (config)
|
||||
|
||||
import Lint exposing (Severity(..))
|
||||
import Lint.Rule exposing (Rule)
|
||||
import Lint.Rule.DefaultPatternPosition as DefaultPatternPosition
|
||||
import Lint.Rule.NoDebug
|
||||
import Lint.Rule.NoExtraBooleanComparison
|
||||
import Lint.Rule.NoImportingEverything
|
||||
@ -15,8 +14,7 @@ import Lint.Rule.NoUnusedVariables
|
||||
|
||||
config : List ( Severity, Rule )
|
||||
config =
|
||||
[ ( Critical, DefaultPatternPosition.rule DefaultPatternPosition.ShouldBeLast )
|
||||
, ( Critical, Lint.Rule.NoDebug.rule )
|
||||
[ ( Critical, Lint.Rule.NoDebug.rule )
|
||||
, ( Critical, Lint.Rule.NoExtraBooleanComparison.rule )
|
||||
, ( Critical, Lint.Rule.NoImportingEverything.rule { exceptions = [] } )
|
||||
, ( Critical, Lint.Rule.NoUnusedVariables.rule )
|
||||
|
@ -41,7 +41,6 @@ Do remember that `elm-lint` is supposed to be run after the Elm compiler has val
|
||||
|
||||
These are the rules that are built-in and available for you to choose from.
|
||||
|
||||
- **DefaultPatternPosition** - Enforce the default pattern to always appear first or last.
|
||||
- **NoDebug** - Forbid the use of `Debug` before it goes into production.
|
||||
- **NoExtraBooleanComparison** - Forbid extraneous comparisons with booleans, like `(isAdmin user) == True`.
|
||||
- **NoImportingEverything** - Forbid importing everything from your module. This can especially be confusing to newcomers when the exposed functions and types are unknown to them.
|
||||
@ -70,7 +69,6 @@ module LintConfig exposing (config)
|
||||
|
||||
import Lint exposing (Severity(..))
|
||||
import Lint.Rule exposing (Rule)
|
||||
import Lint.Rule.DefaultPatternPosition as DefaultPatternPosition
|
||||
import Lint.Rule.NoDebug
|
||||
import Lint.Rule.NoExtraBooleanComparison
|
||||
import Lint.Rule.NoImportingEverything
|
||||
@ -80,12 +78,10 @@ import Lint.Rule.NoUnusedVariables
|
||||
|
||||
config : List ( Severity, Rule )
|
||||
config =
|
||||
[ ( Critical, DefaultPatternPosition.rule DefaultPatternPosition.ShouldBeLast )
|
||||
, ( Critical, Lint.Rule.NoExtraBooleanComparison.rule )
|
||||
[ ( Critical, Lint.Rule.NoExtraBooleanComparison.rule )
|
||||
, ( Critical, Lint.Rule.NoUnusedVariables.rule )
|
||||
, ( Critical, Lint.Rule.NoUnusedTypeConstructors.rule )
|
||||
, ( Warning, Lint.Rule.NoDebug.rule )
|
||||
, ( Critical, Lint.Rule.NoDuplicateImports.rule )
|
||||
, ( Critical, Lint.Rule.NoImportingEverything.rule { exceptions = [ "Html" ] } )
|
||||
]
|
||||
```
|
||||
|
1
elm.json
1
elm.json
@ -7,7 +7,6 @@
|
||||
"exposed-modules": [
|
||||
"Lint",
|
||||
"Lint.Rule",
|
||||
"Lint.Rule.DefaultPatternPosition",
|
||||
"Lint.Rule.NoDebug",
|
||||
"Lint.Rule.NoExtraBooleanComparison",
|
||||
"Lint.Rule.NoImportingEverything",
|
||||
|
@ -6,7 +6,6 @@ import Html.Attributes as Attr
|
||||
import Html.Events as Events
|
||||
import Lint exposing (LintError, Severity(..), lintSource)
|
||||
import Lint.Rule exposing (Rule)
|
||||
import Lint.Rule.DefaultPatternPosition as DefaultPatternPosition exposing (PatternPosition)
|
||||
import Lint.Rule.NoDebug
|
||||
import Lint.Rule.NoExtraBooleanComparison
|
||||
import Lint.Rule.NoImportingEverything
|
||||
@ -23,7 +22,6 @@ config model =
|
||||
[ ( model.noDebugEnabled, ( Critical, Lint.Rule.NoDebug.rule ) )
|
||||
, ( model.noUnusedVariablesEnabled, ( Critical, Lint.Rule.NoUnusedVariables.rule ) )
|
||||
, ( model.noImportingEverythingEnabled, ( Critical, Lint.Rule.NoImportingEverything.rule { exceptions = [ "Html" ] } ) )
|
||||
, ( model.defaultPatternPositionEnabled, ( Critical, DefaultPatternPosition.rule model.defaultPatternPositionPattern ) )
|
||||
, ( model.noExtraBooleanComparisonEnabled, ( Critical, Lint.Rule.NoExtraBooleanComparison.rule ) )
|
||||
, ( model.noUnusedTypeConstructorsEnabled, ( Critical, Lint.Rule.NoUnusedTypeConstructors.rule ) )
|
||||
|
||||
@ -54,8 +52,6 @@ type alias Model =
|
||||
, noUnusedVariablesEnabled : Bool
|
||||
, noImportingEverythingEnabled : Bool
|
||||
, noImportingEverythingExceptions : List String
|
||||
, defaultPatternPositionEnabled : Bool
|
||||
, defaultPatternPositionPattern : PatternPosition
|
||||
, noExtraBooleanComparisonEnabled : Bool
|
||||
, noUnusedTypeConstructorsEnabled : Bool
|
||||
, showConfigurationAsText : Bool
|
||||
@ -88,8 +84,6 @@ g n = n + 1
|
||||
, noUnusedVariablesEnabled = True
|
||||
, noImportingEverythingEnabled = True
|
||||
, noImportingEverythingExceptions = [ "Html", "Html.Attributes" ]
|
||||
, defaultPatternPositionEnabled = True
|
||||
, defaultPatternPositionPattern = DefaultPatternPosition.ShouldBeLast
|
||||
, noExtraBooleanComparisonEnabled = True
|
||||
, noUnusedTypeConstructorsEnabled = True
|
||||
, showConfigurationAsText = False
|
||||
@ -107,10 +101,8 @@ type Msg
|
||||
| UserToggledNoDebugRule
|
||||
| UserToggledNoUnusedVariablesRule
|
||||
| UserToggledNoImportingEverythingRule
|
||||
| UserToggledDefaultPatternPositionRule
|
||||
| UserToggledNoExtraBooleanComparisonRule
|
||||
| UserToggledNoUnusedTypeConstructorsRule
|
||||
| UserChangedDefaultPatternSetting PatternPosition
|
||||
| UserToggledConfigurationAsText
|
||||
|
||||
|
||||
@ -135,14 +127,6 @@ update action model =
|
||||
{ model | noImportingEverythingEnabled = not model.noImportingEverythingEnabled }
|
||||
|> rerunLinting
|
||||
|
||||
UserToggledDefaultPatternPositionRule ->
|
||||
{ model | defaultPatternPositionEnabled = not model.defaultPatternPositionEnabled }
|
||||
|> rerunLinting
|
||||
|
||||
UserChangedDefaultPatternSetting patternPosition ->
|
||||
{ model | defaultPatternPositionPattern = patternPosition }
|
||||
|> rerunLinting
|
||||
|
||||
UserToggledNoExtraBooleanComparisonRule ->
|
||||
{ model | noExtraBooleanComparisonEnabled = not model.noExtraBooleanComparisonEnabled }
|
||||
|> rerunLinting
|
||||
@ -203,21 +187,6 @@ viewConfigurationPanel model =
|
||||
[ viewCheckbox UserToggledNoDebugRule "NoDebug" model.noDebugEnabled
|
||||
, viewCheckbox UserToggledNoUnusedVariablesRule "NoUnusedVariables" model.noUnusedVariablesEnabled
|
||||
, viewCheckbox UserToggledNoImportingEverythingRule "NoImportingEverything" model.noImportingEverythingEnabled
|
||||
, form [ Attr.action "" ]
|
||||
[ viewCheckbox UserToggledDefaultPatternPositionRule "DefaultPatternPosition" model.defaultPatternPositionEnabled
|
||||
, viewRadioButton
|
||||
UserChangedDefaultPatternSetting
|
||||
DefaultPatternPosition.ShouldBeLast
|
||||
"Should be last"
|
||||
model.defaultPatternPositionEnabled
|
||||
model.defaultPatternPositionPattern
|
||||
, viewRadioButton
|
||||
UserChangedDefaultPatternSetting
|
||||
DefaultPatternPosition.ShouldBeFirst
|
||||
"Should be first"
|
||||
model.defaultPatternPositionEnabled
|
||||
model.defaultPatternPositionPattern
|
||||
]
|
||||
, viewCheckbox UserToggledNoExtraBooleanComparisonRule "NoExtraBooleanComparison" model.noExtraBooleanComparisonEnabled
|
||||
, viewCheckbox UserToggledNoUnusedTypeConstructorsRule "NoUnusedTypeConstructors" model.noUnusedTypeConstructorsEnabled
|
||||
]
|
||||
@ -272,19 +241,6 @@ configurationAsText model =
|
||||
, configExpression = "Lint.Rule.NoImportingEverything.rule { exceptions = [] }"
|
||||
}
|
||||
)
|
||||
, ( model.defaultPatternPositionEnabled
|
||||
, { import_ = "Lint.Rule.DefaultPatternPosition as DefaultPatternPosition"
|
||||
, configExpression =
|
||||
"DefaultPatternPosition.rule DefaultPatternPosition."
|
||||
++ (case model.defaultPatternPositionPattern of
|
||||
DefaultPatternPosition.ShouldBeFirst ->
|
||||
"ShouldBeFirst"
|
||||
|
||||
DefaultPatternPosition.ShouldBeLast ->
|
||||
"ShouldBeLast"
|
||||
)
|
||||
}
|
||||
)
|
||||
, ( model.noExtraBooleanComparisonEnabled
|
||||
, { import_ = "Lint.Rule.NoExtraBooleanComparison"
|
||||
, configExpression = "Lint.Rule.NoExtraBooleanComparison.rule"
|
||||
@ -339,23 +295,6 @@ viewCheckbox onClick name checked =
|
||||
]
|
||||
|
||||
|
||||
viewRadioButton : (PatternPosition -> Msg) -> PatternPosition -> String -> Bool -> PatternPosition -> Html Msg
|
||||
viewRadioButton onClick patternPosition name enabled selectedPatternPosition =
|
||||
label
|
||||
[]
|
||||
[ input
|
||||
[ Attr.type_ "radio"
|
||||
, Attr.checked (patternPosition == selectedPatternPosition)
|
||||
, Events.onClick (onClick patternPosition)
|
||||
, Attr.disabled <| not enabled
|
||||
, Attr.name name
|
||||
, Attr.value name
|
||||
]
|
||||
[]
|
||||
, text name
|
||||
]
|
||||
|
||||
|
||||
lintErrors : Model -> List (Html Msg)
|
||||
lintErrors model =
|
||||
let
|
||||
|
@ -1,172 +0,0 @@
|
||||
module Lint.Rule.DefaultPatternPosition exposing (rule, PatternPosition(..))
|
||||
|
||||
{-| Enforce the default pattern to always appear first or last.
|
||||
|
||||
|
||||
## Fail
|
||||
|
||||
import Lint.Rules.DefaultPatternPosition as DefaultPatternPosition
|
||||
|
||||
config =
|
||||
[ ( Critical
|
||||
, DefaultPatternPosition.rule DefaultPatternPosition.ShouldBeLast
|
||||
)
|
||||
]
|
||||
|
||||
a =
|
||||
case value of
|
||||
-- Error: "Expected default pattern to appear last in the list of patterns"
|
||||
_ ->
|
||||
result
|
||||
|
||||
Foo ->
|
||||
bar
|
||||
|
||||
----------------------
|
||||
config =
|
||||
[ ( Critical
|
||||
, DefaultPatternPosition.rule DefaultPatternPosition.ShouldBeFirst
|
||||
)
|
||||
]
|
||||
|
||||
a =
|
||||
case value of
|
||||
Foo ->
|
||||
bar
|
||||
|
||||
-- Error: Expected default pattern to appear first in the list of patterns
|
||||
_ ->
|
||||
result
|
||||
|
||||
|
||||
## Success
|
||||
|
||||
config =
|
||||
[ ( Critical
|
||||
, DefaultPatternPosition.rule DefaultPatternPosition.ShouldBeLast
|
||||
)
|
||||
]
|
||||
|
||||
a =
|
||||
case value of
|
||||
Foo ->
|
||||
bar
|
||||
|
||||
_ ->
|
||||
result
|
||||
a =
|
||||
case value of
|
||||
-- No default pattern
|
||||
Foo ->
|
||||
bar
|
||||
|
||||
Bar ->
|
||||
foo
|
||||
|
||||
-- --------------------
|
||||
config =
|
||||
[ ( Critical
|
||||
, DefaultPatternPosition.rule DefaultPatternPosition.ShouldBeFirst
|
||||
)
|
||||
]
|
||||
|
||||
a =
|
||||
case value of
|
||||
_ ->
|
||||
result
|
||||
|
||||
Foo ->
|
||||
bar
|
||||
|
||||
|
||||
# Rule and configuration
|
||||
|
||||
@docs rule, PatternPosition
|
||||
|
||||
-}
|
||||
|
||||
import Elm.Syntax.Expression exposing (Expression(..))
|
||||
import Elm.Syntax.Node as Node exposing (Node)
|
||||
import Elm.Syntax.Pattern exposing (Pattern(..))
|
||||
import Lint.Rule as Rule exposing (Error, Rule)
|
||||
|
||||
|
||||
{-| Configures whether the default pattern should appear first or last.
|
||||
-}
|
||||
type PatternPosition
|
||||
= ShouldBeFirst
|
||||
| ShouldBeLast
|
||||
|
||||
|
||||
{-| Enforce the default pattern to always appear first or last.
|
||||
-}
|
||||
rule : PatternPosition -> Rule
|
||||
rule patternPosition =
|
||||
Rule.newSchema "DefaultPatternPosition"
|
||||
|> Rule.withSimpleExpressionVisitor (expressionVisitor patternPosition)
|
||||
|> Rule.fromSchema
|
||||
|
||||
|
||||
error : Node a -> String -> Error
|
||||
error node message =
|
||||
Rule.error message (Node.range node)
|
||||
|
||||
|
||||
isDefaultPattern : Pattern -> Bool
|
||||
isDefaultPattern pattern =
|
||||
case pattern of
|
||||
AllPattern ->
|
||||
True
|
||||
|
||||
_ ->
|
||||
False
|
||||
|
||||
|
||||
findDefaultPattern : List ( Node Pattern, Node Expression ) -> Maybe ( Int, Node Pattern )
|
||||
findDefaultPattern patterns =
|
||||
findWithIndex
|
||||
(Node.value >> isDefaultPattern)
|
||||
(List.map Tuple.first patterns)
|
||||
|
||||
|
||||
findWithIndex : (a -> Bool) -> List a -> Maybe ( Int, a )
|
||||
findWithIndex isMatch list =
|
||||
case list of
|
||||
[] ->
|
||||
Nothing
|
||||
|
||||
elem :: rest ->
|
||||
if isMatch elem then
|
||||
Just ( 0, elem )
|
||||
|
||||
else
|
||||
findWithIndex isMatch rest
|
||||
|> Maybe.map (\( index, elem_ ) -> ( index + 1, elem_ ))
|
||||
|
||||
|
||||
expressionVisitor : PatternPosition -> Node Expression -> List Error
|
||||
expressionVisitor patternPosition node =
|
||||
case Node.value node of
|
||||
CaseExpression { cases } ->
|
||||
case findDefaultPattern cases of
|
||||
Nothing ->
|
||||
[]
|
||||
|
||||
Just ( index, patternNode ) ->
|
||||
case patternPosition of
|
||||
ShouldBeFirst ->
|
||||
if index /= 0 then
|
||||
[ error patternNode "Expected default pattern to appear first in the list of patterns" ]
|
||||
|
||||
else
|
||||
[]
|
||||
|
||||
ShouldBeLast ->
|
||||
if index /= List.length cases - 1 then
|
||||
[ error patternNode "Expected default pattern to appear last in the list of patterns" ]
|
||||
|
||||
else
|
||||
[]
|
||||
|
||||
_ ->
|
||||
[]
|
@ -1,123 +0,0 @@
|
||||
module DefaultPatternPositionTest exposing (all)
|
||||
|
||||
import Lint.Rule.DefaultPatternPosition exposing (PatternPosition(..), rule)
|
||||
import Lint.Test exposing (LintResult)
|
||||
import Test exposing (Test, describe, test)
|
||||
|
||||
|
||||
testRule : PatternPosition -> String -> LintResult
|
||||
testRule patternPosition =
|
||||
Lint.Test.run (rule patternPosition)
|
||||
|
||||
|
||||
message : String -> String
|
||||
message position =
|
||||
"Expected default pattern to appear " ++ position ++ " in the list of patterns"
|
||||
|
||||
|
||||
tests : List Test
|
||||
tests =
|
||||
[ test "should not report when default pattern is at the expected position (first)" <|
|
||||
\() ->
|
||||
"""module A exposing(..)
|
||||
a = case b of
|
||||
_ -> 1
|
||||
Bar -> 1
|
||||
Foo -> 1
|
||||
"""
|
||||
|> testRule ShouldBeFirst
|
||||
|> Lint.Test.expectNoErrors
|
||||
, test "should not report when default pattern is at the expected position (last)" <|
|
||||
\() ->
|
||||
"""module A exposing(..)
|
||||
a = case b of
|
||||
Foo -> 1
|
||||
Bar -> 1
|
||||
_ -> 1
|
||||
"""
|
||||
|> testRule ShouldBeLast
|
||||
|> Lint.Test.expectNoErrors
|
||||
, test "should not report when there is no default pattern (first)" <|
|
||||
\() ->
|
||||
"""module A exposing(..)
|
||||
a = case b of
|
||||
Foo -> 1
|
||||
Bar -> 1
|
||||
"""
|
||||
|> testRule ShouldBeFirst
|
||||
|> Lint.Test.expectNoErrors
|
||||
, test "should not report when there is no default pattern (last)" <|
|
||||
\() ->
|
||||
"""module A exposing(..)
|
||||
a = case b of
|
||||
Foo -> 1
|
||||
Bar -> 1
|
||||
"""
|
||||
|> testRule ShouldBeLast
|
||||
|> Lint.Test.expectNoErrors
|
||||
, test "should report an error when the default pattern is not at the expected position (first) (opposite expected position)" <|
|
||||
\() ->
|
||||
"""module A exposing(..)
|
||||
a = case b of
|
||||
Foo -> 1
|
||||
Bar -> 1
|
||||
_ -> 1
|
||||
"""
|
||||
|> testRule ShouldBeFirst
|
||||
|> Lint.Test.expectErrors
|
||||
[ Lint.Test.error
|
||||
{ message = message "first"
|
||||
, under = "_"
|
||||
}
|
||||
]
|
||||
, test "should report an error when the default pattern is not at the expected position (first) (somewhere in the middle)" <|
|
||||
\() ->
|
||||
"""module A exposing(..)
|
||||
a = case b of
|
||||
Foo -> 1
|
||||
_ -> 1
|
||||
Bar -> 1
|
||||
"""
|
||||
|> testRule ShouldBeFirst
|
||||
|> Lint.Test.expectErrors
|
||||
[ Lint.Test.error
|
||||
{ message = message "first"
|
||||
, under = "_"
|
||||
}
|
||||
]
|
||||
, test "should report an error when the default pattern is not at the expected position (last) (opposite expected position)" <|
|
||||
\() ->
|
||||
"""module A exposing(..)
|
||||
a = case b of
|
||||
_ -> 1
|
||||
Foo -> 1
|
||||
Bar -> 1
|
||||
"""
|
||||
|> testRule ShouldBeLast
|
||||
|> Lint.Test.expectErrors
|
||||
[ Lint.Test.error
|
||||
{ message = message "last"
|
||||
, under = "_"
|
||||
}
|
||||
]
|
||||
, test "should report an error when the default pattern is not at the expected position (last) (somewhere in the middle)" <|
|
||||
\() ->
|
||||
"""module A exposing(..)
|
||||
a = case b of
|
||||
Foo -> 1
|
||||
_ -> 1
|
||||
Bar -> 1
|
||||
"""
|
||||
|> testRule ShouldBeLast
|
||||
|> Lint.Test.expectErrors
|
||||
[ Lint.Test.error
|
||||
{ message = message "last"
|
||||
, under = "_"
|
||||
}
|
||||
]
|
||||
]
|
||||
|
||||
|
||||
all : Test
|
||||
all =
|
||||
describe "DefaultPatternPosition" tests
|
Loading…
Reference in New Issue
Block a user