Skip to content
New issue

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

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

Already on GitHub? # to your account

refactor: extract shared code for linting if-else chains #821

Merged
merged 2 commits into from
May 17, 2023

Conversation

mdelah
Copy link
Contributor

@mdelah mdelah commented May 8, 2023

The rules early-return, indent-error-flow and superfluous-else have a similar structure. This moves the common logic for classifying if-else chains to a common package.

A few side benefits:

  • "early-return" now handles os.Exit/log.Panicf/etc
  • "superfluous-else" now handles (builtin) panic
  • "superfluous-else" and "indent-error-flow" now handle if/else chains with 2+ "if" branches

Motivation: preliminary refactor to make #816 easier to implement. Tidying up code I added to early-return rule previously.

I added tests wherever I noticed a change in behaviour (noted above).

I added the new package at internal/ifelse. If there's a better home, let me know.

The rules "early-return", "indent-error-flow" and
"superfluous-else" have a similar structure. This
moves the common logic for classifying if-else chains
to a common package.

A few side benefits:
- "early-return" now handles os.Exit/log.Panicf/etc
- "superfluous-else" now handles (builtin) panic
- "superfluous-else" and "indent-error-flow" now handle if/else
  chains with 2+ "if" branches

// Kind is a classifier for if-else branches. It says whether the branch is empty,
// and whether the branch ends with a statement that can deviate control flow.
type Kind int
Copy link
Collaborator

@chavacava chavacava May 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename to BranchTermination ?

Copy link
Collaborator

@chavacava chavacava left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mdelah thanks for the PR!
It LGTM, just need to fix linting errors (mainly public identifiers without doc) before merging.
Thanks again.

@chavacava chavacava merged commit 4bb48df into mgechev:master May 17, 2023
@mdelah mdelah deleted the ifelse-common branch May 20, 2023 04:29
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants