search

erlang / otp

Erlang/OTP
http://erlang.org
Apache License 2.0
11.32k stars 2.94k forks source link

Fix error messages from `io:format` and related functions #8611

Closed juhlig closed 3 months ago

juhlig commented 3 months ago

Fixes #8568.

This PR introduces a new function erl_lint:check_format_string/2, where the second argument can be used to decide if the check should be strict (true, the default) or relaxed (false).

Strict checking, which is the same as in the current implementation, will check and complain about repeated (eg ~ttp), conflicting (eg ~kKp) or non-applicable (eg ~kb) modifiers in formatting sequences. Those do not cause errors, though, so while it is ok for linting to check and warn about them, it will cause wrong error messages.

Relaxed checking omits the above checks and only reports erroneous format sequences (eg ~q). Relaxed checking is used by erl_stdlib_errors to print out error reasons.

github-actions[bot] commented 3 months ago

CT Test Results

    2 files     95 suites   36m 7s :stopwatch: 2 142 tests 2 094 :white_check_mark: 48 :zzz: 0 :x: 2 451 runs  2 401 :white_check_mark: 50 :zzz: 0 :x:

Results for commit 4c6a99f5.

:recycle: This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

garazdawi commented 3 months ago

I was hoping this PR would arrive soon :)

Would you mind expanding the tests in https://github.com/erlang/otp/blob/OTP-27.0/lib/stdlib/test/io_SUITE.erl#L3125-L3138 and https://github.com/erlang/otp/blob/OTP-27.0/lib/stdlib/test/erl_lint_SUITE.erl#L3820-L3827 (https://github.com/erlang/otp/blob/OTP-27.0/lib/stdlib/test/erl_lint_SUITE_data/format.erl) ?

juhlig commented 3 months ago

I was hoping this PR would arrive soon :)

:sweat_smile:

Would you mind expanding the tests in https://github.com/erlang/otp/blob/OTP-27.0/lib/stdlib/test/io_SUITE.erl#L3125-L3138 and https://github.com/erlang/otp/blob/OTP-27.0/lib/stdlib/test/erl_lint_SUITE.erl#L3820-L3827 (https://github.com/erlang/otp/blob/OTP-27.0/lib/stdlib/test/erl_lint_SUITE_data/format.erl) ?

Sure, but I probably won't get to it today.

juhlig commented 3 months ago

@garazdawi added some tests as requested, please take a look.

garazdawi commented 3 months ago

Looks good and test seem to pass. If you rebase to maint I will merge it for inclusion in 27.1.

juhlig commented 3 months ago

@garazdawi done, squashed the commits and rebased/retargeted to maint :+1:

garazdawi commented 3 months ago

thanks!