WIP Add a bit more rules for strings module
#94
Conversation
|
Looks like there are commits from #93 care to rebase? |
| m.Match(`strings.TrimFunc($s, unicode.IsSpace)`).Suggest(`strings.TrimSpace($s)`) | ||
| m.Match(`strings.Map(unicode.ToUpper, $s)`).Suggest(`strings.ToUpper($s)`) | ||
| m.Match(`strings.Map(unicode.ToLower, $s)`).Suggest(`strings.ToLower($s)`) | ||
| m.Match(`strings.Map(unicode.ToTitle, $s)`).Suggest(`strings.ToTitle($s)`) | ||
|
|
||
| m.Match(`bytes.SplitN($s, $sep, -1)`).Suggest(`bytes.Split($s, $sep)`) | ||
| m.Match(`bytes.Replace($s, $old, $new, -1)`).Suggest(`bytes.ReplaceAll($s, $old, $new)`) |
There was a problem hiding this comment.
The rule is covered by the newly introduced more general rule. Now, we handle all negative numbers, not only -1. See the tests.
| @@ -48,6 +50,8 @@ func _(m fluent.Matcher) { | |||
| Suggest(`strings.Contains($s1, $s2)`) | |||
| m.Match(`strings.Count($s1, $s2) == 0`). | |||
| Suggest(`!strings.Contains($s1, $s2)`) | |||
| m.Match(`strings.Count($s1, $s2) >= 0`). | |||
| Report(`statement always true`) | |||
There was a problem hiding this comment.
looks like bytes.Count deserve same fame ;}
There was a problem hiding this comment.
Great catch! Thank you. Let's add it :)
| func gocriticBadCall(m fluent.Matcher) { | ||
| m.Match(`strings.Replace($_, $_, $_, 0)`, |
There was a problem hiding this comment.
same here - why removed?
There was a problem hiding this comment.
The rule is covered by the newly introduced more general rule. See the tests.
|
I mark PR as WIP until #93 is merged. |
orsinium
changed the title
strings modulestrings module
|
Why not just push only changes related to checks? 👀 |
|
Because there are no tests for checks in master yet. I feel uncomfortable to commit untested changes. |
|
This change includes more than new rules. It would be easier to review if it was separated into independent PRs. I'm 100% fine with changes that cleanup the readme in a separate PR. They're super easy to review and can be merged right away. |
Blocked by #93, contains it in diff (because I need a place to put tests)
Improved checks for
nvalue instrings.Replace,strings.MatchN, andstrings.MatchAfterN:-10.2instead of1becausenmeans "parts count", not "splits count" (it's the opposite in Python).strings.Count($s1, $s2) >= 0, it's always true.