Skip to content

Commit

Permalink
Renames rule and adds better error messages
Browse files Browse the repository at this point in the history
  • Loading branch information
gampleman committed Jul 4, 2024
1 parent eb30d4d commit 78504d5
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 25 deletions.
2 changes: 1 addition & 1 deletion docs.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion elm.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"license": "MIT",
"version": "1.0.0",
"exposed-modules": [
"NoDebug.TodoItForMe",
"Derive",
"CodeGenerator",
"CodeGenerator.Test",
"TypePattern",
Expand Down
4 changes: 2 additions & 2 deletions preview/src/ReviewConfig.elm
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ when inside the directory containing this file.
-}

import NoDebug.TodoItForMe
import Derive
import Review.Rule exposing (Rule)


config : List Rule
config =
[ NoDebug.TodoItForMe.rule True []
[ Derive.rule True []
]
16 changes: 8 additions & 8 deletions src/CodeGenerator/Test.elm
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ module CodeGenerator.Test exposing (codeGenTest, codeGenTestFailsWith, codeGenIn

import Array
import CodeGenerator exposing (CodeGenerator)
import Derive
import Elm.Constraint
import Elm.License
import Elm.Module
Expand All @@ -24,7 +25,6 @@ import Elm.Type
import Elm.Version
import Expect exposing (Expectation)
import Json.Decode
import NoDebug.TodoItForMe
import Review.Project
import Review.Project.Dependency exposing (Dependency)
import Review.Test
Expand Down Expand Up @@ -78,7 +78,7 @@ codeGenTestHelper description dependencies codeGens modules incremental fn =
project =
List.foldl Review.Project.addDependency Review.Test.Dependencies.projectWithElmCore validDependencies
in
fn result (Review.Test.runOnModulesWithProjectData project (NoDebug.TodoItForMe.rule incremental codeGens) inputModules)
fn result (Review.Test.runOnModulesWithProjectData project (Derive.rule incremental codeGens) inputModules)

else
Expect.fail ("Found issues in the following dependencies: \n\n" ++ String.join "\n" failedDeps)
Expand All @@ -100,8 +100,8 @@ codeGenTestFailsWith description dependencies codeGens modules expectedFailureDe
Review.Test.expectErrorsForModules
[ ( result.module_
, [ Review.Test.error
{ message = "Remove the use of `Debug.todo` before shipping to production"
, details = [ "`Debug.todo` 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.", expectedFailureDetails ]
{ message = "Cannot generate the implementation of this `Debug.todo`"
, details = [ expectedFailureDetails ]
, under = result.under
}
]
Expand Down Expand Up @@ -150,8 +150,8 @@ codeGenTest description dependencies codeGens modules expected =
Review.Test.expectErrorsForModules
[ ( result.module_
, [ Review.Test.error
{ message = "Remove the use of `Debug.todo` before shipping to production"
, details = [ "`Debug.todo` 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." ]
{ message = "Remove the use of `Debug.todo`"
, details = [ "We have successfully generated an implementation for this `Debug.todo`" ]
, under = result.under
}
|> Review.Test.whenFixed (String.replace "\u{000D}" "" expected)
Expand All @@ -174,8 +174,8 @@ codeGenIncrementalTest description dependencies codeGens modules expected =
Review.Test.expectErrorsForModules
[ ( result.module_
, [ Review.Test.error
{ message = "Remove the use of `Debug.todo` before shipping to production"
, details = [ "`Debug.todo` 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." ]
{ message = "Remove the use of `Debug.todo`"
, details = [ "We have successfully generated a stub for this `Debug.todo`" ]
, under = result.under
}
|> Review.Test.whenFixed (String.replace "\u{000D}" "" expected)
Expand Down
8 changes: 4 additions & 4 deletions src/NoDebug/TodoItForMe.elm → src/Derive.elm
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module NoDebug.TodoItForMe exposing (rule)
module Derive exposing (rule)

{-|
Expand Down Expand Up @@ -134,7 +134,7 @@ rule incrementalMode generators =
, Internal.Builtin.CsvDecoder.codeGen
]
in
Rule.newProjectRuleSchema "CodeGen" initialProjectContext
Rule.newProjectRuleSchema "Derive" initialProjectContext
|> Rule.withContextFromImportedModules
|> Rule.withModuleVisitor moduleVisitor
|> Rule.withModuleContextUsingContextCreator
Expand Down Expand Up @@ -430,9 +430,9 @@ finalProjectEvaluation incrementalMode projectContext =
List.map
(\( moduleKey, range ) ->
Rule.errorForModule moduleKey
{ message = "Remove the use of `Debug.todo` before shipping to production"
{ message = "Cannot derive implementation for Debug.todo within functions"
, details =
[ "`Debug.todo` 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."
[ "This rule can only derive implementations for you when the `Debug.todo` call is at the top-level and has an explicit supported type declaration."
]
}
range
Expand Down
13 changes: 8 additions & 5 deletions src/Internal/CodeGenTodo.elm
Original file line number Diff line number Diff line change
Expand Up @@ -239,9 +239,13 @@ todoErrors incrementalMode projectContext currentModule todo =
case createFixes incrementalMode projectContext codeGen imports currentModule todo of
Ok fixes ->
Rule.errorForModuleWithFix moduleKey
{ message = "Remove the use of `Debug.todo` before shipping to production"
{ message = "Remove the use of `Debug.todo`"
, details =
[ "`Debug.todo` 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."
[ if incrementalMode then
"We have successfully generated a stub for this `Debug.todo`"

else
"We have successfully generated an implementation for this `Debug.todo`"
]
}
todo.range
Expand All @@ -250,10 +254,9 @@ todoErrors incrementalMode projectContext currentModule todo =

Err reason ->
Rule.errorForModule moduleKey
{ message = "Remove the use of `Debug.todo` before shipping to production"
{ message = "Cannot generate the implementation of this `Debug.todo`"
, details =
[ "`Debug.todo` 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."
, reason
[ reason
]
}
todo.range
Expand Down
24 changes: 20 additions & 4 deletions tests/NoDebugTest.elm
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module NoDebugTest exposing (all)

import NoDebug.TodoItForMe
import Derive
import Review.Test exposing (ReviewResult)
import Review.Test.Dependencies
import Test exposing (Test, describe, test)
Expand All @@ -10,17 +10,17 @@ testRule : String -> ReviewResult
testRule string =
"module A exposing (..)\n\n"
++ string
|> Review.Test.runWithProjectData Review.Test.Dependencies.projectWithElmCore (NoDebug.TodoItForMe.rule False [])
|> Review.Test.runWithProjectData Review.Test.Dependencies.projectWithElmCore (Derive.rule False [])


todoMessage : String
todoMessage =
"Remove the use of `Debug.todo` before shipping to production"
"Cannot derive implementation for Debug.todo within functions"


todoDetails : List String
todoDetails =
[ "`Debug.todo` 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."
[ "This rule can only derive implementations for you when the `Debug.todo` call is at the top-level and has an explicit supported type declaration."
]


Expand Down Expand Up @@ -102,4 +102,20 @@ import Debug exposing (log)
a = todo "" 1
"""
|> Review.Test.expectNoErrors
, test "should report the use of `todo` when it has a non-matching type signature" <|
\() ->
testRule """
import Json.Decode exposing (Decoder)
type Foo var = Foo var
a : Decoder Int
a = Debug.todo ""
"""
|> Review.Test.expectErrors
[ Review.Test.error
{ message = todoMessage
, details = todoDetails
, under = "Debug.todo"
}
]
]

0 comments on commit 78504d5

Please sign in to comment.