`file-sniffer` analyzer should allow string representation for `type_from_binary`

[toy]

Analyzer replied with «Even though there could be other valid ways to solve this exercise, for the sake of mastering bitstrings please match the first few bytes of the argument of type_from_binary/1 with the special form <<>>.» and I expected it to be fine with strings in <<>> instead of only digits.

defmodule FileSniffer do
  @exe_type "application/octet-stream"
  @bmp_type "image/bmp"
  @png_type "image/png"
  @jpg_type "image/jpg"
  @gif_type "image/gif"

  def type_from_binary(<<"\dELF", _rest::binary>>), do: @exe_type
  def type_from_binary(<<"BM", _rest::binary>>), do: @bmp_type
  def type_from_binary(<<"\x89PNG\r\n\x1A\n", _rest::binary>>), do: @png_type
  def type_from_binary(<<"\xFF\xD8\xFF", _rest::binary>>), do: @jpg_type
  def type_from_binary(<<"GIF", _rest::binary>>), do: @gif_type
end

[jiegillet]

Hi @toy, could you please update your description with the analyzer message you got and what you expected?

[angelikatyborska]

@jiegillet I've been thinking about this problem. I asked myself: what is actually important in this exercise? I think the most important part is that people use <<>> and :: at least once to do pattern matching on bitstrings. I think it's less important whether the bitstring chunks get created with integers <<0x42, 0x4D>> or with strings <<"BM">>.

I looked into the current analyzer code and discovered that, after many bug fixes after people complained their specific syntax was not accepted, the analyzer is no longer effective at detecting pattern matching reliably. E.g. this solution would pass the analyzer:

def type_from_binary(file) do
  cond do
    String.starts_with?(file, <<0x42, 0x4D>>) ->
      "image/bmp"

    String.starts_with?(file, <<0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A>>) ->
      "image/png"

    String.starts_with?(file, <<0xFF, 0xD8, 0xFF>>) ->
      "image/jpg"

    String.starts_with?(file, <<0x47, 0x49, 0x46>>) ->
      "image/gif"

    String.starts_with?(file, <<0x7F, 0x45, 0x4C, 0x46>>) ->
      "application/octet-stream"
  end
end

I started rewriting the analyzer for file-sniffer to two simple assert_calls for :: and <<>>, but that will not be enough because our assert_call macro doesn't count calls made when pattern matching in the argument list. Do you think it makes sense to extend the assert_call macro to also look in the argument list? And do you agree with me about the goal of the exercise?

[jiegillet]

I agree with you about the important part being <<>> and ::. I thought it could be achieved with a combination of

    form do
      <<_ignore, _ignore :: _ignore>>
    end

    form do
      <<_ignore::_ignore, _ignore :: _ignore>>
    end

But to match all possible combinations for up to 8 bytes is too much.

So, yes, I also agree that we can try to extend assert_call to check the argument list. I'm a bit surprised it doesn't actually check in the argument though, I'm sure it's that that far out of reach... maybe 😆

[jgabardini]

My code:

  def type_from_binary(<<66, 77, 30, _::binary>>), do: type_from_extension("bmp")
  def type_from_binary(<<71, 73, 70, 56, 57, 97, _::binary>>), do: type_from_extension("gif")
  def type_from_binary(<<255, 216, 255, 219, _::binary>>), do: type_from_extension("jpg")
  def type_from_binary(<<137, 80, 78, 71, 13, 10, 26, 10, _::binary>>), do: type_from_extension("png")
  def type_from_binary(<<127, 69, 76, 70, _::binary>>), do: type_from_extension("exe")
  def type_from_binary(file), do: type_from_extension(file)

The analyzer says:

Essential Even though there could be other valid ways to solve this exercise, for the sake of mastering bitstrings please match the first few bytes of the argument of type_from_binary/1 with the special form <<>>.

It seems to me that its not working as expected

[daviaws]

https://github.com/exercism/elixir-analyzer/issues/407#issuecomment-1821062661

same here