RuboCop for Code Review

Code Review Meetup #1 https://connpass.com/event/75567/
https://docs.google.com/presentation/d/e/2PACX-1vQRTU7twM5Yv32YNvlEHeAb1JAhuJx2oZPrX6_aqXe0fEUooVEi64HFxFKXGbfR3DjtUUYSRmLX562z/pub?start=false&loop=false&delayms=3000

Transcript

1. RuboCop for
   Code Review
   Code Review Meetup #1
   Jan. 19, 2018
   

   View Slide

2. User.find(name: "pocke").inspect
   ● Masataka Kuwabara
   ● Engineer at Actcat, Inc. / SideCI
   ○ Rubyist, Gopher and Vimmer.
   ● RuboCop's core developer.
   ○ Today, I'll talk about RuboCop.
   

   View Slide

3. Goal
   ● Improve code review with RuboCop.
   ● RuboCop is not a “Silver Bullet”.
   ○ When RuboCop reviews your code, you
   should review RuboCop's warnings.
   

   View Slide

4. Note
   ● I’ll talk about Ruby today, but we
   can apply the practices to projects
   that is written in other languages.
   

   View Slide

5. Agenda
   ● What's RuboCop?
   ● The internal of RuboCop
   ● RuboCop for Code Review
   

   View Slide

6. Questions
   

   View Slide

7. Questions
   ● Are you Rubyist?
   ● Do you know RuboCop?
   

   View Slide

8. What's RuboCop?
   

   View Slide

9. What's RuboCop?
   ● A static analyzer for Ruby.
   ● It has 3 type checks.
   ○ Style
   ○ Lint
   ○ Metrics
   

   View Slide

10. Style
    Check coding style
    

    View Slide

11. Style: check coding style
    ● White spaces
    ○ Indentation width
    ○ White spaces around brackets
    ● Syntax vs Method call
    ○ alias vs alias_method
    ○ for vs each
    

    View Slide

12. Style checks based on one of the
    style guides.
    ● https://github.com/bbatsov/ruby-
    style-gudie
    ○ If your coding style has differences, you
    can configure RuboCop.
    

    View Slide

13. Style: example
    # Use `def foo(arr)`
    def foo arr
    for x in arr # Use each
    bar # Bad indentation
    end
    end
    

    View Slide

14. Lint
    Check bugs
    

    View Slide

15. Lint: check bugs
    ● Unreachable code
    ● Debug code
    ● Unused variables
    ● etc...
    

    View Slide

16. Lint: example
    def foo
    binding.irb # debug code
    return true
    # It’s never executed.
    puts 'hi'
    end
    

    View Slide

17. Metrics
    Check complexity
    

    View Slide

18. Metrics: check complexity
    ● ABC size
    ○ Assignment, Branches, Conditional
    ● Line length
    ● Class length
    ● etc...
    

    View Slide

19. The internal of RuboCop
    

    View Slide

20. How does RuboCop check code?
    ● RuboCop analyzes each file
    individually.
    ● RuboCop does not execute code.
    See “Writing Lint for Ruby” in RubyKaigi 2017.
    

    View Slide

21. Analyzing each
    file individually
    

    View Slide

22. Analyzing each file individually
    ● RuboCop does not check relations
    between files.
    ○ e.g. db/schema.rb and models.
    ● For simplicity and performance.
    ○ RuboCop can analyze parallelly.
    

    View Slide

23. Do not execute
    code
    

    View Slide

24. Do not execute code
    ● RuboCop doesn’t execute your
    code.
    ○ Pros: If your code has system('rm -rf
    /'), it does not remove.
    ○ Cons: RuboCop cannot understand
    application completely.
    ■ e.g. eval, method call
    

    View Slide

25. RuboCop for Code Review
    

    View Slide

26. RuboCop for Code Review
    ● What does “RuboCop for Code
    Review” mean?
    ● Advantage of RuboCop in Code
    Review.
    ● Action for RuboCop Review
    

    View Slide

27. What does
    “RuboCop for
    Code Review”
    mean?
    

    View Slide

28. What does “RuboCop for Code
    Review” mean?
    ● RuboCop also reviews your code.
    ○ Human Review + RuboCop Review
    

    View Slide

29. How does RuboCop review
    ● rubocop $(git diff --name-only)
    before human review.
    ● I recommend using CI to run
    RuboCop.
    ○ e.g. SideCI, CodeClimate and
    Reviewdog.
    

    View Slide

30. Advantages of
    RuboCop in
    review
    

    View Slide

31. Advantages of RuboCop in
    review
    ● Reduce human reviews.
    ● Enhancement reviews.
    

    View Slide

32. Reduce human reviews
    ● Layout and style reviews by
    human are no longer necessary.
    ○ e.g. “Remove the spaces”, “Use each
    method instead of for”.
    ○ It’s a RuboCop job, not a human job.
    

    View Slide

33. Enhancement reviews
    ● RuboCop can find bugs that
    human will overlook.
    ○ Duplicate method definitions.
    ○ binding.irb(debug code)
    

    View Slide

34. Action for
    RuboCop Review
    

    View Slide

35. Action for RuboCop Review
    ● You should review “RuboCop
    Review”.
    

    View Slide

36. Action for
    RuboCop Review
    For Style Reviews
    

    View Slide

37. Action for Style Review
    ● Check your coding style.
    ○ If the review does not match with your
    coding style, you should configure or
    ignore the review.
    ○ http://rubocop.readthedocs.io/en/stabl
    e/
    

    View Slide

38. Action for
    RuboCop Review
    For Lint Reviews
    

    View Slide

39. Action for Lint Review
    ● Lint reviews has false positives.
    ○ In other words, Lint review has a
    mistake sometimes.
    ○ Because RuboCop does not know your
    application completely.
    ■ See the slides above.
    ● It is not a bug, it is a restriction.
    

    View Slide

40. How to start and/or
    continue to use RuboCop?
    

    View Slide

41. Start and/or continue to use
    RuboCop
    ● You can get knowledges from
    WEB+DB PRESS vol.102.
    ○ “RuboCopできれいで安全なコード”
    written by me.
    ○ http://amzn.asia/fZ9cgZO
    

    View Slide

42. Conclusion
    

    View Slide

43. Conclusion
    ● RuboCop is not a “Silver Bullet”.
    ○ In many cases, the default coding style
    doesn't match your coding style.
    ○ RuboCop has false positives.
    ○ So when RuboCop reviews your code,
    you should review RuboCop Review.
    

    View Slide