Skip to content

Commit

Permalink
Issue #97: Add duplicate record field update check
Browse files Browse the repository at this point in the history
  • Loading branch information
stil4m committed Aug 4, 2017
1 parent 63814a2 commit f0a2bbe
Show file tree
Hide file tree
Showing 9 changed files with 156 additions and 0 deletions.
25 changes: 25 additions & 0 deletions docs/Docs/MsgDoc.elm
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import Analyser.Checks.UnnecessaryParens
import Analyser.Checks.NoDebug
import Analyser.Checks.DuplicateImport
import Analyser.Checks.DuplicateImportedVariable
import Analyser.Checks.DuplicateRecordFieldUpdate
import Analyser.Checks.UnusedTypeAlias
import Analyser.Checks.OverriddenVariables
import Analyser.Checks.NoUncurriedPrefix
Expand Down Expand Up @@ -104,6 +105,7 @@ allMessages =
, importAll
, singleFieldRecord
, lineLengthExceeded
, duplicateRecordFieldUpdate
]


Expand All @@ -114,6 +116,29 @@ forKey x =
|> List.head


duplicateRecordFieldUpdate : MsgDoc
duplicateRecordFieldUpdate =
{ name = "Duplicate Record Field Update"
, shortDescription = "You only want to update a field once in the record update syntax. Doing twice may only cause bugs."
, key = "DuplicateRecordFieldUpdate"
, arguments =
[ ( "file", FileName )
, ( "fieldName", VariableName )
, ( "ranges", RangeList )
]
, example = Dynamic Analyser.Checks.DuplicateRecordFieldUpdate.checker
, input = """
module Person exposing (Person, changeName)
type alias Person = { name : String }
changeName : Person -> Person
changeName person =
{ person | name = "John", name = "Jane" }
"""
}


lineLengthExceeded : MsgDoc
lineLengthExceeded =
{ name = "Line Length Exceeded"
Expand Down
1 change: 1 addition & 0 deletions elm-package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
"dependencies": {
"Fresheyeball/elm-tuple-extra": "3.0.0 <= v < 4.0.0",
"danielnarey/elm-css-frameworks": "4.0.0 <= v < 5.0.0",
"elm-community/dict-extra": "2.2.0 <= v < 3.0.0",
"elm-community/json-extra": "2.1.0 <= v < 3.0.0",
"elm-community/list-extra": "6.0.0 <= v < 7.0.0",
"elm-community/maybe-extra": "3.1.0 <= v < 4.0.0",
Expand Down
51 changes: 51 additions & 0 deletions src/Analyser/Checks/DuplicateRecordFieldUpdate.elm
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
module Analyser.Checks.DuplicateRecordFieldUpdate exposing (checker)

import Elm.Syntax.Range as Syntax
import Elm.Syntax.Expression exposing (RecordUpdate, Expression)
import Analyser.Messages.Range as Range exposing (RangeContext)
import Analyser.FileContext exposing (FileContext)
import Analyser.Messages.Types exposing (Message, MessageData(DuplicateRecordFieldUpdate), newMessage)
import ASTUtil.Inspector as Inspector exposing (Order(Post), defaultConfig)
import Dict
import Analyser.Configuration exposing (Configuration)
import Analyser.Checks.Base exposing (Checker, keyBasedChecker)
import Dict.Extra as Dict


checker : Checker
checker =
{ check = scan
, shouldCheck = keyBasedChecker [ "DuplicateRecordFieldUpdate" ]
}


type alias Context =
List ( String, List Syntax.Range )


scan : RangeContext -> FileContext -> Configuration -> List Message
scan rangeContext fileContext _ =
Inspector.inspect
{ defaultConfig
| onRecordUpdate = Post onRecordUpdate
}
fileContext.ast
[]
|> List.map (Tuple.mapSecond (List.map (Range.build rangeContext)))
|> List.map (uncurry (DuplicateRecordFieldUpdate fileContext.path))
|> List.map (newMessage [ ( fileContext.sha1, fileContext.path ) ])


onRecordUpdate : RecordUpdate -> Context -> Context
onRecordUpdate { updates } context =
updates
|> Dict.groupBy Tuple.first
|> Dict.filter (\_ v -> List.length v > 1)
|> Dict.map (\_ v -> List.map (Tuple.second >> expressionRange) v)
|> Dict.toList
|> (++) context


expressionRange : Expression -> Syntax.Range
expressionRange ( r, _ ) =
r
13 changes: 13 additions & 0 deletions src/Analyser/Messages/Json.elm
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,12 @@ decodeMessageData =
, ( "CoreArrayUsage", decodeFileAndRange CoreArrayUsage )
, ( "FunctionInLet", decodeFileAndRange FunctionInLet )
, ( "SingleFieldRecord", decodeFileAndRange SingleFieldRecord )
, ( "DuplicateRecordFieldUpdate"
, JD.succeed DuplicateRecordFieldUpdate
|: fileField
|: JD.field "fieldName" JD.string
|: JD.field "ranges" (JD.list Range.decode)
)
]


Expand Down Expand Up @@ -414,3 +420,10 @@ encodeMessageData m =
[ ( "file", JE.string fileName )
, ( "range", Range.encode range )
]

DuplicateRecordFieldUpdate fileName fieldName ranges ->
JE.object
[ ( "file", JE.string fileName )
, ( "fieldName", JE.string fieldName )
, ( "ranges", JE.list <| List.map Range.encode ranges )
]
1 change: 1 addition & 0 deletions src/Analyser/Messages/Types.elm
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ type MessageData
| CoreArrayUsage FileName Range
| FunctionInLet FileName Range
| SingleFieldRecord FileName Range
| DuplicateRecordFieldUpdate FileName String (List Range)


type alias GetFiles =
Expand Down
13 changes: 13 additions & 0 deletions src/Analyser/Messages/Util.elm
Original file line number Diff line number Diff line change
Expand Up @@ -500,3 +500,16 @@ getMessageInfo m =
, [ range ]
, False
)

Analyser.Messages.Types.DuplicateRecordFieldUpdate fileName fieldName ranges ->
( String.concat
[ "The '"
, fieldName
, "' field for a record is updated multiple times in one expression in file "
, fileName
, "."
]
, always [ fileName ]
, ranges
, False
)
49 changes: 49 additions & 0 deletions tests/Analyser/Checks/DuplicateRecordFieldUpdateTests.elm
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
module Analyser.Checks.DuplicateRecordFieldUpdateTests exposing (..)

import Analyser.Checks.DuplicateRecordFieldUpdate as DuplicateRecordFieldUpdate
import Analyser.Checks.CheckTestUtil as CTU
import Analyser.Messages.Types exposing (..)
import Analyser.Messages.Range as Range
import Test exposing (..)


duplicateUpdate : ( String, String, List MessageData )
duplicateUpdate =
( "duplicateUpdate"
, """module Foo exposing (..)
foo = { bar | a = x, a = y}
"""
, [ DuplicateRecordFieldUpdate "./foo.elm"
"a"
[ Range.manual
{ start = { row = 3, column = 18 }, end = { row = 3, column = 19 } }
{ start = { row = 3, column = 17 }, end = { row = 3, column = 18 } }
, Range.manual
{ start = { row = 3, column = 25 }, end = { row = 3, column = 26 } }
{ start = { row = 3, column = 24 }, end = { row = 3, column = 25 } }
]
]
)


nonDuplicateUpdate : ( String, String, List MessageData )
nonDuplicateUpdate =
( "nonDuplicateUpdate"
, """module Foo exposing (..)
foo = { bar | a = x }
"""
, []
)


all : Test
all =
CTU.build "Analyser.Checks.ImportAllTests"
DuplicateRecordFieldUpdate.checker
[ duplicateUpdate
, nonDuplicateUpdate
]
2 changes: 2 additions & 0 deletions tests/Tests.elm
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import Analyser.Checks.NonStaticRegexTests
import Analyser.Checks.FunctionInLetTests
import Analyser.Checks.DuplicateImportedVariableTests
import Analyser.Checks.SingleFieldRecordTests
import Analyser.Checks.DuplicateRecordFieldUpdateTests


all : Test
Expand All @@ -53,6 +54,7 @@ all =
, Analyser.Checks.ImportAllTests.all
, Analyser.Checks.ExposeAllTests.all
, Analyser.Checks.UnnecessaryListConcatTests.all
, Analyser.Checks.DuplicateRecordFieldUpdateTests.all
, InterfaceTest.all
, ASTUtil.PatternOptimizerTests.all
, Analyser.Fixes.FileContentTests.all
Expand Down
1 change: 1 addition & 0 deletions tests/elm-package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"exposed-modules": [],
"dependencies": {
"Fresheyeball/elm-tuple-extra": "3.0.0 <= v < 4.0.0",
"elm-community/dict-extra": "2.2.0 <= v < 3.0.0",
"elm-community/elm-test": "3.0.0 <= v < 4.0.0",
"elm-community/json-extra": "2.0.0 <= v < 3.0.0",
"elm-community/list-extra": "6.0.0 <= v < 7.0.0",
Expand Down

0 comments on commit f0a2bbe

Please sign in to comment.