Backport rules from other projects

This commit is contained in:
Jeroen Engels 2020-09-22 19:40:30 +02:00
parent bb14366da9
commit b1b96c6dcf
24 changed files with 390 additions and 95 deletions

View File

@ -39,7 +39,7 @@ and publishing the package. Otherwise the link for a given version could link to
You can try this rule out by running the following command:
```bash
elm-review --template jfmengels/review-documentation/example --rules NoUselessSubscriptions
elm-review --template jfmengels/elm-review-documentation/example --rules NoUselessSubscriptions
```
-}

112
tests/NoBooleanCaseOf.elm Normal file
View File

@ -0,0 +1,112 @@
module NoBooleanCaseOf exposing (rule)
{-|
@docs rule
-}
import Elm.Syntax.Expression as Expression exposing (Expression)
import Elm.Syntax.Node as Node exposing (Node)
import Elm.Syntax.Pattern as Pattern exposing (Pattern)
import Review.Rule as Rule exposing (Error, Rule)
{-| Reports when pattern matching is used for a boolean value.
The idiomatic way to check for a condition is to use an `if` expression.
Read more about it at: <https://guide.elm-lang.org/core_language.html#if-expressions>
config =
[ NoBooleanCaseOf.rule
]
This won't report pattern matching when a boolean is part of the evaluated value.
## Fail
_ =
case bool of
True ->
expression
False ->
otherExpression
## Success
_ =
if bool then
expression
else
otherExpression
_ =
case ( bool, somethingElse ) of
( True, SomeThingElse ) ->
expression
_ ->
otherExpression
# When (not) to use this rule
You should not use this rule if you do not care about how your boolean values are
evaluated.
## Try it out
You can try this rule out by running the following command:
```bash
elm-review --template jfmengels/elm-review-simplification/example --rules NoBooleanCaseOf
```
-}
rule : Rule
rule =
Rule.newModuleRuleSchema "NoBooleanCaseOf" ()
|> Rule.withSimpleExpressionVisitor expressionVisitor
|> Rule.fromModuleRuleSchema
expressionVisitor : Node Expression -> List (Error {})
expressionVisitor node =
case Node.value node of
Expression.CaseExpression { expression, cases } ->
if List.any (Tuple.first >> isBoolConstructor) cases then
[ error expression ]
else
[]
_ ->
[]
error : Node a -> Error {}
error node =
Rule.error
{ message = "Replace `case..of` by an `if` condition"
, details =
[ "The idiomatic way to check for a condition is to use an `if` expression."
, "Read more about it at: https://guide.elm-lang.org/core_language.html#if-expressions"
]
}
(Node.range node)
isBoolConstructor : Node Pattern -> Bool
isBoolConstructor node =
case Node.value node of
Pattern.NamedPattern { moduleName, name } _ ->
(name == "True" || name == "False")
&& (moduleName == [] || moduleName == [ "Basics" ])
_ ->
False

View File

@ -0,0 +1,111 @@
module NoBooleanCaseOfTest exposing (all)
import NoBooleanCaseOf exposing (rule)
import Review.Test exposing (ReviewResult)
import Test exposing (Test, describe, test)
testRule : String -> ReviewResult
testRule string =
"module A exposing (..)\n\n"
++ string
|> Review.Test.run rule
message : String
message =
"Replace `case..of` by an `if` condition"
details : List String
details =
[ "The idiomatic way to check for a condition is to use an `if` expression."
, "Read more about it at: https://guide.elm-lang.org/core_language.html#if-expressions"
]
tests : List Test
tests =
[ test "should not report pattern matches for non-boolean values" <|
\() ->
testRule """
a = case thing of
Thing -> 1"""
|> Review.Test.expectNoErrors
, test "should not report pattern matches when the evaluated expression is a tuple of with a boolean" <|
\() ->
testRule """
a = case ( bool1, bool2 ) of
( True, True ) -> 1
_ -> 2"""
|> Review.Test.expectNoErrors
, test "should report pattern matches when one of the patterns is a bool constructor" <|
\() ->
testRule """
a = case boolTrue of
True -> 1
_ -> 2
b = case boolFalse of
False -> 1
_ -> 2
c = case boolAll of
False -> 1
True -> 2"""
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "boolTrue"
}
, Review.Test.error
{ message = message
, details = details
, under = "boolFalse"
}
, Review.Test.error
{ message = message
, details = details
, under = "boolAll"
}
]
, test "should report pattern matches for booleans even when one of the patterns starts with `Basics.`" <|
\() ->
testRule """
a = case boolTrue of
Basics.True -> 1
_ -> 2
b = case boolFalse of
Basics.False -> 1
_ -> 2"""
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "boolTrue"
}
, Review.Test.error
{ message = message
, details = details
, under = "boolFalse"
}
]
, test "should report pattern matches for booleans even when the constructor seems to be for booleans but comes from an unknown module" <|
\() ->
testRule """
a = case boolTrue of
OtherModule.True -> 1
_ -> 2
b = case boolFalse of
OtherModule.False -> 1
_ -> 2"""
|> Review.Test.expectNoErrors
]
all : Test
all =
describe "NoBooleanCaseOf" tests

