From 96a73fbecd15dd7c63bcd4f2ab10d10dc17c028d Mon Sep 17 00:00:00 2001 From: Denis Date: Sun, 16 Jan 2022 21:55:50 +0300 Subject: [PATCH 01/11] add strings.Compare checker --- checkers/rules/rules.go | 32 +++++++++++ checkers/rulesdata/rulesdata.go | 56 +++++++++++++++++++ .../testdata/stringsCompare/negative_tests.go | 16 ++++++ .../testdata/stringsCompare/positive_tests.go | 53 ++++++++++++++++++ 4 files changed, 157 insertions(+) create mode 100644 checkers/testdata/stringsCompare/negative_tests.go create mode 100644 checkers/testdata/stringsCompare/positive_tests.go diff --git a/checkers/rules/rules.go b/checkers/rules/rules.go index a612c90b6..074001358 100644 --- a/checkers/rules/rules.go +++ b/checkers/rules/rules.go @@ -736,3 +736,35 @@ func dynamicFmtString(m dsl.Matcher) { Suggest("errors.New($f($*args))"). Report(`use errors.New($f($*args)) or fmt.Errorf("%s", $f($*args)) instead`) } + +//doc:summary Detects strings.Compare usage +//doc:tags performance experimental +//doc:before strings.Compare(x, y) +//doc:after x < y +func stringsCompare(m dsl.Matcher) { + m.Match(`strings.Compare($s1, $s2) == 0`). + Report(`don't use strings.Compare on hot path, change it to built-in operators`). + Suggest(`$s1 == $s2`). + At(m["s1"]) + + m.Match(`strings.Compare($s1, $s2) < 0`, + `strings.Compare($s1, $s2) == -1`). + Report(`don't use strings.Compare on hot path, change it to built-in operators`). + Suggest(`$s1 < $s2`). + At(m["s1"]) + + m.Match(`strings.Compare($s1, $s2) > 0`, + `strings.Compare($s1, $s2) == 1`). + Report(`don't use strings.Compare on hot path, change it to built-in operators`). + Suggest(`$s1 > $s2`). + At(m["s1"]) + + m.Match(`$_($*_,strings.Compare($s1, $_), $*_)`, + `$_ = strings.Compare($s1, $_)`, + `$_ := strings.Compare($s1, $_)`, + `var $_ = strings.Compare($s1, $_)`, + `var $_ $_ = strings.Compare($s1, $_)`, + `switch strings.Compare($s1, $_) { $*_ }`). + Report(`don't use strings.Compare on hot path, change it to built-in operators`). + At(m["s1"]) +} diff --git a/checkers/rulesdata/rulesdata.go b/checkers/rulesdata/rulesdata.go index 8a952b7c2..c325389d1 100644 --- a/checkers/rulesdata/rulesdata.go +++ b/checkers/rulesdata/rulesdata.go @@ -2745,6 +2745,62 @@ var PrecompiledRules = &ir.File{ }, }, }, + ir.RuleGroup{ + Line: 744, + Name: "stringsCompare", + MatcherName: "m", + DocTags: []string{ + "performance", + "experimental", + }, + DocSummary: "Detects strings.Compare usage", + DocBefore: "strings.Compare(x, y)", + DocAfter: "x < y", + Rules: []ir.Rule{ + ir.Rule{ + Line: 745, + SyntaxPatterns: []ir.PatternString{ + ir.PatternString{Line: 745, Value: "strings.Compare($s1, $s2) == 0"}, + }, + ReportTemplate: "don't use strings.Compare on hot path, change it to built-in operators", + SuggestTemplate: "$s1 == $s2", + LocationVar: "s1", + }, + ir.Rule{ + Line: 750, + SyntaxPatterns: []ir.PatternString{ + ir.PatternString{Line: 750, Value: "strings.Compare($s1, $s2) < 0"}, + ir.PatternString{Line: 751, Value: "strings.Compare($s1, $s2) == -1"}, + }, + ReportTemplate: "don't use strings.Compare on hot path, change it to built-in operators", + SuggestTemplate: "$s1 < $s2", + LocationVar: "s1", + }, + ir.Rule{ + Line: 756, + SyntaxPatterns: []ir.PatternString{ + ir.PatternString{Line: 756, Value: "strings.Compare($s1, $s2) > 0"}, + ir.PatternString{Line: 757, Value: "strings.Compare($s1, $s2) == 1"}, + }, + ReportTemplate: "don't use strings.Compare on hot path, change it to built-in operators", + SuggestTemplate: "$s1 > $s2", + LocationVar: "s1", + }, + ir.Rule{ + Line: 762, + SyntaxPatterns: []ir.PatternString{ + ir.PatternString{Line: 762, Value: "$_($*_,strings.Compare($s1, $_), $*_)"}, + ir.PatternString{Line: 763, Value: "$_ = strings.Compare($s1, $_)"}, + ir.PatternString{Line: 764, Value: "$_ := strings.Compare($s1, $_)"}, + ir.PatternString{Line: 765, Value: "var $_ = strings.Compare($s1, $_)"}, + ir.PatternString{Line: 766, Value: "var $_ $_ = strings.Compare($s1, $_)"}, + ir.PatternString{Line: 767, Value: "switch strings.Compare($s1, $_) { $*_ }"}, + }, + ReportTemplate: "don't use strings.Compare on hot path, change it to built-in operators", + LocationVar: "s1", + }, + }, + }, }, } diff --git a/checkers/testdata/stringsCompare/negative_tests.go b/checkers/testdata/stringsCompare/negative_tests.go new file mode 100644 index 000000000..073802111 --- /dev/null +++ b/checkers/testdata/stringsCompare/negative_tests.go @@ -0,0 +1,16 @@ +package checker_test + +import ( + "bytes" +) + +type s []string + +func (s) Compare(x, y string) bool { return x > y } + +func negative() { + bytes.Compare([]byte{}, []byte{}) + + strings := s{} + print(strings.Compare("1", "3")) +} diff --git a/checkers/testdata/stringsCompare/positive_tests.go b/checkers/testdata/stringsCompare/positive_tests.go new file mode 100644 index 000000000..fd9199f83 --- /dev/null +++ b/checkers/testdata/stringsCompare/positive_tests.go @@ -0,0 +1,53 @@ +package checker_test + +import ( + "strings" +) + +func foo() { + f, b := "aaa", "bbb" + + /*! don't use strings.Compare on hot path, change it to built-in operators */ + if strings.Compare(f, b) == 0 { + } + + /*! don't use strings.Compare on hot path, change it to built-in operators */ + switch strings.Compare(f, b) { + case 0: + print(0) + case 1: + print(1) + case -1: + print(-1) + } + + /*! don't use strings.Compare on hot path, change it to built-in operators */ + switch dd := strings.Compare(f, b); dd { + case 0: + print(0) + case 1: + print(1) + case -1: + print(-1) + } + + /*! don't use strings.Compare on hot path, change it to built-in operators */ + kk(1, strings.Compare(f, b)) + + /*! don't use strings.Compare on hot path, change it to built-in operators */ + kk(1, strings.Compare(f, b), 2323) + + /*! don't use strings.Compare on hot path, change it to built-in operators */ + kk(strings.Compare(f, b), 2323) + + /*! don't use strings.Compare on hot path, change it to built-in operators */ + kk(strings.Compare(f, b)) +} + +func kk(b ...int) { print(b) } + +func bar() { + /*! don't use strings.Compare on hot path, change it to built-in operators */ + kk := strings.Compare("s", "ww") + print(kk) +} From b3072eccf63dc1902f296c9c7a7fab8d0125dfba Mon Sep 17 00:00:00 2001 From: Denis Date: Sun, 16 Jan 2022 22:27:20 +0300 Subject: [PATCH 02/11] upd --- checkers/rules/rules.go | 3 +-- checkers/rulesdata/rulesdata.go | 11 +++++------ .../testdata/stringsCompare/positive_tests.go | 16 ++++------------ 3 files changed, 10 insertions(+), 20 deletions(-) diff --git a/checkers/rules/rules.go b/checkers/rules/rules.go index 074001358..86e6df60c 100644 --- a/checkers/rules/rules.go +++ b/checkers/rules/rules.go @@ -759,8 +759,7 @@ func stringsCompare(m dsl.Matcher) { Suggest(`$s1 > $s2`). At(m["s1"]) - m.Match(`$_($*_,strings.Compare($s1, $_), $*_)`, - `$_ = strings.Compare($s1, $_)`, + m.Match(`$_ = strings.Compare($s1, $_)`, `$_ := strings.Compare($s1, $_)`, `var $_ = strings.Compare($s1, $_)`, `var $_ $_ = strings.Compare($s1, $_)`, diff --git a/checkers/rulesdata/rulesdata.go b/checkers/rulesdata/rulesdata.go index c325389d1..edbbc3b97 100644 --- a/checkers/rulesdata/rulesdata.go +++ b/checkers/rulesdata/rulesdata.go @@ -2789,12 +2789,11 @@ var PrecompiledRules = &ir.File{ ir.Rule{ Line: 762, SyntaxPatterns: []ir.PatternString{ - ir.PatternString{Line: 762, Value: "$_($*_,strings.Compare($s1, $_), $*_)"}, - ir.PatternString{Line: 763, Value: "$_ = strings.Compare($s1, $_)"}, - ir.PatternString{Line: 764, Value: "$_ := strings.Compare($s1, $_)"}, - ir.PatternString{Line: 765, Value: "var $_ = strings.Compare($s1, $_)"}, - ir.PatternString{Line: 766, Value: "var $_ $_ = strings.Compare($s1, $_)"}, - ir.PatternString{Line: 767, Value: "switch strings.Compare($s1, $_) { $*_ }"}, + ir.PatternString{Line: 762, Value: "$_ = strings.Compare($s1, $_)"}, + ir.PatternString{Line: 763, Value: "$_ := strings.Compare($s1, $_)"}, + ir.PatternString{Line: 764, Value: "var $_ = strings.Compare($s1, $_)"}, + ir.PatternString{Line: 765, Value: "var $_ $_ = strings.Compare($s1, $_)"}, + ir.PatternString{Line: 766, Value: "switch strings.Compare($s1, $_) { $*_ }"}, }, ReportTemplate: "don't use strings.Compare on hot path, change it to built-in operators", LocationVar: "s1", diff --git a/checkers/testdata/stringsCompare/positive_tests.go b/checkers/testdata/stringsCompare/positive_tests.go index fd9199f83..b3514a75d 100644 --- a/checkers/testdata/stringsCompare/positive_tests.go +++ b/checkers/testdata/stringsCompare/positive_tests.go @@ -4,6 +4,10 @@ import ( "strings" ) +type aliasInt struct { + int +} + func foo() { f, b := "aaa", "bbb" @@ -30,18 +34,6 @@ func foo() { case -1: print(-1) } - - /*! don't use strings.Compare on hot path, change it to built-in operators */ - kk(1, strings.Compare(f, b)) - - /*! don't use strings.Compare on hot path, change it to built-in operators */ - kk(1, strings.Compare(f, b), 2323) - - /*! don't use strings.Compare on hot path, change it to built-in operators */ - kk(strings.Compare(f, b), 2323) - - /*! don't use strings.Compare on hot path, change it to built-in operators */ - kk(strings.Compare(f, b)) } func kk(b ...int) { print(b) } From cdea2f1115bd5c604f5dfa24f84107cc71879a1f Mon Sep 17 00:00:00 2001 From: Denis Date: Sun, 16 Jan 2022 22:33:49 +0300 Subject: [PATCH 03/11] delete unused --- checkers/testdata/stringsCompare/positive_tests.go | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/checkers/testdata/stringsCompare/positive_tests.go b/checkers/testdata/stringsCompare/positive_tests.go index b3514a75d..32d6b7b36 100644 --- a/checkers/testdata/stringsCompare/positive_tests.go +++ b/checkers/testdata/stringsCompare/positive_tests.go @@ -4,10 +4,6 @@ import ( "strings" ) -type aliasInt struct { - int -} - func foo() { f, b := "aaa", "bbb" @@ -34,12 +30,7 @@ func foo() { case -1: print(-1) } -} - -func kk(b ...int) { print(b) } -func bar() { /*! don't use strings.Compare on hot path, change it to built-in operators */ - kk := strings.Compare("s", "ww") - print(kk) + _ = strings.Compare("s", "ww") } From 33f4fe0f41cd0d66399d1a4a2acdcbffcef4c0e6 Mon Sep 17 00:00:00 2001 From: Denis Date: Sun, 16 Jan 2022 22:39:48 +0300 Subject: [PATCH 04/11] upd negative test --- checkers/testdata/stringsCompare/negative_tests.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/checkers/testdata/stringsCompare/negative_tests.go b/checkers/testdata/stringsCompare/negative_tests.go index 073802111..82f90b2d3 100644 --- a/checkers/testdata/stringsCompare/negative_tests.go +++ b/checkers/testdata/stringsCompare/negative_tests.go @@ -6,11 +6,16 @@ import ( type s []string -func (s) Compare(x, y string) bool { return x > y } +func (s) Compare(x, y string) int { + if x < y { + return 1 + } + return 0 +} func negative() { bytes.Compare([]byte{}, []byte{}) strings := s{} - print(strings.Compare("1", "3")) + _ = strings.Compare("1", "3") == 0 } From 6fcd6a3dc9b7edd705778b07661478cb92d92981 Mon Sep 17 00:00:00 2001 From: Denis Date: Sun, 16 Jan 2022 23:15:37 +0300 Subject: [PATCH 05/11] upd rules --- checkers/rules/rules.go | 27 +++---- checkers/rulesdata/rulesdata.go | 73 +++++++++++++------ .../testdata/stringsCompare/positive_tests.go | 29 +++----- 3 files changed, 75 insertions(+), 54 deletions(-) diff --git a/checkers/rules/rules.go b/checkers/rules/rules.go index 86e6df60c..bb2628643 100644 --- a/checkers/rules/rules.go +++ b/checkers/rules/rules.go @@ -742,28 +742,23 @@ func dynamicFmtString(m dsl.Matcher) { //doc:before strings.Compare(x, y) //doc:after x < y func stringsCompare(m dsl.Matcher) { - m.Match(`strings.Compare($s1, $s2) == 0`). - Report(`don't use strings.Compare on hot path, change it to built-in operators`). + m.Match(`strings.Compare($s1, $s2) == 0`, `0 == strings.Compare($s1, $s2)`). Suggest(`$s1 == $s2`). At(m["s1"]) - m.Match(`strings.Compare($s1, $s2) < 0`, - `strings.Compare($s1, $s2) == -1`). - Report(`don't use strings.Compare on hot path, change it to built-in operators`). + m.Match(`strings.Compare($s1, $s2) < $i`, + `$i > strings.Compare($s1, $s2)`, + `strings.Compare($s1, $s2) == -1`, + `-1 == strings.Compare($s1, $s2)`). + Where(m["i"].Value.Int() < 0). Suggest(`$s1 < $s2`). At(m["s1"]) - m.Match(`strings.Compare($s1, $s2) > 0`, - `strings.Compare($s1, $s2) == 1`). - Report(`don't use strings.Compare on hot path, change it to built-in operators`). + m.Match(`strings.Compare($s1, $s2) > $i`, + `$i < strings.Compare($s1, $s2)`, + `strings.Compare($s1, $s2) == 1`, + `1 == strings.Compare($s1, $s2)`). + Where(m["i"].Value.Int() > 0). Suggest(`$s1 > $s2`). At(m["s1"]) - - m.Match(`$_ = strings.Compare($s1, $_)`, - `$_ := strings.Compare($s1, $_)`, - `var $_ = strings.Compare($s1, $_)`, - `var $_ $_ = strings.Compare($s1, $_)`, - `switch strings.Compare($s1, $_) { $*_ }`). - Report(`don't use strings.Compare on hot path, change it to built-in operators`). - At(m["s1"]) } diff --git a/checkers/rulesdata/rulesdata.go b/checkers/rulesdata/rulesdata.go index edbbc3b97..e3338c90b 100644 --- a/checkers/rulesdata/rulesdata.go +++ b/checkers/rulesdata/rulesdata.go @@ -2761,42 +2761,73 @@ var PrecompiledRules = &ir.File{ Line: 745, SyntaxPatterns: []ir.PatternString{ ir.PatternString{Line: 745, Value: "strings.Compare($s1, $s2) == 0"}, + ir.PatternString{Line: 745, Value: "0 == strings.Compare($s1, $s2)"}, }, - ReportTemplate: "don't use strings.Compare on hot path, change it to built-in operators", + ReportTemplate: "suggestion: $s1 == $s2", SuggestTemplate: "$s1 == $s2", LocationVar: "s1", }, ir.Rule{ - Line: 750, + Line: 749, SyntaxPatterns: []ir.PatternString{ - ir.PatternString{Line: 750, Value: "strings.Compare($s1, $s2) < 0"}, + ir.PatternString{Line: 749, Value: "strings.Compare($s1, $s2) < $i"}, + ir.PatternString{Line: 750, Value: "$i > strings.Compare($s1, $s2)"}, ir.PatternString{Line: 751, Value: "strings.Compare($s1, $s2) == -1"}, + ir.PatternString{Line: 752, Value: "-1 == strings.Compare($s1, $s2)"}, }, - ReportTemplate: "don't use strings.Compare on hot path, change it to built-in operators", + ReportTemplate: "suggestion: $s1 < $s2", SuggestTemplate: "$s1 < $s2", - LocationVar: "s1", + WhereExpr: ir.FilterExpr{ + Line: 753, + Op: ir.FilterLtOp, + Src: "m[\"i\"].Value.Int() < 0", + Args: []ir.FilterExpr{ + ir.FilterExpr{ + Line: 753, + Op: ir.FilterVarValueIntOp, + Src: "m[\"i\"].Value.Int()", + Value: "i", + }, + ir.FilterExpr{ + Line: 753, + Op: ir.FilterIntOp, + Src: "0", + Value: int64(0), + }, + }, + }, + LocationVar: "s1", }, ir.Rule{ - Line: 756, + Line: 757, SyntaxPatterns: []ir.PatternString{ - ir.PatternString{Line: 756, Value: "strings.Compare($s1, $s2) > 0"}, - ir.PatternString{Line: 757, Value: "strings.Compare($s1, $s2) == 1"}, + ir.PatternString{Line: 757, Value: "strings.Compare($s1, $s2) > $i"}, + ir.PatternString{Line: 758, Value: "$i < strings.Compare($s1, $s2)"}, + ir.PatternString{Line: 759, Value: "strings.Compare($s1, $s2) == 1"}, + ir.PatternString{Line: 760, Value: "1 == strings.Compare($s1, $s2)"}, }, - ReportTemplate: "don't use strings.Compare on hot path, change it to built-in operators", + ReportTemplate: "suggestion: $s1 > $s2", SuggestTemplate: "$s1 > $s2", - LocationVar: "s1", - }, - ir.Rule{ - Line: 762, - SyntaxPatterns: []ir.PatternString{ - ir.PatternString{Line: 762, Value: "$_ = strings.Compare($s1, $_)"}, - ir.PatternString{Line: 763, Value: "$_ := strings.Compare($s1, $_)"}, - ir.PatternString{Line: 764, Value: "var $_ = strings.Compare($s1, $_)"}, - ir.PatternString{Line: 765, Value: "var $_ $_ = strings.Compare($s1, $_)"}, - ir.PatternString{Line: 766, Value: "switch strings.Compare($s1, $_) { $*_ }"}, + WhereExpr: ir.FilterExpr{ + Line: 761, + Op: ir.FilterGtOp, + Src: "m[\"i\"].Value.Int() > 0", + Args: []ir.FilterExpr{ + ir.FilterExpr{ + Line: 761, + Op: ir.FilterVarValueIntOp, + Src: "m[\"i\"].Value.Int()", + Value: "i", + }, + ir.FilterExpr{ + Line: 761, + Op: ir.FilterIntOp, + Src: "0", + Value: int64(0), + }, + }, }, - ReportTemplate: "don't use strings.Compare on hot path, change it to built-in operators", - LocationVar: "s1", + LocationVar: "s1", }, }, }, diff --git a/checkers/testdata/stringsCompare/positive_tests.go b/checkers/testdata/stringsCompare/positive_tests.go index 32d6b7b36..1596403e9 100644 --- a/checkers/testdata/stringsCompare/positive_tests.go +++ b/checkers/testdata/stringsCompare/positive_tests.go @@ -7,30 +7,25 @@ import ( func foo() { f, b := "aaa", "bbb" - /*! don't use strings.Compare on hot path, change it to built-in operators */ + /*! suggestion: f == b */ if strings.Compare(f, b) == 0 { } - /*! don't use strings.Compare on hot path, change it to built-in operators */ - switch strings.Compare(f, b) { - case 0: - print(0) - case 1: - print(1) - case -1: - print(-1) + /*! suggestion: f == b */ + if 0 == strings.Compare(f, b) { } - /*! don't use strings.Compare on hot path, change it to built-in operators */ - switch dd := strings.Compare(f, b); dd { - case 0: + /*! suggestion: f > b */ + switch dd := strings.Compare(f, b) > 100; dd { + case true: print(0) - case 1: + case false: print(1) - case -1: - print(-1) } - /*! don't use strings.Compare on hot path, change it to built-in operators */ - _ = strings.Compare("s", "ww") + _ = strings.Compare("s", "ww") < 10 + _ = 10 > strings.Compare("s", "ww") + + _ = strings.Compare(f, b) < 10 + _ = 10 > strings.Compare(f, b) } From 04c71dfb0ebb94a04afb0225cef365da5c4bec49 Mon Sep 17 00:00:00 2001 From: Denis Date: Sat, 22 Jan 2022 00:37:08 +0300 Subject: [PATCH 06/11] fix --- checkers/rules/rules.go | 18 +++--- checkers/rulesdata/rulesdata.go | 56 +++---------------- .../testdata/stringsCompare/positive_tests.go | 20 +++++-- 3 files changed, 31 insertions(+), 63 deletions(-) diff --git a/checkers/rules/rules.go b/checkers/rules/rules.go index bb2628643..029b1011a 100644 --- a/checkers/rules/rules.go +++ b/checkers/rules/rules.go @@ -746,19 +746,17 @@ func stringsCompare(m dsl.Matcher) { Suggest(`$s1 == $s2`). At(m["s1"]) - m.Match(`strings.Compare($s1, $s2) < $i`, - `$i > strings.Compare($s1, $s2)`, - `strings.Compare($s1, $s2) == -1`, - `-1 == strings.Compare($s1, $s2)`). - Where(m["i"].Value.Int() < 0). + m.Match(`strings.Compare($s1, $s2) == -1`, + `-1 == strings.Compare($s1, $s2)`, + `strings.Compare($s1, $s2) < 0`, + `0 > strings.Compare($s1, $s2)`). Suggest(`$s1 < $s2`). At(m["s1"]) - m.Match(`strings.Compare($s1, $s2) > $i`, - `$i < strings.Compare($s1, $s2)`, - `strings.Compare($s1, $s2) == 1`, - `1 == strings.Compare($s1, $s2)`). - Where(m["i"].Value.Int() > 0). + m.Match(`strings.Compare($s1, $s2) == 1`, + `1 == strings.Compare($s1, $s2)`, + `strings.Compare($s1, $s2) > 0`, + `0 < strings.Compare($s1, $s2)`). Suggest(`$s1 > $s2`). At(m["s1"]) } diff --git a/checkers/rulesdata/rulesdata.go b/checkers/rulesdata/rulesdata.go index e3338c90b..da0f0526e 100644 --- a/checkers/rulesdata/rulesdata.go +++ b/checkers/rulesdata/rulesdata.go @@ -2761,73 +2761,35 @@ var PrecompiledRules = &ir.File{ Line: 745, SyntaxPatterns: []ir.PatternString{ ir.PatternString{Line: 745, Value: "strings.Compare($s1, $s2) == 0"}, - ir.PatternString{Line: 745, Value: "0 == strings.Compare($s1, $s2)"}, + ir.PatternString{Line: 746, Value: "0 == strings.Compare($s1, $s2)"}, }, ReportTemplate: "suggestion: $s1 == $s2", SuggestTemplate: "$s1 == $s2", LocationVar: "s1", }, ir.Rule{ - Line: 749, + Line: 751, SyntaxPatterns: []ir.PatternString{ - ir.PatternString{Line: 749, Value: "strings.Compare($s1, $s2) < $i"}, - ir.PatternString{Line: 750, Value: "$i > strings.Compare($s1, $s2)"}, ir.PatternString{Line: 751, Value: "strings.Compare($s1, $s2) == -1"}, ir.PatternString{Line: 752, Value: "-1 == strings.Compare($s1, $s2)"}, + ir.PatternString{Line: 753, Value: "strings.Compare($s1, $s2) < 0"}, + ir.PatternString{Line: 754, Value: "0 > strings.Compare($s1, $s2)"}, }, ReportTemplate: "suggestion: $s1 < $s2", SuggestTemplate: "$s1 < $s2", - WhereExpr: ir.FilterExpr{ - Line: 753, - Op: ir.FilterLtOp, - Src: "m[\"i\"].Value.Int() < 0", - Args: []ir.FilterExpr{ - ir.FilterExpr{ - Line: 753, - Op: ir.FilterVarValueIntOp, - Src: "m[\"i\"].Value.Int()", - Value: "i", - }, - ir.FilterExpr{ - Line: 753, - Op: ir.FilterIntOp, - Src: "0", - Value: int64(0), - }, - }, - }, - LocationVar: "s1", + LocationVar: "s1", }, ir.Rule{ - Line: 757, + Line: 759, SyntaxPatterns: []ir.PatternString{ - ir.PatternString{Line: 757, Value: "strings.Compare($s1, $s2) > $i"}, - ir.PatternString{Line: 758, Value: "$i < strings.Compare($s1, $s2)"}, ir.PatternString{Line: 759, Value: "strings.Compare($s1, $s2) == 1"}, ir.PatternString{Line: 760, Value: "1 == strings.Compare($s1, $s2)"}, + ir.PatternString{Line: 761, Value: "strings.Compare($s1, $s2) > 0"}, + ir.PatternString{Line: 762, Value: "0 < strings.Compare($s1, $s2)"}, }, ReportTemplate: "suggestion: $s1 > $s2", SuggestTemplate: "$s1 > $s2", - WhereExpr: ir.FilterExpr{ - Line: 761, - Op: ir.FilterGtOp, - Src: "m[\"i\"].Value.Int() > 0", - Args: []ir.FilterExpr{ - ir.FilterExpr{ - Line: 761, - Op: ir.FilterVarValueIntOp, - Src: "m[\"i\"].Value.Int()", - Value: "i", - }, - ir.FilterExpr{ - Line: 761, - Op: ir.FilterIntOp, - Src: "0", - Value: int64(0), - }, - }, - }, - LocationVar: "s1", + LocationVar: "s1", }, }, }, diff --git a/checkers/testdata/stringsCompare/positive_tests.go b/checkers/testdata/stringsCompare/positive_tests.go index 1596403e9..e19219c69 100644 --- a/checkers/testdata/stringsCompare/positive_tests.go +++ b/checkers/testdata/stringsCompare/positive_tests.go @@ -4,7 +4,7 @@ import ( "strings" ) -func foo() { +func warning() { f, b := "aaa", "bbb" /*! suggestion: f == b */ @@ -16,16 +16,24 @@ func foo() { } /*! suggestion: f > b */ - switch dd := strings.Compare(f, b) > 100; dd { + switch foo := strings.Compare(f, b) > 0; foo { case true: print(0) case false: print(1) } - _ = strings.Compare("s", "ww") < 10 - _ = 10 > strings.Compare("s", "ww") + /*! suggestion: "s" < "ww" */ + _ = strings.Compare("s", "ww") < 0 + /*! suggestion: "s" == "ww" */ + _ = strings.Compare("s", "ww") == 0 + /*! suggestion: "s" > "ww" */ + _ = strings.Compare("s", "ww") > 0 - _ = strings.Compare(f, b) < 10 - _ = 10 > strings.Compare(f, b) + /*! suggestion: "s" < "ww" */ + _ = 0 > strings.Compare("s", "ww") + /*! suggestion: "s" == "ww" */ + _ = 0 == strings.Compare("s", "ww") + /*! suggestion: "s" > "ww" */ + _ = 0 < strings.Compare("s", "ww") } From 676c8eacda67baa310c299281bfe68e14da06561 Mon Sep 17 00:00:00 2001 From: Denis Date: Sat, 22 Jan 2022 00:39:16 +0300 Subject: [PATCH 07/11] update category --- checkers/rules/rules.go | 2 +- checkers/rulesdata/rulesdata.go | 24 ++++++++++++------------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/checkers/rules/rules.go b/checkers/rules/rules.go index 029b1011a..f7e713f18 100644 --- a/checkers/rules/rules.go +++ b/checkers/rules/rules.go @@ -738,7 +738,7 @@ func dynamicFmtString(m dsl.Matcher) { } //doc:summary Detects strings.Compare usage -//doc:tags performance experimental +//doc:tags style experimental //doc:before strings.Compare(x, y) //doc:after x < y func stringsCompare(m dsl.Matcher) { diff --git a/checkers/rulesdata/rulesdata.go b/checkers/rulesdata/rulesdata.go index da0f0526e..07f479e7b 100644 --- a/checkers/rulesdata/rulesdata.go +++ b/checkers/rulesdata/rulesdata.go @@ -2750,7 +2750,7 @@ var PrecompiledRules = &ir.File{ Name: "stringsCompare", MatcherName: "m", DocTags: []string{ - "performance", + "style", "experimental", }, DocSummary: "Detects strings.Compare usage", @@ -2761,31 +2761,31 @@ var PrecompiledRules = &ir.File{ Line: 745, SyntaxPatterns: []ir.PatternString{ ir.PatternString{Line: 745, Value: "strings.Compare($s1, $s2) == 0"}, - ir.PatternString{Line: 746, Value: "0 == strings.Compare($s1, $s2)"}, + ir.PatternString{Line: 745, Value: "0 == strings.Compare($s1, $s2)"}, }, ReportTemplate: "suggestion: $s1 == $s2", SuggestTemplate: "$s1 == $s2", LocationVar: "s1", }, ir.Rule{ - Line: 751, + Line: 749, SyntaxPatterns: []ir.PatternString{ - ir.PatternString{Line: 751, Value: "strings.Compare($s1, $s2) == -1"}, - ir.PatternString{Line: 752, Value: "-1 == strings.Compare($s1, $s2)"}, - ir.PatternString{Line: 753, Value: "strings.Compare($s1, $s2) < 0"}, - ir.PatternString{Line: 754, Value: "0 > strings.Compare($s1, $s2)"}, + ir.PatternString{Line: 749, Value: "strings.Compare($s1, $s2) == -1"}, + ir.PatternString{Line: 750, Value: "-1 == strings.Compare($s1, $s2)"}, + ir.PatternString{Line: 751, Value: "strings.Compare($s1, $s2) < 0"}, + ir.PatternString{Line: 752, Value: "0 > strings.Compare($s1, $s2)"}, }, ReportTemplate: "suggestion: $s1 < $s2", SuggestTemplate: "$s1 < $s2", LocationVar: "s1", }, ir.Rule{ - Line: 759, + Line: 756, SyntaxPatterns: []ir.PatternString{ - ir.PatternString{Line: 759, Value: "strings.Compare($s1, $s2) == 1"}, - ir.PatternString{Line: 760, Value: "1 == strings.Compare($s1, $s2)"}, - ir.PatternString{Line: 761, Value: "strings.Compare($s1, $s2) > 0"}, - ir.PatternString{Line: 762, Value: "0 < strings.Compare($s1, $s2)"}, + ir.PatternString{Line: 756, Value: "strings.Compare($s1, $s2) == 1"}, + ir.PatternString{Line: 757, Value: "1 == strings.Compare($s1, $s2)"}, + ir.PatternString{Line: 758, Value: "strings.Compare($s1, $s2) > 0"}, + ir.PatternString{Line: 759, Value: "0 < strings.Compare($s1, $s2)"}, }, ReportTemplate: "suggestion: $s1 > $s2", SuggestTemplate: "$s1 > $s2", From 60892b9b478a9d4855bc034fc8066b38f718cf43 Mon Sep 17 00:00:00 2001 From: Denis Date: Sat, 22 Jan 2022 00:42:49 +0300 Subject: [PATCH 08/11] fix autofix block --- checkers/rules/rules.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/checkers/rules/rules.go b/checkers/rules/rules.go index f7e713f18..89c7ac1ea 100644 --- a/checkers/rules/rules.go +++ b/checkers/rules/rules.go @@ -743,20 +743,17 @@ func dynamicFmtString(m dsl.Matcher) { //doc:after x < y func stringsCompare(m dsl.Matcher) { m.Match(`strings.Compare($s1, $s2) == 0`, `0 == strings.Compare($s1, $s2)`). - Suggest(`$s1 == $s2`). - At(m["s1"]) + Suggest(`$s1 == $s2`) m.Match(`strings.Compare($s1, $s2) == -1`, `-1 == strings.Compare($s1, $s2)`, `strings.Compare($s1, $s2) < 0`, `0 > strings.Compare($s1, $s2)`). - Suggest(`$s1 < $s2`). - At(m["s1"]) + Suggest(`$s1 < $s2`) m.Match(`strings.Compare($s1, $s2) == 1`, `1 == strings.Compare($s1, $s2)`, `strings.Compare($s1, $s2) > 0`, `0 < strings.Compare($s1, $s2)`). - Suggest(`$s1 > $s2`). - At(m["s1"]) + Suggest(`$s1 > $s2`) } From 9dda0db7d6f9e32d5d6eac95c5c8067205a6ae25 Mon Sep 17 00:00:00 2001 From: Denis Date: Sat, 22 Jan 2022 00:43:49 +0300 Subject: [PATCH 09/11] upd generated --- checkers/rulesdata/rulesdata.go | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/checkers/rulesdata/rulesdata.go b/checkers/rulesdata/rulesdata.go index 07f479e7b..392c03de1 100644 --- a/checkers/rulesdata/rulesdata.go +++ b/checkers/rulesdata/rulesdata.go @@ -2765,31 +2765,28 @@ var PrecompiledRules = &ir.File{ }, ReportTemplate: "suggestion: $s1 == $s2", SuggestTemplate: "$s1 == $s2", - LocationVar: "s1", }, ir.Rule{ - Line: 749, + Line: 748, SyntaxPatterns: []ir.PatternString{ - ir.PatternString{Line: 749, Value: "strings.Compare($s1, $s2) == -1"}, - ir.PatternString{Line: 750, Value: "-1 == strings.Compare($s1, $s2)"}, - ir.PatternString{Line: 751, Value: "strings.Compare($s1, $s2) < 0"}, - ir.PatternString{Line: 752, Value: "0 > strings.Compare($s1, $s2)"}, + ir.PatternString{Line: 748, Value: "strings.Compare($s1, $s2) == -1"}, + ir.PatternString{Line: 749, Value: "-1 == strings.Compare($s1, $s2)"}, + ir.PatternString{Line: 750, Value: "strings.Compare($s1, $s2) < 0"}, + ir.PatternString{Line: 751, Value: "0 > strings.Compare($s1, $s2)"}, }, ReportTemplate: "suggestion: $s1 < $s2", SuggestTemplate: "$s1 < $s2", - LocationVar: "s1", }, ir.Rule{ - Line: 756, + Line: 754, SyntaxPatterns: []ir.PatternString{ - ir.PatternString{Line: 756, Value: "strings.Compare($s1, $s2) == 1"}, - ir.PatternString{Line: 757, Value: "1 == strings.Compare($s1, $s2)"}, - ir.PatternString{Line: 758, Value: "strings.Compare($s1, $s2) > 0"}, - ir.PatternString{Line: 759, Value: "0 < strings.Compare($s1, $s2)"}, + ir.PatternString{Line: 754, Value: "strings.Compare($s1, $s2) == 1"}, + ir.PatternString{Line: 755, Value: "1 == strings.Compare($s1, $s2)"}, + ir.PatternString{Line: 756, Value: "strings.Compare($s1, $s2) > 0"}, + ir.PatternString{Line: 757, Value: "0 < strings.Compare($s1, $s2)"}, }, ReportTemplate: "suggestion: $s1 > $s2", SuggestTemplate: "$s1 > $s2", - LocationVar: "s1", }, }, }, From c0906549050c392d965499659be93083a52eab59 Mon Sep 17 00:00:00 2001 From: Denis Date: Sat, 22 Jan 2022 16:11:11 +0300 Subject: [PATCH 10/11] delete yoda style expr --- checkers/rules/rules.go | 10 +++------- checkers/rulesdata/rulesdata.go | 13 ++++--------- checkers/testdata/stringsCompare/positive_tests.go | 10 +--------- 3 files changed, 8 insertions(+), 25 deletions(-) diff --git a/checkers/rules/rules.go b/checkers/rules/rules.go index 89c7ac1ea..ca5a5c067 100644 --- a/checkers/rules/rules.go +++ b/checkers/rules/rules.go @@ -742,18 +742,14 @@ func dynamicFmtString(m dsl.Matcher) { //doc:before strings.Compare(x, y) //doc:after x < y func stringsCompare(m dsl.Matcher) { - m.Match(`strings.Compare($s1, $s2) == 0`, `0 == strings.Compare($s1, $s2)`). + m.Match(`strings.Compare($s1, $s2) == 0`). Suggest(`$s1 == $s2`) m.Match(`strings.Compare($s1, $s2) == -1`, - `-1 == strings.Compare($s1, $s2)`, - `strings.Compare($s1, $s2) < 0`, - `0 > strings.Compare($s1, $s2)`). + `strings.Compare($s1, $s2) < 0`). Suggest(`$s1 < $s2`) m.Match(`strings.Compare($s1, $s2) == 1`, - `1 == strings.Compare($s1, $s2)`, - `strings.Compare($s1, $s2) > 0`, - `0 < strings.Compare($s1, $s2)`). + `strings.Compare($s1, $s2) > 0`). Suggest(`$s1 > $s2`) } diff --git a/checkers/rulesdata/rulesdata.go b/checkers/rulesdata/rulesdata.go index 392c03de1..b759c4abb 100644 --- a/checkers/rulesdata/rulesdata.go +++ b/checkers/rulesdata/rulesdata.go @@ -2761,7 +2761,6 @@ var PrecompiledRules = &ir.File{ Line: 745, SyntaxPatterns: []ir.PatternString{ ir.PatternString{Line: 745, Value: "strings.Compare($s1, $s2) == 0"}, - ir.PatternString{Line: 745, Value: "0 == strings.Compare($s1, $s2)"}, }, ReportTemplate: "suggestion: $s1 == $s2", SuggestTemplate: "$s1 == $s2", @@ -2770,20 +2769,16 @@ var PrecompiledRules = &ir.File{ Line: 748, SyntaxPatterns: []ir.PatternString{ ir.PatternString{Line: 748, Value: "strings.Compare($s1, $s2) == -1"}, - ir.PatternString{Line: 749, Value: "-1 == strings.Compare($s1, $s2)"}, - ir.PatternString{Line: 750, Value: "strings.Compare($s1, $s2) < 0"}, - ir.PatternString{Line: 751, Value: "0 > strings.Compare($s1, $s2)"}, + ir.PatternString{Line: 749, Value: "strings.Compare($s1, $s2) < 0"}, }, ReportTemplate: "suggestion: $s1 < $s2", SuggestTemplate: "$s1 < $s2", }, ir.Rule{ - Line: 754, + Line: 752, SyntaxPatterns: []ir.PatternString{ - ir.PatternString{Line: 754, Value: "strings.Compare($s1, $s2) == 1"}, - ir.PatternString{Line: 755, Value: "1 == strings.Compare($s1, $s2)"}, - ir.PatternString{Line: 756, Value: "strings.Compare($s1, $s2) > 0"}, - ir.PatternString{Line: 757, Value: "0 < strings.Compare($s1, $s2)"}, + ir.PatternString{Line: 752, Value: "strings.Compare($s1, $s2) == 1"}, + ir.PatternString{Line: 753, Value: "strings.Compare($s1, $s2) > 0"}, }, ReportTemplate: "suggestion: $s1 > $s2", SuggestTemplate: "$s1 > $s2", diff --git a/checkers/testdata/stringsCompare/positive_tests.go b/checkers/testdata/stringsCompare/positive_tests.go index e19219c69..f5ce8b333 100644 --- a/checkers/testdata/stringsCompare/positive_tests.go +++ b/checkers/testdata/stringsCompare/positive_tests.go @@ -11,10 +11,6 @@ func warning() { if strings.Compare(f, b) == 0 { } - /*! suggestion: f == b */ - if 0 == strings.Compare(f, b) { - } - /*! suggestion: f > b */ switch foo := strings.Compare(f, b) > 0; foo { case true: @@ -30,10 +26,6 @@ func warning() { /*! suggestion: "s" > "ww" */ _ = strings.Compare("s", "ww") > 0 - /*! suggestion: "s" < "ww" */ - _ = 0 > strings.Compare("s", "ww") - /*! suggestion: "s" == "ww" */ - _ = 0 == strings.Compare("s", "ww") /*! suggestion: "s" > "ww" */ - _ = 0 < strings.Compare("s", "ww") + _ = strings.Compare("s", "ww") > 0 } From 051400966642b63436941db9fb8a0f37edaffb5f Mon Sep 17 00:00:00 2001 From: Denis Date: Mon, 24 Jan 2022 01:13:39 +0300 Subject: [PATCH 11/11] add more test cases --- .../testdata/stringsCompare/negative_tests.go | 57 +++++++++++++++++++ .../testdata/stringsCompare/positive_tests.go | 5 ++ 2 files changed, 62 insertions(+) diff --git a/checkers/testdata/stringsCompare/negative_tests.go b/checkers/testdata/stringsCompare/negative_tests.go index 82f90b2d3..5cc0420d7 100644 --- a/checkers/testdata/stringsCompare/negative_tests.go +++ b/checkers/testdata/stringsCompare/negative_tests.go @@ -2,6 +2,7 @@ package checker_test import ( "bytes" + "strings" ) type s []string @@ -19,3 +20,59 @@ func negative() { strings := s{} _ = strings.Compare("1", "3") == 0 } + +func negative2() { + var a, b = "aaa", "bbb" + if a > b { + print(1) + } + + if a == b { + print(0) + } + + if a < b { + print(-1) + } + + switch a > b { + case true: + print(1) + case false: + print(0, -1) + } +} + +func negative3() { + if "aaa" > "bbb" { + print(1) + } + + if "aaa" == "bbb" { + print(0) + } + + if "aaa" < "bbb" { + print(-1) + } + + switch "aaa" > "bbb" { + case true: + print(1) + case false: + print(0, -1) + } +} + +func negative4() { + f, b := "aaa", "bbb" + + _ = strings.Compare(f, b) > -100 + _ = strings.Compare(f, b) < 100 + _ = strings.Compare(f, b) >= -1 + _ = strings.Compare(f, b) <= -1 + _ = strings.Compare(f, b) >= 1 + _ = strings.Compare(f, b) <= 1 + _ = strings.Compare(f, b) >= 0 + _ = strings.Compare(f, b) <= 0 +} diff --git a/checkers/testdata/stringsCompare/positive_tests.go b/checkers/testdata/stringsCompare/positive_tests.go index f5ce8b333..4d1cb51c4 100644 --- a/checkers/testdata/stringsCompare/positive_tests.go +++ b/checkers/testdata/stringsCompare/positive_tests.go @@ -28,4 +28,9 @@ func warning() { /*! suggestion: "s" > "ww" */ _ = strings.Compare("s", "ww") > 0 + + /*! suggestion: "s" > "ww" */ + _ = strings.Compare("s", "ww") == 1 + /*! suggestion: "s" < "ww" */ + _ = strings.Compare("s", "ww") == -1 }