Backport elm-review-debug

This commit is contained in:
Jeroen Engels 2021-01-24 16:43:58 +01:00
parent fa8bc02068
commit 63819d3fb7
4 changed files with 365 additions and 238 deletions

View File

@ -6,10 +6,11 @@ module NoDebug.Log exposing (rule)
-}
import Elm.Syntax.Exposing as Exposing
import Elm.Syntax.Expression as Expression exposing (Expression)
import Elm.Syntax.Import exposing (Import)
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
import Review.ModuleNameLookupTable as ModuleNameLookupTable exposing (ModuleNameLookupTable)
import Review.Rule as Rule exposing (Error, Rule)
@ -63,75 +64,223 @@ elm-review --template jfmengels/elm-review-debug/example --rules NoDebug.Log
-}
rule : Rule
rule =
Rule.newModuleRuleSchema "NoDebug.Log" { hasLogBeenImported = False }
|> Rule.withImportVisitor importVisitor
|> Rule.withExpressionVisitor expressionVisitor
Rule.newModuleRuleSchemaUsingContextCreator "NoDebug.Log" initContext
|> Rule.withExpressionEnterVisitor expressionVisitor
|> Rule.fromModuleRuleSchema
type alias Context =
{ hasLogBeenImported : Bool
{ lookupTable : ModuleNameLookupTable
, rangesToIgnore : List Range
}
error : Node a -> Error {}
error node =
Rule.error
initContext : Rule.ContextCreator () Context
initContext =
Rule.initContextCreator
(\lookupTable () ->
{ lookupTable = lookupTable
, rangesToIgnore = []
}
)
|> Rule.withModuleNameLookupTable
error : Node a -> Maybe Range -> Error {}
error node rangeToRemove =
Rule.errorWithFix
{ message = "Remove the use of `Debug.log` before shipping to production"
, details =
[ "`Debug.log` is useful when developing, but is not meant to be shipped to production or published in a package. I suggest removing its use before committing and attempting to push to production."
]
}
(Node.range node)
importVisitor : Node Import -> Context -> ( List nothing, Context )
importVisitor node context =
let
moduleName : List String
moduleName =
node
|> Node.value
|> .moduleName
|> Node.value
in
if moduleName == [ "Debug" ] then
case node |> Node.value |> .exposingList |> Maybe.map Node.value of
Just (Exposing.All _) ->
( [], { hasLogBeenImported = True } )
Just (Exposing.Explicit importedNames) ->
( [], { hasLogBeenImported = List.any isLog importedNames } )
(case rangeToRemove of
Just range ->
[ Review.Fix.removeRange range ]
Nothing ->
( [], context )
else
( [], context )
[]
)
isLog : Node Exposing.TopLevelExpose -> Bool
isLog node =
handleWhenSingleArg : Range -> Context -> Node Expression -> ( List (Error {}), Context )
handleWhenSingleArg rangeToPotentiallyRemove context node =
case Node.value node of
Exposing.FunctionExpose "log" ->
True
Expression.Application (((Node logFunctionRange (Expression.FunctionOrValue _ "log")) as logFunctionNode) :: logArguments) ->
case ModuleNameLookupTable.moduleNameAt context.lookupTable logFunctionRange of
Just [ "Debug" ] ->
let
rangeToRemove : Maybe Range
rangeToRemove =
case logArguments of
[ _ ] ->
Just rangeToPotentiallyRemove
_ ->
False
_ ->
Nothing
in
( [ error logFunctionNode rangeToRemove ]
, { context | rangesToIgnore = Node.range node :: logFunctionRange :: context.rangesToIgnore }
)
expressionVisitor : Node Expression -> Rule.Direction -> Context -> ( List (Error {}), Context )
expressionVisitor node direction context =
case ( direction, Node.value node ) of
( Rule.OnEnter, Expression.FunctionOrValue [ "Debug" ] "log" ) ->
( [ error node ], context )
( Rule.OnEnter, Expression.FunctionOrValue [] "log" ) ->
if context.hasLogBeenImported then
( [ error node ], context )
else
( [], context )
_ ->
( [], context )
_ ->
( [], context )
collectPipeline : Node Expression -> ( List Range, List (Node Expression) )
collectPipeline node =
collectPipelineHelp node
collectPipelineHelp : Node Expression -> ( List Range, List (Node Expression) )
collectPipelineHelp node =
case Node.value node of
Expression.OperatorApplication "|>" _ left right ->
let
( x1, y1 ) =
collectPipelineHelp left
( x2, y2 ) =
collectPipelineHelp right
in
( Node.range node :: (x1 ++ x2), y1 ++ y2 )
_ ->
( [], [ node ] )
expressionVisitor : Node Expression -> Context -> ( List (Error {}), Context )
expressionVisitor node context =
if List.member (Node.range node) context.rangesToIgnore then
( [], context )
else
expressionVisitorHelp node context
expressionVisitorHelp : Node Expression -> Context -> ( List (Error {}), Context )
expressionVisitorHelp node context =
case Node.value node of
Expression.OperatorApplication "|>" _ left right ->
let
( pipelineOperators, pipelineOperations ) =
collectPipeline node
contextWithIgnoredElements : Context
contextWithIgnoredElements =
{ context | rangesToIgnore = List.concat [ pipelineOperators, List.map Node.range pipelineOperations, context.rangesToIgnore ] }
( errorsAfterLeft, contextAfterLeft ) =
handleWhenSingleArg
{ start = (Node.range left).start
, end = (Node.range right).start
}
contextWithIgnoredElements
left
in
List.foldl
(\pipelineItem ( prev, ( errors, ctx ) ) ->
let
( newErrors, newContext ) =
handleWhenSingleArg
{ start = (Node.range prev).end
, end = (Node.range pipelineItem).end
}
ctx
pipelineItem
in
( pipelineItem, ( newErrors ++ errors, newContext ) )
)
( left, ( errorsAfterLeft, contextAfterLeft ) )
pipelineOperations
|> Tuple.second
Expression.OperatorApplication "<|" _ left right ->
handleWhenSingleArg
{ start = (Node.range left).start
, end = (Node.range right).start
}
context
left
Expression.OperatorApplication "<<" _ left right ->
let
( errorsLeft, contextAfterLeft ) =
handleWhenSingleArg
{ start = (Node.range left).start
, end = (Node.range right).start
}
context
left
( errorsRight, contextAfterRight ) =
handleWhenSingleArg
{ start = (Node.range left).end
, end = (Node.range right).end
}
contextAfterLeft
right
in
( errorsLeft ++ errorsRight, contextAfterRight )
Expression.OperatorApplication ">>" _ left right ->
let
( errorsLeft, contextAfterLeft ) =
handleWhenSingleArg
{ start = (Node.range left).start
, end = (Node.range right).start
}
context
left
( errorsRight, contextAfterRight ) =
handleWhenSingleArg
{ start = (Node.range left).end
, end = (Node.range right).end
}
contextAfterLeft
right
in
( errorsLeft ++ errorsRight, contextAfterRight )
Expression.Application (((Node logFunctionRange (Expression.FunctionOrValue _ "log")) as logFunctionNode) :: logArguments) ->
let
rangeToRemove : Maybe Range
rangeToRemove =
case logArguments of
[ _, valueToLog ] ->
Just
{ start = logFunctionRange.start
, end = (Node.range valueToLog).start
}
_ ->
Nothing
in
reportIfDebugLog logFunctionNode context rangeToRemove
Expression.FunctionOrValue _ "log" ->
reportIfDebugLog node context Nothing
_ ->
( [], context )
reportIfDebugLog : Node Expression -> Context -> Maybe Range -> ( List (Error {}), Context )
reportIfDebugLog node context rangeToRemove =
if List.member (Node.range node) context.rangesToIgnore then
( [], context )
else
case ModuleNameLookupTable.moduleNameFor context.lookupTable node of
Just [ "Debug" ] ->
( [ error node rangeToRemove ]
, { context | rangesToIgnore = Node.range node :: context.rangesToIgnore }
)
_ ->
( [], context )

View File

@ -1,6 +1,8 @@
module NoDebug.LogTest exposing (all)
import Dependencies.ElmCore
import NoDebug.Log exposing (rule)
import Review.Project as Project exposing (Project)
import Review.Test exposing (ReviewResult)
import Test exposing (Test, describe, test)
@ -9,7 +11,25 @@ testRule : String -> ReviewResult
testRule string =
"module A exposing (..)\n\n"
++ string
|> Review.Test.run rule
|> Review.Test.runWithProjectData project rule
errorDetails : { message : String, details : List String, under : String }
errorDetails =
{ message = message
, details = details
, under = "Debug.log"
}
project : Project
project =
Project.addDependency Dependencies.ElmCore.dependency Project.new
whenFixed : String -> Review.Test.ExpectedError -> Review.Test.ExpectedError
whenFixed string =
Review.Test.whenFixed ("module A exposing (..)\n\n" ++ string)
message : String
@ -47,21 +67,13 @@ b = Debug.todo ""
\() ->
testRule "a = Debug.log"
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
[ Review.Test.error errorDetails
]
, test "should report Debug.log calls" <|
\() ->
testRule "a = Debug.log z"
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
[ Review.Test.error errorDetails
]
, test "should report multiple Debug.log calls" <|
\() ->
@ -70,170 +82,176 @@ a = Debug.log z
b = Debug.log z
"""
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
[ Review.Test.error errorDetails
|> Review.Test.atExactly { start = { row = 4, column = 5 }, end = { row = 4, column = 14 } }
, Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
, Review.Test.error errorDetails
|> Review.Test.atExactly { start = { row = 5, column = 5 }, end = { row = 5, column = 14 } }
]
, test "should report Debug.log in a binary expression" <|
\() ->
testRule "a = ( Debug.log z ) + 2"
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
[ Review.Test.error errorDetails
]
, test "should report Debug.log in a << binary expression" <|
, test "should report Debug.log in a << binary expression (on the right)" <|
\() ->
testRule "a = fn << Debug.log"
testRule "a = fn << Debug.log b"
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
[ Review.Test.error errorDetails
|> whenFixed "a = fn"
]
, test "should report Debug.log in a pipe expression" <|
, test "should report Debug.log in a << binary expression (on the left)" <|
\() ->
testRule "a = Debug.log b << fn"
|> Review.Test.expectErrors
[ Review.Test.error errorDetails
|> whenFixed "a = fn"
]
, test "should report Debug.log in a >> binary expression (on the right)" <|
\() ->
testRule "a = fn >> Debug.log b"
|> Review.Test.expectErrors
[ Review.Test.error errorDetails
|> whenFixed "a = fn"
]
, test "should report Debug.log in a >> binary expression (on the left)" <|
\() ->
testRule "a = Debug.log b >> fn"
|> Review.Test.expectErrors
[ Review.Test.error errorDetails
|> whenFixed "a = fn"
]
, test "should report Debug.log in a |> pipe expression" <|
\() ->
testRule "a = fn |> Debug.log z"
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
[ Review.Test.error errorDetails
|> whenFixed "a = fn"
]
, test "should report Debug.log in a large |> pipe expression (pipes on each side)" <|
\() ->
testRule "a = fn |> Debug.log z |> fn2"
|> Review.Test.expectErrors
[ Review.Test.error errorDetails
|> whenFixed "a = fn |> fn2"
]
, test "should report Debug.log multiple times in a pipeline expression with multiple Debug.log calls" <|
\() ->
testRule "a = fn |> Debug.log z |> fn2 |> Debug.log y"
|> Review.Test.expectErrors
[ Review.Test.error errorDetails
|> Review.Test.atExactly { start = { row = 3, column = 11 }, end = { row = 3, column = 20 } }
|> whenFixed "a = fn |> fn2 |> Debug.log y"
, Review.Test.error errorDetails
|> Review.Test.atExactly { start = { row = 3, column = 33 }, end = { row = 3, column = 42 } }
|> whenFixed "a = fn |> Debug.log z |> fn2"
]
, test "should report Debug.log in a <| pipe expression" <|
\() ->
testRule "a = Debug.log z <| fn"
|> Review.Test.expectErrors
[ Review.Test.error errorDetails
|> whenFixed "a = fn"
]
, test "should report Debug.log in a large <| pipe expression" <|
\() ->
testRule "a = foo <| Debug.log z <| fn"
|> Review.Test.expectErrors
[ Review.Test.error errorDetails
|> whenFixed "a = foo <| fn"
]
, test "should report Debug.log in an list expression" <|
\() ->
testRule "a = [ Debug.log z y ]"
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
[ Review.Test.error errorDetails
|> whenFixed "a = [ y ]"
]
, test "should report Debug.log in an record expression" <|
, test "should report Debug.log in a record expression" <|
\() ->
testRule "a = { foo = Debug.log z y }"
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
[ Review.Test.error errorDetails
|> whenFixed "a = { foo = y }"
]
, test "should report Debug.log in an record update expression" <|
, test "should report Debug.log in a record update expression" <|
\() ->
testRule "a = { model | foo = Debug.log z y }"
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
[ Review.Test.error errorDetails
|> whenFixed "a = { model | foo = y }"
]
, test "should report Debug.log in an lambda expression" <|
\() ->
testRule "a = (\\foo -> Debug.log z foo)"
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
[ Review.Test.error errorDetails
|> whenFixed "a = (\\foo -> foo)"
]
, test "should report Debug.log in an if expression condition" <|
\() ->
testRule "a = if Debug.log a b then True else False"
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
[ Review.Test.error errorDetails
|> whenFixed "a = if b then True else False"
]
, test "should report Debug.log in an if expression then branch" <|
\() ->
testRule "a = if True then Debug.log a b else False"
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
[ Review.Test.error errorDetails
|> whenFixed "a = if True then b else False"
]
, test "should report Debug.log in an if expression else branch" <|
\() ->
testRule "a = if True then True else Debug.log a b"
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
[ Review.Test.error errorDetails
|> whenFixed "a = if True then True else b"
]
, test "should report Debug.log in a case value" <|
\() ->
testRule """
a = case Debug.log a b of
_ -> []
"""
_ -> []"""
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
[ Review.Test.error errorDetails
|> whenFixed """
a = case b of
_ -> []"""
]
, test "should report Debug.log in a case body" <|
\() ->
testRule """
a = case a of
_ -> Debug.log a b
"""
_ -> Debug.log a b"""
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
[ Review.Test.error errorDetails
|> whenFixed """
a = case a of
_ -> b"""
]
, test "should report Debug.log in let declaration section" <|
\() ->
testRule """
a = let b = Debug.log a b
in b
"""
a = let b = Debug.log a c
in b"""
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
[ Review.Test.error errorDetails
|> whenFixed """
a = let b = c
in b"""
]
, test "should report Debug.log in let body" <|
\() ->
testRule """
a = let b = c
in Debug.log a b
"""
in Debug.log a b"""
|> Review.Test.expectErrors
[ Review.Test.error
{ message = message
, details = details
, under = "Debug.log"
}
[ Review.Test.error errorDetails
|> whenFixed """
a = let b = c
in b"""
]
, test "should not report calls from a module containing Debug but that is not Debug" <|
\() ->
@ -259,6 +277,10 @@ a = log "" 1
, under = "log"
}
|> Review.Test.atExactly { start = { row = 5, column = 5 }, end = { row = 5, column = 8 } }
|> whenFixed """
import Debug exposing (log)
a = 1
"""
]
, test "should report the use of `log` when it has been implicitly imported" <|
\() ->
@ -272,6 +294,10 @@ a = log "" 1
, details = details
, under = "log"
}
|> whenFixed """
import Debug exposing (..)
a = 1
"""
]
, test "should not report the use of `log` when it has not been imported" <|
\() ->

View File

@ -6,10 +6,9 @@ module NoDebug.TodoOrToString exposing (rule)
-}
import Elm.Syntax.Exposing as Exposing
import Elm.Syntax.Expression as Expression exposing (Expression)
import Elm.Syntax.Import exposing (Import)
import Elm.Syntax.Node as Node exposing (Node)
import Review.ModuleNameLookupTable as ModuleNameLookupTable exposing (ModuleNameLookupTable)
import Review.Rule as Rule exposing (Error, Rule)
@ -84,95 +83,41 @@ elm-review --template jfmengels/elm-review-debug/example --rules NoDebug.TodoOrT
-}
rule : Rule
rule =
Rule.newModuleRuleSchema "NoDebug.TodoOrToString" init
|> Rule.withImportVisitor importVisitor
|> Rule.withExpressionVisitor expressionVisitor
Rule.newModuleRuleSchemaUsingContextCreator "NoDebug.TodoOrToString" init
|> Rule.withExpressionEnterVisitor expressionVisitor
|> Rule.fromModuleRuleSchema
type alias Context =
{ hasTodoBeenImported : Bool
, hasToStringBeenImported : Bool
}
ModuleNameLookupTable
init : Context
init : Rule.ContextCreator () Context
init =
{ hasTodoBeenImported = False
, hasToStringBeenImported = False
}
Rule.initContextCreator (\lookupTable () -> lookupTable)
|> Rule.withModuleNameLookupTable
error : Node a -> String -> Error {}
error node name =
Rule.error
{ message = "Remove the use of `Debug." ++ name ++ "` before shipping to production"
, details =
[ "`Debug." ++ name ++ "` can be useful when developing, but is not meant to be shipped to production or published in a package. I suggest removing its use before committing and attempting to push to production."
]
}
(Node.range node)
importVisitor : Node Import -> Context -> ( List nothing, Context )
importVisitor node context =
let
moduleName : List String
moduleName =
node
|> Node.value
|> .moduleName
|> Node.value
in
if moduleName == [ "Debug" ] then
case node |> Node.value |> .exposingList |> Maybe.map Node.value of
Just (Exposing.All _) ->
( [], { hasTodoBeenImported = True, hasToStringBeenImported = True } )
Just (Exposing.Explicit importedNames) ->
( []
, { context
| hasTodoBeenImported = List.any (is "todo") importedNames
, hasToStringBeenImported = List.any (is "toString") importedNames
}
)
Nothing ->
( [], context )
else
( [], context )
is : String -> Node Exposing.TopLevelExpose -> Bool
is name node =
expressionVisitor : Node Expression -> Context -> ( List (Error {}), Context )
expressionVisitor node context =
case Node.value node of
Exposing.FunctionExpose functionName ->
name == functionName
Expression.FunctionOrValue _ name ->
if name == "todo" || name == "toString" then
case ModuleNameLookupTable.moduleNameFor context node of
Just [ "Debug" ] ->
( [ Rule.error
{ message = "Remove the use of `Debug." ++ name ++ "` before shipping to production"
, details =
[ "`Debug." ++ name ++ "` can be useful when developing, but is not meant to be shipped to production or published in a package. I suggest removing its use before committing and attempting to push to production."
]
}
(Node.range node)
]
, context
)
_ ->
False
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 if name == "toString" then
( [ error node name ], 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 )
_ ->
( [], context )
else
( [], context )

View File

@ -1,6 +1,8 @@
module NoDebug.TodoOrToStringTest exposing (all)
import Dependencies.ElmCore
import NoDebug.TodoOrToString exposing (rule)
import Review.Project as Project exposing (Project)
import Review.Test exposing (ReviewResult)
import Test exposing (Test, describe, test)
@ -9,7 +11,12 @@ testRule : String -> ReviewResult
testRule string =
"module A exposing (..)\n\n"
++ string
|> Review.Test.run rule
|> Review.Test.runWithProjectData project rule
project : Project
project =
Project.addDependency Dependencies.ElmCore.dependency Project.new
todoMessage : String