View File

@ -57,7 +57,7 @@ do not ship to production.
You can try this rule out by running the following command:
```bash
elm-review --template jfmengels/review-debug/example --rules NoDebug.Log
elm-review --template jfmengels/elm-review-debug/example --rules NoDebug.Log
```
-}

View File

@ -74,7 +74,7 @@ can configure the rule like this.
You can try this rule out by running the following command:
```bash
elm-review --template jfmengels/review-debug/example --rules NoDebug.TodoOrToString
elm-review --template jfmengels/elm-review-debug/example --rules NoDebug.TodoOrToString
```
[`Debug.log`]: https://package.elm-lang.org/packages/elm/core/latest/Debug#log
@ -84,28 +84,22 @@ elm-review --template jfmengels/review-debug/example --rules NoDebug.TodoOrToStr
-}
rule : Rule
rule =
Rule.newModuleRuleSchemaUsingContextCreator "NoDebug.TodoOrToString" contextCreator
Rule.newModuleRuleSchema "NoDebug.TodoOrToString" init
|> Rule.withImportVisitor importVisitor
|> Rule.withExpressionEnterVisitor expressionVisitor
|> Rule.withExpressionVisitor expressionVisitor
|> Rule.fromModuleRuleSchema
contextCreator : Rule.ContextCreator () Context
contextCreator =
Rule.initContextCreator
(\metadata () ->
{ hasTodoBeenImported = False
, hasToStringBeenImported = False
, isInSourceDirectories = Rule.isInSourceDirectories metadata
}
)
|> Rule.withMetadata
type alias Context =
{ hasTodoBeenImported : Bool
, hasToStringBeenImported : Bool
, isInSourceDirectories : Bool
}
init : Context
init =
{ hasTodoBeenImported = False
, hasToStringBeenImported = False
}
@ -133,13 +127,13 @@ importVisitor node context =
if moduleName == [ "Debug" ] then
case node |> Node.value |> .exposingList |> Maybe.map Node.value of
Just (Exposing.All _) ->
( [], { hasTodoBeenImported = True, hasToStringBeenImported = True, isInSourceDirectories = context.isInSourceDirectories } )
( [], { hasTodoBeenImported = True, hasToStringBeenImported = True } )
Just (Exposing.Explicit importedNames) ->
( []
, { hasTodoBeenImported = List.any (is "todo") importedNames
, hasToStringBeenImported = List.any (is "toString") importedNames
, isInSourceDirectories = context.isInSourceDirectories
, { context
| hasTodoBeenImported = List.any (is "todo") importedNames
, hasToStringBeenImported = List.any (is "toString") importedNames
}
)
@ -160,32 +154,28 @@ is name node =
False
expressionVisitor : Node Expression -> Context -> ( List (Error {}), Context )
expressionVisitor node context =
if not context.isInSourceDirectories then
( [], context )
expressionVisitor : Node Expression -> Rule.Direction -> Context -> ( List (Error {}), Context )
expressionVisitor node direction context =
case ( direction, Node.value node ) of
( Rule.OnEnter, Expression.FunctionOrValue [ "Debug" ] name ) ->
if name == "todo" then
( [ error node name ], context )
else
case Node.value node of
Expression.FunctionOrValue [ "Debug" ] name ->
if name == "todo" then
( [ error node name ], context )
else if name == "toString" then
( [ error node name ], context )
else if name == "toString" then
( [ error node name ], context )
else
( [], context )
Expression.FunctionOrValue [] name ->
if name == "todo" && context.hasTodoBeenImported then
( [ error node name ], context )
else if name == "toString" && context.hasToStringBeenImported then
( [ error node name ], context )
else
( [], context )
_ ->
else
( [], context )
( Rule.OnEnter, Expression.FunctionOrValue [] name ) ->
if name == "todo" && context.hasTodoBeenImported then
( [ error node name ], context )
else if name == "toString" && context.hasToStringBeenImported then
( [ error node name ], context )
else
( [], context )
_ ->
( [], context )

View File

@ -46,7 +46,7 @@ in the following manner:
You can try this rule out by running the following command:
```bash
elm-review --template jfmengels/review-common/example --rules NoExposingEverything
elm-review --template jfmengels/elm-review-common/example --rules NoExposingEverything
```
-}

