From 73262f6f47397e84ec4c0fdcd43fa90e41b5cb29 Mon Sep 17 00:00:00 2001 From: Jeroen Engels Date: Thu, 8 Dec 2022 16:14:58 +0100 Subject: [PATCH] Backport rules from packages --- tests/NoUnapprovedLicense.elm | 183 ++++++++++-------- tests/NoUnapprovedLicenseTest.elm | 52 +++-- tests/NoUnoptimizedRecursion.elm | 163 ++++++++-------- tests/NoUnused/CustomTypeConstructors.elm | 140 +++----------- tests/NoUnused/CustomTypeConstructorsTest.elm | 12 ++ 5 files changed, 258 insertions(+), 292 deletions(-) diff --git a/tests/NoUnapprovedLicense.elm b/tests/NoUnapprovedLicense.elm index 0e8de7fe..0becd5bb 100644 --- a/tests/NoUnapprovedLicense.elm +++ b/tests/NoUnapprovedLicense.elm @@ -10,6 +10,7 @@ import Dict exposing (Dict) import Elm.License import Elm.Project import Elm.Syntax.Range exposing (Range) +import Json.Encode as Encode import Review.Project.Dependency as Dependency exposing (Dependency) import Review.Rule as Rule exposing (Error, Rule) import Set exposing (Set) @@ -25,43 +26,68 @@ import Set exposing (Set) ] If the license of a dependency is in the `allowed` list, the dependency will not be reported. -If it's in the `forbidden`, the dependency will be reported as an error. +If it's in the `forbidden` list, the dependency will be reported as an error. If it's in neither, the dependency will be reported but with a different message asking you to add the license to either list. + +## Usage as an insight rule + +If instead of enforcing a restriction on the licenses, you wish to have an overview of the licenses used in your project, +you can run the rule as an insight rule (using `elm-review --report=json --extract`), which would yield an output like +the following: + +```json +{ + "NoRedInk/elm-json-decode-pipeline": "BSD-3-Clause", + "elm-explorations/markdown": "BSD-3-Clause", + "elm-explorations/test": "BSD-3-Clause", + "elm/browser": "BSD-3-Clause", + "elm/core": "BSD-3-Clause", + "elm/html": "BSD-3-Clause", + "elm/http": "BSD-3-Clause", + "elm/json": "BSD-3-Clause", + "elm/parser": "BSD-3-Clause", + "elm/random": "BSD-3-Clause", + "elm/time": "BSD-3-Clause", + "elm/url": "BSD-3-Clause", + "elm/virtual-dom": "BSD-3-Clause", + "rtfeldman/elm-iso8601-date-strings": "BSD-3-Clause" +} +``` + -} rule : { allowed : List String, forbidden : List String } -> Rule rule configuration = Rule.newProjectRuleSchema "NoUnapprovedLicense" initialProjectContext |> Rule.withElmJsonProjectVisitor elmJsonVisitor |> Rule.withDependenciesProjectVisitor dependenciesVisitor - |> Rule.withFinalProjectEvaluation (finalEvaluationForProject configuration) + |> Rule.withFinalProjectEvaluation + (finalEvaluationForProject + { allowed = Set.fromList configuration.allowed + , forbidden = Set.fromList configuration.forbidden + } + ) + |> Rule.withDataExtractor dataExtractor |> Rule.fromProjectRuleSchema -type alias Configuration = - { allowed : List String - , forbidden : List String - } - - dependenciesVisitor : Dict String Dependency -> ProjectContext -> ( List nothing, ProjectContext ) dependenciesVisitor dependencies projectContext = let licenses : Dict String String licenses = - dependencies - |> Dict.toList - |> List.filterMap - (\( packageName, dependency ) -> - case Dependency.elmJson dependency of - Elm.Project.Package { license } -> - Just ( packageName, Elm.License.toString license ) + Dict.foldl + (\packageName dependency acc -> + case Dependency.elmJson dependency of + Elm.Project.Package { license } -> + Dict.insert packageName (Elm.License.toString license) acc - Elm.Project.Application _ -> - Nothing - ) - |> Dict.fromList + Elm.Project.Application _ -> + acc + ) + Dict.empty + dependencies in ( [], { projectContext | licenses = licenses } ) @@ -101,76 +127,73 @@ initialProjectContext = -- FINAL EVALUATION -finalEvaluationForProject : Configuration -> ProjectContext -> List (Error { useErrorForModule : () }) -finalEvaluationForProject configuration projectContext = +finalEvaluationForProject : { allowed : Set String, forbidden : Set String } -> ProjectContext -> List (Error { useErrorForModule : () }) +finalEvaluationForProject { allowed, forbidden } projectContext = case projectContext.elmJsonKey of Just elmJsonKey -> - let - allowed : Set String - allowed = - Set.fromList configuration.allowed + Dict.foldl + (\name license acc -> + if Set.member license allowed then + acc - forbidden : Set String - forbidden = - Set.fromList configuration.forbidden + else if Set.member license forbidden then + Rule.errorForElmJson elmJsonKey + (\elmJson -> + { message = "Forbidden license `" ++ license ++ "` for dependency `" ++ name ++ "`" + , details = [ "This license has been marked as forbidden and you should therefore not use this package." ] + , range = findPackageNameInElmJson name elmJson + } + ) + :: acc - unknownOrForbiddenLicenses : Dict String String - unknownOrForbiddenLicenses = - projectContext.licenses - |> Dict.filter (\_ license -> not <| Set.member license allowed) - in - unknownOrForbiddenLicenses - |> Dict.toList - |> List.map - (\( name, license ) -> - if Set.member license forbidden then - Rule.errorForElmJson elmJsonKey - (\elmJson -> - { message = "Forbidden license `" ++ license ++ "` for dependency `" ++ name ++ "`" - , details = [ "This license has been marked as forbidden and you should therefore not use this package." ] - , range = findPackageNameInElmJson name elmJson - } - ) - - else - Rule.errorForElmJson elmJsonKey - (\elmJson -> - { message = "Unknown license `" ++ license ++ "` for dependency `" ++ name ++ "`" - , details = - [ "Talk to your legal team and see if this license is allowed. If it is allowed, add it to the list of allowed licenses. Otherwise, add it to the list of forbidden licenses and remove this dependency." - , "More info about licenses at https://spdx.org/licenses." - ] - , range = findPackageNameInElmJson name elmJson - } - ) - ) + else + Rule.errorForElmJson elmJsonKey + (\elmJson -> + { message = "Unknown license `" ++ license ++ "` for dependency `" ++ name ++ "`" + , details = + [ "Talk to your legal team and see if this license is allowed. If it is allowed, add it to the list of allowed licenses. Otherwise, add it to the list of forbidden licenses and remove this dependency." + , "More info about licenses at https://spdx.org/licenses." + ] + , range = findPackageNameInElmJson name elmJson + } + ) + :: acc + ) + [] + projectContext.licenses Nothing -> [] +dataExtractor : ProjectContext -> Encode.Value +dataExtractor projectContext = + Encode.dict identity Encode.string projectContext.licenses + + findPackageNameInElmJson : String -> String -> Range findPackageNameInElmJson packageName elmJson = - elmJson - |> String.lines - |> List.indexedMap Tuple.pair - |> List.filterMap - (\( row, line ) -> - case String.indexes ("\"" ++ packageName ++ "\"") line of - [] -> - Nothing + findPackageNameInElmJsonHelp packageName (String.lines elmJson) 0 - column :: _ -> - Just - { start = - { row = row + 1 - , column = column + 2 - } - , end = - { row = row + 1 - , column = column + String.length packageName + 2 - } - } - ) - |> List.head - |> Maybe.withDefault { start = { row = 1, column = 1 }, end = { row = 10000, column = 1 } } + +findPackageNameInElmJsonHelp : String -> List String -> Int -> Range +findPackageNameInElmJsonHelp packageName lines row = + case lines of + [] -> + { start = { row = 1, column = 1 }, end = { row = 10000, column = 1 } } + + line :: rest -> + case String.indexes ("\"" ++ packageName ++ "\"") line of + [] -> + findPackageNameInElmJsonHelp packageName rest (row + 1) + + column :: _ -> + { start = + { row = row + 1 + , column = column + 2 + } + , end = + { row = row + 1 + , column = column + String.length packageName + 2 + } + } diff --git a/tests/NoUnapprovedLicenseTest.elm b/tests/NoUnapprovedLicenseTest.elm index bea17888..47dd15b6 100644 --- a/tests/NoUnapprovedLicenseTest.elm +++ b/tests/NoUnapprovedLicenseTest.elm @@ -113,35 +113,53 @@ all = \() -> sourceCode |> Review.Test.run (rule { allowed = [], forbidden = [] }) - |> Review.Test.expectNoErrors + |> Review.Test.expectDataExtract """ +{ + "elm/core": "BSD-3-Clause" +}""" , test "should not report anything if all dependencies have a license that is allowed" <| \() -> sourceCode |> Review.Test.runWithProjectData (createProject "MIT") (rule { allowed = [ "MIT" ], forbidden = [] }) - |> Review.Test.expectNoErrors + |> Review.Test.expectDataExtract """ +{ + "author/dependency": "MIT" +}""" , test "should report an error if a dependency has an unknown license" <| \() -> sourceCode |> Review.Test.runWithProjectData (createProject "BSD-3-Clause") (rule { allowed = [ "MIT" ], forbidden = [] }) - |> Review.Test.expectErrorsForElmJson - [ Review.Test.error - { message = "Unknown license `BSD-3-Clause` for dependency `author/dependency`" - , details = - [ "Talk to your legal team and see if this license is allowed. If it is allowed, add it to the list of allowed licenses. Otherwise, add it to the list of forbidden licenses and remove this dependency." - , "More info about licenses at https://spdx.org/licenses." - ] - , under = "author/dependency" - } + |> Review.Test.expect + [ Review.Test.elmJsonErrors + [ Review.Test.error + { message = "Unknown license `BSD-3-Clause` for dependency `author/dependency`" + , details = + [ "Talk to your legal team and see if this license is allowed. If it is allowed, add it to the list of allowed licenses. Otherwise, add it to the list of forbidden licenses and remove this dependency." + , "More info about licenses at https://spdx.org/licenses." + ] + , under = "author/dependency" + } + ] + , Review.Test.dataExtract """ +{ + "author/dependency": "BSD-3-Clause" +}""" ] , test "should report an error if a dependency has a forbidden license" <| \() -> sourceCode |> Review.Test.runWithProjectData (createProject "BSD-3-Clause") (rule { allowed = [ "MIT" ], forbidden = [ "BSD-3-Clause" ] }) - |> Review.Test.expectErrorsForElmJson - [ Review.Test.error - { message = "Forbidden license `BSD-3-Clause` for dependency `author/dependency`" - , details = [ "This license has been marked as forbidden and you should therefore not use this package." ] - , under = "author/dependency" - } + |> Review.Test.expect + [ Review.Test.elmJsonErrors + [ Review.Test.error + { message = "Forbidden license `BSD-3-Clause` for dependency `author/dependency`" + , details = [ "This license has been marked as forbidden and you should therefore not use this package." ] + , under = "author/dependency" + } + ] + , Review.Test.dataExtract """ +{ + "author/dependency": "BSD-3-Clause" +}""" ] ] diff --git a/tests/NoUnoptimizedRecursion.elm b/tests/NoUnoptimizedRecursion.elm index 0b84b369..d0d141ab 100644 --- a/tests/NoUnoptimizedRecursion.elm +++ b/tests/NoUnoptimizedRecursion.elm @@ -66,7 +66,7 @@ This function won't be reported because it has not been tagged. -- With opt-in configuration config = - [ NoUnoptimizedRecursion.rule (NoUnoptimizedRecursion.optInWithComment "CHECK TCO") + [ NoUnoptimizedRecursion.rule (NoUnoptimizedRecursion.optInWithComment "ENSURE TCO") ] fun n = @@ -272,13 +272,13 @@ optOutWithComment comment = {-| Reports only the functions tagged with a comment. config = - [ NoUnoptimizedRecursion.rule (NoUnoptimizedRecursion.optInWithComment "CHECK TCO") + [ NoUnoptimizedRecursion.rule (NoUnoptimizedRecursion.optInWithComment "ENSURE TCO") ] With the configuration above, the following function would be reported. fun n = - -- CHECK TCO + -- ENSURE TCO if condition n then fun n * n @@ -291,19 +291,28 @@ optInWithComment comment = OptIn comment -shouldReportFunction : Configuration -> Context -> Range -> Bool -shouldReportFunction configuration context range = - let - foundComment : Bool - foundComment = - Set.member (range.start.row + 1) context.comments - in - case configuration of - OptOut _ -> - not foundComment +hasNoArguments : Expression.FunctionImplementation -> Bool +hasNoArguments declaration = + List.isEmpty declaration.arguments - OptIn _ -> - foundComment + +shouldReportFunction : Configuration -> Context -> Node Expression.FunctionImplementation -> Bool +shouldReportFunction configuration context (Node range declaration) = + if hasNoArguments declaration then + False + + else + let + foundComment : Bool + foundComment = + Set.member (range.start.row + 1) context.comments + in + case configuration of + OptOut _ -> + not foundComment + + OptIn _ -> + foundComment @@ -361,10 +370,16 @@ commentsVisitor configuration comments context = ( [] , { context | comments = - comments - |> List.filter (Node.value >> String.contains commentTag) - |> List.map (Node.range >> .start >> .row) - |> Set.fromList + List.foldl + (\(Node range value) acc -> + if String.contains commentTag value then + Set.insert range.start.row acc + + else + acc + ) + Set.empty + comments } ) @@ -375,16 +390,7 @@ declarationVisitor configuration node context = Declaration.FunctionDeclaration function -> ( [] , { currentFunctionName = - let - hasArguments : Bool - hasArguments = - function.declaration - |> Node.value - |> .arguments - |> List.isEmpty - |> not - in - if hasArguments && shouldReportFunction configuration context (Node.range function.declaration) then + if shouldReportFunction configuration context function.declaration then function.declaration |> Node.value |> .name @@ -503,53 +509,7 @@ addAllowedLocation configuration node context = } Expression.LetExpression { declarations, expression } -> - let - newScopes : List ( Range, String ) - newScopes = - List.filterMap - (\decl -> - case Node.value decl of - Expression.LetFunction function -> - let - functionDeclaration : Expression.FunctionImplementation - functionDeclaration = - Node.value function.declaration - - hasArguments : Bool - hasArguments = - function.declaration - |> Node.value - |> .arguments - |> List.isEmpty - |> not - in - Just - ( Node.range functionDeclaration.expression - , if hasArguments && shouldReportFunction configuration context (Node.range function.declaration) then - Node.value functionDeclaration.name - - else - "" - ) - - Expression.LetDestructuring _ _ -> - Nothing - ) - declarations - in - { context - | newScopesForLet = newScopes - - {- The following translates to TCO code - - let - fun x = - fun x - in - fun 1 - -} - , tcoLocations = Node.range expression :: context.tcoLocations - } + addAllowedLocationForLetExpression configuration context declarations expression Expression.ParenthesizedExpression expr -> {- The following translates to TCO code @@ -561,12 +521,12 @@ addAllowedLocation configuration node context = Expression.CaseExpression { cases } -> let - caseBodies : List Range - caseBodies = - List.map (Tuple.second >> Node.range) cases + tcoLocations : List Range + tcoLocations = + List.foldl (\( _, Node range _ ) acc -> range :: acc) context.tcoLocations cases in { context - | tcoLocations = caseBodies ++ context.tcoLocations + | tcoLocations = tcoLocations , deOptimizationRange = Just (Node.range node) , deOptimizationReason = [ "Among other possible reasons, the recursive call should not appear in the pattern to evaluate for a case expression." ] } @@ -656,6 +616,49 @@ expressionExitVisitor node context = ( [], removeDeOptimizationRangeIfNeeded node context ) +addAllowedLocationForLetExpression : Configuration -> Context -> List (Node Expression.LetDeclaration) -> Node a -> Context +addAllowedLocationForLetExpression configuration context declarations expression = + let + newScopes : List ( Range, String ) + newScopes = + List.filterMap + (\decl -> + case Node.value decl of + Expression.LetFunction function -> + let + functionDeclaration : Expression.FunctionImplementation + functionDeclaration = + Node.value function.declaration + in + Just + ( Node.range functionDeclaration.expression + , if shouldReportFunction configuration context function.declaration then + Node.value functionDeclaration.name + + else + "" + ) + + Expression.LetDestructuring _ _ -> + Nothing + ) + declarations + in + { context + | newScopesForLet = newScopes + + {- The following translates to TCO code + + let + fun x = + fun x + in + fun 1 + -} + , tcoLocations = Node.range expression :: context.tcoLocations + } + + removeDeOptimizationRangeIfNeeded : Node Expression -> Context -> Context removeDeOptimizationRangeIfNeeded node context = if Just (Node.range node) == context.deOptimizationRange then diff --git a/tests/NoUnused/CustomTypeConstructors.elm b/tests/NoUnused/CustomTypeConstructors.elm index d3b066b3..1805d66d 100644 --- a/tests/NoUnused/CustomTypeConstructors.elm +++ b/tests/NoUnused/CustomTypeConstructors.elm @@ -149,7 +149,6 @@ moduleVisitor schema = |> Rule.withExpressionEnterVisitor (\node context -> ( [], expressionVisitor node context )) |> Rule.withCaseBranchEnterVisitor (\caseBlock casePattern context -> ( [], caseBranchEnterVisitor caseBlock casePattern context )) |> Rule.withCaseBranchExitVisitor (\caseBlock casePattern context -> ( [], caseBranchExitVisitor caseBlock casePattern context )) - |> Rule.withFinalModuleEvaluation finalModuleEvaluation @@ -197,7 +196,6 @@ type alias ProjectContext = type alias ModuleContext = { lookupTable : ModuleNameLookupTable , exposedCustomTypesWithConstructors : Set CustomTypeName - , exposedCustomTypesWithoutConstructors : Set CustomTypeName , isExposed : Bool , exposesEverything : Bool , exposedConstructors : Dict ModuleNameAsString ExposedConstructors @@ -238,7 +236,6 @@ fromProjectToModule = (\lookupTable moduleName projectContext -> { lookupTable = lookupTable , exposedCustomTypesWithConstructors = Set.empty - , exposedCustomTypesWithoutConstructors = Set.empty , isExposed = Set.member (String.join "." moduleName) projectContext.exposedModules , exposedConstructors = projectContext.declaredConstructors , exposesEverything = False @@ -291,11 +288,7 @@ fromModuleToProject = { moduleKey = moduleKey , customTypes = moduleContext.declaredTypesWithConstructors - |> Dict.filter - (\typeName _ -> - not (Set.member typeName moduleContext.exposedCustomTypesWithConstructors) - && not (Set.member typeName moduleContext.exposedCustomTypesWithoutConstructors) - ) + |> Dict.filter (\typeName _ -> not <| Set.member typeName moduleContext.exposedCustomTypesWithConstructors) } ) @@ -304,13 +297,7 @@ fromModuleToProject = moduleNameAsString (ExposedConstructors { moduleKey = moduleKey - , customTypes = - moduleContext.declaredTypesWithConstructors - |> Dict.filter - (\typeName _ -> - Set.member typeName moduleContext.exposedCustomTypesWithConstructors - || Set.member typeName moduleContext.exposedCustomTypesWithoutConstructors - ) + , customTypes = moduleContext.declaredTypesWithConstructors } ) , usedConstructors = @@ -470,26 +457,21 @@ moduleDefinitionVisitor moduleNode context = Exposing.Explicit list -> let - ( exposedCustomTypesWithConstructors, exposedCustomTypesWithoutConstructors ) = + exposedCustomTypesWithConstructors : Set String + exposedCustomTypesWithConstructors = List.foldl - (\node (( withConstructors, withoutConstructors ) as acc) -> + (\node acc -> case Node.value node of Exposing.TypeExpose { name } -> - ( Set.insert name withConstructors, withoutConstructors ) - - Exposing.TypeOrAliasExpose name -> - ( withConstructors, Set.insert name withoutConstructors ) + Set.insert name acc _ -> acc ) - ( context.exposedCustomTypesWithConstructors, context.exposedCustomTypesWithoutConstructors ) + context.exposedCustomTypesWithConstructors list in - { context - | exposedCustomTypesWithConstructors = exposedCustomTypesWithConstructors - , exposedCustomTypesWithoutConstructors = exposedCustomTypesWithoutConstructors - } + { context | exposedCustomTypesWithConstructors = exposedCustomTypesWithConstructors } @@ -1090,44 +1072,6 @@ isCapitalized name = --- FINAL MODULE EVALUATION - - -finalModuleEvaluation : ModuleContext -> List (Error {}) -finalModuleEvaluation moduleContext = - let - customTypes : Dict CustomTypeName (Dict ConstructorName ConstructorInformation) - customTypes = - if moduleContext.isExposed then - moduleContext.declaredTypesWithConstructors - |> Dict.filter - (\typeName _ -> - Set.member typeName moduleContext.exposedCustomTypesWithoutConstructors - ) - - else - moduleContext.declaredTypesWithConstructors - |> Dict.filter - (\typeName _ -> - not (Set.member typeName moduleContext.exposedCustomTypesWithConstructors) - && not (Set.member typeName moduleContext.exposedCustomTypesWithoutConstructors) - ) - - getFixes : ConstructorName -> List Fix - getFixes name = - Dict.get name moduleContext.fixesForRemovingConstructor |> Maybe.withDefault [] - in - errorsForCustomTypes - moduleContext - Rule.errorWithFix - getFixes - (Dict.get "" moduleContext.usedFunctionsOrValues |> Maybe.withDefault Set.empty) - "" - customTypes - [] - - - -- FINAL PROJECT EVALUATION @@ -1136,76 +1080,41 @@ finalProjectEvaluation projectContext = Dict.foldl (\moduleName (ExposedConstructors { moduleKey, customTypes }) acc -> let - getFixes : ConstructorName -> List Fix - getFixes name = - Dict.get ( moduleName, name ) projectContext.fixesForRemovingConstructor |> Maybe.withDefault [] - usedConstructors : Set ConstructorName usedConstructors = Dict.get moduleName projectContext.usedConstructors |> Maybe.withDefault Set.empty in - errorsForCustomTypes - projectContext - (Rule.errorForModuleWithFix moduleKey) - getFixes - usedConstructors - moduleName - customTypes - acc + errorsForCustomTypes projectContext usedConstructors moduleName moduleKey customTypes acc ) [] projectContext.declaredConstructors -type alias DataForReportingConstructors a = - { a - | wasUsedInLocationThatNeedsItself : Set ( ModuleNameAsString, String ) - , wasUsedInComparisons : Set ( ModuleNameAsString, String ) - , wasUsedInOtherModules : Set ( ModuleNameAsString, String ) - } - - -errorsForCustomTypes : - DataForReportingConstructors a - -> ({ message : String, details : List String } -> Range -> List Fix -> Error scope) - -> (ConstructorName -> List Fix) - -> Set String - -> String - -> Dict CustomTypeName (Dict ConstructorName ConstructorInformation) - -> List (Error scope) - -> List (Error scope) -errorsForCustomTypes projectContext createError getFixes usedConstructors moduleName customTypes acc = +errorsForCustomTypes : ProjectContext -> Set String -> String -> Rule.ModuleKey -> Dict CustomTypeName (Dict ConstructorName ConstructorInformation) -> List (Error scope) -> List (Error scope) +errorsForCustomTypes projectContext usedConstructors moduleName moduleKey customTypes acc = Dict.foldl (\_ constructors subAcc -> - errorsForConstructors projectContext createError getFixes usedConstructors moduleName constructors subAcc + errorsForConstructors projectContext usedConstructors moduleName moduleKey constructors subAcc ) acc customTypes -errorsForConstructors : - DataForReportingConstructors a - -> ({ message : String, details : List String } -> Range -> List Fix -> Error scope) - -> (ConstructorName -> List Fix) - -> Set String - -> String - -> Dict ConstructorName ConstructorInformation - -> List (Error scope) - -> List (Error scope) -errorsForConstructors projectContext createError getFixes usedConstructors moduleName constructors acc = +errorsForConstructors : ProjectContext -> Set String -> String -> Rule.ModuleKey -> Dict ConstructorName ConstructorInformation -> List (Error scope) -> List (Error scope) +errorsForConstructors projectContext usedConstructors moduleName moduleKey constructors acc = Dict.foldl (\constructorName constructorInformation subAcc -> if Set.member constructorName usedConstructors then subAcc else - reportError - createError - { wasUsedInLocationThatNeedsItself = Set.member ( moduleName, constructorName ) projectContext.wasUsedInLocationThatNeedsItself - , wasUsedInComparisons = Set.member ( moduleName, constructorName ) projectContext.wasUsedInComparisons - , isUsedInOtherModules = Set.member ( moduleName, constructorName ) projectContext.wasUsedInOtherModules - , fixesForRemovingConstructor = getFixes constructorName + errorForModule + moduleKey + { wasUsedInLocationThatNeedsItself = Set.member ( moduleName, constructorInformation.name ) projectContext.wasUsedInLocationThatNeedsItself + , wasUsedInComparisons = Set.member ( moduleName, constructorInformation.name ) projectContext.wasUsedInComparisons + , isUsedInOtherModules = Set.member ( moduleName, constructorInformation.name ) projectContext.wasUsedInOtherModules + , fixesForRemovingConstructor = Dict.get ( moduleName, constructorInformation.name ) projectContext.fixesForRemovingConstructor |> Maybe.withDefault [] } constructorInformation :: subAcc @@ -1236,8 +1145,8 @@ defaultDetails = "This type constructor is never used. It might be handled everywhere it might appear, but there is no location where this value actually gets created." -reportError : - ({ message : String, details : List String } -> Range -> List Fix -> Error scope) +errorForModule : + Rule.ModuleKey -> { wasUsedInLocationThatNeedsItself : Bool , wasUsedInComparisons : Bool @@ -1246,8 +1155,9 @@ reportError : } -> ConstructorInformation -> Error scope -reportError createError params constructorInformation = - createError +errorForModule moduleKey params constructorInformation = + Rule.errorForModuleWithFix + moduleKey (errorInformation { wasUsedInLocationThatNeedsItself = params.wasUsedInLocationThatNeedsItself , wasUsedInComparisons = params.wasUsedInComparisons diff --git a/tests/NoUnused/CustomTypeConstructorsTest.elm b/tests/NoUnused/CustomTypeConstructorsTest.elm index 11165c74..571d45b3 100644 --- a/tests/NoUnused/CustomTypeConstructorsTest.elm +++ b/tests/NoUnused/CustomTypeConstructorsTest.elm @@ -924,6 +924,18 @@ module MyModule exposing (..) type Foo = Bar """ ] + , test "should not report used type constructors when an application module is exposing all but it's used elsewhere" <| + \() -> + [ """ +module MyModule exposing (..) +type Foo = Bar | Baz +""", """ +module Main exposing (main) +import MyModule +main = [MyModule.Bar, MyModule.Baz] +""" ] + |> Review.Test.runOnModulesWithProjectData applicationProject (rule []) + |> Review.Test.expectNoErrors , test "should not report unused type constructors when package module is exposing the constructors of that type and module is exposed" <| \() -> """