exercism / elixir-analyzer

GNU Affero General Public License v3.0
30 stars 32 forks source link

Accept defdelegates as valid equivalents of function calls in assert_call #363

Closed angelikatyborska closed 1 year ago

angelikatyborska commented 1 year ago

Resolves https://github.com/exercism/elixir-analyzer/issues/296

angelikatyborska commented 1 year ago

@jiegillet I need your help here. I can't get the last two tests from this commit to pass: https://github.com/exercism/elixir-analyzer/commit/40ad1679bd4eac1634edb3fc5371e206ceedd8e6

I am very confused about how assert_call compiler works. Somehow it looks for the right function call twice, once in a find function, and then again in indirect_call?? Is that right? And indirect_call? doesn't seem to use modules_in_scope at all, where the alias information is stored. Why?

jiegillet commented 1 year ago

Man, it's been a while :)

Yes, it's right, it's looking for the function call twice, but the second time seems to be very lacking. Your approach was great and should have worked.

I think we should refactor so that the first pass with find finds the function call leaves a tag in the metadata if it finds it, and then the second pass with indirect_call? can find it it's called in the right place. I will try to find the time to do that this week :)

jiegillet commented 1 year ago

OK, I think I found a solution. It took me a while to remember how this all works.

There were two issues:

  1. track_all_functions usually doesn't need to know about aliases, because when function_def? finds a match, it is added to the tree directly with a de-aliased module. However, that breaks down when the call is in a different module that is aliased.
  2. function_def? wouldn't detect the function call in defdelegate, leaving it to the very limited track_all_functions

So I made the following changes:

  1. Give track_all_functions the list of all aliases
  2. Call track_all_functions in annotate_and_find to make sure the list of aliases is complete
  3. Transform all defdelegate into def that function_def? can analyze
angelikatyborska commented 1 year ago

Woohoo! Thank you 🙏 all the tests are passing now