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

Rule request: identical operands in comparison operator rule #1371

Closed
marcelofabri opened this issue Mar 19, 2017 · 9 comments
Closed

Rule request: identical operands in comparison operator rule #1371

marcelofabri opened this issue Mar 19, 2017 · 9 comments
Labels
rule-request Requests for a new rules.

Comments

@marcelofabri
Copy link
Collaborator

// not ok
rhs.applyToDictionaries == rhs.applyToDictionaries

// ok
lhs.applyToDictionaries == rhs.applyToDictionaries

See #1368 for more history.

@masters3d
Copy link
Contributor

masters3d commented May 15, 2017

What do you think of this regex? (\b\S*\b)\s*(==|!=|<|>|<=|>=)\s*\1
escaped: "(\\b\\S*\\b)\\s*(==|!=|<|>|<=|>=)\\s*\\1"
image

@masters3d
Copy link
Contributor

I think instead of matching anything \S we should probably match only valid operands [A-Za-z0-9_\.]+
That would give us this:
(\b[A-Za-z0-9_\.]+\b)\s*(==|!=|<|>|<=|>=)\s*\1

@masters3d
Copy link
Contributor

@marcelofabri Do you think this rule should be on by default?

@jpsim
Copy link
Collaborator

jpsim commented May 19, 2017

Seems like a reasonable place to start to me! OSSCheck would undoubtedly catch if this produces common false positives.

@masters3d
Copy link
Contributor

@jpsim You had an idea of more rules around this sort of line of thinking? (I think I remember you posted something on twitter with a document that pointed out some of these common issues.)

@masters3d
Copy link
Contributor

@ZevEisenberg
Copy link
Contributor

Just lost a half hour to this exact issue. Came here to post my +1.

@masters3d
Copy link
Contributor

Equal to (a == a)
Not equal to (a != a)
Greater than (a > a)
Less than (a < a)
Greater than or equal to (a >= a)
Less than or equal to (a <= a)

In addition to the basic operators above

we should also include:
Logical AND (a && b) // avoid a && a
Logical OR (a || b) // avoid a || a

surprisingly Logical NOT (!a) can be essentially ignored which is good since this could have been tripped the unwrap operator.

ternary conditional operator:
bool ? a : a // Avoid same value for true and false case.

@realm-probot
Copy link

realm-probot bot commented Sep 10, 2018

Hey - looks like you forgot to add a T:* label - could you please add one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule-request Requests for a new rules.
Projects
None yet
Development

No branches or pull requests

4 participants