Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Metrics/LineLength のより良いデフォルト値を模索したい #32

Closed
onk opened this issue Dec 4, 2017 · 6 comments
Closed

Comments

@onk
Copy link
Collaborator

onk commented Dec 4, 2017

統計で殴るのが変更を訴える理由として一番有効そうかなと思っているんですが、
第一報としてこんな感じです。

雑に star 数 1500 以上で .rubocop.yml を持っているプロダクトを
見て回ったところ、以下のような感じになりました。

(いっぱい取りこぼしがあるので雰囲気として掴んでください)
(追試可能なようにコード化していきます……)

max repo
Disabled DavyJonesLocker/client_side_validations
Disabled discourse/discourse
Disabled erikhuda/thor
Disabled fatfreecrm/fat_free_crm
Disabled hanami/hanami
Disabled helpyio/helpy
Disabled intridea/oauth2
Disabled jwt/ruby-jwt
Disabled kpumuk/meta-tags
Disabled lsegal/yard
Disabled mbj/mutant
Disabled michenriksen/gitrob
Disabled middleman/middleman
Disabled mongodb/mongoid
Disabled mroth/lolcommits
Disabled nanoc/nanoc
Disabled neo4jrb/neo4j
Disabled neonichu/ThisCouldBeUsButYouPlaying
Disabled octobox/octobox
Disabled omniauth/omniauth
Disabled prat0318/json_resume
Disabled publify/publify
Disabled rails/rails
Disabled rails/webpacker
Disabled realm/jazzy
Disabled refile/refile
Disabled resque/resque-scheduler
Disabled rest-client/rest-client
Disabled rmosolgo/graphql-ruby
Disabled roidrage/lograge
Disabled rpush/rpush
Disabled ruboto/ruboto
Disabled ruby-grape/grape
Disabled rubygems/rubygems.org
Disabled sensu/sensu
Disabled sferik/rails_admin
Disabled sferik/t
Disabled sferik/twitter
Disabled shakacode/react-webpack-rails-tutorial
Disabled shakacode/react_on_rails
Disabled sharetribe/sharetribe
Disabled shoes/shoes4
Disabled skywinder/github-changelog-generator
Disabled solidusio/solidus
Disabled soundcloud/lhm
Disabled spree/spree
Disabled swanson/stringer
Disabled technicalpickles/homesick
Disabled tenex/rails-assets
Disabled test-kitchen/test-kitchen
Disabled theforeman/foreman
Disabled thoughtbot/administrate
Disabled thoughtbot/gitsh
Disabled tootsuite/mastodon
Disabled toptal/chewy
Disabled troessner/reek
Disabled welaika/wordmove
Disabled zdennis/activerecord-import
9000 jordansissel/fpm
483 hexorx/countries
309 cucumber/cucumber-ruby
279 CocoaPods/CocoaPods
276 ctran/annotate_models
266 jnunemaker/httparty
200 celluloid/celluloid
187 comfy/comfortable-mexican-sofa
186 geokit/geokit
180 rapid7/metasploit-framework
170 intridea/hashie
143 httprb/http
140 dtan4/terraforming
120 Casecommons/pg_search
120 Shopify/bootsnap
120 ambethia/recaptcha
120 athityakumar/colorls
120 binarylogic/authlogic
120 churchio/onebody
120 diaspora/diaspora
120 feedjira/feedjira
120 glebm/i18n-tasks
120 google/google-api-ruby-client
110 galetahub/ckeditor
100 airblade/paper_trail
100 drapergem/draper
100 ffaker/ffaker
90 jekyll/jekyll
80 BBC-News/wraith
80 awesome-print/awesome_print
80 bbatsov/rubocop
80 bkeepers/dotenv
80 chef/chef
80 deivid-rodriguez/byebug
80 deivid-rodriguez/pry-byebug
80 doorkeeper-gem/doorkeeper
80 fnando/browser
80 houndci/hound
80 thoughtbot/bourbon
80 thoughtbot/griddler
80 thoughtbot/guides
80 thoughtbot/scenic
80 vcr/vcr
@pocke
Copy link
Contributor

pocke commented Dec 4, 2017

rubocop/rubocop#4588
これ、めんどくさくて挫折してたんですが、実は手元に

  1. RubyGemsのランキング上位n件をfetchする
  2. fetchしてきたコード全体に対してRuboCopを実行する
  3. Metrics copの警告から、実際の複雑度を取り出してリストにする
  4. リストから「mパーセンタイルのコードに対して警告が出なくなる」には設定値をいくつにしたらいいか算出する

みたいなコードがあります。
この時はLineLengthはターゲットにしていなかったのでいい感じの結果が出るかはわかりませんが、あとでコードを公開しますね

@pocke
Copy link
Contributor

pocke commented Dec 4, 2017

↑のコメントで言及したコードです。

クローラー

環境に依存するかも知れないです。回数は適当に調整してください。

require 'pathname'

STATS_URL = 'https://rubygems.org/stats?page=%{page}'

def sh!(cmd)
  puts "$ #{cmd}"
  system(cmd)
end

GEM_HOME = Pathname(Dir.pwd).join('gem_home')
GEM_HOME.mkdir unless GEM_HOME.exist?

1.upto(50) do |i|
  puts "page: #{i}"

  gems = `curl '#{STATS_URL % {page: i}}'`
    .each_line
    .grep(/class="stats__graph__gem__name"/)
    .map {|line| line[%r!([^>]+)\</a\>\</h3\>$!, 1]}
    .map(&:chomp)

  sh! "GEMSRC_SKIP=true GEM_HOME=#{GEM_HOME} gem install #{gems.join(' ')} --no-user-install"
  sleep 1
end

RuboCop の実行の仕方

なお、この手法にはdisable commentを無視できないというバグがあります。(disable commentで無効化されているコードを計測できない)

1. rm .rubocop.yml

各ライブラリの設定を無視するため

rm **/*rubocop*yml

2. put .rubocop.yml

計測したいCopのMaxを0にする

MethodLength:
  Max: 0
AbcSize:
  Max: 0
AllCops:
  TargetRubyVersion: 2.4

なお、実はこの辺の変更によって、masterとv0.51.0では違う結果が出る気がします。

3. 実行

only に対象のCopを指定、formatはJSONで。parallelはあると便利

rubocop --parallel --only MethodLength,AbcSize --format json . > out.json

4. データの整形

outputというファイルに結果を吐き出します。これは、Metrics/MethodLengthの例

< out.json| ruby -rjson -e 'puts JSON[ARGF.read]["files"].flat_map{|file|file["offenses"].map{|offense| offense["cop_name"] == "Metrics/MethodLength" ? offense["message"][/([\d.]+)\/0\]$/, 1].to_i : nil}.compact}' > output

5. 集計

ちょっと何をやっているか覚えていないのですが、多分なにか数字が出てきます。
多分このコードは「Nに指定した数値に.rubocop.yml内のMaxを指定した時、何パーセンタイルのコードをカバーできるか」という意味です。

sort -n < output | uniq -c | ruby -e 'N=17; a=STDIN.read.each_line.map(&:split).map{|x|x.map{|y|y.to_i}}; sum = a.map{|x|x[0]}.sum; sum2 = a.select{|_, n| n<=N}.map{|x|x[0]}.sum; p(sum2 / sum.to_f)'

@pocke
Copy link
Contributor

pocke commented Dec 4, 2017

ちなみに、Metrics/LineLengthに対して上記コードを適用してみたところ、Maxが80の時92.34パーセンタイルのコードに対しては警告が出ない、ということが分かりました。
この辺もし見てみたい人がいたらデータをアップロードするので、声をかけてください。上のスクリプトは結構時間がかかるので最初から実行するのはたいへんです

@onk
Copy link
Collaborator Author

onk commented Dec 5, 2017

LineLength もパーセンタイルで許容するのが有効なのかな……?
であればそっちを進めると他の Metrics Cop でも便利になるので十分かもしれない。

余談:
rubocop 本体の場合は 80 ギリギリに調整してあるコード/コメントが異常に多く、Max: 79 にするだけで 1062 files inspected, 339 offenses detected となって明確に人の手による調整コストが掛かっているので、このコストは下げたいなと思う。

@onk
Copy link
Collaborator Author

onk commented Dec 5, 2017

各プロジェクトの rubocop 設定値の確認方法

1. GitHub から stars:>1500 language:ruby, filename:.rubocop.yml repo:foo/bar で検索してダウンロードする

# 概念コード。sleep とかページネートとか要ります。
client = Octokit::Client.new(access_token: ENV["GITHUB_TOKEN"])
client.search_repos("stars:>1500 language:ruby").each do |repo|
  client.search_code("filename:.rubocop.yml repo:#{repo.full_name}") do |code|
    f = client.contents(code.repository.full_name, path: code.path)
    FileUtils.mkdir_p(path.dirname)
    path.write(Base64.decode64(f.content))
  end
end

2. 古い config でも読めるように RuboCop::ConfigLoader にパッチを当てる

mry すごい簡単便利

https://github.com/codeclimate/codeclimate-rubocop/blob/channel/rubocop-0-51/lib/cc/engine/config_upgrader.rb

3. RuboCop::CLI から config を読む

Dir.glob("repos/*/*").each do |dir|
  Dir.chdir(dir) do
    cli = RuboCop::CLI.new
    config_for_line_length = cli.config_store.for(Dir.pwd).for_cop("Metrics/LineLength")
    if config_for_line_length["Enabled"]
      p config_for_line_length["Max"]
    else
      p "Disabled"
    end
  end
rescue  => e
  # nop
end

的な感じでやっています。

RuboCop::ValidationError に引っかかったり、inherit_gem が書かれていたりと例外対応が結構要ります。ここを綺麗に書けてない 😢

tunepolo added a commit to tunepolo/railstutorial that referenced this issue Feb 15, 2019
sanfrecce-osaka added a commit to fjordllc/bootcamp that referenced this issue May 7, 2020
refs #1541 (comment)

### 概要

Searchable#target_column_of_keyword で動的に定義される _grouping_condition を privateメソッド化 するために define_singleton_method からメソッドチェーンで privateメソッド化 していた
しかし、レビューで「横に長くて読みにくい&tapがごちゃごちゃしてわかりづらい」という意見をいただいたため、 tap で privateメソッド化 していた箇所を削除した
レビューではブロックが curly brace での one-line から do end での multi-line も提案されていたが、以下の理由からそのまま curry brace での記述にしている

- curly brace での記述の場合、line length は 111文字
- Style/LineLength を設定している場合、 120文字 に設定しているプロジェクトが結構多いように感じる
  - c.f.  rubocop/rubocop-jp#32 (comment)
- RubyMine の Code Style でのデフォルト設定が 120文字
@koic
Copy link
Member

koic commented May 12, 2020

RuboCop 1.0 でデフォルト 120 になります。
rubocop/rubocop#7952

デフォルト 80 からのアップデートということでイシューの役割を果たしたと思うためクローズします。

@koic koic closed this as completed May 12, 2020
tash-2s added a commit to tash-2s/onchain-game-2019_archive that referenced this issue Oct 5, 2023
I believe this is more easy to see.

ref. rubocop/rubocop-jp#32

---

Original commit (web-client): 8beeccea338013285e963f041fb53b41563b385c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants