Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect error messages from io:format and related functions #8568

Closed
juhlig opened this issue Jun 12, 2024 · 1 comment · Fixed by #8611
Closed

Incorrect error messages from io:format and related functions #8568

juhlig opened this issue Jun 12, 2024 · 1 comment · Fixed by #8611
Assignees
Labels
bug Issue is reported as a bug team:VM Assigned to OTP team VM

Comments

@juhlig
Copy link
Contributor

juhlig commented Jun 12, 2024

Describe the bug
Modifiers that make no sense for a given control character (like the modifier k (map key order ordered) for the control character b (integer)) are silently ignored if the formatting is successful. However, if there is an error, the reported error reason is "format string invalid (invalid modifier/control combination ~kb)". And while it is true that the modifier makes no sense for the given control character, it is arguably not an error (in the sense that by itself it works), and it is usually not the reason for the error at hand.

The reason is that erl_stdlib_errors relies on erl_lint:check_format_string for io error reporting. While it makes sense for a linter to detect invalid modifier/control character combinations, it poses a problem for error cause detection when the questionable combination is not actually an error.

Related, the value for the K modifier (expected to be either undefined, ordered, reversed or a fun/2) is allowed to be anything unless the value to be formatted is a non-empty map. This should actually be an error.

To Reproduce

1> io:format("~kb~n", [123]).
123
ok
2> io:format("~kb~n", [x]).
** exception error: bad argument
     in function  io:format/2
        called as io:format("~kb~n",[x])
        *** argument 1: format string invalid (invalid modifier/control combination ~kb)
3> io:format("~kb~n", [123, x]).
** exception error: bad argument
     in function  io:format/2
        called as io:format("~kb~n",[123,x])
        *** argument 1: format string invalid (invalid modifier/control combination ~kb)
4> io:format("~kb~q", [123, x]).
** exception error: bad argument
     in function  io:format/2
        called as io:format("~kb~q",[123,x])
        *** argument 1: format string invalid (invalid modifier/control combination ~kb)
  • at 1>, the format string is accepted even though the k modifier is invalid for the b control character.
  • at 2>, the real reason for the error is that the atom x can not be formatted with the b control character
  • at 3>, the real reason for the error is that the number of values does not match the number of format sequences
  • at 4>, the real reason for the error is the invalid control character ~q

Regarding the K modifier:

5> io:format("~Kp~n", [foo, 123]).
123
ok
6> io:format("~Kp~n", [foo, #{}]).
#{}
ok
7> io:format("~Kp~n", [foo, #{x => y}]).
** exception error: bad argument
     in function  io:format/2
        called as io:format("~Kp~n",[foo,#{x => y}])
  • at 5>, foo is incorrectly allowed as K-value when the value to be formatted is not a map
  • at 6>, foo is incorrectly even allowed as K-value even though the value to be formatted is a map
  • at 7>, the incorrect K-value is detected, but no error message is created

Expected behavior

  • correct error reasons are created (cases 2>, 3> and 4>)
  • invalid K-values are detected (cases 5> and 6>), at least when the value to be formatted is a map even if empty (case 6>)
  • if the K-value is invalid, an appropriate error message should be displayed.

Affected versions
OTP 27.0 and earlier

@juhlig juhlig added the bug Issue is reported as a bug label Jun 12, 2024
@IngelaAndin IngelaAndin added priority:medium team:VM Assigned to OTP team VM and removed priority:medium labels Jun 13, 2024
@garazdawi
Copy link
Contributor

Yes, erl_lint:check_format_string needs to be updated. I'll add it to our list of things to do. A PR would as always be very welcome :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is reported as a bug team:VM Assigned to OTP team VM
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants