From 5cc72c3991ec9f4f0a86ed98ba7074381fba0023 Mon Sep 17 00:00:00 2001 From: Jeremie Gillet Date: Thu, 23 Nov 2023 19:09:26 +0900 Subject: [PATCH 01/17] Add destructuring helper --- src/ElmSyntaxHelpers.elm | 27 ++++++++++++++++++++++++++- tests/ElmSyntaxHelpersTest.elm | 31 +++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/src/ElmSyntaxHelpers.elm b/src/ElmSyntaxHelpers.elm index ca21018..5e923cd 100644 --- a/src/ElmSyntaxHelpers.elm +++ b/src/ElmSyntaxHelpers.elm @@ -1,4 +1,4 @@ -module ElmSyntaxHelpers exposing (hasGenericRecord, traversePattern, typeAnnotationsMatch) +module ElmSyntaxHelpers exposing (hasDestructuringPattern, hasGenericRecord, traversePattern, typeAnnotationsMatch) import Elm.Syntax.Node as Node exposing (Node(..)) import Elm.Syntax.Pattern exposing (Pattern(..)) @@ -76,6 +76,31 @@ hasGenericRecord annotation = False +hasDestructuringPattern : Node Pattern -> Bool +hasDestructuringPattern fullPattern = + let + destructuringPattern pattern = + case Node.value pattern of + RecordPattern _ -> + True + + UnConsPattern _ _ -> + True + + TuplePattern _ -> + True + + NamedPattern _ _ -> + True + + _ -> + False + in + fullPattern + |> traversePattern + |> List.any destructuringPattern + + traversePattern : Node Pattern -> List (Node Pattern) traversePattern pattern = case Node.value pattern of diff --git a/tests/ElmSyntaxHelpersTest.elm b/tests/ElmSyntaxHelpersTest.elm index 6804c0a..daabd2a 100644 --- a/tests/ElmSyntaxHelpersTest.elm +++ b/tests/ElmSyntaxHelpersTest.elm @@ -50,6 +50,37 @@ hasGenericRecordTests = ] +hasDestructuringPatternTests : Test +hasDestructuringPatternTests = + describe "hasDestructuringPattern" + [ test "no destructuring" <| + \_ -> + Node.empty (ParenthesizedPattern (Node.empty (ListPattern [ Node.empty UnitPattern ]))) + |> ElmSyntaxHelpers.hasDestructuringPattern + |> Expect.equal False + , test "tuple destructuring" <| + \_ -> + Node.empty (TuplePattern []) + |> ElmSyntaxHelpers.hasDestructuringPattern + |> Expect.equal True + , test "record destructuring" <| + \_ -> + Node.empty (RecordPattern []) + |> ElmSyntaxHelpers.hasDestructuringPattern + |> Expect.equal True + , test "named destructuring" <| + \_ -> + Node.empty (NamedPattern { moduleName = [], name = "Thing" } []) + |> ElmSyntaxHelpers.hasDestructuringPattern + |> Expect.equal True + , test "uncons destructuring" <| + \_ -> + Node.empty (UnConsPattern (Node.empty UnitPattern) (Node.empty UnitPattern)) + |> ElmSyntaxHelpers.hasDestructuringPattern + |> Expect.equal True + ] + + traversePatternTests : Test traversePatternTests = describe "traversePattern" From 395f0dd88678515fe67f15f81782a25ffe89fb95 Mon Sep 17 00:00:00 2001 From: Jeremie Gillet Date: Thu, 23 Nov 2023 19:10:50 +0900 Subject: [PATCH 02/17] generate tags --- .gitignore | 1 + bin/run.sh | 13 ++- src/ReviewConfig.elm | 8 +- src/RuleConfig.elm | 18 +++- src/Tags.elm | 218 ++++++++++++++++++++++++++++++++++++++++++ tests/TagsTest.elm | 220 +++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 470 insertions(+), 8 deletions(-) create mode 100644 src/Tags.elm create mode 100644 tests/TagsTest.elm diff --git a/.gitignore b/.gitignore index 3f1f810..6b02fdc 100644 --- a/.gitignore +++ b/.gitignore @@ -2,4 +2,5 @@ elm-stuff bin/*.js src/main.js *analysis.json +*tags.json node_modules diff --git a/bin/run.sh b/bin/run.sh index 13fc7b6..18e90f1 100755 --- a/bin/run.sh +++ b/bin/run.sh @@ -20,14 +20,23 @@ if [ -f solution_cache.tar ]; then INPUT_DIR=/tmp/sol fi -# Run analysis # Temporarily disable -e mode set +e + +# Run analysis npx elm-review $INPUT_DIR \ --elmjson $INPUT_DIR/elm.json \ --config . \ --report=json \ - | node ./bin/cli.js > $OUTPUT_DIR/analysis.json + --extract \ + > /tmp/elm-review-report.json + +# Get comments +cat /tmp/elm-review-report.json | node ./bin/cli.js > $OUTPUT_DIR/analysis.json + +# Get tags +jq '.extracts .Tags' /tmp/elm-review-report.json > $OUTPUT_DIR/tags.json + set -e echo Finished diff --git a/src/ReviewConfig.elm b/src/ReviewConfig.elm index f27215a..a08e470 100644 --- a/src/ReviewConfig.elm +++ b/src/ReviewConfig.elm @@ -23,12 +23,16 @@ import Exercise.ValentinesDay import Exercise.ZebraPuzzle import Review.Rule as Rule exposing (Rule) import RuleConfig exposing (RuleConfig) +import Tags ruleConfigs : List RuleConfig ruleConfigs = - [ -- Common Rules - Common.NoUnused.ruleConfig + [ -- Tags + Tags.ruleConfig + + -- Common Rules + , Common.NoUnused.ruleConfig , Common.Simplify.ruleConfig , Common.NoDebug.ruleConfig , Common.UseCamelCase.ruleConfig diff --git a/src/RuleConfig.elm b/src/RuleConfig.elm index dc26351..eb7e720 100644 --- a/src/RuleConfig.elm +++ b/src/RuleConfig.elm @@ -14,6 +14,7 @@ type alias RuleConfig = type AnalyzerRule = CustomRule (Comment -> Rule) Comment | ImportedRule Rule (Comment -> Decoder Comment) Comment + | TagRule Rule analyzerRuleToRule : AnalyzerRule -> Rule @@ -25,6 +26,9 @@ analyzerRuleToRule analyzerRule = ImportedRule rule _ _ -> rule + TagRule rule -> + rule + analyzerRuleToDecoder : AnalyzerRule -> Maybe (Decoder Comment) analyzerRuleToDecoder analyzerRule = @@ -35,15 +39,21 @@ analyzerRuleToDecoder analyzerRule = ImportedRule _ toDecoder comment -> Just (toDecoder comment) + TagRule _ -> + Nothing + -analyzerRuleToComment : AnalyzerRule -> Comment +analyzerRuleToComment : AnalyzerRule -> Maybe Comment analyzerRuleToComment analyzerRule = case analyzerRule of CustomRule _ comment -> - comment + Just comment ImportedRule _ _ comment -> - comment + Just comment + + TagRule _ -> + Nothing getRules : RuleConfig -> List Rule @@ -63,7 +73,7 @@ getDecoders = getComments : List RuleConfig -> List Comment getComments = - List.concatMap (.rules >> List.map analyzerRuleToComment) + List.concatMap (.rules >> List.filterMap analyzerRuleToComment) makeConfig : List RuleConfig -> List Rule diff --git a/src/Tags.elm b/src/Tags.elm new file mode 100644 index 0000000..86ab782 --- /dev/null +++ b/src/Tags.elm @@ -0,0 +1,218 @@ +module Tags exposing (commonTagsRule, expressionTagsRule, ruleConfig) + +import Elm.Syntax.Expression exposing (Expression(..), LetDeclaration(..)) +import Elm.Syntax.Node as Node exposing (Node(..)) +import Elm.Syntax.Pattern exposing (Pattern(..)) +import ElmSyntaxHelpers +import Json.Encode as Encode exposing (Value) +import Review.ModuleNameLookupTable as LookupTable exposing (ModuleNameLookupTable) +import Review.Rule as Rule exposing (Rule) +import RuleConfig exposing (AnalyzerRule(..), RuleConfig) +import Set exposing (Set) + + +type alias Tag = + String + + +type alias ProjectContext = + Set Tag + + +type alias ModuleContext = + { lookupTable : ModuleNameLookupTable + , tags : Set Tag + } + + +ruleConfig : RuleConfig +ruleConfig = + { restrictToFiles = Nothing + , rules = + [ TagRule commonTagsRule + , TagRule expressionTagsRule + ] + } + + +commonTagsRule : Rule +commonTagsRule = + Rule.newProjectRuleSchema "Tags" commonTags + |> Rule.withModuleVisitor (Rule.withSimpleModuleDefinitionVisitor (always [])) + |> Rule.withModuleContextUsingContextCreator + { fromModuleToProject = fromModuleToProject + , fromProjectToModule = fromProjectToModule + , foldProjectContexts = Set.union + } + |> Rule.withDataExtractor dataExtractor + |> Rule.fromProjectRuleSchema + + +commonTags : Set Tag +commonTags = + Set.fromList + [ "paradigm:functional" + , "technique:immutability" + , "uses:module" + ] + + +fromProjectToModule : Rule.ContextCreator ProjectContext ModuleContext +fromProjectToModule = + Rule.initContextCreator + (\lookupTable _ -> + { lookupTable = lookupTable + , tags = Set.empty + } + ) + |> Rule.withModuleNameLookupTable + + +fromModuleToProject : Rule.ContextCreator ModuleContext ProjectContext +fromModuleToProject = + Rule.initContextCreator (\{ tags } -> tags) + + +dataExtractor : ProjectContext -> Value +dataExtractor = + Set.toList >> Encode.list Encode.string + + +expressionTagsRule : Rule +expressionTagsRule = + Rule.newProjectRuleSchema "Tags" Set.empty + |> Rule.withModuleVisitor (Rule.withExpressionEnterVisitor expressionVisitor) + |> Rule.withModuleContextUsingContextCreator + { fromModuleToProject = fromModuleToProject + , fromProjectToModule = fromProjectToModule + , foldProjectContexts = Set.union + } + |> Rule.withDataExtractor dataExtractor + |> Rule.fromProjectRuleSchema + + +expressionVisitor : Node Expression -> ModuleContext -> ( List never, ModuleContext ) +expressionVisitor ((Node range expression) as node) ({ lookupTable, tags } as context) = + case expression of + FunctionOrValue _ name -> + case LookupTable.moduleNameFor lookupTable node of + Nothing -> + ( [], { context | tags = Set.union tags (matchExpression node) } ) + + Just originalModuleName -> + ( [], { context | tags = Set.union tags (matchExpression (Node range (FunctionOrValue originalModuleName name))) } ) + + _ -> + ( [], { context | tags = Set.union tags (matchExpression node) } ) + + +matchExpression : Node Expression -> Set Tag +matchExpression (Node range expression) = + case expression of + FunctionOrValue [ "Set" ] _ -> + Set.fromList [ "construct:set", "technique:immutable-collection", "technique:sorted-collection" ] + + FunctionOrValue [ "Dict" ] _ -> + Set.fromList [ "construct:dictionary", "technique:immutable-collection", "technique:sorted-collection" ] + + FunctionOrValue [ "Random" ] _ -> + Set.singleton "technique:randomness" + + FunctionOrValue [ "Regex" ] _ -> + Set.singleton "technique:regular-expression" + + FunctionOrValue [ "Debug" ] _ -> + Set.singleton "uses:debug" + + PrefixOperator "+" -> + Set.singleton "construct:add" + + OperatorApplication "+" _ _ _ -> + Set.singleton "construct:add" + + PrefixOperator "-" -> + Set.singleton "construct:subtract" + + OperatorApplication "-" _ _ _ -> + Set.singleton "construct:subtract" + + PrefixOperator "*" -> + Set.singleton "construct:multiply" + + OperatorApplication "*" _ _ _ -> + Set.singleton "construct:multiply" + + PrefixOperator "/" -> + Set.singleton "construct:divide" + + OperatorApplication "/" _ _ _ -> + Set.singleton "construct:divide" + + PrefixOperator "//" -> + Set.singleton "construct:divide" + + OperatorApplication "//" _ _ _ -> + Set.singleton "construct:divide" + + PrefixOperator "==" -> + Set.fromList [ "construct:equality", "technique:equality-comparison", "construct:boolean" ] + + OperatorApplication "==" _ _ _ -> + Set.fromList [ "construct:equality", "technique:equality-comparison", "construct:boolean" ] + + PrefixOperator "/=" -> + Set.fromList [ "construct:inequality", "technique:equality-comparison", "construct:boolean" ] + + OperatorApplication "/=" _ _ _ -> + Set.fromList [ "construct:inequality", "technique:equality-comparison", "construct:boolean" ] + + UnitExpr -> + Set.singleton "uses:unit" + + Floatable _ -> + Set.fromList [ "construct:float", "construct:floating-point-number" ] + + Integer _ -> + Set.fromList [ "construct:integral-number", "construct:int" ] + + Hex _ -> + Set.fromList [ "construct:hexadecimal-number", "construct:integral-number", "construct:int" ] + + Negation _ -> + Set.singleton "construct:unary-minus" + + Literal _ -> + if range.end.row > range.start.row then + Set.fromList [ "construct:string", "construct:multiline-string" ] + + else + Set.singleton "construct:string" + + LambdaExpression _ -> + Set.singleton "construct:lambda" + + IfBlock _ _ _ -> + Set.fromList [ "construct:if", "construct:boolean" ] + + LetExpression { declarations } -> + if List.any (Node.value >> usesProperDestructuring) declarations then + Set.fromList [ "uses:destructure", "construct:pattern-matching" ] + + else + Set.empty + + _ -> + Set.empty + + +usesProperDestructuring : LetDeclaration -> Bool +usesProperDestructuring letDeclaration = + case letDeclaration of + LetDestructuring _ _ -> + True + + LetFunction { declaration } -> + declaration + |> Node.value + |> .arguments + |> List.any ElmSyntaxHelpers.hasDestructuringPattern diff --git a/tests/TagsTest.elm b/tests/TagsTest.elm new file mode 100644 index 0000000..d6f8347 --- /dev/null +++ b/tests/TagsTest.elm @@ -0,0 +1,220 @@ +module TagsTest exposing (tests) + +import Comment exposing (CommentType(..)) +import Review.Test +import Tags +import Test exposing (Test, describe, test) + + +tests : Test +tests = + describe "TagsTest tests" + [ commonTags + , expressionTags + ] + + +commonTags : Test +commonTags = + let + data = + """ +[ + "paradigm:functional", + "technique:immutability", + "uses:module" +] +""" + in + describe "should always return the initialTags" + [ test "for an empty module" <| + \() -> + "module A exposing (..)" + |> Review.Test.run Tags.commonTagsRule + |> Review.Test.expectDataExtract data + , test "for an non-empty module" <| + \() -> + """ +module TwoFer exposing (twoFer) + +twoFer : Maybe String -> String +twoFer name = + "One for " + ++ Maybe.withDefault "you" name + ++ ", one for me." +""" + |> Review.Test.run Tags.commonTagsRule + |> Review.Test.expectDataExtract data + ] + + +expressionTags : Test +expressionTags = + let + commonStart = + """ +module A exposing (..) + +import Set +import Dict +import Random +import Regex +import Debug + """ + + expectData function data = + commonStart + ++ function + |> Review.Test.run Tags.expressionTagsRule + |> Review.Test.expectDataExtract data + in + describe "matching expressions" + [ test "using Set function" <| + \() -> + expectData "f = Set.empty" + "[ \"construct:set\", \"technique:immutable-collection\", \"technique:sorted-collection\" ]" + , test "using Dict function" <| + \() -> + expectData "f = Dict.empty" + "[ \"construct:dictionary\", \"technique:immutable-collection\", \"technique:sorted-collection\" ]" + , test "using Random function" <| + \() -> + expectData "f = Random.constant" + "[ \"technique:randomness\" ]" + , test "using Regex function" <| + \() -> + expectData "f = Regex.fromString" + "[ \"technique:regular-expression\" ]" + , test "using inline +" <| + \() -> + expectData "f a b = a + b" + "[ \"construct:add\" ]" + , test "using prefix +" <| + \() -> + expectData "f = (+)" + "[ \"construct:add\" ]" + , test "using inline -" <| + \() -> + expectData "f a b = a - b" + "[ \"construct:subtract\" ]" + , test "using prefix -" <| + \() -> + expectData "f = (-)" + "[ \"construct:subtract\" ]" + , test "using inline *" <| + \() -> + expectData "f a b = a * b" + "[ \"construct:multiply\" ]" + , test "using prefix *" <| + \() -> + expectData "f = (*)" + "[ \"construct:multiply\" ]" + , test "using inline /" <| + \() -> + expectData "f a b = a / b" + "[ \"construct:divide\" ]" + , test "using prefix /" <| + \() -> + expectData "f = (/)" + "[ \"construct:divide\" ]" + , test "using inline //" <| + \() -> + expectData "f a b = a // b" + "[ \"construct:divide\" ]" + , test "using prefix //" <| + \() -> + expectData "f = (//)" + "[ \"construct:divide\" ]" + , test "using inline ==" <| + \() -> + expectData "f a b = a == b" + "[ \"construct:boolean\", \"construct:equality\", \"technique:equality-comparison\" ]" + , test "using prefix ==" <| + \() -> + expectData "f = (==)" + "[ \"construct:boolean\", \"construct:equality\", \"technique:equality-comparison\" ]" + , test "using inline /=" <| + \() -> + expectData "f a b = a /= b" + "[ \"construct:boolean\", \"construct:inequality\", \"technique:equality-comparison\" ]" + , test "using ()" <| + \() -> + expectData "f = ()" + "[ \"uses:unit\" ]" + , test "using float" <| + \() -> + expectData "f = 3.14" + "[ \"construct:float\", \"construct:floating-point-number\" ]" + , test "using float scientific notation" <| + \() -> + expectData "f = 314e-2" + "[ \"construct:float\", \"construct:floating-point-number\" ]" + , test "using int" <| + \() -> + expectData "f = 42" + "[ \"construct:int\", \"construct:integral-number\" ]" + , test "using int with hex notation" <| + \() -> + expectData "f = 0x42" + "[ \"construct:hexadecimal-number\", \"construct:int\", \"construct:integral-number\" ]" + , test "using negation and int" <| + \() -> + expectData "f = -42" + "[ \"construct:int\", \"construct:integral-number\", \"construct:unary-minus\" ]" + , test "using a string literal" <| + \() -> + expectData "f = \"hello\"" + "[ \"construct:string\" ]" + , test "using mutiline string literal" <| + \() -> + expectData "f = \"\"\"\nhello\nworld\"\"\"" + "[ \"construct:multiline-string\", \"construct:string\" ]" + , test "using lambda" <| + \() -> + expectData "f x = (\\_ -> x)" + "[ \"construct:lambda\" ]" + , test "using if block" <| + \() -> + expectData "f x y z = if x then y else z" + "[ \"construct:boolean\", \"construct:if\" ]" + , test "let block without proper destructuring" <| + \() -> + expectData "f x y = let z = y in z" + "[]" + , test "let block with tuple destructuring" <| + \() -> + expectData "f y = let (a, b) = y in a" + "[ \"construct:pattern-matching\", \"uses:destructure\"]" + , test "let block with record destructuring" <| + \() -> + expectData "f y = let {a, b} = y in a" + "[ \"construct:pattern-matching\", \"uses:destructure\"]" + , test "let block with uncons destructuring" <| + \() -> + expectData "f y = let a :: b = y in a" + "[ \"construct:pattern-matching\", \"uses:destructure\"]" + , test "let block with named destructuring" <| + \() -> + expectData "f y = let Thing a = y in a" + "[ \"construct:pattern-matching\", \"uses:destructure\"]" + , test "let block with nested destructuring" <| + \() -> + expectData "f y = let (Thing a) = y in a" + "[ \"construct:pattern-matching\", \"uses:destructure\"]" + , test "let block with a function with record destructuring" <| + \() -> + expectData "f y = let f2 { a } = a in f2 y" + "[ \"construct:pattern-matching\", \"uses:destructure\"]" + , test "let block with a function with tuple destructuring" <| + \() -> + expectData "f y = let f2 (a, b) = a in f2 y" + "[ \"construct:pattern-matching\", \"uses:destructure\"]" + , test "let block with a function with uncons destructuring" <| + \() -> + expectData "f y = let f2 (a :: b) = a in f2 y" + "[ \"construct:pattern-matching\", \"uses:destructure\"]" + , test "let block with a function with named destructuring" <| + \() -> + expectData "f y = let f2 (Thing a) = a in f2 y" + "[ \"construct:pattern-matching\", \"uses:destructure\"]" + ] From 9daf799cc4de1dfecf79980bf85abe2603e9fbf1 Mon Sep 17 00:00:00 2001 From: Jeremie Gillet Date: Thu, 23 Nov 2023 19:11:10 +0900 Subject: [PATCH 03/17] compare sorted expected values --- bin/check_files.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/check_files.sh b/bin/check_files.sh index 2be02c3..e71c763 100755 --- a/bin/check_files.sh +++ b/bin/check_files.sh @@ -37,8 +37,8 @@ function main { exit 1 fi - jq -S . ${exercise}/expected_analysis.json > /tmp/expected.json - jq -S . ${exercise}/analysis.json > /tmp/actual.json + jq -S '.comments |= sort' ${exercise}/expected_analysis.json > /tmp/expected.json + jq -S '.comments |= sort' ${exercise}/analysis.json > /tmp/actual.json if ! diff /tmp/expected.json /tmp/actual.json ;then echo "🔥 ${exercise}: expected ${exercise}/analysis.json to equal ${exercise}/expected_analysis.json on successful run 🔥" exit 1 From 8f6c54e46282a0437cf0c97596f723713971fadc Mon Sep 17 00:00:00 2001 From: Jeremie Gillet Date: Thu, 23 Nov 2023 20:49:23 +0900 Subject: [PATCH 04/17] remove unused things --- src/Tags.elm | 1 - tests/TagsTest.elm | 1 - 2 files changed, 2 deletions(-) diff --git a/src/Tags.elm b/src/Tags.elm index 86ab782..1a02b6e 100644 --- a/src/Tags.elm +++ b/src/Tags.elm @@ -2,7 +2,6 @@ module Tags exposing (commonTagsRule, expressionTagsRule, ruleConfig) import Elm.Syntax.Expression exposing (Expression(..), LetDeclaration(..)) import Elm.Syntax.Node as Node exposing (Node(..)) -import Elm.Syntax.Pattern exposing (Pattern(..)) import ElmSyntaxHelpers import Json.Encode as Encode exposing (Value) import Review.ModuleNameLookupTable as LookupTable exposing (ModuleNameLookupTable) diff --git a/tests/TagsTest.elm b/tests/TagsTest.elm index d6f8347..d1fb83d 100644 --- a/tests/TagsTest.elm +++ b/tests/TagsTest.elm @@ -1,6 +1,5 @@ module TagsTest exposing (tests) -import Comment exposing (CommentType(..)) import Review.Test import Tags import Test exposing (Test, describe, test) From aa906dcf60afd8525d61d2ea4f29653b63019bca Mon Sep 17 00:00:00 2001 From: Jeremie Gillet Date: Sat, 25 Nov 2023 09:21:40 +0900 Subject: [PATCH 05/17] ProjectContext become record type --- src/Tags.elm | 41 +++++++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/src/Tags.elm b/src/Tags.elm index 1a02b6e..7282de0 100644 --- a/src/Tags.elm +++ b/src/Tags.elm @@ -15,7 +15,8 @@ type alias Tag = type alias ProjectContext = - Set Tag + { tags : Set Tag + } type alias ModuleContext = @@ -36,24 +37,26 @@ ruleConfig = commonTagsRule : Rule commonTagsRule = - Rule.newProjectRuleSchema "Tags" commonTags + Rule.newProjectRuleSchema "Tags" commonTagsProjectContext |> Rule.withModuleVisitor (Rule.withSimpleModuleDefinitionVisitor (always [])) |> Rule.withModuleContextUsingContextCreator { fromModuleToProject = fromModuleToProject , fromProjectToModule = fromProjectToModule - , foldProjectContexts = Set.union + , foldProjectContexts = foldProjectContexts } |> Rule.withDataExtractor dataExtractor |> Rule.fromProjectRuleSchema -commonTags : Set Tag -commonTags = - Set.fromList - [ "paradigm:functional" - , "technique:immutability" - , "uses:module" - ] +commonTagsProjectContext : ProjectContext +commonTagsProjectContext = + ProjectContext + (Set.fromList + [ "paradigm:functional" + , "technique:immutability" + , "uses:module" + ] + ) fromProjectToModule : Rule.ContextCreator ProjectContext ModuleContext @@ -69,27 +72,37 @@ fromProjectToModule = fromModuleToProject : Rule.ContextCreator ModuleContext ProjectContext fromModuleToProject = - Rule.initContextCreator (\{ tags } -> tags) + Rule.initContextCreator (\{ tags } -> ProjectContext tags) + + +foldProjectContexts : ProjectContext -> ProjectContext -> ProjectContext +foldProjectContexts a b = + ProjectContext (Set.union a.tags b.tags) dataExtractor : ProjectContext -> Value dataExtractor = - Set.toList >> Encode.list Encode.string + .tags >> Set.toList >> Encode.list Encode.string expressionTagsRule : Rule expressionTagsRule = - Rule.newProjectRuleSchema "Tags" Set.empty + Rule.newProjectRuleSchema "Tags" emptyProjectContext |> Rule.withModuleVisitor (Rule.withExpressionEnterVisitor expressionVisitor) |> Rule.withModuleContextUsingContextCreator { fromModuleToProject = fromModuleToProject , fromProjectToModule = fromProjectToModule - , foldProjectContexts = Set.union + , foldProjectContexts = foldProjectContexts } |> Rule.withDataExtractor dataExtractor |> Rule.fromProjectRuleSchema +emptyProjectContext : ProjectContext +emptyProjectContext = + ProjectContext Set.empty + + expressionVisitor : Node Expression -> ModuleContext -> ( List never, ModuleContext ) expressionVisitor ((Node range expression) as node) ({ lookupTable, tags } as context) = case expression of From bd4af55b6e3791c3abb373bf8ecebb0d45c7a576 Mon Sep 17 00:00:00 2001 From: Jeremie Gillet Date: Sat, 25 Nov 2023 09:43:35 +0900 Subject: [PATCH 06/17] Add smoke test mechanism --- bin/check_files.sh | 17 +++++++++++++++++ bin/smoke_test.sh | 2 +- .../imperfect_solution/expected_tags.json | 5 +++++ .../strain/perfect_solution/expected_tags.json | 5 +++++ .../imperfect_solution/expected_tags.json | 5 +++++ .../two-fer/perfect_solution/expected_tags.json | 5 +++++ 6 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 test_data/strain/imperfect_solution/expected_tags.json create mode 100644 test_data/strain/perfect_solution/expected_tags.json create mode 100644 test_data/two-fer/imperfect_solution/expected_tags.json create mode 100644 test_data/two-fer/perfect_solution/expected_tags.json diff --git a/bin/check_files.sh b/bin/check_files.sh index e71c763..cffdae3 100755 --- a/bin/check_files.sh +++ b/bin/check_files.sh @@ -44,6 +44,23 @@ function main { exit 1 fi + if [[ ! -f "${exercise}/expected_tags.json" ]]; then + echo "🔥 ${exercise}: expected expected_tags.json to exist 🔥" + exit 1 + fi + + if [[ ! -f "${exercise}/tags.json" ]]; then + echo "🔥 ${exercise}: expected tags.json to exist on successful run 🔥" + exit 1 + fi + + jq -S 'sort' ${exercise}/expected_tags.json > /tmp/expected.json + jq -S 'sort' ${exercise}/tags.json > /tmp/actual.json + if ! diff /tmp/expected.json /tmp/actual.json ;then + echo "🔥 ${exercise}: expected ${exercise}/tags.json to equal ${exercise}/expected_tags.json on successful run 🔥" + exit 1 + fi + echo "🏁 ${exercise}: expected files present and correct after successful run 🏁" } diff --git a/bin/smoke_test.sh b/bin/smoke_test.sh index 8b1748c..31c6d36 100755 --- a/bin/smoke_test.sh +++ b/bin/smoke_test.sh @@ -8,7 +8,7 @@ set -o pipefail # Catch failures in pipes. for solution in test_data/*/* ; do slug=$(basename $(dirname $solution)) # run analysis - bin/run.sh $slug $solution $solution + bin/run.sh $slug $solution $solution > /dev/null # check result bin/check_files.sh $solution done diff --git a/test_data/strain/imperfect_solution/expected_tags.json b/test_data/strain/imperfect_solution/expected_tags.json new file mode 100644 index 0000000..7f6b1ec --- /dev/null +++ b/test_data/strain/imperfect_solution/expected_tags.json @@ -0,0 +1,5 @@ +[ + "paradigm:functional", + "technique:immutability", + "uses:module" +] diff --git a/test_data/strain/perfect_solution/expected_tags.json b/test_data/strain/perfect_solution/expected_tags.json new file mode 100644 index 0000000..7f6b1ec --- /dev/null +++ b/test_data/strain/perfect_solution/expected_tags.json @@ -0,0 +1,5 @@ +[ + "paradigm:functional", + "technique:immutability", + "uses:module" +] diff --git a/test_data/two-fer/imperfect_solution/expected_tags.json b/test_data/two-fer/imperfect_solution/expected_tags.json new file mode 100644 index 0000000..7f6b1ec --- /dev/null +++ b/test_data/two-fer/imperfect_solution/expected_tags.json @@ -0,0 +1,5 @@ +[ + "paradigm:functional", + "technique:immutability", + "uses:module" +] diff --git a/test_data/two-fer/perfect_solution/expected_tags.json b/test_data/two-fer/perfect_solution/expected_tags.json new file mode 100644 index 0000000..7f6b1ec --- /dev/null +++ b/test_data/two-fer/perfect_solution/expected_tags.json @@ -0,0 +1,5 @@ +[ + "paradigm:functional", + "technique:immutability", + "uses:module" +] From 6a64b85a9c0279ce13469ae4927ffb73f586a137 Mon Sep 17 00:00:00 2001 From: Jeremie Gillet Date: Sat, 25 Nov 2023 11:59:24 +0900 Subject: [PATCH 07/17] Bug fix: don't overwrite extracts --- bin/run.sh | 2 +- src/Tags.elm | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bin/run.sh b/bin/run.sh index 18e90f1..82b3c9d 100755 --- a/bin/run.sh +++ b/bin/run.sh @@ -35,7 +35,7 @@ npx elm-review $INPUT_DIR \ cat /tmp/elm-review-report.json | node ./bin/cli.js > $OUTPUT_DIR/analysis.json # Get tags -jq '.extracts .Tags' /tmp/elm-review-report.json > $OUTPUT_DIR/tags.json +jq '.extracts | to_entries | map(.value) | add' /tmp/elm-review-report.json > $OUTPUT_DIR/tags.json set -e diff --git a/src/Tags.elm b/src/Tags.elm index 7282de0..ef5149a 100644 --- a/src/Tags.elm +++ b/src/Tags.elm @@ -37,7 +37,7 @@ ruleConfig = commonTagsRule : Rule commonTagsRule = - Rule.newProjectRuleSchema "Tags" commonTagsProjectContext + Rule.newProjectRuleSchema "commonTags" commonTagsProjectContext |> Rule.withModuleVisitor (Rule.withSimpleModuleDefinitionVisitor (always [])) |> Rule.withModuleContextUsingContextCreator { fromModuleToProject = fromModuleToProject @@ -87,7 +87,7 @@ dataExtractor = expressionTagsRule : Rule expressionTagsRule = - Rule.newProjectRuleSchema "Tags" emptyProjectContext + Rule.newProjectRuleSchema "expressionTags" emptyProjectContext |> Rule.withModuleVisitor (Rule.withExpressionEnterVisitor expressionVisitor) |> Rule.withModuleContextUsingContextCreator { fromModuleToProject = fromModuleToProject From b9afae730d58f0b0c5cfc1a65a7ffb9d8c7426a4 Mon Sep 17 00:00:00 2001 From: Jeremie Gillet Date: Sat, 25 Nov 2023 12:13:14 +0900 Subject: [PATCH 08/17] tags ignore files that errors ignore --- src/Tags.elm | 72 ++++++++++++++++++++++++++++++---------------------- 1 file changed, 42 insertions(+), 30 deletions(-) diff --git a/src/Tags.elm b/src/Tags.elm index ef5149a..5e8e56f 100644 --- a/src/Tags.elm +++ b/src/Tags.elm @@ -1,6 +1,7 @@ module Tags exposing (commonTagsRule, expressionTagsRule, ruleConfig) import Elm.Syntax.Expression exposing (Expression(..), LetDeclaration(..)) +import Elm.Syntax.Module exposing (Module) import Elm.Syntax.Node as Node exposing (Node(..)) import ElmSyntaxHelpers import Json.Encode as Encode exposing (Value) @@ -37,8 +38,8 @@ ruleConfig = commonTagsRule : Rule commonTagsRule = - Rule.newProjectRuleSchema "commonTags" commonTagsProjectContext - |> Rule.withModuleVisitor (Rule.withSimpleModuleDefinitionVisitor (always [])) + Rule.newProjectRuleSchema "commonTags" emptyProjectContext + |> Rule.withModuleVisitor (Rule.withModuleDefinitionVisitor commonModuleVisitor) |> Rule.withModuleContextUsingContextCreator { fromModuleToProject = fromModuleToProject , fromProjectToModule = fromProjectToModule @@ -48,15 +49,36 @@ commonTagsRule = |> Rule.fromProjectRuleSchema -commonTagsProjectContext : ProjectContext -commonTagsProjectContext = - ProjectContext - (Set.fromList - [ "paradigm:functional" - , "technique:immutability" - , "uses:module" - ] - ) +expressionTagsRule : Rule +expressionTagsRule = + Rule.newProjectRuleSchema "expressionTags" emptyProjectContext + |> Rule.withModuleVisitor (Rule.withExpressionEnterVisitor expressionVisitor) + |> Rule.withModuleContextUsingContextCreator + { fromModuleToProject = fromModuleToProject + , fromProjectToModule = fromProjectToModule + , foldProjectContexts = foldProjectContexts + } + |> Rule.withDataExtractor dataExtractor + |> Rule.fromProjectRuleSchema + + +emptyProjectContext : ProjectContext +emptyProjectContext = + ProjectContext Set.empty + + +commonModuleVisitor : Node Module -> ModuleContext -> ( List never, ModuleContext ) +commonModuleVisitor _ context = + ( [], { context | tags = commonTags } ) + + +commonTags : Set Tag +commonTags = + Set.fromList + [ "paradigm:functional" + , "technique:immutability" + , "uses:module" + ] fromProjectToModule : Rule.ContextCreator ProjectContext ModuleContext @@ -72,7 +94,15 @@ fromProjectToModule = fromModuleToProject : Rule.ContextCreator ModuleContext ProjectContext fromModuleToProject = - Rule.initContextCreator (\{ tags } -> ProjectContext tags) + Rule.initContextCreator + (\isFileIgnored { tags } -> + if isFileIgnored then + emptyProjectContext + + else + ProjectContext tags + ) + |> Rule.withIsFileIgnored foldProjectContexts : ProjectContext -> ProjectContext -> ProjectContext @@ -85,24 +115,6 @@ dataExtractor = .tags >> Set.toList >> Encode.list Encode.string -expressionTagsRule : Rule -expressionTagsRule = - Rule.newProjectRuleSchema "expressionTags" emptyProjectContext - |> Rule.withModuleVisitor (Rule.withExpressionEnterVisitor expressionVisitor) - |> Rule.withModuleContextUsingContextCreator - { fromModuleToProject = fromModuleToProject - , fromProjectToModule = fromProjectToModule - , foldProjectContexts = foldProjectContexts - } - |> Rule.withDataExtractor dataExtractor - |> Rule.fromProjectRuleSchema - - -emptyProjectContext : ProjectContext -emptyProjectContext = - ProjectContext Set.empty - - expressionVisitor : Node Expression -> ModuleContext -> ( List never, ModuleContext ) expressionVisitor ((Node range expression) as node) ({ lookupTable, tags } as context) = case expression of From ad9738c5b164929f0242bd6d51193269ff28bf17 Mon Sep 17 00:00:00 2001 From: Jeremie Gillet Date: Sat, 25 Nov 2023 12:34:33 +0900 Subject: [PATCH 09/17] Remove Tag alias --- src/Tags.elm | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/Tags.elm b/src/Tags.elm index 5e8e56f..9b6a94d 100644 --- a/src/Tags.elm +++ b/src/Tags.elm @@ -11,18 +11,14 @@ import RuleConfig exposing (AnalyzerRule(..), RuleConfig) import Set exposing (Set) -type alias Tag = - String - - type alias ProjectContext = - { tags : Set Tag + { tags : Set String } type alias ModuleContext = { lookupTable : ModuleNameLookupTable - , tags : Set Tag + , tags : Set String } @@ -72,7 +68,7 @@ commonModuleVisitor _ context = ( [], { context | tags = commonTags } ) -commonTags : Set Tag +commonTags : Set String commonTags = Set.fromList [ "paradigm:functional" @@ -130,7 +126,7 @@ expressionVisitor ((Node range expression) as node) ({ lookupTable, tags } as co ( [], { context | tags = Set.union tags (matchExpression node) } ) -matchExpression : Node Expression -> Set Tag +matchExpression : Node Expression -> Set String matchExpression (Node range expression) = case expression of FunctionOrValue [ "Set" ] _ -> From 2d620b1335c20a9c92e8eee0aa12f59a7f5c31fc Mon Sep 17 00:00:00 2001 From: Jeremie Gillet Date: Sat, 25 Nov 2023 13:53:18 +0900 Subject: [PATCH 10/17] Separate type expressions --- src/Tags.elm | 155 +++++++++++++++++++++++------- tests/TagsTest.elm | 230 ++++++++++++++++++++++++++++++--------------- 2 files changed, 274 insertions(+), 111 deletions(-) diff --git a/src/Tags.elm b/src/Tags.elm index 9b6a94d..ef17984 100644 --- a/src/Tags.elm +++ b/src/Tags.elm @@ -113,22 +113,133 @@ dataExtractor = expressionVisitor : Node Expression -> ModuleContext -> ( List never, ModuleContext ) expressionVisitor ((Node range expression) as node) ({ lookupTable, tags } as context) = + let + matches n = + Set.union (matchExpressionType n) (matchExpression n) + in case expression of FunctionOrValue _ name -> case LookupTable.moduleNameFor lookupTable node of Nothing -> - ( [], { context | tags = Set.union tags (matchExpression node) } ) + ( [], { context | tags = Set.union tags (matches node) } ) Just originalModuleName -> - ( [], { context | tags = Set.union tags (matchExpression (Node range (FunctionOrValue originalModuleName name))) } ) + ( [], { context | tags = Set.union tags (matches (Node range (FunctionOrValue originalModuleName name))) } ) _ -> - ( [], { context | tags = Set.union tags (matchExpression node) } ) + ( [], { context | tags = Set.union tags (matches node) } ) + + +{-| Only looks at the type of the expression, not the content +-} +matchExpressionType : Node Expression -> Set String +matchExpressionType (Node range expression) = + case expression of + Application _ -> + Set.singleton "uses:function-application" + + OperatorApplication _ _ _ _ -> + Set.singleton "uses:function-application" + + PrefixOperator _ -> + Set.singleton "uses:prefix-operator" + + UnitExpr -> + Set.singleton "uses:unit" + + Floatable _ -> + Set.fromList [ "construct:float", "construct:floating-point-number" ] + + Integer _ -> + Set.fromList [ "construct:integral-number", "construct:int" ] + + Hex _ -> + Set.fromList [ "construct:hexadecimal-number", "construct:integral-number", "construct:int" ] + + Negation _ -> + Set.singleton "construct:unary-minus" + + Literal _ -> + if range.end.row > range.start.row then + Set.fromList [ "construct:string", "construct:multiline-string" ] + + else + Set.singleton "construct:string" + + LambdaExpression _ -> + Set.singleton "construct:lambda" + + IfBlock _ _ _ -> + Set.fromList [ "construct:if", "construct:boolean" ] + + LetExpression _ -> + Set.singleton "construct:assignment" + + CharLiteral _ -> + Set.singleton "construct:char" + + TupledExpression _ -> + Set.singleton "construct:tuple" + + CaseExpression _ -> + Set.singleton "construct:pattern-matching" + + RecordExpr _ -> + Set.singleton "construct:record" + + RecordAccess _ _ -> + Set.fromList [ "construct:record", "uses:record-access" ] + + RecordAccessFunction _ -> + Set.fromList [ "construct:record", "uses:record-access", "uses:record-access-function" ] + + RecordUpdateExpression _ _ -> + Set.fromList [ "construct:record", "uses:record-update" ] + + ListExpr _ -> + Set.singleton "construct:list" + + GLSLExpression _ -> + Set.singleton "uses:glsl" + + FunctionOrValue _ _ -> + Set.empty + + ParenthesizedExpression _ -> + Set.empty + + -- not possible to get in practice + Operator _ -> + Set.empty matchExpression : Node Expression -> Set String -matchExpression (Node range expression) = +matchExpression (Node _ expression) = case expression of + FunctionOrValue [ "Bitwise" ] "and" -> + Set.fromList [ "construct:bit-manipulation", "construct:bitwise-and" ] + + FunctionOrValue [ "Bitwise" ] "or" -> + Set.fromList [ "construct:bit-manipulation", "construct:bitwise-or" ] + + FunctionOrValue [ "Bitwise" ] "xor" -> + Set.fromList [ "construct:bit-manipulation", "construct:bitwise-xor" ] + + FunctionOrValue [ "Bitwise" ] "complement" -> + Set.fromList [ "construct:bit-manipulation", "construct:bitwise-not" ] + + FunctionOrValue [ "Bitwise" ] "shiftLeftBy" -> + Set.fromList [ "construct:bit-manipulation", "technique:bit-shifting", "construct:bitwise-left-shift" ] + + FunctionOrValue [ "Bitwise" ] "shiftRightBy" -> + Set.fromList [ "construct:bit-manipulation", "technique:bit-shifting", "construct:bitwise-right-shift" ] + + FunctionOrValue [ "Bitwise" ] "shiftRightZfBy" -> + Set.fromList [ "construct:bit-manipulation", "technique:bit-shifting" ] + + FunctionOrValue [ "Array" ] _ -> + Set.fromList [ "construct:array", "technique:immutable-collection" ] + FunctionOrValue [ "Set" ] _ -> Set.fromList [ "construct:set", "technique:immutable-collection", "technique:sorted-collection" ] @@ -144,6 +255,12 @@ matchExpression (Node range expression) = FunctionOrValue [ "Debug" ] _ -> Set.singleton "uses:debug" + PrefixOperator "&&" -> + Set.fromList [ "construct:construct:boolean", "technique:boolean-logic" ] + + OperatorApplication "&&" _ _ _ -> + Set.fromList [ "construct:construct:boolean", "technique:boolean-logic" ] + PrefixOperator "+" -> Set.singleton "construct:add" @@ -186,37 +303,9 @@ matchExpression (Node range expression) = OperatorApplication "/=" _ _ _ -> Set.fromList [ "construct:inequality", "technique:equality-comparison", "construct:boolean" ] - UnitExpr -> - Set.singleton "uses:unit" - - Floatable _ -> - Set.fromList [ "construct:float", "construct:floating-point-number" ] - - Integer _ -> - Set.fromList [ "construct:integral-number", "construct:int" ] - - Hex _ -> - Set.fromList [ "construct:hexadecimal-number", "construct:integral-number", "construct:int" ] - - Negation _ -> - Set.singleton "construct:unary-minus" - - Literal _ -> - if range.end.row > range.start.row then - Set.fromList [ "construct:string", "construct:multiline-string" ] - - else - Set.singleton "construct:string" - - LambdaExpression _ -> - Set.singleton "construct:lambda" - - IfBlock _ _ _ -> - Set.fromList [ "construct:if", "construct:boolean" ] - LetExpression { declarations } -> if List.any (Node.value >> usesProperDestructuring) declarations then - Set.fromList [ "uses:destructure", "construct:pattern-matching" ] + Set.fromList [ "construct:destructuring", "construct:pattern-matching" ] else Set.empty diff --git a/tests/TagsTest.elm b/tests/TagsTest.elm index d1fb83d..f061694 100644 --- a/tests/TagsTest.elm +++ b/tests/TagsTest.elm @@ -1,5 +1,6 @@ module TagsTest exposing (tests) +import Expect exposing (Expectation) import Review.Test import Tags import Test exposing (Test, describe, test) @@ -9,6 +10,7 @@ tests : Test tests = describe "TagsTest tests" [ commonTags + , expressionTypeTags , expressionTags ] @@ -47,11 +49,9 @@ twoFer name = ] -expressionTags : Test -expressionTags = - let - commonStart = - """ +commonStart : String +commonStart = + """ module A exposing (..) import Set @@ -61,14 +61,120 @@ import Regex import Debug """ - expectData function data = - commonStart - ++ function - |> Review.Test.run Tags.expressionTagsRule - |> Review.Test.expectDataExtract data - in + +expectData : String -> String -> Expectation +expectData function data = + commonStart + ++ function + |> Review.Test.run Tags.expressionTagsRule + |> Review.Test.expectDataExtract data + + +expressionTypeTags : Test +expressionTypeTags = + describe "matching expression types" + [ test "FunctionOrValue is too general for a tag" <| + \() -> expectData "f x = x" "[]" + , test "ParenthesizedExpression is too general for a tag" <| + \() -> expectData "f x = (x)" "[]" + , test "GLSLExpression" <| + \() -> expectData "f = [glsl| void main () {} |]" "[ \"uses:glsl\" ]" + , test "Application" <| + \() -> expectData "f x y = x y" "[ \"uses:function-application\" ]" + , test "PrefixOperator" <| + \() -> expectData "f = (^)" "[ \"uses:prefix-operator\" ]" + , test "using ()" <| + \() -> expectData "f = ()" "[ \"uses:unit\" ]" + , test "using float" <| + \() -> + expectData "f = 3.14" "[ \"construct:float\", \"construct:floating-point-number\" ]" + , test "using float scientific notation" <| + \() -> + expectData "f = 314e-2" "[ \"construct:float\", \"construct:floating-point-number\" ]" + , test "using int" <| + \() -> + expectData "f = 42" "[ \"construct:int\", \"construct:integral-number\" ]" + , test "using int with hex notation" <| + \() -> + expectData "f = 0x42" + "[ \"construct:hexadecimal-number\", \"construct:int\", \"construct:integral-number\" ]" + , test "using negation and int" <| + \() -> + expectData "f = -42" + "[ \"construct:int\", \"construct:integral-number\", \"construct:unary-minus\" ]" + , test "using a string literal" <| + \() -> + expectData "f = \"hello\"" "[ \"construct:string\" ]" + , test "using mutiline string literal" <| + \() -> + expectData "f = \"\"\"\nhello\nworld\"\"\"" + "[ \"construct:multiline-string\", \"construct:string\" ]" + , test "using lambda" <| + \() -> expectData "f x = (\\_ -> x)" "[ \"construct:lambda\" ]" + , test "using if block" <| + \() -> + expectData "f x y z = if x then y else z" + "[ \"construct:boolean\", \"construct:if\" ]" + , test "using let block" <| + \() -> + expectData "f x y = let z = y in z" "[ \"construct:assignment\" ]" + , test "using char" <| + \() -> expectData "f = 'a'" "[ \"construct:char\" ]" + , test "using tuple" <| + \() -> expectData "f a b = (a, b)" "[ \"construct:tuple\" ]" + , test "using list" <| + \() -> expectData "f a b = [a, b]" "[ \"construct:list\" ]" + , test "using case" <| + \() -> expectData "f a = case a of\n b -> b" "[ \"construct:pattern-matching\" ]" + , test "using record" <| + \() -> expectData "f b = {a = b}" "[ \"construct:record\" ]" + , test "using record access" <| + \() -> expectData "f rec = rec.field" "[ \"construct:record\", \"uses:record-access\" ]" + , test "using record access function" <| + \() -> + expectData "f = .field" + "[ \"construct:record\", \"uses:record-access\", \"uses:record-access-function\" ]" + , test "using record update" <| + \() -> expectData "f a = {a | b = a}" "[ \"construct:record\", \"uses:record-update\" ]" + ] + + +expressionTags : Test +expressionTags = describe "matching expressions" - [ test "using Set function" <| + [ test "using Bitwise.and" <| + \() -> + expectData "f = Bitwise.and" + "[ \"construct:bit-manipulation\", \"construct:bitwise-and\" ]" + , test "using Bitwise.or" <| + \() -> + expectData "f = Bitwise.or" + "[ \"construct:bit-manipulation\", \"construct:bitwise-or\" ]" + , test "using Bitwise.xor" <| + \() -> + expectData "f = Bitwise.xor" + "[ \"construct:bit-manipulation\", \"construct:bitwise-xor\" ]" + , test "using Bitwise.complement" <| + \() -> + expectData "f = Bitwise.complement" + "[ \"construct:bit-manipulation\", \"construct:bitwise-not\" ]" + , test "using Bitwise.shiftLeftBy" <| + \() -> + expectData "f = Bitwise.shiftLeftBy" + "[ \"construct:bit-manipulation\", \"construct:bitwise-left-shift\" , \"technique:bit-shifting\"]" + , test "using Bitwise.shiftRightBy" <| + \() -> + expectData "f = Bitwise.shiftRightBy" + "[ \"construct:bit-manipulation\", \"construct:bitwise-right-shift\", \"technique:bit-shifting\" ]" + , test "using Bitwise.shiftRightZfBy" <| + \() -> + expectData "f = Bitwise.shiftRightZfBy" + "[ \"construct:bit-manipulation\", \"technique:bit-shifting\" ]" + , test "using Array function" <| + \() -> + expectData "f = Array.empty" + "[ \"construct:array\", \"technique:immutable-collection\" ]" + , test "using Set function" <| \() -> expectData "f = Set.empty" "[ \"construct:set\", \"technique:immutable-collection\", \"technique:sorted-collection\" ]" @@ -87,133 +193,101 @@ import Debug , test "using inline +" <| \() -> expectData "f a b = a + b" - "[ \"construct:add\" ]" + "[ \"construct:add\" , \"uses:function-application\"]" , test "using prefix +" <| \() -> expectData "f = (+)" - "[ \"construct:add\" ]" + "[ \"construct:add\", \"uses:prefix-operator\" ]" , test "using inline -" <| \() -> expectData "f a b = a - b" - "[ \"construct:subtract\" ]" + "[ \"construct:subtract\" , \"uses:function-application\"]" , test "using prefix -" <| \() -> expectData "f = (-)" - "[ \"construct:subtract\" ]" + "[ \"construct:subtract\", \"uses:prefix-operator\" ]" , test "using inline *" <| \() -> expectData "f a b = a * b" - "[ \"construct:multiply\" ]" + "[ \"construct:multiply\" , \"uses:function-application\"]" , test "using prefix *" <| \() -> expectData "f = (*)" - "[ \"construct:multiply\" ]" + "[ \"construct:multiply\", \"uses:prefix-operator\" ]" , test "using inline /" <| \() -> expectData "f a b = a / b" - "[ \"construct:divide\" ]" + "[ \"construct:divide\" , \"uses:function-application\"]" , test "using prefix /" <| \() -> expectData "f = (/)" - "[ \"construct:divide\" ]" + "[ \"construct:divide\", \"uses:prefix-operator\" ]" , test "using inline //" <| \() -> expectData "f a b = a // b" - "[ \"construct:divide\" ]" + "[ \"construct:divide\" , \"uses:function-application\"]" , test "using prefix //" <| \() -> expectData "f = (//)" - "[ \"construct:divide\" ]" + "[ \"construct:divide\", \"uses:prefix-operator\" ]" , test "using inline ==" <| \() -> expectData "f a b = a == b" - "[ \"construct:boolean\", \"construct:equality\", \"technique:equality-comparison\" ]" + "[ \"construct:boolean\", \"construct:equality\", \"technique:equality-comparison\" , \"uses:function-application\"]" , test "using prefix ==" <| \() -> expectData "f = (==)" - "[ \"construct:boolean\", \"construct:equality\", \"technique:equality-comparison\" ]" + "[ \"construct:boolean\", \"construct:equality\", \"technique:equality-comparison\", \"uses:prefix-operator\" ]" , test "using inline /=" <| \() -> expectData "f a b = a /= b" - "[ \"construct:boolean\", \"construct:inequality\", \"technique:equality-comparison\" ]" - , test "using ()" <| + "[ \"construct:boolean\", \"construct:inequality\", \"technique:equality-comparison\" , \"uses:function-application\"]" + , test "using prefix /=" <| \() -> - expectData "f = ()" - "[ \"uses:unit\" ]" - , test "using float" <| - \() -> - expectData "f = 3.14" - "[ \"construct:float\", \"construct:floating-point-number\" ]" - , test "using float scientific notation" <| - \() -> - expectData "f = 314e-2" - "[ \"construct:float\", \"construct:floating-point-number\" ]" - , test "using int" <| - \() -> - expectData "f = 42" - "[ \"construct:int\", \"construct:integral-number\" ]" - , test "using int with hex notation" <| - \() -> - expectData "f = 0x42" - "[ \"construct:hexadecimal-number\", \"construct:int\", \"construct:integral-number\" ]" - , test "using negation and int" <| - \() -> - expectData "f = -42" - "[ \"construct:int\", \"construct:integral-number\", \"construct:unary-minus\" ]" - , test "using a string literal" <| - \() -> - expectData "f = \"hello\"" - "[ \"construct:string\" ]" - , test "using mutiline string literal" <| - \() -> - expectData "f = \"\"\"\nhello\nworld\"\"\"" - "[ \"construct:multiline-string\", \"construct:string\" ]" - , test "using lambda" <| - \() -> - expectData "f x = (\\_ -> x)" - "[ \"construct:lambda\" ]" - , test "using if block" <| - \() -> - expectData "f x y z = if x then y else z" - "[ \"construct:boolean\", \"construct:if\" ]" + expectData "f = (/=)" + "[ \"construct:boolean\", \"construct:inequality\", \"technique:equality-comparison\", \"uses:prefix-operator\" ]" , test "let block without proper destructuring" <| \() -> expectData "f x y = let z = y in z" - "[]" + "[ \"construct:assignment\" ]" , test "let block with tuple destructuring" <| \() -> expectData "f y = let (a, b) = y in a" - "[ \"construct:pattern-matching\", \"uses:destructure\"]" + "[ \"construct:assignment\", \"construct:destructuring\", \"construct:pattern-matching\"]" , test "let block with record destructuring" <| \() -> expectData "f y = let {a, b} = y in a" - "[ \"construct:pattern-matching\", \"uses:destructure\"]" + "[ \"construct:assignment\", \"construct:destructuring\", \"construct:pattern-matching\"]" , test "let block with uncons destructuring" <| \() -> expectData "f y = let a :: b = y in a" - "[ \"construct:pattern-matching\", \"uses:destructure\"]" + "[ \"construct:assignment\", \"construct:destructuring\", \"construct:pattern-matching\"]" , test "let block with named destructuring" <| \() -> expectData "f y = let Thing a = y in a" - "[ \"construct:pattern-matching\", \"uses:destructure\"]" + "[ \"construct:assignment\", \"construct:destructuring\", \"construct:pattern-matching\"]" , test "let block with nested destructuring" <| \() -> expectData "f y = let (Thing a) = y in a" - "[ \"construct:pattern-matching\", \"uses:destructure\"]" + "[ \"construct:assignment\", \"construct:destructuring\", \"construct:pattern-matching\"]" , test "let block with a function with record destructuring" <| \() -> - expectData "f y = let f2 { a } = a in f2 y" - "[ \"construct:pattern-matching\", \"uses:destructure\"]" + expectData "f y = let f2 { a } = a in f2" + "[ \"construct:assignment\", \"construct:destructuring\", \"construct:pattern-matching\"]" , test "let block with a function with tuple destructuring" <| \() -> - expectData "f y = let f2 (a, b) = a in f2 y" - "[ \"construct:pattern-matching\", \"uses:destructure\"]" + expectData "f y = let f2 (a, b) = a in b" + "[ \"construct:assignment\", \"construct:destructuring\", \"construct:pattern-matching\"]" , test "let block with a function with uncons destructuring" <| \() -> - expectData "f y = let f2 (a :: b) = a in f2 y" - "[ \"construct:pattern-matching\", \"uses:destructure\"]" + expectData "f y = let f2 (a :: b) = a in b" + "[ \"construct:assignment\", \"construct:destructuring\", \"construct:pattern-matching\"]" , test "let block with a function with named destructuring" <| \() -> - expectData "f y = let f2 (Thing a) = a in f2 y" - "[ \"construct:pattern-matching\", \"uses:destructure\"]" + expectData "f y = let f2 (Thing a) = a in f2" + "[ \"construct:assignment\", \"construct:destructuring\", \"construct:pattern-matching\"]" + , test "function application" <| + \() -> + expectData "f x y = x y" + "[ \"uses:function-application\"]" ] From beea31b27186ec3fd6b9075a1e0fe05a3901dd89 Mon Sep 17 00:00:00 2001 From: Jeremie Gillet Date: Sat, 25 Nov 2023 16:12:57 +0900 Subject: [PATCH 11/17] Adding some tagd --- src/Tags.elm | 47 +++++++++++++++++++++++++++++++++++---- tests/TagsTest.elm | 55 +++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 93 insertions(+), 9 deletions(-) diff --git a/src/Tags.elm b/src/Tags.elm index ef17984..d21fd13 100644 --- a/src/Tags.elm +++ b/src/Tags.elm @@ -202,8 +202,17 @@ matchExpressionType (Node range expression) = GLSLExpression _ -> Set.singleton "uses:glsl" - FunctionOrValue _ _ -> - Set.empty + FunctionOrValue _ value -> + case String.uncons value of + Nothing -> + Set.empty + + Just ( first, _ ) -> + if Char.isUpper first then + Set.singleton "construct:constructor" + + else + Set.empty ParenthesizedExpression _ -> Set.empty @@ -240,12 +249,18 @@ matchExpression (Node _ expression) = FunctionOrValue [ "Array" ] _ -> Set.fromList [ "construct:array", "technique:immutable-collection" ] + FunctionOrValue ("Bytes" :: _) _ -> + Set.singleton "construct:byte" + FunctionOrValue [ "Set" ] _ -> Set.fromList [ "construct:set", "technique:immutable-collection", "technique:sorted-collection" ] FunctionOrValue [ "Dict" ] _ -> Set.fromList [ "construct:dictionary", "technique:immutable-collection", "technique:sorted-collection" ] + FunctionOrValue [ "List" ] _ -> + Set.singleton "construct:list" + FunctionOrValue [ "Random" ] _ -> Set.singleton "technique:randomness" @@ -256,10 +271,34 @@ matchExpression (Node _ expression) = Set.singleton "uses:debug" PrefixOperator "&&" -> - Set.fromList [ "construct:construct:boolean", "technique:boolean-logic" ] + Set.fromList [ "construct:boolean", "construct:logical-and", "technique:boolean-logic" ] OperatorApplication "&&" _ _ _ -> - Set.fromList [ "construct:construct:boolean", "technique:boolean-logic" ] + Set.fromList [ "construct:boolean", "construct:logical-and", "technique:boolean-logic" ] + + PrefixOperator "||" -> + Set.fromList [ "construct:boolean", "construct:logical-or", "technique:boolean-logic" ] + + OperatorApplication "||" _ _ _ -> + Set.fromList [ "construct:boolean", "construct:logical-or", "technique:boolean-logic" ] + + FunctionOrValue [ "Basics" ] "not" -> + Set.fromList [ "construct:boolean", "construct:logical-not", "technique:boolean-logic" ] + + FunctionOrValue [ "Basics" ] "xor" -> + Set.fromList [ "construct:boolean", "technique:boolean-logic" ] + + FunctionOrValue [ "Basics" ] "True" -> + Set.singleton "construct:boolean" + + FunctionOrValue [ "Basics" ] "False" -> + Set.singleton "construct:boolean" + + FunctionOrValue [ "Basics" ] "isNaN" -> + Set.fromList [ "construct:boolean", "construct:float", "construct:floating-point-number" ] + + FunctionOrValue [ "Basics" ] "isInfinite" -> + Set.fromList [ "construct:boolean", "construct:float", "construct:floating-point-number" ] PrefixOperator "+" -> Set.singleton "construct:add" diff --git a/tests/TagsTest.elm b/tests/TagsTest.elm index f061694..88eeecf 100644 --- a/tests/TagsTest.elm +++ b/tests/TagsTest.elm @@ -73,7 +73,9 @@ expectData function data = expressionTypeTags : Test expressionTypeTags = describe "matching expression types" - [ test "FunctionOrValue is too general for a tag" <| + [ test "Type constructors " <| + \() -> expectData "f = Just" "[ \"construct:constructor\"]" + , test "other functions and values too general for a tag" <| \() -> expectData "f x = x" "[]" , test "ParenthesizedExpression is too general for a tag" <| \() -> expectData "f x = (x)" "[]" @@ -174,6 +176,17 @@ expressionTags = \() -> expectData "f = Array.empty" "[ \"construct:array\", \"technique:immutable-collection\" ]" + , test "using Bytes function" <| + \() -> + expectData "f = Bytes.width" + "[ \"construct:byte\" ]" + , test "using Bytes.Encode function" <| + \() -> + expectData "f = Bytes.Encode.encode" + "[ \"construct:byte\" ]" + , test "using List function" <| + \() -> + expectData "f = List.all" "[ \"construct:list\" ]" , test "using Set function" <| \() -> expectData "f = Set.empty" @@ -286,8 +299,40 @@ expressionTags = \() -> expectData "f y = let f2 (Thing a) = a in f2" "[ \"construct:assignment\", \"construct:destructuring\", \"construct:pattern-matching\"]" - , test "function application" <| - \() -> - expectData "f x y = x y" - "[ \"uses:function-application\"]" + , test "using inline &&" <| + \() -> + expectData "f a b = a && b" + "[ \"construct:boolean\", \"construct:logical-and\", \"technique:boolean-logic\", \"uses:function-application\"]" + , test "using prefix &&" <| + \() -> + expectData "f = (&&)" + "[ \"construct:boolean\", \"construct:logical-and\", \"technique:boolean-logic\", \"uses:prefix-operator\" ]" + , test "using inline ||" <| + \() -> + expectData "f a b = a || b" + "[ \"construct:boolean\", \"construct:logical-or\", \"technique:boolean-logic\", \"uses:function-application\"]" + , test "using prefix ||" <| + \() -> + expectData "f = (||)" + "[ \"construct:boolean\", \"construct:logical-or\", \"technique:boolean-logic\", \"uses:prefix-operator\" ]" + , test "using not" <| + \() -> + expectData "f x = not x" + "[ \"construct:boolean\", \"construct:logical-not\", \"technique:boolean-logic\", \"uses:function-application\" ]" + , test "using xor" <| + \() -> + expectData "f x y = xor x y" + "[ \"construct:boolean\", \"technique:boolean-logic\", \"uses:function-application\" ]" + , test "using True" <| + \() -> expectData "f = True" "[ \"construct:boolean\", \"construct:constructor\" ]" + , test "using False" <| + \() -> expectData "f = False" "[ \"construct:boolean\", \"construct:constructor\" ]" + , test "using isNaN" <| + \() -> + expectData "f x = isNaN x" + "[ \"construct:boolean\", \"construct:float\", \"construct:floating-point-number\", \"uses:function-application\" ]" + , test "using isInfinite" <| + \() -> + expectData "f x = isInfinite x" + "[ \"construct:boolean\", \"construct:float\", \"construct:floating-point-number\", \"uses:function-application\" ]" ] From ca1a0134928d026d320523d30241b929ff384154 Mon Sep 17 00:00:00 2001 From: Jeremie Gillet Date: Sat, 25 Nov 2023 16:58:49 +0900 Subject: [PATCH 12/17] tagging comments --- src/Tags.elm | 42 +++++++++++++++++++++++++++++++++++++- tests/TagsTest.elm | 50 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 1 deletion(-) diff --git a/src/Tags.elm b/src/Tags.elm index d21fd13..bb318cc 100644 --- a/src/Tags.elm +++ b/src/Tags.elm @@ -1,5 +1,6 @@ module Tags exposing (commonTagsRule, expressionTagsRule, ruleConfig) +import Elm.Syntax.Declaration exposing (Declaration(..)) import Elm.Syntax.Expression exposing (Expression(..), LetDeclaration(..)) import Elm.Syntax.Module exposing (Module) import Elm.Syntax.Node as Node exposing (Node(..)) @@ -48,7 +49,12 @@ commonTagsRule = expressionTagsRule : Rule expressionTagsRule = Rule.newProjectRuleSchema "expressionTags" emptyProjectContext - |> Rule.withModuleVisitor (Rule.withExpressionEnterVisitor expressionVisitor) + |> Rule.withModuleVisitor + (Rule.withDeclarationEnterVisitor declarationVisitor + >> Rule.withExpressionEnterVisitor expressionVisitor + >> Rule.withModuleDocumentationVisitor documentationVisitor + >> Rule.withCommentsVisitor commentsVisitor + ) |> Rule.withModuleContextUsingContextCreator { fromModuleToProject = fromModuleToProject , fromProjectToModule = fromProjectToModule @@ -111,6 +117,40 @@ dataExtractor = .tags >> Set.toList >> Encode.list Encode.string +declarationVisitor : Node Declaration -> ModuleContext -> ( List never, ModuleContext ) +declarationVisitor node context = + case Node.value node of + FunctionDeclaration { documentation } -> + documentationVisitor documentation context + + _ -> + ( [], context ) + + +documentationVisitor : Maybe documentation -> ModuleContext -> ( List never, ModuleContext ) +documentationVisitor maybeDoc ({ tags } as context) = + let + docTags = + Set.fromList [ "construct:comment", "construct:documentation" ] + in + case maybeDoc of + Nothing -> + ( [], context ) + + Just _ -> + ( [], { context | tags = Set.union docTags tags } ) + + +commentsVisitor : List (Node String) -> ModuleContext -> ( List never, ModuleContext ) +commentsVisitor comments ({ tags } as context) = + case comments of + [] -> + ( [], context ) + + _ -> + ( [], { context | tags = Set.insert "construct:comment" tags } ) + + expressionVisitor : Node Expression -> ModuleContext -> ( List never, ModuleContext ) expressionVisitor ((Node range expression) as node) ({ lookupTable, tags } as context) = let diff --git a/tests/TagsTest.elm b/tests/TagsTest.elm index 88eeecf..24839f3 100644 --- a/tests/TagsTest.elm +++ b/tests/TagsTest.elm @@ -10,6 +10,7 @@ tests : Test tests = describe "TagsTest tests" [ commonTags + , commentsTags , expressionTypeTags , expressionTags ] @@ -49,6 +50,55 @@ twoFer name = ] +commentsTags : Test +commentsTags = + describe "comments" + [ test "module documentation" <| + \() -> + """ +module A exposing (..) +{-| This a module documentation blurb +-} + +f x = x +""" + |> Review.Test.run Tags.expressionTagsRule + |> Review.Test.expectDataExtract "[ \"construct:comment\", \"construct:documentation\" ]" + , test "top level comment" <| + \() -> + """ +module A exposing (..) + +-- this is a top level comment +f x = x +""" + |> Review.Test.run Tags.expressionTagsRule + |> Review.Test.expectDataExtract "[ \"construct:comment\" ]" + , test "internal comment" <| + \() -> + """ +module A exposing (..) + +f x = + -- this is an internal comment + x +""" + |> Review.Test.run Tags.expressionTagsRule + |> Review.Test.expectDataExtract "[ \"construct:comment\" ]" + , test "function documentation" <| + \() -> + """ +module A exposing (..) + +{-| This is a function documentation comment +-} +f x = x +""" + |> Review.Test.run Tags.expressionTagsRule + |> Review.Test.expectDataExtract "[ \"construct:comment\", \"construct:documentation\" ]" + ] + + commonStart : String commonStart = """ From 2be7eecd131e33ea3bba8a98c4b234817ebf6c9b Mon Sep 17 00:00:00 2001 From: Jeremie Gillet Date: Sat, 25 Nov 2023 18:55:30 +0900 Subject: [PATCH 13/17] type declaration tags --- src/ElmSyntaxHelpers.elm | 33 ++++++++++++++- src/Tags.elm | 48 ++++++++++++++++++++- tests/ElmSyntaxHelpersTest.elm | 28 +++++++++++++ tests/TagsTest.elm | 76 ++++++++++++++++++++++++++++++---- 4 files changed, 175 insertions(+), 10 deletions(-) diff --git a/src/ElmSyntaxHelpers.elm b/src/ElmSyntaxHelpers.elm index 5e923cd..7fc5fcb 100644 --- a/src/ElmSyntaxHelpers.elm +++ b/src/ElmSyntaxHelpers.elm @@ -1,5 +1,6 @@ -module ElmSyntaxHelpers exposing (hasDestructuringPattern, hasGenericRecord, traversePattern, typeAnnotationsMatch) +module ElmSyntaxHelpers exposing (hasDestructuringPattern, hasGenericRecord, hasTyped, traversePattern, typeAnnotationsMatch) +import Elm.Syntax.ModuleName exposing (ModuleName) import Elm.Syntax.Node as Node exposing (Node(..)) import Elm.Syntax.Pattern exposing (Pattern(..)) import Elm.Syntax.TypeAnnotation exposing (TypeAnnotation(..)) @@ -76,6 +77,36 @@ hasGenericRecord annotation = False +hasTyped : ModuleName -> String -> Node TypeAnnotation -> Bool +hasTyped moduleName name annotation = + case Node.value annotation of + Typed (Node _ ( typeModule, typeName )) annotations -> + (typeModule == moduleName && typeName == name) + || List.any (hasTyped moduleName name) annotations + + Record recordFields -> + recordFields + |> List.map (Node.value >> Tuple.second) + |> List.any (hasTyped moduleName name) + + GenericRecord _ (Node _ recordFields) -> + recordFields + |> List.map (Node.value >> Tuple.second) + |> List.any (hasTyped moduleName name) + + Tupled annotations -> + List.any (hasTyped moduleName name) annotations + + FunctionTypeAnnotation a b -> + hasTyped moduleName name a || hasTyped moduleName name b + + GenericType _ -> + False + + Unit -> + False + + hasDestructuringPattern : Node Pattern -> Bool hasDestructuringPattern fullPattern = let diff --git a/src/Tags.elm b/src/Tags.elm index bb318cc..8e58c98 100644 --- a/src/Tags.elm +++ b/src/Tags.elm @@ -3,7 +3,10 @@ module Tags exposing (commonTagsRule, expressionTagsRule, ruleConfig) import Elm.Syntax.Declaration exposing (Declaration(..)) import Elm.Syntax.Expression exposing (Expression(..), LetDeclaration(..)) import Elm.Syntax.Module exposing (Module) +import Elm.Syntax.ModuleName exposing (ModuleName) import Elm.Syntax.Node as Node exposing (Node(..)) +import Elm.Syntax.Type exposing (Type) +import Elm.Syntax.TypeAnnotation exposing (TypeAnnotation(..)) import ElmSyntaxHelpers import Json.Encode as Encode exposing (Value) import Review.ModuleNameLookupTable as LookupTable exposing (ModuleNameLookupTable) @@ -118,15 +121,55 @@ dataExtractor = declarationVisitor : Node Declaration -> ModuleContext -> ( List never, ModuleContext ) -declarationVisitor node context = +declarationVisitor node ({ tags } as context) = case Node.value node of FunctionDeclaration { documentation } -> documentationVisitor documentation context + AliasDeclaration _ -> + ( [], { context | tags = Set.insert "uses:type-alias" tags } ) + + CustomTypeDeclaration customType -> + ( [], { context | tags = Set.union (analyzeCustomType customType) tags } ) + _ -> ( [], context ) +analyzeCustomType : Type -> Set String +analyzeCustomType { name, generics, constructors } = + let + unionTags = + case constructors of + _ :: _ :: _ -> + [ "uses:union-type" ] + + _ -> + [] + + genericsTags = + case generics of + [] -> + [] + + _ -> + [ "construct:generic-type" ] + + isRecursive = + constructors + |> List.concatMap (Node.value >> .arguments) + |> List.any (ElmSyntaxHelpers.hasTyped [] (Node.value name)) + + recursiveTags = + if isRecursive then + [ "construct:recursive-type" ] + + else + [] + in + Set.fromList ("uses:custom-type" :: genericsTags ++ unionTags ++ recursiveTags) + + documentationVisitor : Maybe documentation -> ModuleContext -> ( List never, ModuleContext ) documentationVisitor maybeDoc ({ tags } as context) = let @@ -295,6 +338,9 @@ matchExpression (Node _ expression) = FunctionOrValue [ "Set" ] _ -> Set.fromList [ "construct:set", "technique:immutable-collection", "technique:sorted-collection" ] + FunctionOrValue [ "Time" ] _ -> + Set.fromList [ "construct:date-time" ] + FunctionOrValue [ "Dict" ] _ -> Set.fromList [ "construct:dictionary", "technique:immutable-collection", "technique:sorted-collection" ] diff --git a/tests/ElmSyntaxHelpersTest.elm b/tests/ElmSyntaxHelpersTest.elm index daabd2a..623cc6c 100644 --- a/tests/ElmSyntaxHelpersTest.elm +++ b/tests/ElmSyntaxHelpersTest.elm @@ -50,6 +50,34 @@ hasGenericRecordTests = ] +hasTypedTests : Test +hasTypedTests = + describe "hasTyped" + [ test "no typed" <| + \_ -> + Node.empty + (Tupled + [ Node.empty (FunctionTypeAnnotation (Node.empty Unit) (Node.empty (GenericType "x"))) + , Node.empty (Record [ Node.empty ( Node.empty "x", Node.empty Unit ) ]) + , Node.empty (Typed (Node.empty ( [ "X" ], "y" )) []) + ] + ) + |> ElmSyntaxHelpers.hasTyped [ "X" ] "x" + |> Expect.equal False + , test "one typed" <| + \_ -> + Node.empty + (Tupled + [ Node.empty (FunctionTypeAnnotation (Node.empty Unit) (Node.empty (GenericType "x"))) + , Node.empty (Record [ Node.empty ( Node.empty "x", Node.empty Unit ) ]) + , Node.empty (Typed (Node.empty ( [ "X" ], "x" )) []) + ] + ) + |> ElmSyntaxHelpers.hasTyped [ "X" ] "x" + |> Expect.equal True + ] + + hasDestructuringPatternTests : Test hasDestructuringPatternTests = describe "hasDestructuringPattern" diff --git a/tests/TagsTest.elm b/tests/TagsTest.elm index 24839f3..8501bff 100644 --- a/tests/TagsTest.elm +++ b/tests/TagsTest.elm @@ -11,6 +11,7 @@ tests = describe "TagsTest tests" [ commonTags , commentsTags + , typesTags , expressionTypeTags , expressionTags ] @@ -99,6 +100,68 @@ f x = x ] +typesTags : Test +typesTags = + describe "types" + [ test "type alias" <| + \() -> + """ +module A exposing (..) +type alias MyString = String +""" + |> Review.Test.run Tags.expressionTagsRule + |> Review.Test.expectDataExtract "[ \"uses:type-alias\" ]" + , test "custom type" <| + \() -> + """ +module A exposing (..) +type MyType = MyType +""" + |> Review.Test.run Tags.expressionTagsRule + |> Review.Test.expectDataExtract "[ \"uses:custom-type\" ]" + , test "union type" <| + \() -> + """ +module A exposing (..) +type MyType = A | B +""" + |> Review.Test.run Tags.expressionTagsRule + |> Review.Test.expectDataExtract "[ \"uses:custom-type\", \"uses:union-type\" ]" + , test "generics type" <| + \() -> + """ +module A exposing (..) +type MyType a = MyType +""" + |> Review.Test.run Tags.expressionTagsRule + |> Review.Test.expectDataExtract "[ \"construct:generic-type\", \"uses:custom-type\" ]" + , test "direct recursive type" <| + \() -> + """ +module A exposing (..) +type MyType = A MyType +""" + |> Review.Test.run Tags.expressionTagsRule + |> Review.Test.expectDataExtract "[ \"construct:recursive-type\", \"uses:custom-type\" ]" + , test "nested recursive type" <| + \() -> + """ +module A exposing (..) +type MyType = A | B | C { c: (String, MyType) } +""" + |> Review.Test.run Tags.expressionTagsRule + |> Review.Test.expectDataExtract "[ \"construct:recursive-type\", \"uses:custom-type\", \"uses:union-type\" ]" + , test "nested generic recursive type" <| + \() -> + """ +module A exposing (..) +type MyType a x = A { c : ( String, { a | b : MyType a Int } ) } +""" + |> Review.Test.run Tags.expressionTagsRule + |> Review.Test.expectDataExtract "[ \"construct:generic-type\", \"construct:recursive-type\", \"uses:custom-type\" ]" + ] + + commonStart : String commonStart = """ @@ -227,20 +290,17 @@ expressionTags = expectData "f = Array.empty" "[ \"construct:array\", \"technique:immutable-collection\" ]" , test "using Bytes function" <| - \() -> - expectData "f = Bytes.width" - "[ \"construct:byte\" ]" + \() -> expectData "f = Bytes.width" "[ \"construct:byte\" ]" , test "using Bytes.Encode function" <| - \() -> - expectData "f = Bytes.Encode.encode" - "[ \"construct:byte\" ]" + \() -> expectData "f = Bytes.Encode.encode" "[ \"construct:byte\" ]" , test "using List function" <| - \() -> - expectData "f = List.all" "[ \"construct:list\" ]" + \() -> expectData "f = List.all" "[ \"construct:list\" ]" , test "using Set function" <| \() -> expectData "f = Set.empty" "[ \"construct:set\", \"technique:immutable-collection\", \"technique:sorted-collection\" ]" + , test "using Time function" <| + \() -> expectData "f = Time.now" "[ \"construct:date-time\" ]" , test "using Dict function" <| \() -> expectData "f = Dict.empty" From 2c21bcf1e1f5bf3da23df396cef90e89671e5292 Mon Sep 17 00:00:00 2001 From: Jeremie Gillet Date: Sun, 26 Nov 2023 16:21:56 +0900 Subject: [PATCH 14/17] destructuring --- src/Tags.elm | 43 ++++++++++++--- tests/TagsTest.elm | 134 +++++++++++++++++++++++++++++++++++---------- 2 files changed, 141 insertions(+), 36 deletions(-) diff --git a/src/Tags.elm b/src/Tags.elm index 8e58c98..0dc04d1 100644 --- a/src/Tags.elm +++ b/src/Tags.elm @@ -1,9 +1,8 @@ module Tags exposing (commonTagsRule, expressionTagsRule, ruleConfig) import Elm.Syntax.Declaration exposing (Declaration(..)) -import Elm.Syntax.Expression exposing (Expression(..), LetDeclaration(..)) +import Elm.Syntax.Expression exposing (Expression(..), FunctionImplementation, LetDeclaration(..)) import Elm.Syntax.Module exposing (Module) -import Elm.Syntax.ModuleName exposing (ModuleName) import Elm.Syntax.Node as Node exposing (Node(..)) import Elm.Syntax.Type exposing (Type) import Elm.Syntax.TypeAnnotation exposing (TypeAnnotation(..)) @@ -123,8 +122,15 @@ dataExtractor = declarationVisitor : Node Declaration -> ModuleContext -> ( List never, ModuleContext ) declarationVisitor node ({ tags } as context) = case Node.value node of - FunctionDeclaration { documentation } -> - documentationVisitor documentation context + FunctionDeclaration { documentation, declaration } -> + let + ( _, docContext ) = + documentationVisitor documentation context + + argTags = + functionImplementationTags declaration + in + ( [], { context | tags = Set.union tags (Set.union docContext.tags argTags) } ) AliasDeclaration _ -> ( [], { context | tags = Set.insert "uses:type-alias" tags } ) @@ -194,6 +200,15 @@ commentsVisitor comments ({ tags } as context) = ( [], { context | tags = Set.insert "construct:comment" tags } ) +functionImplementationTags : Node FunctionImplementation -> Set String +functionImplementationTags (Node _ { arguments }) = + if List.any ElmSyntaxHelpers.hasDestructuringPattern arguments then + Set.fromList [ "construct:destructuring", "construct:pattern-matching" ] + + else + Set.empty + + expressionVisitor : Node Expression -> ModuleContext -> ( List never, ModuleContext ) expressionVisitor ((Node range expression) as node) ({ lookupTable, tags } as context) = let @@ -429,7 +444,21 @@ matchExpression (Node _ expression) = Set.fromList [ "construct:inequality", "technique:equality-comparison", "construct:boolean" ] LetExpression { declarations } -> - if List.any (Node.value >> usesProperDestructuring) declarations then + if List.any (Node.value >> letUsesDestructuring) declarations then + Set.fromList [ "construct:destructuring", "construct:pattern-matching" ] + + else + Set.empty + + LambdaExpression { args } -> + if List.any ElmSyntaxHelpers.hasDestructuringPattern args then + Set.fromList [ "construct:destructuring", "construct:pattern-matching" ] + + else + Set.empty + + CaseExpression { cases } -> + if List.any (Tuple.first >> ElmSyntaxHelpers.hasDestructuringPattern) cases then Set.fromList [ "construct:destructuring", "construct:pattern-matching" ] else @@ -439,8 +468,8 @@ matchExpression (Node _ expression) = Set.empty -usesProperDestructuring : LetDeclaration -> Bool -usesProperDestructuring letDeclaration = +letUsesDestructuring : LetDeclaration -> Bool +letUsesDestructuring letDeclaration = case letDeclaration of LetDestructuring _ _ -> True diff --git a/tests/TagsTest.elm b/tests/TagsTest.elm index 8501bff..ae83516 100644 --- a/tests/TagsTest.elm +++ b/tests/TagsTest.elm @@ -14,6 +14,7 @@ tests = , typesTags , expressionTypeTags , expressionTags + , destructuringTags ] @@ -369,7 +370,49 @@ expressionTags = \() -> expectData "f = (/=)" "[ \"construct:boolean\", \"construct:inequality\", \"technique:equality-comparison\", \"uses:prefix-operator\" ]" - , test "let block without proper destructuring" <| + , test "using inline &&" <| + \() -> + expectData "f a b = a && b" + "[ \"construct:boolean\", \"construct:logical-and\", \"technique:boolean-logic\", \"uses:function-application\"]" + , test "using prefix &&" <| + \() -> + expectData "f = (&&)" + "[ \"construct:boolean\", \"construct:logical-and\", \"technique:boolean-logic\", \"uses:prefix-operator\" ]" + , test "using inline ||" <| + \() -> + expectData "f a b = a || b" + "[ \"construct:boolean\", \"construct:logical-or\", \"technique:boolean-logic\", \"uses:function-application\"]" + , test "using prefix ||" <| + \() -> + expectData "f = (||)" + "[ \"construct:boolean\", \"construct:logical-or\", \"technique:boolean-logic\", \"uses:prefix-operator\" ]" + , test "using not" <| + \() -> + expectData "f x = not x" + "[ \"construct:boolean\", \"construct:logical-not\", \"technique:boolean-logic\", \"uses:function-application\" ]" + , test "using xor" <| + \() -> + expectData "f x y = xor x y" + "[ \"construct:boolean\", \"technique:boolean-logic\", \"uses:function-application\" ]" + , test "using True" <| + \() -> expectData "f = True" "[ \"construct:boolean\", \"construct:constructor\" ]" + , test "using False" <| + \() -> expectData "f = False" "[ \"construct:boolean\", \"construct:constructor\" ]" + , test "using isNaN" <| + \() -> + expectData "f x = isNaN x" + "[ \"construct:boolean\", \"construct:float\", \"construct:floating-point-number\", \"uses:function-application\" ]" + , test "using isInfinite" <| + \() -> + expectData "f x = isInfinite x" + "[ \"construct:boolean\", \"construct:float\", \"construct:floating-point-number\", \"uses:function-application\" ]" + ] + + +destructuringTags : Test +destructuringTags = + describe "destructuring" + [ test "let block without proper destructuring" <| \() -> expectData "f x y = let z = y in z" "[ \"construct:assignment\" ]" @@ -409,40 +452,73 @@ expressionTags = \() -> expectData "f y = let f2 (Thing a) = a in f2" "[ \"construct:assignment\", \"construct:destructuring\", \"construct:pattern-matching\"]" - , test "using inline &&" <| + , test "top-level function without proper destructuring" <| \() -> - expectData "f a b = a && b" - "[ \"construct:boolean\", \"construct:logical-and\", \"technique:boolean-logic\", \"uses:function-application\"]" - , test "using prefix &&" <| + expectData "f x y = x" "[]" + , test "top-level function with tuple destructuring" <| \() -> - expectData "f = (&&)" - "[ \"construct:boolean\", \"construct:logical-and\", \"technique:boolean-logic\", \"uses:prefix-operator\" ]" - , test "using inline ||" <| + expectData "f (a, b) = a" + "[ \"construct:destructuring\", \"construct:pattern-matching\"]" + , test "top-level function with record destructuring" <| \() -> - expectData "f a b = a || b" - "[ \"construct:boolean\", \"construct:logical-or\", \"technique:boolean-logic\", \"uses:function-application\"]" - , test "using prefix ||" <| + expectData "f {a, b} = a" + "[ \"construct:destructuring\", \"construct:pattern-matching\"]" + , test "top-level function with uncons destructuring" <| \() -> - expectData "f = (||)" - "[ \"construct:boolean\", \"construct:logical-or\", \"technique:boolean-logic\", \"uses:prefix-operator\" ]" - , test "using not" <| + expectData "f (a :: b) = a" + "[ \"construct:destructuring\", \"construct:pattern-matching\"]" + , test "top-level function with named destructuring" <| \() -> - expectData "f x = not x" - "[ \"construct:boolean\", \"construct:logical-not\", \"technique:boolean-logic\", \"uses:function-application\" ]" - , test "using xor" <| + expectData "f (Thing a) = a" + "[ \"construct:destructuring\", \"construct:pattern-matching\"]" + , test "top-level function with nested destructuring" <| \() -> - expectData "f x y = xor x y" - "[ \"construct:boolean\", \"technique:boolean-logic\", \"uses:function-application\" ]" - , test "using True" <| - \() -> expectData "f = True" "[ \"construct:boolean\", \"construct:constructor\" ]" - , test "using False" <| - \() -> expectData "f = False" "[ \"construct:boolean\", \"construct:constructor\" ]" - , test "using isNaN" <| + expectData "f (Thing { a }) = a" + "[ \"construct:destructuring\", \"construct:pattern-matching\"]" + , test "lambda without proper destructuring" <| \() -> - expectData "f x = isNaN x" - "[ \"construct:boolean\", \"construct:float\", \"construct:floating-point-number\", \"uses:function-application\" ]" - , test "using isInfinite" <| + expectData "f = \\x y -> x" "[\"construct:lambda\"]" + , test "lambda with tuple destructuring" <| \() -> - expectData "f x = isInfinite x" - "[ \"construct:boolean\", \"construct:float\", \"construct:floating-point-number\", \"uses:function-application\" ]" + expectData "f = \\(a, b) -> a" + "[ \"construct:destructuring\", \"construct:lambda\", \"construct:pattern-matching\"]" + , test "lambda with record destructuring" <| + \() -> + expectData "f = \\{a, b} -> a" + "[ \"construct:destructuring\", \"construct:lambda\", \"construct:pattern-matching\"]" + , test "lambda with uncons destructuring" <| + \() -> + expectData "f = \\(a :: b) -> a" + "[ \"construct:destructuring\", \"construct:lambda\", \"construct:pattern-matching\"]" + , test "lambda with named destructuring" <| + \() -> + expectData "f = \\(Thing a) -> a" + "[ \"construct:destructuring\", \"construct:lambda\", \"construct:pattern-matching\"]" + , test "lambda with nested destructuring" <| + \() -> + expectData "f = \\(Thing { a }) -> a" + "[ \"construct:destructuring\", \"construct:lambda\", \"construct:pattern-matching\"]" + , test "case without proper destructuring" <| + \() -> + expectData "f x = case x of\n x -> x" "[\"construct:pattern-matching\"]" + , test "case with tuple destructuring" <| + \() -> + expectData "f x = case x of\n (a, b) -> a" + "[ \"construct:destructuring\", \"construct:pattern-matching\"]" + , test "case with record destructuring" <| + \() -> + expectData "f x = case x of\n {a, b} -> a" + "[ \"construct:destructuring\", \"construct:pattern-matching\"]" + , test "case with uncons destructuring" <| + \() -> + expectData "f x = case x of\n (a :: b) -> a" + "[ \"construct:destructuring\", \"construct:pattern-matching\"]" + , test "case with named destructuring" <| + \() -> + expectData "f x = case x of\n (Thing a) -> a" + "[ \"construct:destructuring\", \"construct:pattern-matching\"]" + , test "case with nested destructuring" <| + \() -> + expectData "f x = case x of\n (Thing { a }) -> a" + "[ \"construct:destructuring\", \"construct:pattern-matching\"]" ] From 7efeab8a295f94baf2117893bdf4062b2496615b Mon Sep 17 00:00:00 2001 From: Jeremie Gillet Date: Sat, 9 Dec 2023 10:34:48 +0900 Subject: [PATCH 15/17] smoke tests pass --- bin/run.sh | 2 +- .../strain/imperfect_solution/expected_tags.json | 6 ++++++ .../strain/perfect_solution/expected_tags.json | 7 +++++++ .../two-fer/imperfect_solution/expected_tags.json | 14 ++++++++++++++ .../two-fer/perfect_solution/expected_tags.json | 2 ++ 5 files changed, 30 insertions(+), 1 deletion(-) diff --git a/bin/run.sh b/bin/run.sh index 82b3c9d..48f2b84 100755 --- a/bin/run.sh +++ b/bin/run.sh @@ -35,7 +35,7 @@ npx elm-review $INPUT_DIR \ cat /tmp/elm-review-report.json | node ./bin/cli.js > $OUTPUT_DIR/analysis.json # Get tags -jq '.extracts | to_entries | map(.value) | add' /tmp/elm-review-report.json > $OUTPUT_DIR/tags.json +jq '.extracts | to_entries | map(.value) | add | sort' /tmp/elm-review-report.json > $OUTPUT_DIR/tags.json set -e diff --git a/test_data/strain/imperfect_solution/expected_tags.json b/test_data/strain/imperfect_solution/expected_tags.json index 7f6b1ec..78ab901 100644 --- a/test_data/strain/imperfect_solution/expected_tags.json +++ b/test_data/strain/imperfect_solution/expected_tags.json @@ -1,5 +1,11 @@ [ + "construct:boolean", + "construct:comment", + "construct:list", + "construct:logical-not", "paradigm:functional", + "technique:boolean-logic", "technique:immutability", + "uses:function-application", "uses:module" ] diff --git a/test_data/strain/perfect_solution/expected_tags.json b/test_data/strain/perfect_solution/expected_tags.json index 7f6b1ec..d4607ac 100644 --- a/test_data/strain/perfect_solution/expected_tags.json +++ b/test_data/strain/perfect_solution/expected_tags.json @@ -1,5 +1,12 @@ [ + "construct:boolean", + "construct:if", + "construct:lambda", + "construct:list", + "construct:logical-not", "paradigm:functional", + "technique:boolean-logic", "technique:immutability", + "uses:function-application", "uses:module" ] diff --git a/test_data/two-fer/imperfect_solution/expected_tags.json b/test_data/two-fer/imperfect_solution/expected_tags.json index 7f6b1ec..98df01b 100644 --- a/test_data/two-fer/imperfect_solution/expected_tags.json +++ b/test_data/two-fer/imperfect_solution/expected_tags.json @@ -1,5 +1,19 @@ [ + "construct:boolean", + "construct:comment", + "construct:constructor", + "construct:destructuring", + "construct:if", + "construct:int", + "construct:integral-number", + "construct:logical-and", + "construct:multiply", + "construct:pattern-matching", + "construct:string", "paradigm:functional", + "technique:boolean-logic", "technique:immutability", + "uses:custom-type", + "uses:function-application", "uses:module" ] diff --git a/test_data/two-fer/perfect_solution/expected_tags.json b/test_data/two-fer/perfect_solution/expected_tags.json index 7f6b1ec..55b39b2 100644 --- a/test_data/two-fer/perfect_solution/expected_tags.json +++ b/test_data/two-fer/perfect_solution/expected_tags.json @@ -1,5 +1,7 @@ [ + "construct:string", "paradigm:functional", "technique:immutability", + "uses:function-application", "uses:module" ] From 27dffadf6bd4b0140e10f558e561af77b0bbb3f9 Mon Sep 17 00:00:00 2001 From: Jeremie Gillet Date: Sat, 9 Dec 2023 11:12:49 +0900 Subject: [PATCH 16/17] Add feature flag --- bin/run.sh | 12 +++++++----- bin/smoke_test.sh | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/bin/run.sh b/bin/run.sh index 48f2b84..f192d06 100755 --- a/bin/run.sh +++ b/bin/run.sh @@ -10,6 +10,7 @@ set -o pipefail # Catch failures in pipes. # Remove trailing slash for elm-review INPUT_DIR=${2%/} OUTPUT_DIR=${3%/} +OUTPUT_TAGS=${4:-false} # Check if script running in docker if [ -f solution_cache.tar ]; then @@ -30,13 +31,14 @@ npx elm-review $INPUT_DIR \ --report=json \ --extract \ > /tmp/elm-review-report.json +set -e -# Get comments +# Output comments cat /tmp/elm-review-report.json | node ./bin/cli.js > $OUTPUT_DIR/analysis.json -# Get tags -jq '.extracts | to_entries | map(.value) | add | sort' /tmp/elm-review-report.json > $OUTPUT_DIR/tags.json - -set -e +if [ $OUTPUT_TAGS = "--tags" ]; then + # Output tags + jq '.extracts | to_entries | map(.value) | add | sort' /tmp/elm-review-report.json > $OUTPUT_DIR/tags.json +fi echo Finished diff --git a/bin/smoke_test.sh b/bin/smoke_test.sh index 31c6d36..720eba9 100755 --- a/bin/smoke_test.sh +++ b/bin/smoke_test.sh @@ -8,7 +8,7 @@ set -o pipefail # Catch failures in pipes. for solution in test_data/*/* ; do slug=$(basename $(dirname $solution)) # run analysis - bin/run.sh $slug $solution $solution > /dev/null + bin/run.sh $slug $solution $solution --tags > /dev/null # check result bin/check_files.sh $solution done From 335aa1bfca4ef061cd2b3c7e3df2c4c777e43a8a Mon Sep 17 00:00:00 2001 From: Jeremie Gillet Date: Sat, 9 Dec 2023 11:14:58 +0900 Subject: [PATCH 17/17] fix elm-review errors --- src/Tags.elm | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Tags.elm b/src/Tags.elm index 0dc04d1..3fe6f2e 100644 --- a/src/Tags.elm +++ b/src/Tags.elm @@ -5,7 +5,6 @@ import Elm.Syntax.Expression exposing (Expression(..), FunctionImplementation, L import Elm.Syntax.Module exposing (Module) import Elm.Syntax.Node as Node exposing (Node(..)) import Elm.Syntax.Type exposing (Type) -import Elm.Syntax.TypeAnnotation exposing (TypeAnnotation(..)) import ElmSyntaxHelpers import Json.Encode as Encode exposing (Value) import Review.ModuleNameLookupTable as LookupTable exposing (ModuleNameLookupTable) @@ -354,7 +353,7 @@ matchExpression (Node _ expression) = Set.fromList [ "construct:set", "technique:immutable-collection", "technique:sorted-collection" ] FunctionOrValue [ "Time" ] _ -> - Set.fromList [ "construct:date-time" ] + Set.singleton "construct:date-time" FunctionOrValue [ "Dict" ] _ -> Set.fromList [ "construct:dictionary", "technique:immutable-collection", "technique:sorted-collection" ]