Backport rules from elm-review-common

This commit is contained in:
Jeroen Engels 2020-12-25 17:23:07 +01:00
parent 84c7828604
commit fa8bc02068
2 changed files with 175 additions and 19 deletions

View File

@ -6,9 +6,13 @@ module NoExposingEverything exposing (rule)
-}
import Elm.Syntax.Declaration as Declaration exposing (Declaration)
import Elm.Syntax.Exposing as Exposing
import Elm.Syntax.Expression as Expression
import Elm.Syntax.Module as Module exposing (Module)
import Elm.Syntax.Node as Node exposing (Node)
import Elm.Syntax.Node as Node exposing (Node(..))
import Elm.Syntax.Range exposing (Range)
import Review.Fix as Fix
import Review.Rule as Rule exposing (Error, Rule)
@ -52,25 +56,79 @@ elm-review --template jfmengels/elm-review-common/example --rules NoExposingEver
-}
rule : Rule
rule =
Rule.newModuleRuleSchema "NoExposingEverything" ()
|> Rule.withSimpleModuleDefinitionVisitor moduleDefinitionVisitor
Rule.newModuleRuleSchema "NoExposingEverything" ExposingOk
|> Rule.withModuleDefinitionVisitor moduleDefinitionVisitor
|> Rule.withDeclarationListVisitor declarationListVisitor
|> Rule.fromModuleRuleSchema
moduleDefinitionVisitor : Node Module -> List (Error {})
moduleDefinitionVisitor moduleNode =
type ExposingContext
= ExposingOk
| ExposingAll Range
moduleDefinitionVisitor : Node Module -> ExposingContext -> ( List (Error {}), ExposingContext )
moduleDefinitionVisitor moduleNode _ =
case Module.exposingList <| Node.value moduleNode of
Exposing.All range ->
[ Rule.error
{ message = "Module exposes everything implicitly \"(..)\""
, details =
[ "Modules should have hidden implementation details with an explicit API so that the module is used in a proper and controlled way. The users of this module should not have to know about what is inside a module it is using, and they shouldn't need to access it's internal details. Therefore, the API should be explicitly defined and ideally as small as possible."
]
}
{ start = { row = range.start.row, column = range.start.column - 1 }
, end = { row = range.end.row, column = range.end.column + 1 }
}
]
( [], ExposingAll range )
Exposing.Explicit _ ->
[]
( [], ExposingOk )
declarationListVisitor : List (Node Declaration) -> ExposingContext -> ( List (Error {}), ExposingContext )
declarationListVisitor declarations context =
case context of
ExposingAll range ->
( [ Rule.errorWithFix
{ message = "Module exposes everything implicitly \"(..)\""
, details =
[ "Modules should have hidden implementation details with an explicit API so that the module is used in a proper and controlled way. The users of this module should not have to know about what is inside a module it is using, and they shouldn't need to access it's internal details. Therefore, the API should be explicitly defined and ideally as small as possible."
]
}
{ start = { row = range.start.row, column = range.start.column - 1 }
, end = { row = range.end.row, column = range.end.column + 1 }
}
[ exposingDeclarationList declarations
|> String.join ", "
|> Fix.replaceRangeBy range
]
]
, context
)
_ ->
( [], context )
exposingDeclarationList : List (Node Declaration) -> List String
exposingDeclarationList declarations =
List.map exposingDeclarationName declarations
exposingDeclarationName : Node Declaration -> String
exposingDeclarationName (Node _ declaration) =
case declaration of
Declaration.AliasDeclaration { name } ->
Node.value name
Declaration.CustomTypeDeclaration { name } ->
Node.value name ++ "(..)"
Declaration.FunctionDeclaration function ->
functionDeclarationName function
Declaration.InfixDeclaration { operator } ->
"(" ++ Node.value operator ++ ")"
Declaration.PortDeclaration { name } ->
Node.value name
Declaration.Destructuring _ _ ->
""
functionDeclarationName : Expression.Function -> String
functionDeclarationName { declaration } =
declaration |> Node.value |> .name |> Node.value

View File

@ -17,11 +17,12 @@ d = 1
"""
|> Review.Test.run rule
|> Review.Test.expectNoErrors
, test "should report when a module exposes everything" <|
, test "should offer a fix listing all variables" <|
\() ->
"""
module A exposing (..)
import B exposing (..)
foo = 1
bar = 2
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
@ -32,6 +33,103 @@ import B exposing (..)
]
, under = "(..)"
}
|> Review.Test.atExactly { start = { row = 2, column = 19 }, end = { row = 2, column = 23 } }
|> Review.Test.whenFixed
"""
module A exposing (foo, bar)
foo = 1
bar = 2
"""
]
, test "should offer a fix listing all types" <|
\() ->
"""
module A exposing (..)
type Foo = Foo
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Module exposes everything implicitly \"(..)\""
, details =
[ "Modules should have hidden implementation details with an explicit API so that the module is used in a proper and controlled way. The users of this module should not have to know about what is inside a module it is using, and they shouldn't need to access it's internal details. Therefore, the API should be explicitly defined and ideally as small as possible."
]
, under = "(..)"
}
|> Review.Test.whenFixed
"""
module A exposing (Foo(..))
type Foo = Foo
"""
]
, test "should offer a fix listing all type aliases" <|
\() ->
"""
module A exposing (..)
type alias Foo = String
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Module exposes everything implicitly \"(..)\""
, details =
[ "Modules should have hidden implementation details with an explicit API so that the module is used in a proper and controlled way. The users of this module should not have to know about what is inside a module it is using, and they shouldn't need to access it's internal details. Therefore, the API should be explicitly defined and ideally as small as possible."
]
, under = "(..)"
}
|> Review.Test.whenFixed
"""
module A exposing (Foo)
type alias Foo = String
"""
]
, test "should offer a fix listing all ports" <|
\() ->
"""
port module A exposing (..)
port foo : String
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Module exposes everything implicitly \"(..)\""
, details =
[ "Modules should have hidden implementation details with an explicit API so that the module is used in a proper and controlled way. The users of this module should not have to know about what is inside a module it is using, and they shouldn't need to access it's internal details. Therefore, the API should be explicitly defined and ideally as small as possible."
]
, under = "(..)"
}
|> Review.Test.whenFixed
"""
port module A exposing (foo)
port foo : String
"""
]
, test "should offer a fix listing all infixes" <|
\() ->
"""
module List exposing (..)
import Elm.Kernel.List
infix right 5 (::) = cons
cons : a -> List a -> List a
cons =
Elm.Kernel.List.cons
"""
|> Review.Test.run rule
|> Review.Test.expectErrors
[ Review.Test.error
{ message = "Module exposes everything implicitly \"(..)\""
, details =
[ "Modules should have hidden implementation details with an explicit API so that the module is used in a proper and controlled way. The users of this module should not have to know about what is inside a module it is using, and they shouldn't need to access it's internal details. Therefore, the API should be explicitly defined and ideally as small as possible."
]
, under = "(..)"
}
|> Review.Test.whenFixed
"""
module List exposing ((::), cons)
import Elm.Kernel.List
infix right 5 (::) = cons
cons : a -> List a -> List a
cons =
Elm.Kernel.List.cons
"""
]
]