Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/elm_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
# Re-use node_modules between runs until package.json or package-lock.json changes.
- name: Cache node_modules
id: cache-node_modules
uses: actions/cache@9b0c1fce7a93df8e3bb8926b0d6e9d89e92f20a7
uses: actions/cache@7de21022a7b6824c106a9847befcbd8154b45b6a
with:
path: node_modules
key: node_modules-${{ hashFiles('package.json', 'package-lock.json') }}
Expand All @@ -28,7 +28,7 @@ jobs:
# review/elm.json changes. The Elm compiler saves downloaded Elm packages
# to ~/.elm, and elm-tooling saves downloaded tool executables there.
- name: Cache ~/.elm
uses: actions/cache@9b0c1fce7a93df8e3bb8926b0d6e9d89e92f20a7
uses: actions/cache@7de21022a7b6824c106a9847befcbd8154b45b6a
with:
path: ~/.elm
key: elm-${{ hashFiles('elm-tooling.json', 'elm.json', 'review/elm.json') }}
Expand Down Expand Up @@ -83,7 +83,7 @@ jobs:

- name: Cache node_modules
id: cache-node_modules
uses: actions/cache@9b0c1fce7a93df8e3bb8926b0d6e9d89e92f20a7
uses: actions/cache@7de21022a7b6824c106a9847befcbd8154b45b6a
with:
path: node_modules
key: node_modules-${{ hashFiles('package.json', 'package-lock.json') }}
Expand All @@ -92,7 +92,7 @@ jobs:
node_modules-

- name: Cache ~/.elm
uses: actions/cache@9b0c1fce7a93df8e3bb8926b0d6e9d89e92f20a7
uses: actions/cache@7de21022a7b6824c106a9847befcbd8154b45b6a
with:
path: ~/.elm
key: elm-${{ hashFiles('elm-tooling.json', 'elm.json', 'review/elm.json') }}
Expand Down
26 changes: 26 additions & 0 deletions src/Exercise/BirdCount.elm
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
module Exercise.BirdCount exposing (doNotUseListModule, ruleConfig)

import Analyzer exposing (CalledExpression(..), CalledFrom(..), Find(..))
import Comment exposing (Comment, CommentType(..))
import Dict
import Review.Rule exposing (Rule)
import RuleConfig exposing (AnalyzerRule(..), RuleConfig)


ruleConfig : RuleConfig
ruleConfig =
{ restrictToFiles = Just [ "src/BirdCount.elm" ]
, rules =
[ CustomRule doNotUseListModule
(Comment "elm.bird-count.do_not_use_list" Essential Dict.empty)
]
}


doNotUseListModule : Comment -> Rule
doNotUseListModule =
Analyzer.functionCalls
{ calledFrom = Anywhere
, findExpressions = [ AnyFromExternalModule [ "List" ] ]
, find = None
}
2 changes: 2 additions & 0 deletions src/ReviewConfig.elm
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import Common.UseCamelCase
import Exercise.AnnalynsInfiltration
import Exercise.Bandwagoner
import Exercise.BettysBikeShop
import Exercise.BirdCount
import Exercise.BlorkemonCards
import Exercise.CustomSet
import Exercise.GottaSnatchEmAll
Expand Down Expand Up @@ -56,6 +57,7 @@ ruleConfigs =
, Exercise.GottaSnatchEmAll.ruleConfig
, Exercise.AnnalynsInfiltration.ruleConfig
, Exercise.LuciansLusciousLasagna.ruleConfig
, Exercise.BirdCount.ruleConfig

-- Practice Exercises
, Exercise.Strain.ruleConfig
Expand Down
170 changes: 170 additions & 0 deletions tests/Exercise/BirdCountTest.elm
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
module Exercise.BirdCountTest exposing (tests)

import Comment exposing (Comment, CommentType(..))
import Dict
import Exercise.BirdCount as BirdCount
import Review.Rule exposing (Rule)
import Review.Test
import RuleConfig
import Test exposing (Test, describe, test)
import TestHelper


tests : Test
tests =
describe "BirdCountTest"
[ exemplar
, usingList
]


