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

False positive on no_nested_try_catch #351

Open
elbrujohalcon opened this issue Jul 15, 2024 · 1 comment
Open

False positive on no_nested_try_catch #351

elbrujohalcon opened this issue Jul 15, 2024 · 1 comment
Labels
Milestone

Comments

@elbrujohalcon
Copy link
Member

elbrujohalcon commented Jul 15, 2024

Bug Description

no_nested_try_catch reports a warning on a try…catch block that's not really nested within another.

To Reproduce

Run Elvis on the following module:

-module(example).

-export([example/0]).

%% @doc If this fails in do:something/0, we handle the error.
%%      But if it fails in do:something_else/2, we ignore it.
example() ->
    try do:something() of
        {ok, KeepGoing} ->
            try
                do:something_else("and", KeepGoing)
            catch
                _:_ -> {ignore, errors, here}
            end
    catch
        Kind:Error ->
            {Kind, Error}
    end.

You'll get the following warning:

# src/example.erl [FAIL]
  - no_nested_try_catch (https://github.com/inaka/elvis_core/tree/main/doc_rules/elvis_style/no_nested_try_catch.md)
    - Nested try...catch block starting at line 10.

Expected Behavior

No warnings, since the second try…catch is not really nested within the first one (i.e., it's not try … try … catch … end … catch … end, which is what the rule tries to prevent).

Additional Context

This equivalent version of the code works:

-module(example).

-export([example/0]).

%% @doc If this fails in do:something/0, we handle the error.
%%      But if it fails in do:something_else/2, we ignore it.
example() ->
    Result =
        try
            do:something()
        catch
            Kind:Error ->
                {Kind, Error}
        end,
    case Result of
        {ok, KeepGoing} ->
            try
                do:something_else("and", KeepGoing)
            catch
                _:_ -> {ignore, errors, here}
            end;
        {Kind, Error} ->
            {Kind, Error}
    end.
@paulo-ferraz-oliveira
Copy link
Collaborator

Yeah, still I think that even if wrongly implemented the first piece of code is ugly ugly.

@elbrujohalcon elbrujohalcon added this to the 4.0.0 milestone Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants