Expect non_neg_integer instead of pos_integer

[RemiBardon]

As String — Elixir v1.18.3 states, String.length/1 returns a non_neg_integer(), not a pos_integer().

[github-actions]

Thank you for contributing to exercism/elixir 💜 🎉. This is an automated PR comment 🤖 for the maintainers of this repository that helps with the PR review process. You can safely ignore it and wait for a maintainer to review your changes.

Based on the files changed in this PR, it would be good to pay attention to the following details when reviewing the PR:

• General steps

  • 🏆 Does this PR need to receive a label with a reputation modifier (x:size/{tiny,small,medium,large,massive})? (A medium reputation amount is awarded by default, see docs)

• Any exercise changed

  • 👤 Does the author of the PR need to be added as an author or contributor in <exercise>/.meta/config.json (see docs)?
  • 🔬 Do the analyzer and the analyzer comments exist for this exercise? Do they need to be changed?
  • 📜 Does the design file (<exercise>/.meta/design.md) need to be updated to document new implementation decisions?

• Concept exercise changed

  • 🌲 Do prerequisites and practices in config.json need to be updated?
  • 📖 Does the concept introduction provide all necessary information to solve this exercise?

• Concept exercise tests changed

  • ⚪️ Are all tests un-skipped?
  • 🔢 Are all tests annotated with @tag task_id?
  • 🐈 Can all tests be understood by reading the test's block only (e.g. no module attributes, no setup functions)?

---

Automated comment created by PR Commenter 🤖.

[github-actions]

This PR touches files which potentially affect the outcome of the tests of an exercise. This will cause all students' solutions to affected exercises to be re-tested.

If this PR does not affect the result of the test (or, for example, adds an edge case that is not worth rerunning all tests for), please add the following to the merge-commit message which will stops student's tests from re-running. Please copy-paste to avoid typos.

[no important files changed]

For more information, refer to the documentation. If you are unsure whether to add the message or not, please ping @exercism/maintainers-admin in a comment. Thank you!

[neenjaw]

This task instructs a student to define a typespec for this function:

This is needed to check that the values of fields do not exceed the maximum allowed length.
It also tells you by how much the value exceeds the maximum.

A string can't exceed the maximum length by 0 and still be an error. So it was correct for the typespec to be a positive integer to exclude 0 from valid responses.

[RemiBardon]

If we look at implementation details, you’re correct, but in terms of types diff is a non_neg_integer(), per the spec of String.length/1. I was purely reasoning in types here.

[RemiBardon]

Coming from a Rust background, the tests didn’t behave as I was expecting, but maybe I’m wrong and it’s just not how it works in Elixir 🤷🏻‍♂️

[RemiBardon]

I’m not the experienced one here so I’ll be fine with whatever you find more correct.

[neenjaw]

I don't think pos_integer() or non_neg_integer() is a type in the same sense as it is in rust. It's not a different data format, and pos_ and non_neg_ are communicating something about the expected value of the integer when it is part of the returned value. At this point there's no strict connection between the value returned and the type specified in the typespec attribute.

if we have non_neg_integer, we would have to cover the possibility of a {:error, 0} response when that's not an expected response from check_length/2

I think this will be more obvious in the future as set theoretic types are further fleshed out in future releases of the language, but for now this is is used as information for dialyzer, which performs static analysis on these types, function constructs, etc.

For now, I think it makes most sense to stick with pos_integer since 0 is excluded by the implementation.

[RemiBardon]

if we have non_neg_integer, we would have to cover the possibility of a {:error, 0} response when that's not an expected response from check_length/2

Good point. Makes sense why use pos_integer. Sorry for the disturbance and thank you for the links!

[neenjaw]

No problem, no disturbance, thanks for the discussion!

[neenjaw]

See above comment.

There may be some uncertainty around the task which has prompted this PR, so I'm hesitant to close it without discussion. If there is something else in the wording of this task that might improve the task's clarity, I'm open to it.