rules : List Rule
rules =
BirdCount.ruleConfig |> .rules |> List.map RuleConfig.analyzerRuleToRule


exemplar : Test
exemplar =
test "should not report anything for the example solution" <|
\() ->
TestHelper.expectNoErrorsForRules rules
"""
module BirdCount exposing (busyDays, hasDayWithoutBirds, incrementDayCount, today, total)


today : List Int -> Maybe Int
today counts =
case counts of
[] ->
Nothing

head :: _ ->
Just head


incrementDayCount : List Int -> List Int
incrementDayCount counts =
case counts of
[] ->
[ 1 ]

head :: tail ->
(head + 1) :: tail


hasDayWithoutBirds : List Int -> Bool
hasDayWithoutBirds counts =
case counts of
[] ->
False

0 :: _ ->
True

_ :: tail ->
hasDayWithoutBirds tail


total : List Int -> Int
total counts =
case counts of
[] ->
0

head :: tail ->
head + total tail


busyDays : List Int -> Int
busyDays counts =
case counts of
[] ->
0

head :: tail ->
if head >= 5 then
1 + busyDays tail

else
busyDays tail
"""


usingList : Test
usingList =
let
comment =
Comment "elm.bird-count.do_not_use_list" Essential Dict.empty
in
describe "solutions that use the List function" <|
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I would test all the different uses of List, being as the rule disallows the module completely, and AnyFromExternalModule and similar are already tested, but its not a hill I am going to die on.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point of these tests is not to check that the rule works properly, like you said this has been tested a lot by now, the point is to show examples of "wrong" solutions that justify why we have those rules in place.

With these code examples, you can conclude "right, if someone writes a solution like that, they won't learn anything about recursion." Conversely, if you are not able to come up with an reasonable example of a solution breaking a rule, then maybe the rule isn't meaningful.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can agree to differ on this, but I do take your point.

[ test "using List.head for today" <|
\() ->
"""
module BirdCount exposing (today)

today : List Int -> Maybe Int
today = List.head
"""
|> Review.Test.run (BirdCount.doNotUseListModule comment)
|> Review.Test.expectErrors
[ TestHelper.createExpectedErrorUnder comment "List.head" ]
, test "using List.any for hasDayWithoutBirds" <|
\() ->
"""
module BirdCount exposing (hasDayWithoutBirds)

hasDayWithoutBirds : List Int -> Bool
hasDayWithoutBirds = List.any ((==) 0)
"""
|> Review.Test.run (BirdCount.doNotUseListModule comment)
|> Review.Test.expectErrors
[ TestHelper.createExpectedErrorUnder comment "List.any" ]
, test "using List.sum for total" <|
\() ->
"""
module BirdCount exposing (total)

total : List Int -> Int
total = List.sum
"""
|> Review.Test.run (BirdCount.doNotUseListModule comment)
|> Review.Test.expectErrors
[ TestHelper.createExpectedErrorUnder comment "List.sum" ]
, test "using List.filter and List.length for busyDays" <|
\() ->
"""
module BirdCount exposing (busyDays)

busyDays : List Int -> Int
busyDays = List.filter ((>=) 5) >> List.length
"""
|> Review.Test.run (BirdCount.doNotUseListModule comment)
|> Review.Test.expectErrors
[ TestHelper.createExpectedErrorUnder comment "List.filter" ]
, test "using List with an alias" <|
\() ->
"""
module BirdCount exposing (today)
import List as L

today : List Int -> Maybe Int
today = L.head
"""
|> Review.Test.run (BirdCount.doNotUseListModule comment)
|> Review.Test.expectErrors
[ TestHelper.createExpectedErrorUnder comment "L.head" ]
, test "using List with exposing" <|
\() ->
"""
module BirdCount exposing (today)
import List exposing (head)

today : List Int -> Maybe Int
today = head
"""
|> Review.Test.run (BirdCount.doNotUseListModule comment)
|> Review.Test.expectErrors
[ TestHelper.createExpectedErrorUnder comment "head"
|> Review.Test.atExactly { start = { row = 6, column = 9 }, end = { row = 6, column = 13 } }
]
]