View File

@ -50,7 +50,7 @@ you can configure a list of exceptions.
You can try this rule out by running the following command:
```bash
elm-review --template jfmengels/review-common/example --rules NoImportingEverything
elm-review --template jfmengels/elm-review-common/example --rules NoImportingEverything
```
-}

View File

@ -58,7 +58,7 @@ This won't fail if `SomeModule` does not define a `subscriptions` function.
You can try this rule out by running the following command:
```bash
elm - review --template jfmengels/review-tea/example --rules NoMissingSubscriptionsCall
elm-review --template jfmengels/elm-review-the-elm-architecture/example --rules NoMissingSubscriptionsCall
```
-}
@ -67,8 +67,8 @@ rule =
Rule.newProjectRuleSchema "NoMissingSubscriptionsCall" initialProjectContext
|> Rule.withModuleVisitor moduleVisitor
|> Rule.withModuleContextUsingContextCreator
{ fromProjectToModule = Rule.initContextCreator fromProjectToModule |> Rule.withModuleNameLookupTable
, fromModuleToProject = Rule.initContextCreator fromModuleToProject |> Rule.withMetadata
{ fromProjectToModule = fromProjectToModule
, fromModuleToProject = fromModuleToProject
, foldProjectContexts = foldProjectContexts
}
|> Rule.withContextFromImportedModules
@ -89,15 +89,12 @@ type alias ProjectContext =
type alias ModuleContext =
{ modulesThatExposeSubscriptionsAndUpdate : Set ModuleName
--, usesUpdate : Bool
--, usesSubscription : Bool
{ lookupTable : ModuleNameLookupTable
, modulesThatExposeSubscriptionsAndUpdate : Set ModuleName
, definesUpdate : Bool
, definesSubscriptions : Bool
, usesUpdateOfModule : Dict ModuleName Range
, usesSubscriptionsOfModule : Set ModuleName
, lookupTable : ModuleNameLookupTable
}
@ -107,26 +104,34 @@ initialProjectContext =
}
fromProjectToModule : ModuleNameLookupTable -> ProjectContext -> ModuleContext
fromProjectToModule lookupTable projectContext =
{ modulesThatExposeSubscriptionsAndUpdate = projectContext.modulesThatExposeSubscriptionsAndUpdate
, definesUpdate = False
, definesSubscriptions = False
, usesUpdateOfModule = Dict.empty
, usesSubscriptionsOfModule = Set.empty
, lookupTable = lookupTable
}
fromProjectToModule : Rule.ContextCreator ProjectContext ModuleContext
fromProjectToModule =
Rule.initContextCreator
(\lookupTable projectContext ->
{ lookupTable = lookupTable
, modulesThatExposeSubscriptionsAndUpdate = projectContext.modulesThatExposeSubscriptionsAndUpdate
, definesUpdate = False
, definesSubscriptions = False
, usesUpdateOfModule = Dict.empty
, usesSubscriptionsOfModule = Set.empty
}
)
|> Rule.withModuleNameLookupTable
fromModuleToProject : Rule.Metadata -> ModuleContext -> ProjectContext
fromModuleToProject metadata moduleContext =
{ modulesThatExposeSubscriptionsAndUpdate =
if moduleContext.definesSubscriptions && moduleContext.definesUpdate then
Set.singleton (Rule.moduleNameFromMetadata metadata)
fromModuleToProject : Rule.ContextCreator ModuleContext ProjectContext
fromModuleToProject =
Rule.initContextCreator
(\metadata moduleContext ->
{ modulesThatExposeSubscriptionsAndUpdate =
if moduleContext.definesSubscriptions && moduleContext.definesUpdate then
Set.singleton (Rule.moduleNameFromMetadata metadata)
else
Set.empty
}
else
Set.empty
}
)
|> Rule.withMetadata
foldProjectContexts : ProjectContext -> ProjectContext -> ProjectContext
@ -166,9 +171,9 @@ expressionVisitor node moduleContext =
case Node.value node of
Expression.FunctionOrValue _ "update" ->
case ModuleNameLookupTable.moduleNameFor moduleContext.lookupTable node of
Just realModuleName ->
if Set.member realModuleName moduleContext.modulesThatExposeSubscriptionsAndUpdate then
( [], { moduleContext | usesUpdateOfModule = Dict.insert realModuleName (Node.range node) moduleContext.usesUpdateOfModule } )
Just moduleName ->
if Set.member moduleName moduleContext.modulesThatExposeSubscriptionsAndUpdate then
( [], { moduleContext | usesUpdateOfModule = Dict.insert moduleName (Node.range node) moduleContext.usesUpdateOfModule } )
else
( [], moduleContext )
@ -178,9 +183,9 @@ expressionVisitor node moduleContext =
Expression.FunctionOrValue _ "subscriptions" ->
case ModuleNameLookupTable.moduleNameFor moduleContext.lookupTable node of
Just realModuleName ->
if Set.member realModuleName moduleContext.modulesThatExposeSubscriptionsAndUpdate then
( [], { moduleContext | usesSubscriptionsOfModule = Set.insert realModuleName moduleContext.usesSubscriptionsOfModule } )
Just moduleName ->
if Set.member moduleName moduleContext.modulesThatExposeSubscriptionsAndUpdate then
( [], { moduleContext | usesSubscriptionsOfModule = Set.insert moduleName moduleContext.usesSubscriptionsOfModule } )
else
( [], moduleContext )

View File

@ -49,7 +49,7 @@ For that, enable [`NoMissingTypeAnnotationInLetIn`](./NoMissingTypeAnnotationInL
You can try this rule out by running the following command:
```bash
elm-review --template jfmengels/review-common/example --rules NoMissingTypeAnnotation
elm-review --template jfmengels/elm-review-common/example --rules NoMissingTypeAnnotation
```
-}

View File

@ -53,7 +53,7 @@ For that, enable [`NoMissingTypeAnnotation`](./NoMissingTypeAnnotation).
You can try this rule out by running the following command:
```bash
elm-review --template jfmengels/review-common/example --rules NoMissingTypeAnnotationInLetIn
elm-review --template jfmengels/elm-review-common/example --rules NoMissingTypeAnnotationInLetIn
```
-}

