From b1b96c6dcf2654f02f864de485e215098ccb0790 Mon Sep 17 00:00:00 2001 From: Jeroen Engels Date: Tue, 22 Sep 2020 19:40:30 +0200 Subject: [PATCH] Backport rules from other projects --- .../ReadmeLinksPointToCurrentVersion.elm | 2 +- tests/NoBooleanCaseOf.elm | 112 ++++++++++++++++++ tests/NoBooleanCaseOfTest.elm | 111 +++++++++++++++++ tests/NoDebug/Log.elm | 2 +- tests/NoDebug/TodoOrToString.elm | 82 ++++++------- tests/NoExposingEverything.elm | 2 +- tests/NoImportingEverything.elm | 2 +- tests/NoMissingSubscriptionsCall.elm | 67 ++++++----- tests/NoMissingTypeAnnotation.elm | 2 +- tests/NoMissingTypeAnnotationInLetIn.elm | 2 +- tests/NoMissingTypeExpose.elm | 2 +- tests/NoMissingTypeExposeTest.elm | 2 +- tests/NoRecursiveUpdate.elm | 9 ++ tests/NoUnused/CustomTypeConstructorArgs.elm | 8 +- .../CustomTypeConstructorArgsTest.elm | 24 ++++ tests/NoUnused/CustomTypeConstructors.elm | 2 +- tests/NoUnused/Dependencies.elm | 2 +- tests/NoUnused/Exports.elm | 14 ++- tests/NoUnused/ExportsTest.elm | 28 +++++ tests/NoUnused/Modules.elm | 2 +- tests/NoUnused/Parameters.elm | 2 +- tests/NoUnused/Patterns.elm | 2 +- tests/NoUnused/Variables.elm | 2 +- tests/NoUselessSubscriptions.elm | 2 +- 24 files changed, 390 insertions(+), 95 deletions(-) create mode 100644 tests/NoBooleanCaseOf.elm create mode 100644 tests/NoBooleanCaseOfTest.elm diff --git a/tests/Documentation/ReadmeLinksPointToCurrentVersion.elm b/tests/Documentation/ReadmeLinksPointToCurrentVersion.elm index f41331eb..bc403cbc 100644 --- a/tests/Documentation/ReadmeLinksPointToCurrentVersion.elm +++ b/tests/Documentation/ReadmeLinksPointToCurrentVersion.elm @@ -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 ``` -} diff --git a/tests/NoBooleanCaseOf.elm b/tests/NoBooleanCaseOf.elm new file mode 100644 index 00000000..fc1a8a2c --- /dev/null +++ b/tests/NoBooleanCaseOf.elm @@ -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: + + 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 diff --git a/tests/NoBooleanCaseOfTest.elm b/tests/NoBooleanCaseOfTest.elm new file mode 100644 index 00000000..bb72e029 --- /dev/null +++ b/tests/NoBooleanCaseOfTest.elm @@ -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 diff --git a/tests/NoDebug/Log.elm b/tests/NoDebug/Log.elm index 6250633e..9794b43c 100644 --- a/tests/NoDebug/Log.elm +++ b/tests/NoDebug/Log.elm @@ -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 ``` -} diff --git a/tests/NoDebug/TodoOrToString.elm b/tests/NoDebug/TodoOrToString.elm index a808a078..9932ef8a 100644 --- a/tests/NoDebug/TodoOrToString.elm +++ b/tests/NoDebug/TodoOrToString.elm @@ -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 ) diff --git a/tests/NoExposingEverything.elm b/tests/NoExposingEverything.elm index 3f1caea4..41b2a6e5 100644 --- a/tests/NoExposingEverything.elm +++ b/tests/NoExposingEverything.elm @@ -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 ``` -} diff --git a/tests/NoImportingEverything.elm b/tests/NoImportingEverything.elm index 02716774..9f0cb254 100644 --- a/tests/NoImportingEverything.elm +++ b/tests/NoImportingEverything.elm @@ -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 ``` -} diff --git a/tests/NoMissingSubscriptionsCall.elm b/tests/NoMissingSubscriptionsCall.elm index 626ced78..ae8925a5 100644 --- a/tests/NoMissingSubscriptionsCall.elm +++ b/tests/NoMissingSubscriptionsCall.elm @@ -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 ) diff --git a/tests/NoMissingTypeAnnotation.elm b/tests/NoMissingTypeAnnotation.elm index 1f8d81cd..cfa41463 100644 --- a/tests/NoMissingTypeAnnotation.elm +++ b/tests/NoMissingTypeAnnotation.elm @@ -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 ``` -} diff --git a/tests/NoMissingTypeAnnotationInLetIn.elm b/tests/NoMissingTypeAnnotationInLetIn.elm index 79dd3b32..dd6571dd 100644 --- a/tests/NoMissingTypeAnnotationInLetIn.elm +++ b/tests/NoMissingTypeAnnotationInLetIn.elm @@ -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 ``` -} diff --git a/tests/NoMissingTypeExpose.elm b/tests/NoMissingTypeExpose.elm index f3d289e4..ba9f8092 100644 --- a/tests/NoMissingTypeExpose.elm +++ b/tests/NoMissingTypeExpose.elm @@ -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 ``` -} diff --git a/tests/NoMissingTypeExposeTest.elm b/tests/NoMissingTypeExposeTest.elm index f6cde048..2f6f21a0 100644 --- a/tests/NoMissingTypeExposeTest.elm +++ b/tests/NoMissingTypeExposeTest.elm @@ -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", diff --git a/tests/NoRecursiveUpdate.elm b/tests/NoRecursiveUpdate.elm index d17bb00b..a617bb10 100644 --- a/tests/NoRecursiveUpdate.elm +++ b/tests/NoRecursiveUpdate.elm @@ -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 = diff --git a/tests/NoUnused/CustomTypeConstructorArgs.elm b/tests/NoUnused/CustomTypeConstructorArgs.elm index 6188274a..5820d99d 100644 --- a/tests/NoUnused/CustomTypeConstructorArgs.elm +++ b/tests/NoUnused/CustomTypeConstructorArgs.elm @@ -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 } ) diff --git a/tests/NoUnused/CustomTypeConstructorArgsTest.elm b/tests/NoUnused/CustomTypeConstructorArgsTest.elm index d0ce932e..815c7b99 100644 --- a/tests/NoUnused/CustomTypeConstructorArgsTest.elm +++ b/tests/NoUnused/CustomTypeConstructorArgsTest.elm @@ -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 ] diff --git a/tests/NoUnused/CustomTypeConstructors.elm b/tests/NoUnused/CustomTypeConstructors.elm index 2280a3c0..3f11327c 100644 --- a/tests/NoUnused/CustomTypeConstructors.elm +++ b/tests/NoUnused/CustomTypeConstructors.elm @@ -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 ``` -} diff --git a/tests/NoUnused/Dependencies.elm b/tests/NoUnused/Dependencies.elm index 615b774e..bada6bd3 100644 --- a/tests/NoUnused/Dependencies.elm +++ b/tests/NoUnused/Dependencies.elm @@ -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 ``` -} diff --git a/tests/NoUnused/Exports.elm b/tests/NoUnused/Exports.elm index 88eb72c3..b69c54ed 100644 --- a/tests/NoUnused/Exports.elm +++ b/tests/NoUnused/Exports.elm @@ -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 ) diff --git a/tests/NoUnused/ExportsTest.elm b/tests/NoUnused/ExportsTest.elm index bf37f456..59b01886 100644 --- a/tests/NoUnused/ExportsTest.elm +++ b/tests/NoUnused/ExportsTest.elm @@ -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 diff --git a/tests/NoUnused/Modules.elm b/tests/NoUnused/Modules.elm index 3fabc9ea..5a35806d 100644 --- a/tests/NoUnused/Modules.elm +++ b/tests/NoUnused/Modules.elm @@ -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 ``` -} diff --git a/tests/NoUnused/Parameters.elm b/tests/NoUnused/Parameters.elm index 8fe10dfb..8432f58b 100644 --- a/tests/NoUnused/Parameters.elm +++ b/tests/NoUnused/Parameters.elm @@ -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 ``` -} diff --git a/tests/NoUnused/Patterns.elm b/tests/NoUnused/Patterns.elm index 6b865205..3be89b38 100644 --- a/tests/NoUnused/Patterns.elm +++ b/tests/NoUnused/Patterns.elm @@ -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 ``` -} diff --git a/tests/NoUnused/Variables.elm b/tests/NoUnused/Variables.elm index 421c59bc..91ce7b67 100644 --- a/tests/NoUnused/Variables.elm +++ b/tests/NoUnused/Variables.elm @@ -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 ``` -} diff --git a/tests/NoUselessSubscriptions.elm b/tests/NoUselessSubscriptions.elm index f1b9a499..ce8a3228 100644 --- a/tests/NoUselessSubscriptions.elm +++ b/tests/NoUselessSubscriptions.elm @@ -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 ``` -}