HairyFotr / linter

Static Analysis Compiler Plugin for Scala
Apache License 2.0
268 stars 34 forks source link

New reverse-related checks #34

Closed vdichev closed 8 years ago

vdichev commented 8 years ago

New checks with reverse (ideas from https://pavelfatin.com/scala-collections-tips-and-tricks/):

HairyFotr commented 8 years ago

Looks good, I'll merge now... and I hope to take a closer look at the article in a few days - looks like it has plenty of possible static check ideas contained in it.

Thanks!

vdichev commented 8 years ago

Actually most of these have already been implemented! I might open an issue for a quick discussion on some low-hanging fruit from the article which might be a good idea to implement.

For this pull request I haven't changed the README, are the correct links to the appropriate line in the test generated?

I've also considered merging some checks, like reverse.{head,headOption,map,iterator} into one and then reverse.{head,take(n)}.reverse into one similarly to UseFuncNot{Fold,Reduce}. Not sure it's worth it though, what do you think?

HairyFotr commented 8 years ago

Firstly, sorry for the slow reply.

Yes, please open a new issue, and I'll try get through the article soon :)

The list is generated by the shell script printWarnings and copy pasted into the readme.

I'd prefer if the checks stay separate to enable more fine-grained control. If there's a request to have them joined, a feature for tagging/grouping checks could be implemented, which would also solve a few other popular requests (e.g. tagging checks as optimization / style / probable bug, or to skip slowest checks, or all checks on a certain compiler phase, etc.).

Btw, I've expanded these and some other checks to also include String (the AST is the same as for Array, just with "augmentString" instead of "arrayOps").

vdichev commented 8 years ago

Yes, the strings checks are useful.

I already have a few more checks implemented like using .head instead of seq(0) and not implementing headOption manually with an if statement.

What I was interested in is a place to discuss which checks are more appropriate to implement in terms of code style and weeding out the more controversial advise, and also which have a higher ratio of usefulness to false positives.

I'll submit my next pull request in the next couple of days and try with issues for the next checks.