View File

@ -78,7 +78,7 @@ If a type is not exposed then it can be impossible to annotate functions or valu
You can try this rule out by running the following command:
```bash
elm-review --template jfmengels/review-common/example --rules NoMissingTypeExpose
elm-review --template jfmengels/elm-review-common/example --rules NoMissingTypeExpose
```
-}

View File

@ -887,7 +887,7 @@ packageElmJson : String
packageElmJson =
"""{
"type": "package",
"name": "jfmengels/review-common-tests",
"name": "jfmengels/elm-review-common-tests",
"summary": "A test package",
"license": "MIT",
"version": "1.0.0",

View File

@ -51,6 +51,15 @@ To add the rule to your configuration:
[ NoRecursiveUpdate.rule
]
## Try it out
You can try this rule out by running the following command:
```bash
elm-review --template jfmengels/elm-review-the-elm-architecture/example --rules NoRecursiveUpdate
```
-}
rule : Rule
rule =

View File

@ -33,6 +33,9 @@ This rule will warn arguments that are always pattern matched using a wildcard (
For package projects, custom types whose constructors are exposed as part of the package API are not reported.
Note that this rule may report false positives if you compare custom types with the `==`/`/=` operators (and never destructure the custom type), like when you do `value == Just 0`.
If you have a solution in mind to better handle these cases in a manageable way, please open an issue so we can talk about it.
## Fail
@ -57,6 +60,8 @@ For package projects, custom types whose constructors are exposed as part of the
If you like giving names to all arguments when pattern matching, then this rule will not found many problems.
This rule will work well when enabled along with [`NoUnused.Patterns`](./NoUnused-Patterns).
Also, if you like comparing custom types in the way described above, you might pass on this rule, or want to be very careful when enabling it.
## Try it out
@ -323,8 +328,7 @@ expressionVisitor node context =
let
usedArguments : List ( ( ModuleName, String ), Set Int )
usedArguments =
cases
|> List.concatMap (Tuple.first >> collectUsedCustomTypeArgs context.lookupTable)
List.concatMap (Tuple.first >> collectUsedCustomTypeArgs context.lookupTable) cases
in
( [], { context | usedArguments = registerUsedPatterns usedArguments context.usedArguments } )

View File

@ -360,6 +360,30 @@ something =
, under = "SomeData"
}
]
, test "should not report args if they are used in a different module" <|
\() ->
[ """
module Main exposing (Model, main)
import Messages exposing (Msg(..))
update : Msg -> Model -> Model
update msg model =
case msg of
Content s ->
{ model | content = "content " ++ s }
Search string ->
{ model | content = "search " ++ string }
"""
, """
module Messages exposing (Msg(..))
type Msg
= Content String
| Search String
"""
]
|> Review.Test.runOnModules rule
|> Review.Test.expectNoErrors
]

View File

@ -109,7 +109,7 @@ I would love help with improving this :)
You can try this rule out by running the following command:
```bash
elm - review --template jfmengels/review-unused/example --rules NoUnused.CustomTypeConstructors
elm-review --template jfmengels/elm-review-unused/example --rules NoUnused.CustomTypeConstructors
```
-}

View File

@ -35,7 +35,7 @@ A dependency is considered unused if none of its modules are imported in the pro
You can try this rule out by running the following command:
```bash
elm-review --template jfmengels/review-unused/example --rules NoUnused.Dependencies
elm-review --template jfmengels/elm-elm-review-unused/example --rules NoUnused.Dependencies
```
-}

View File

@ -46,7 +46,7 @@ then nothing will be reported.
You can try this rule out by running the following command:
```bash
elm - review --template jfmengels/review-unused/example --rules NoUnused.Exports
elm-review --template jfmengels/elm-review-unused/example --rules NoUnused.Exports
```
-}
@ -635,5 +635,17 @@ expressionVisitor node moduleContext =
Nothing ->
( [], moduleContext )
Expression.RecordUpdateExpression (Node range name) _ ->
case ModuleNameLookupTable.moduleNameAt moduleContext.lookupTable range of
Just moduleName ->
( []
, registerAsUsed
( moduleName, name )
moduleContext
)
Nothing ->
( [], moduleContext )
_ ->
( [], moduleContext )

View File

@ -165,6 +165,34 @@ a = 1
module B exposing (main)
import A exposing (..)
main = a
""" ]
|> Review.Test.runOnModulesWithProjectData application rule
|> Review.Test.expectNoErrors
, test "should not report an exposed value when it is used in other modules (using record update syntax, importing explicitly)" <|
\() ->
[ """
module A exposing (..)
import B exposing (person)
otherPerson =
{ person | age = 30 }
""", """
module B exposing (person)
person =
{ age = 29 }
""" ]
|> Review.Test.runOnModulesWithProjectData application rule
|> Review.Test.expectNoErrors
, test "should not report an exposed value when it is used in other modules (using record update syntax, importing all)" <|
\() ->
[ """
module A exposing (..)
import B exposing (..)
otherPerson =
{ person | age = 30 }
""", """
module B exposing (person)
person =
{ age = 29 }
""" ]
|> Review.Test.runOnModulesWithProjectData application rule
|> Review.Test.expectNoErrors

View File

@ -43,7 +43,7 @@ config =
You can try this rule out by running the following command:
```bash
elm-review --template jfmengels/review-unused/example --rules NoUnused.Modules
elm-review --template jfmengels/elm-review-unused/example --rules NoUnused.Modules
```
-}

View File

@ -55,7 +55,7 @@ Value `something` is not used:
You can try this rule out by running the following command:
```bash
elm-review --template jfmengels/review-unused/example --rules NoUnused.Parameters
elm-review --template jfmengels/elm-review-unused/example --rules NoUnused.Parameters
```
-}

View File

@ -57,7 +57,7 @@ Value `something` is not used:
You can try this rule out by running the following command:
```bash
elm-review --template jfmengels/review-unused/example --rules NoUnused.Patterns
elm-review --template jfmengels/elm-review-unused/example --rules NoUnused.Patterns
```
-}

View File

@ -56,7 +56,7 @@ import Set exposing (Set)
You can try this rule out by running the following command:
```bash
elm-review --template jfmengels/review-unused/example --rules NoUnused.Variables
elm-review --template jfmengels/elm-review-unused/example --rules NoUnused.Variables
```
-}

View File

@ -47,7 +47,7 @@ that turn out to be unnecessary later.
You can try this rule out by running the following command:
```bash
elm-review --template jfmengels/review-tea/example --rules NoUselessSubscriptions
elm-review --template jfmengels/elm-review-the-elm-architecture/example --rules NoUselessSubscriptions
```
-}