Report missing core modules in NoContractViolations review check.

This commit is contained in:
Dillon Kearns 2022-03-24 15:18:28 -07:00
parent 58ba422c2a
commit a6870989a1
2 changed files with 207 additions and 82 deletions

View File

@ -52,13 +52,54 @@ elm-review --template dillonkearns/elm-review-elm-pages/example --rules Pages.Re
-} -}
rule : Rule rule : Rule
rule = rule =
Rule.newModuleRuleSchema "Pages.Review.NoContractViolations" Rule.newProjectRuleSchema "Pages.Review.NoContractViolations"
{ moduleName = [] { visitedCoreModules = Set.empty
, isRouteModule = False
} }
|> Rule.withModuleDefinitionVisitor moduleDefinitionVisitor |> Rule.withModuleVisitor
|> Rule.withDeclarationVisitor declarationVisitor (Rule.withModuleDefinitionVisitor moduleDefinitionVisitor
|> Rule.fromModuleRuleSchema >> Rule.withDeclarationVisitor declarationVisitor
)
|> Rule.withModuleContext
{ foldProjectContexts = \a b -> { visitedCoreModules = Set.union a.visitedCoreModules b.visitedCoreModules }
, fromModuleToProject = \moduleKey moduleName moduleContext -> { visitedCoreModules = Set.singleton (Node.value moduleName) }
, fromProjectToModule =
\moduleKey moduleName projectContext ->
{ moduleName = Node.value moduleName
, isRouteModule = (Node.value moduleName |> List.take 1) == [ "Route" ] && ((Node.value moduleName |> List.length) > 1)
}
}
|> Rule.withFinalProjectEvaluation
(\context ->
let
missingCoreModules =
context.visitedCoreModules
|> Set.diff coreModules
in
if missingCoreModules |> Set.isEmpty then
[]
else
[ Rule.globalError
{ message = "Missing core modules"
, details =
missingCoreModules
|> Set.toList
|> List.map (String.join ".")
}
]
)
|> Rule.fromProjectRuleSchema
coreModules : Set (List String)
coreModules =
Set.fromList
[ [ "Api" ]
, [ "Effect" ]
, [ "Shared" ]
, [ "Site" ]
, [ "View" ]
]
type alias Context = type alias Context =
@ -67,32 +108,22 @@ type alias Context =
} }
type alias ProjectContext =
{ visitedCoreModules : Set (List String)
}
moduleDefinitionVisitor : Node Module -> Context -> ( List (Error {}), Context ) moduleDefinitionVisitor : Node Module -> Context -> ( List (Error {}), Context )
moduleDefinitionVisitor node _ = moduleDefinitionVisitor node context =
let
isRouteModule : Bool
isRouteModule =
(Node.value node |> Module.moduleName |> List.take 1)
== [ "Route" ]
&& ((Node.value node |> Module.moduleName |> List.length) > 1)
in
case Node.value node |> Module.exposingList of case Node.value node |> Module.exposingList of
Exposing.All _ -> Exposing.All _ ->
( [] ( [], context )
, { moduleName = Node.value node |> Module.moduleName
, isRouteModule = isRouteModule
}
)
Exposing.Explicit exposedValues -> Exposing.Explicit exposedValues ->
if isRouteModule then if context.isRouteModule then
case Set.diff (Set.fromList [ "Data", "Msg", "Model", "route" ]) (exposedNames exposedValues) |> Set.toList of case Set.diff (Set.fromList [ "Data", "Msg", "Model", "route" ]) (exposedNames exposedValues) |> Set.toList of
[] -> [] ->
( [] ( [], context )
, { moduleName = Node.value node |> Module.moduleName
, isRouteModule = isRouteModule
}
)
nonEmpty -> nonEmpty ->
( [ Rule.error ( [ Rule.error
@ -111,17 +142,11 @@ But it is not exposing: """
} }
(Node.range (exposingListNode (Node.value node))) (Node.range (exposingListNode (Node.value node)))
] ]
, { moduleName = Node.value node |> Module.moduleName , context
, isRouteModule = isRouteModule
}
) )
else else
( [] ( [], context )
, { moduleName = Node.value node |> Module.moduleName
, isRouteModule = isRouteModule
}
)
routeParamsMatchesNameOrError : Node TypeAnnotation -> List String -> List (Error {}) routeParamsMatchesNameOrError : Node TypeAnnotation -> List String -> List (Error {})

View File

@ -14,12 +14,13 @@ all =
a = 1 a = 1
""" """
|> Review.Test.run rule |> testRouteModule
|> Review.Test.expectErrors |> Review.Test.expectErrorsForModules
[ Review.Test.error [ ( "Route.Blog.Slug_"
{ message = "Unexposed Declaration in Route Module" , [ Review.Test.error
, details = { message = "Unexposed Declaration in Route Module"
[ """Route Modules need to expose the following values: , details =
[ """Route Modules need to expose the following values:
- route - route
- Data - Data
@ -27,9 +28,11 @@ a = 1
- Msg - Msg
But it is not exposing: Model""" But it is not exposing: Model"""
] ]
, under = "exposing (Data, Msg, route)" , under = "exposing (Data, Msg, route)"
} }
]
)
] ]
, test "reports RouteParams mismatch" <| , test "reports RouteParams mismatch" <|
\() -> \() ->
@ -39,18 +42,21 @@ type alias RouteParams = { blogPostName : String }
route = {} route = {}
""" """
|> Review.Test.run rule |> testRouteModule
|> Review.Test.expectErrors |> Review.Test.expectErrorsForModules
[ Review.Test.error [ ( "Route.Blog.Slug_"
{ message = "RouteParams don't match Route Module name" , [ Review.Test.error
, details = { message = "RouteParams don't match Route Module name"
[ """Expected , details =
[ """Expected
type alias RouteParams = { slug : String } type alias RouteParams = { slug : String }
""" """
] ]
, under = "{ blogPostName : String }" , under = "{ blogPostName : String }"
} }
]
)
] ]
, test "reports incorrect types for optional RouteParams" <| , test "reports incorrect types for optional RouteParams" <|
\() -> \() ->
@ -60,18 +66,21 @@ type alias RouteParams = { section : String, subSection : String }
route = {} route = {}
""" """
|> Review.Test.run rule |> testRouteModule
|> Review.Test.expectErrors |> Review.Test.expectErrorsForModules
[ Review.Test.error [ ( "Route.Docs.Section_.SubSection__"
{ message = "RouteParams don't match Route Module name" , [ Review.Test.error
, details = { message = "RouteParams don't match Route Module name"
[ """Expected , details =
[ """Expected
type alias RouteParams = { section : String, subSection : Maybe String } type alias RouteParams = { section : String, subSection : Maybe String }
""" """
] ]
, under = "{ section : String, subSection : String }" , under = "{ section : String, subSection : String }"
} }
]
)
] ]
, test "reports incorrect types for required splat RouteParams" <| , test "reports incorrect types for required splat RouteParams" <|
\() -> \() ->
@ -81,18 +90,21 @@ type alias RouteParams = { section : String, splat : List String }
route = {} route = {}
""" """
|> Review.Test.run rule |> testRouteModule
|> Review.Test.expectErrors |> Review.Test.expectErrorsForModules
[ Review.Test.error [ ( "Route.Docs.Section_.SPLAT_"
{ message = "RouteParams don't match Route Module name" , [ Review.Test.error
, details = { message = "RouteParams don't match Route Module name"
[ """Expected , details =
[ """Expected
type alias RouteParams = { section : String, splat : ( String, List String ) } type alias RouteParams = { section : String, splat : ( String, List String ) }
""" """
] ]
, under = "{ section : String, splat : List String }" , under = "{ section : String, splat : List String }"
} }
]
)
] ]
, test "no error for valid SPLAT_ RouteParams" <| , test "no error for valid SPLAT_ RouteParams" <|
\() -> \() ->
@ -102,7 +114,7 @@ type alias RouteParams = { section : String, splat : ( String, List String ) }
route = {} route = {}
""" """
|> Review.Test.run rule |> testRouteModule
|> Review.Test.expectNoErrors |> Review.Test.expectNoErrors
, test "no error for valid SPLAT__ RouteParams" <| , test "no error for valid SPLAT__ RouteParams" <|
\() -> \() ->
@ -112,7 +124,7 @@ type alias RouteParams = { section : String, splat : List String }
route = {} route = {}
""" """
|> Review.Test.run rule |> testRouteModule
|> Review.Test.expectNoErrors |> Review.Test.expectNoErrors
, test "no error for matching RouteParams name" <| , test "no error for matching RouteParams name" <|
\() -> \() ->
@ -122,7 +134,7 @@ type alias RouteParams = { slug : String }
route = {} route = {}
""" """
|> Review.Test.run rule |> testRouteModule
|> Review.Test.expectNoErrors |> Review.Test.expectNoErrors
, test "error when RouteParams type is not a record" <| , test "error when RouteParams type is not a record" <|
\() -> \() ->
@ -132,15 +144,18 @@ type alias RouteParams = ()
route = {} route = {}
""" """
|> Review.Test.run rule |> testRouteModule
|> Review.Test.expectErrors |> Review.Test.expectErrorsForModules
[ Review.Test.error [ ( "Route.Blog.Slug_"
{ message = "RouteParams must be a record type alias." , [ Review.Test.error
, details = { message = "RouteParams must be a record type alias."
[ """Expected a record type alias.""" , details =
] [ """Expected a record type alias."""
, under = "()" ]
} , under = "()"
}
]
)
] ]
, test "no error for modules that don't start with Route prefix" <| , test "no error for modules that don't start with Route prefix" <|
\() -> \() ->
@ -150,6 +165,91 @@ type alias RouteParams = ()
route = {} route = {}
""" """
|> Review.Test.run rule |> testRouteModule
|> Review.Test.expectNoErrors
, test "error for missing application module definitions" <|
\() ->
[ """module Route.Index exposing (Data, route, Model, Msg)
type alias RouteParams = {}
route = {}
"""
, """module Site exposing (config)
config : SiteConfig
config =
{ canonicalUrl = canonicalUrl
, head = head
}
"""
]
|> Review.Test.runOnModules rule
|> Review.Test.expectGlobalErrors
[ { message = "Missing core modules"
, details =
[ "Api"
, "Effect"
, "Shared"
, "View"
]
}
]
, test "no error when all core modules are defined" <|
\() ->
[ """module Route.Index exposing (Data, route, Model, Msg)
type alias RouteParams = {}
route = {}
"""
, """module Site exposing (config)
config : SiteConfig
config =
{ canonicalUrl = canonicalUrl
, head = head
}
"""
, """module Api exposing (routes)
routes = Debug.todo ""
"""
, """module Effect exposing (routes)
routes = Debug.todo ""
"""
, """module Shared exposing (routes)
routes = Debug.todo ""
"""
, """module View exposing (routes)
routes = Debug.todo ""
"""
]
|> Review.Test.runOnModules rule
|> Review.Test.expectNoErrors |> Review.Test.expectNoErrors
] ]
testRouteModule : String -> Review.Test.ReviewResult
testRouteModule routeModule =
Review.Test.runOnModules rule
(routeModule :: validCoreModules)
validCoreModules : List String
validCoreModules =
[ """module Api exposing (routes)
routes = Debug.todo ""
"""
, """module Effect exposing (routes)
routes = Debug.todo ""
"""
, """module Shared exposing (routes)
routes = Debug.todo ""
"""
, """module Site exposing (routes)
routes = Debug.todo ""
"""
, """module View exposing (routes)
routes = Debug.todo ""
"""
]