Skip to content

Conversation

@angelikatyborska
Copy link
Member

@angelikatyborska angelikatyborska commented Apr 5, 2021

The bird-count exercise is supposed to teach recursion and should be solved using recursion-only.

A corresponding website-copy PR: exercism/website-copy#1960

Comment on lines 17 to 23
Enum.map(not_allowed_functions, fn {module, function} ->
assert_no_call "does not call #{module}.#{function}" do
type :essential
called_fn module: module, name: function
comment Constants.bird_count_use_recursion()
end
end)
Copy link
Member Author

Choose a reason for hiding this comment

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

@neenjaw I'm unable to get this to work. I'm getting this error:

$ mix test test/elixir_analyzer/test_suite/bird_count_test.exs
Compiling 1 file (.ex)

== Compilation error in file lib/test_suite/bird_count.ex ==
** (ArgumentError) calling function signature requires :module to be nil or a module atom, got: {:module, [line: 20], nil}
    lib/elixir_analyzer/exercise_test/assert_call.ex:121: ElixirAnalyzer.ExerciseTest.AssertCall.validate_module/1
    lib/elixir_analyzer/exercise_test/assert_call.ex:105: ElixirAnalyzer.ExerciseTest.AssertCall.validate_signature/1
    lib/elixir_analyzer/exercise_test/assert_call.ex:67: ElixirAnalyzer.ExerciseTest.AssertCall.do_walk_assert_call_block/2
    (elixir 1.11.3) lib/macro.ex:482: anonymous fn/4 in Macro.do_traverse_args/4
    (elixir 1.11.3) lib/enum.ex:1533: Enum."-map_reduce/3-lists^mapfoldl/2-0-"/3
    (elixir 1.11.3) lib/enum.ex:1533: Enum."-map_reduce/3-lists^mapfoldl/2-0-"/3
    (elixir 1.11.3) lib/macro.ex:448: Macro.do_traverse/4
    lib/elixir_analyzer/exercise_test/assert_call.ex:57: ElixirAnalyzer.ExerciseTest.AssertCall.walk_assert_call_block/2

I'm sure it's something obvious and I'm probably attempting to use macros incorrectly. I tried various combinations of quote/unquote and I just don't get it (I'm having a bad day probably...) Could you advise? How do I achieve what I need? (Dynamically pass all Enum, List, and Stream functions to assert_no_call)

Copy link
Member Author

@angelikatyborska angelikatyborska Apr 5, 2021

Choose a reason for hiding this comment

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

I arrived at this monster that uses Code.eval_quoted: 77ced7a (#50)

@angelikatyborska
Copy link
Member Author

I'm marking this as ready for review because it works, but I expect some code quality improvement might be necessary before a merge. It feels very dirty to use eval for code that we 100% control. I'm sure there's a better way.

@neenjaw
Copy link
Contributor

neenjaw commented Apr 6, 2021

#51 <- should make this cleaner, being able to specify function name wildcards.

@neenjaw
Copy link
Contributor

neenjaw commented Apr 7, 2021

I merged #51, you're good to rebase to make use of the changes

assert_no_call "does not call any Enum functions" do
type :essential
called_fn module: Enum, name: :_
comment Constants.bird_count_use_recursion()
Copy link
Member Author

Choose a reason for hiding this comment

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

@neenjaw This doesn't work. I get the error about comment missing because it's:

[{{:., [line: 13], [{:__aliases__, [line: 13], [:Constants]}, :bird_count_use_recursion]}, [line: 13], []}]

And not a binary, so it doesn't get added (fails the guard when is_binary(comment) in do_walk_assert_call_block).

@angelikatyborska
Copy link
Member Author

I merged your patch branch into here and then I gave up on the remaining problem by just not using any aliases for the comments module... I also already merged #54 into this PR to see that everything will work together, and added a simple detection for import/alias.

@angelikatyborska
Copy link
Member Author

This PR needs #57 and #54

@neenjaw
Copy link
Contributor

neenjaw commented May 16, 2021

I think this is good to go. Merged in main to get the latest changes.

I'm going to merge it so that all of the changes are present for the other PRs

@neenjaw neenjaw merged commit c201cea into main May 16, 2021
@neenjaw neenjaw deleted the analyze-bird-count branch May 16, 2021 02:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants