Feature #1269
closed
warning when Kernel#p is used
Description
=begin
遠藤です。
Kernel#p は非常に便利ですが、デバッグ後に p の呼び出しをすべて消せたか
不安になることがあります。
p という名前は使うときは非常に便利ですが、残念ながら検索するには非常に
不便です。単語区切りで検索できないエディタだと検索は限りなく困難ですし、
単語区切りで検索できるエディタであっても、
- p という変数や attr を使っている場合
- rdoc 中のサンプルコードとして p を使っている場合
- html 断片の
や sprint の %p などが文字列リテラル中にある場合
など、false positive がとても多いです。
p を再定義して出力を止めれば p の呼び出しが残っても実害はなくせますが、
余計なコードが残るのは気持ち悪いですし、後でほかのデバッグをするときに
邪魔になる恐れもあります。
そこで、$DEBUG = true の場合に p を関数呼び出ししている箇所があったら
警告することを提案します。
通常時は何も言わない¶
$ ruby19 -e 'p :debug'
:debug
-d をつけると警告する¶
$ ruby19 -d -e 'p :debug'
-e:1: warning: Kernel#p is used
:debug
パーサでチェックするので実行されない場合でも検出できる¶
$ ruby19 -d -e 'p :debug if rand > 0.999'
-e:1: warning: Kernel#p is used
関数呼び出しでなければ警告しない¶
$ ruby19 -d -e 's = Struct.new(:p); p = s.new; p.p = "
"; puts p.p'
もちろん、eval される文字列中の p の呼び出しは実行しないと検出できない
ので完璧ではないですが、それでも多くの場合はカバーできると思います。
問題点は
- p という名前のメソッドだけ特別扱いで警告されるのが気持ち悪い
- 互換性
くらいかと思いますが、$DEBUG = true なら本番環境でないので、個人的には
許容範囲かなと思いました。どうでしょうか。
Index: id.c¶
--- id.c (revision 22892)
+++ id.c (working copy)
@@ -47,4 +47,5 @@
REGISTER_SYMID(idSend, "send");
REGISTER_SYMID(id__send__, "send");
REGISTER_SYMID(idInitialize, "initialize");
- REGISTER_SYMID(idP, "p");
}
Index: parse.y
===================================================================
--- parse.y (revision 22892)
+++ parse.y (working copy)
@@ -1257,6 +1257,9 @@
command : operation command_args %prec tLOWEST
{
/%%%/ -
if ($1 == idP && RTEST(ruby_debug)) { -
rb_warning0("Kernel#p is used"); -
} $$ = NEW_FCALL($1, $2); fixpos($$, $2); /*%
@@ -3525,6 +3528,9 @@
method_call : operation paren_args
{
/%%%/
-
if ($1 == idP && RTEST(ruby_debug)) { -
rb_warning0("Kernel#p is used"); -
} $$ = NEW_FCALL($1, $2); fixpos($$, $2); /*%
Index: id.h¶
--- id.h (revision 22892)
+++ id.h (working copy)
@@ -99,6 +99,7 @@
tSend,
t__send__,
tInitialize,
- tP,
#if SUPPORT_JOKE
tBitblt,
tAnswer,
@@ -118,7 +119,8 @@
TOKEN2ID(Lambda),
TOKEN2ID(Send),
TOKEN2ID(send),
- TOKEN2ID(Initialize)
- TOKEN2ID(Initialize),
- TOKEN2ID(P)
};
#ifdef tLAST_TOKEN
Index: template/id.h.tmpl
--- template/id.h.tmpl (revision 22892)
+++ template/id.h.tmpl (working copy)
@@ -92,6 +92,7 @@
tSend,
t__send__,
tInitialize,
- tP,
#if SUPPORT_JOKE
tBitblt,
tAnswer,
@@ -111,7 +112,8 @@
TOKEN2ID(Lambda),
TOKEN2ID(Send),
TOKEN2ID(send),
- TOKEN2ID(Initialize)
- TOKEN2ID(Initialize),
- TOKEN2ID(P)
};
#ifdef tLAST_TOKEN
--
Yusuke ENDOH mame@tsg.ne.jp
=end
Updated by usa (Usaku NAKAMURA) about 15 years ago
=begin
こんにちは、なかむら(う)です。
In message "[ruby-dev:38153] [feature:trunk] warning when Kernel#p is used"
on Mar.11,2009 21:29:22, mame@tsg.ne.jp wrote:
問題点は
- p という名前のメソッドだけ特別扱いで警告されるのが気持ち悪い
- 互換性
くらいかと思いますが、$DEBUG = true なら本番環境でないので、個人的には
許容範囲かなと思いました。どうでしょうか。
私は
p foo if $DEBUG
というコードを頻繁に書くんですが、ダメなんでしょうか。
pを使うのはデバッグ時なわけですから、当然$DEBUGがtrueの時にこ
そpに本来の仕事を余計な妨害なしでこなしてもらうべきだと思いま
す。
アイデア自体は悪くないと思いますが、パーサレベルでチェックす
るのであれば、syntax check (-c)などでひっかけるのがよいのでは
ないかと思います。
それでは。¶
U.Nakamura usa@garbagecollect.jp
=end
Updated by mame (Yusuke Endoh) about 15 years ago
=begin
遠藤です。
2009/03/11 21:50 U.Nakamura usa@garbagecollect.jp:
私は
p foo if $DEBUG
というコードを頻繁に書くんですが、ダメなんでしょうか。
p というすばらしい名前の利点が掻き消えるのでダメです。
さらに、ある箇所のデバッグ中に p foo if $DEBUG と書いて残して
しまうと、別の箇所をデバッグするために p bar if $DEBUG としたら
出力が混ざってわけわからなくなります。
pを使うのはデバッグ時なわけですから、当然$DEBUGがtrueの時にこ
そpに本来の仕事を余計な妨害なしでこなしてもらうべきだと思いま
す。
確かに $DEBUG を使うのはいまいちですね。
p の検出機能の有効・無効を切り替えられればなんでもいいので、
- 新しいオプションを追加する
- 環境変数で指定する
- RubyVM.warn_p = true という warn-p.rb を require する
など、特にこだわりはないです。(最後のは名前以外わりといいかも？)
アイデア自体は悪くないと思いますが、パーサレベルでチェックす
るのであれば、syntax check (-c)などでひっかけるのがよいのでは
ないかと思います。
それでもいいと思います。
--
Yusuke ENDOH mame@tsg.ne.jp
=end
Updated by ko1 (Koichi Sasada) about 15 years ago
=begin
ささだです．
Yusuke ENDOH wrote::
- 新しいオプションを追加する
- 環境変数で指定する
- RubyVM.warn_p = true という warn-p.rb を require する
など、特にこだわりはないです。(最後のは名前以外わりといいかも？)
warn-p.rb を使いたい人が require する，だと足りないですかね．で，p を
再定義してしまえば．
アイデア自体は悪くないと思いますが、パーサレベルでチェックす
るのであれば、syntax check (-c)などでひっかけるのがよいのでは
ないかと思います。それでもいいと思います。
パーサレベルでは難しいんじゃないですかね．
--
// SASADA Koichi at atdot dot net
=end
Updated by mame (Yusuke Endoh) about 15 years ago
=begin
遠藤です。
2009/03/11 23:48 SASADA Koichi ko1@atdot.net:
ささだです．
Yusuke ENDOH wrote::
- 新しいオプションを追加する
- 環境変数で指定する
- RubyVM.warn_p = true という warn-p.rb を require する
など、特にこだわりはないです。(最後のは名前以外わりといいかも？)
warn-p.rb を使いたい人が require する，だと足りないですかね．で，p を
再定義してしまえば．
ええと、提案のポイントは「p という名前のメソッドを関数呼び出しする
箇所を (静的に) 検出して警告する機能を入れよう」です。
alias my_p p として my_p を使うなどすると検出できなくなりますし、
自分で p というメソッドを定義して関数呼び出しすれば誤検出します。
が、(検出回避など以外の目的で) そんなことすることはなさそうですし、
この機能を使わなければ現状と何も変わりません。
で、この機能を有効にするトリガの案として、最初は「$DEBUG = true」を
挙げていましたが、これだと、検出は要らないけれど $DEBUG は使いたい
人が困るのでいまいちでした。
そこで改良案として、上では「warn-p を require する」をトリガとして
挙げています。
アイデア自体は悪くないと思いますが、パーサレベルでチェックす
るのであれば、syntax check (-c)などでひっかけるのがよいのでは
ないかと思います。それでもいいと思います。
パーサレベルでは難しいんじゃないですかね．
「p という名前のメソッドを関数呼び出しするかどうか」を検査するだけ
なのでできます。最初のメールにパッチがあります。
--
Yusuke ENDOH mame@tsg.ne.jp
=end
Updated by marcandre (Marc-Andre Lafortune) over 14 years ago
- Category set to core
- Assignee set to matz (Yukihiro Matsumoto)
Updated by naruse (Yui NARUSE) over 14 years ago
- Status changed from Open to Assigned
Updated by mame (Yusuke Endoh) about 14 years ago
- Target version set to 2.0.0
=begin
遠藤です。
2009年3月11日21:31 Yusuke ENDOH mame@tsg.ne.jp:
Kernel#p は非常に便利ですが、デバッグ後に p の呼び出しをすべて消せたか
不安になることがあります。
p という名前は使うときは非常に便利ですが、残念ながら検索するには非常に
不便です。単語区切りで検索できないエディタだと検索は限りなく困難ですし、
単語区切りで検索できるエディタであっても、
- p という変数や attr を使っている場合
- rdoc 中のサンプルコードとして p を使っている場合
- html 断片の
や sprint の %p などが文字列リテラル中にある場合
など、false positive がとても多いです。
p を再定義して出力を止めれば p の呼び出しが残っても実害はなくせますが、
余計なコードが残るのは気持ち悪いですし、後でほかのデバッグをするときに
邪魔になる恐れもあります。そこで、$DEBUG = true の場合に p を関数呼び出ししている箇所があったら
警告することを提案します。
という提案を 1 年前にしましたが、$DEBUG というインターフェイス
にケチがついていました。
2009年3月11日21:50 U.Nakamura usa@garbagecollect.jp:
アイデア自体は悪くないと思いますが、パーサレベルでチェックす
るのであれば、syntax check (-c)などでひっかけるのがよいのでは
ないかと思います。
[ruby-dev:40768] を見て思いついたのですが、ruby -c -v の時だけ
警告してくれるといいんじゃないかと思いました。
蛇足ですが、他にも簡単な lint を実装するなど面白そう。
ローカル変数は全部小文字で _ 区切りとか、クラス名は Camel Case
とか。一行 80 桁とか、1 メソッドの行数は 24 行までとか。異論は
ありそう。
--
Yusuke ENDOH mame@tsg.ne.jp
=end
Updated by yugui (Yuki Sonoda) about 14 years ago
=begin
2010/3/26 Yusuke Endoh redmine@ruby-lang.org:
蛇足ですが、他にも簡単な lint を実装するなど面白そう。
ローカル変数は全部小文字で _ 区切りとか、クラス名は Camel Case
とか。一行 80 桁とか、1 メソッドの行数は 24 行までとか。異論は
ありそう。
ripperを使ったlint実装の例を公開するほうが、汎用性があって弊害が少なく、その後に続くcheckerやらindentやらの礎になりそうですね。
--
Yuki Sonoda (Yugui)
yugui@yugui.jp
http://yugui.jp
=end
Updated by matz (Yukihiro Matsumoto) almost 13 years ago
まつもと ゆきひろです
In message "Re: [ruby-dev:43705] [Ruby 1.9 - Feature #1269] warning when Kernel#p is used"
on Sat, 11 Jun 2011 14:21:18 +0900, Koichi Sasada redmine@ruby-lang.org writes:
|これ，どうしましょう．
pをコンパイル時に特別扱いというのがなんとなく納得できないので、
1.9.3には入れない方向で。
Updated by matz (Yukihiro Matsumoto) almost 13 years ago
まつもと ゆきひろです
In message "Re: [ruby-dev:43705] [Ruby 1.9 - Feature #1269] warning when Kernel#p is used"
on Sat, 11 Jun 2011 14:21:18 +0900, Koichi Sasada redmine@ruby-lang.org writes:
|これ，どうしましょう．
pをコンパイル時に特別扱いというのがなんとなく納得できないので、
1.9.3には入れない方向で。
Updated by kosaki (Motohiro KOSAKI) about 12 years ago
- Status changed from Assigned to Rejected
これも問答無用で閉じてしまいます。蒸し返したい人がreopenしてください。個人的には １）ｐで警告を出したい と ２）パーサーでｐを検出したい の間に距離があるので２つの事をパラで議論するチケットになってしまっていて収束する傾向に見えないです