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

Read closing bracket(s) as empty lines #112

Closed
jtagcat opened this issue Mar 1, 2022 · 4 comments
Closed

Read closing bracket(s) as empty lines #112

jtagcat opened this issue Mar 1, 2022 · 4 comments

Comments

@jtagcat
Copy link

jtagcat commented Mar 1, 2022

I see this as just fine:

	// ...
	}
}
return
	// ...

	if err != nil
		return nil, err
	}
	return // "bad"
}

return statements should not be cuddled if block has more than two lines

@bombsimon
Copy link
Owner

I don't think I agree here, it still doesn't separate control flow which is the intent here. Also in general cuddling anything with another block (a closing }) feels like something against the wsl philosophy.

But either way, as noted in #110 I'm more likely to drop this rule completely since it overlaps with nlreturn. I won't close this because I haven't decided yet.

@jtagcat
Copy link
Author

jtagcat commented Mar 2, 2022

For the first example, I think I could agree with you, but for the second, the control flow, is like this:

	if err != nil
		return nil, err
	} else { // this
		return
	} // this

^ I would say the second return is part of the same flow.

@bombsimon
Copy link
Owner

Both of these are valid:

if err != nil {
    return 0, err
} else {
    return 1, nil
}
if err != nil {
    return 0, err
}

return 1, nil

Just because something fits in an else block does not mean I want to reduce separation of control flow. The point of this rule is similar to the philosophy of nlreturn and for any non single line function wsl will require an empty line.

The ticket mentions to read any bracket as an empty line even though the example is for error checking. I think that's also against the idea with wsl since that would allow something like this which is one of the most important rules in my opinion:

if expr {
     //
}
if expr {
    //
}

I will close this as wontfix but feel free to add more context or thoughts if you think this would fit wsl or if I misinterpreted your thoughts here.

@jtagcat
Copy link
Author

jtagcat commented Mar 18, 2023

I disagree with my OP, you are right. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants