$30 off During Our Annual Pro Sale. View Details »

RuboCop for Code Review

pocke
January 19, 2018

RuboCop for Code Review

pocke

January 19, 2018
Tweet

More Decks by pocke

Other Decks in Programming

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