This repository has been archived by the owner on Mar 25, 2021. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added unnecessary-else Rule #4502
Added unnecessary-else Rule #4502
Changes from 10 commits
5bd58d2
021b97c
90e9d7a
ee45076
8d4f035
c2db576
a1ec00c
b46d538
89c814e
0f7d798
2e0825f
7578c43
b281653
be2f8b4
f36d420
a2842a9
d7425c2
717baef
4c29b43
418c234
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a purely stylistic rule; is there any functional difference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like you have a few different
ts.forEachChild
s: individual ones for the node'selseStatement
andthenStatement
if it's anIfStatement
, but just a regular recursion otherwise? You should be able to simplify this a bit by only having one call tots.forEachChild
that's always called.Also, if
jumpStatement
is undefined butnode.elseStatement
isn't, this code will skip it. That seems like a potential bug?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done be2f8b4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry @debsmita1 I think I wasn't very clear here. I'm suggesting you don't need to have
ts.forEachChild
statements in the bigif (utils.isIfStatement
block:Note the lack of
else
statement: a singlets.forEachChild
should recurse on all the node's children no matter what. Could you please switch to that? It's a bit less code and better practice to minimize recursive calls (since they can get tricky in larger rules).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done f36d420