[Rejected] warning when Kernel#p is used

Issue #1269 has been updated by Motohiro KOSAKI.

Status changed from Assigned to Rejected

これも問答無用で閉じてしまいます。蒸し返したい人がreopenしてください。個人的には 1)pで警告を出したい と 2)パーサーでpを検出したい
の間に距離があるので2つの事をパラで議論するチケットになってしまっていて収束する傾向に見えないです

Feature #1269: warning when Kernel#p is used

Author: Yusuke E.
Status: Rejected
Priority: Low
Assignee: Yukihiro M.
Category: core
Target version: 2.0.0

=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 [email protected]
=end