Add optimize-operands-order rule (#599)

[doniacld]

Description

Add swap conditional expression rule that closes #599.

Test

Add a unit test with several cases that should be spotted and other not.
Tested on a repository, here is the output:

cmd/kidlectl/create.go
  (24, 10)  https://revive.run/r#swap-conditional-expr  swap 'tito(y)' and 'y' around '&&' operator  
  (25, 9)   https://revive.run/r#swap-conditional-expr  swap 'tito(y)' and 'y' around '||' operator  

Closes #599

[chavacava]

Thanks for the PR @doniacld, I've made some comments

[doniacld]

@chavacava Thanks for the code review.
Until we did not agree on a better name, please do not merge. :)

[chavacava]

I've tested the new rule on revive's code. There are some good warnings:

rule/defer.go:91:6

but also some false positives:

rule/var-naming.go:148:6

False positives are generated in cases where the right subexpression contains a function call, for example

caller(x, y) && (x && caller(y, x))

The conditions in

revive/rule/swap-conditional-expr.go

Lines 51 to 60 in 5eaf79c

	// check left expression is a caller
	x, xCaller := binExpr.X.(*ast.CallExpr)
	if !xCaller {
	return w
	}
	// check right expression is not a caller
	_, yCaller := binExpr.Y.(*ast.CallExpr)
	if yCaller {
	return w
	}

should check if the sub expression tree has a function call (the current check is only on the root of the sub-expression tree) To do that you can use

revive/rule/utils.go

Lines 112 to 116 in 0ee7866

	// pick yields a list of nodes by picking them from a sub-ast with root node n.
	// Nodes are selected by applying the fselect function
	// f function is applied to each selected node before inseting it in the final result.
	// If f==nil then it defaults to the identity function (ie it returns the node itself)
	func pick(n ast.Node, fselect func(n ast.Node) bool, f func(n ast.Node) []ast.Node) []ast.Node {

to pick function calls in the sub expression and then check if there is >1 (left) and 0 (right)

[chavacava]

I've tested the rule over some open source repositories and it looks great!
I've modified the failure's message, let me know if you are OK with it, we just need to find a good name for the rule :)