Feature #1269
warning when Kernel#p is used
| Status: | Rejected | Start date: | ||
|---|---|---|---|---|
| Priority: | Low | Due date: | ||
| Assignee: |  matz (Yukihiro Matsumoto) |
% Done: | 0% |
|
| Category: | core | |||
| Target version: | 2.0.0 |
Description
遠藤です。
Kernel#p は非常に便利ですが、デバッグ後に p の呼び出しをすべて消せたか
不安になることがあります。
p という名前は使うときは非常に便利ですが、残念ながら検索するには非常に
不便です。単語区切りで検索できないエディタだと検索は限りなく困難ですし、
単語区切りで検索できるエディタであっても、
- p という変数や attr を使っている場合
- rdoc 中のサンプルコードとして p を使っている場合
- html 断片の <p> や 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 = "<p>"; puts p.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>
History
Updated by usa (Usaku NAKAMURA) about 3 years ago
こんにちは、なかむら(う)です。 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>
Updated by mame (Yusuke Endoh) about 3 years ago
遠藤です。 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>
Updated by ko1 (Koichi Sasada) about 3 years ago
ささだです． 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
Updated by mame (Yusuke Endoh) about 3 years ago
遠藤です。 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>
Updated by marcandre (Marc-Andre Lafortune) over 2 years ago
- Category set to core
- Assignee set to matz (Yukihiro Matsumoto)
Updated by naruse (Yui NARUSE) over 2 years ago
- Status changed from Open to Assigned
Updated by mame (Yusuke Endoh) about 2 years ago
- Target version set to 2.0.0
遠藤です。 2009年3月11日21:31 Yusuke ENDOH <mame@tsg.ne.jp>: > Kernel#p は非常に便利ですが、デバッグ後に p の呼び出しをすべて消せたか > 不安になることがあります。 > p という名前は使うときは非常に便利ですが、残念ながら検索するには非常に > 不便です。単語区切りで検索できないエディタだと検索は限りなく困難ですし、 > 単語区切りで検索できるエディタであっても、 > > - p という変数や attr を使っている場合 > - rdoc 中のサンプルコードとして p を使っている場合 > - html 断片の <p> や 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>
Updated by yugui (Yuki Sonoda) about 2 years ago
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
Updated by ko1 (Koichi Sasada) 12 months ago
これ，どうしましょう．
Updated by matz (Yukihiro Matsumoto) 12 months 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) 12 months 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) 3 months ago
- Status changed from Assigned to Rejected
これも問答無用で閉じてしまいます。蒸し返したい人がreopenしてください。個人的には １）ｐで警告を出したい と ２）パーサーでｐを検出したい の間に距離があるので２つの事をパラで議論するチケットになってしまっていて収束する傾向に見えないです