私が歌川です

@utgwkk が書いている

Twitter::Text のバグ取り風景

Twitter::Text のバグ取りの風景をお伝えします。このバグは 2020/11/21 17:46 時点の最新バージョンである 0.07 では修正されています。

やりたいこと

この CPAN Testers report の環境 (win32, Perl 5.16.0) で Twitter::Text のテストが通るようにします。普段の開発環境はLinux (Ubnutu)で、Perlのバージョンは5.30.0なので、そこそこ違いがあります。

やったこと

JSON モジュールの最低バージョンを2.90まで上げる (効果なし、取り下げた)

JSONモジュールは、バージョン 2.90 と 4.00 (3.99_01) で非互換変更を行っています*1。reportを見ると、JSONのバージョンが2.53 (1回目の非互換変更が行われるより前) だったので、これが原因か? とアタリをつけて、最低バージョンを2.90まで上げました。

これは結論から言うと全く関係なかったです。JSON 2.90での非互換変更は、 JSON::{true,false}JSON::PP::Boolean のインスタンスになっていること・それらが eq をoverrideしてないこと*2でした。JSONモジュールは、ツイートのバリデーションのルールを決める設定ファイルを読み込むためだけに使っているので、ここで非互換変更があっても関係ないです。そのため、JSONのバージョン制限はいったん導入した後にすぐ取り下げました

JSONのバージョンを制限すると、先述したCPAN Testers reportでは Twitter::Text が使えなくなる、ということになります。できるだけ制限しない方向でなんとか解決できないものか、と思って次の作戦を考えました。

正規表現の /o 修飾子を外す (効果なし)

Twitter::Text で使う正規表現は、本家twitter-textのRuby実装の正規表現を参考にして、Perlの正規表現に書き直しています。

このとき、正規表現の修飾子をほとんどそのまま持ってきていましたが、そこには /o という修飾子が含まれていました。perldoc perlre には、 /o 修飾子について以下のように書いています。

o - pretend to optimize your code, but actually introduce bugs

バグを引き起す、と書いてあるのを見て、これが原因ではないかと思いました。Twitter::Text で使っている正規表現から /o を取り除いてみました。

この段階で、reportの環境を手元に再現できたのでテストを走らせてみましたが、どうやら /o 修飾子も関係ないようです。バグを引き起す、と書いてあるので取り除くのは悪くないかもしれませんが、問題は解決していません。

2020/11/23 追記

Rubyの正規表現についていた /o 修飾子の意味は、正規表現中での #{} による式展開を一度だけ行う、というものでした*3

Net::IDN::Encode モジュールの最低バージョンを2.100まで上げる (効果あり)

そもそもどういうテストが落ちているのか、じっくり観察することにしました。落ちているのは、長いURLやドメインは23文字としてカウントしない*4、ということを確かめるテストでした。このことから、URLを抽出する関数 extract_urls_with_indices の実装に不備があるのではないか、とアタリをつけます。

warn によるデバッグを駆使することで、どうやら extract_urls_with_indices 内で呼び出している _is_valid_domain という関数の実装がおかしそうだ、ということが分かりました。 _is_valid_domain は、名前の通り、与えられたドメイン名がvalidかどうかの真偽値を返す関数です。この関数の中に、以下のような処理があります。

my $encoded_domain = eval { domain_to_ascii($domain) };

if ($@) {
    return 0;
}

度重なる warn デバッグの結果、落ちているテストでは、与えられるURLがinvalidなのでここで return 0 されるべきだが、 return 0 されずにvalidなURLとしてチェックをすり抜けている、ということが分かりました。いよいよ、どこにバグがあるのかが特定できました!

domain_to_asciiNet::IDN::Encode モジュールがexportしている関数です。eval ブロックで囲んでいることからも分かるように、この関数は、与えられたドメイン名がinvalidであれば例外を送出することが期待されています。

reportにある環境では Net::IDN::Encode 2.005 がインストールされており、どうやらこのバージョンだとテストが落ちるようです。また、Net::IDN::Encode 2.500が入っている環境ではテストが通ることが分かっています。したがって、2.005より大きく2.500以下の範囲で、Twitter::Text のテストが通る Net::IDN::Encode の最低バージョンを特定できればよさそうです。

Net::IDN::Encode のバージョンに対して、テストが通るかどうかで二分探索を行えば最低バージョンを決定できそうです。が、その前に他のreportで、2.500より低いバージョンのNet::IDN::Encodeでテストが通ってないかを見ることにしました。すると、win32, Perl 5.16.3, Net::IDN::Encode 2.100 でテストが通っているreportを発見できました。落ちていたのは2.005だったので、バージョンを1つ上げて2.100以上にすることを要求すればよさそうです。幸いにも、手元のWindows環境のPerl 5.16.0でも Net::IDN::Encode 2.100 をインストールすることができました。

github.com

原因

さて、このように問題を解決することができましたが、原因は何だったのでしょうか?

またもや warn デバッグをすることで、label too long という croakが呼ばれるべきだが呼ばれていない、ということが分かりました。そして、この croak は、Net::IDN::Encode 2.100 で追加されたものでした。Net::IDN::Encode のcommit logを見ると、preserve case in pure-ASCII labels (bypass en-/decoding) · cfaerber/Net-IDN-Encode@9fed007 · GitHub というcommitで croak されるようになっています*5

label (そもそもこの時点ではlabelが何かも分かってなかった) が63文字より長いと例外になるが、63文字とはどこから来た数字なのか? 答えは、RFC 1034 でした。ドメイン名を . で区切ったときの各サブドメイン名を表す文字列のことを label と呼び、各 label は最大63文字、と規定されています。つまり、64文字以上の label を含む場合はドメイン名としてはinvalidである、ということです。

おわりに

CPAN Testers reportがなければバグに気づくことはなかったと思います。環境を再現するのがひと苦労でしたが、再現してしまえば手元でいくらでも調査できるので、素早く環境を作るのがよさそうです。

Net::IDN::Encode::domain_to_ascii が例外を送出することを期待しているけど、呼び出し元でlabelの長さを見てバリデーションする、という手も取れるかもしれませんが、依存モジュールの最低バージョンを指定して直ることが分かったのでそのようにしました。PODにも、不正な入力に対しては例外を送出する、と書いてあるのでまあよいのではないでしょうか。

ドメイン名に関する知識がないままバグに直面して調査した結果、学びがあった、という記事でした。

*1:https://metacpan.org/changes/distribution/JSON

*2:https://metacpan.org/source/ISHIGAKI/JSON-4.02/Changes#L76-83

*3:https://ruby-doc.org/core-2.7.2/Regexp.html#class-Regexp-label-Options

*4:Twitterでは、URLは一律23文字としてカウントします。Counting characters | Docs | Twitter Developer

*5:しかしテストケースにはlabelが長すぎるときのテストがないように見える

睡眠失敗 🔜 悪夢

夜中の3時半ぐらいに腹痛で目が覚める。トイレに行って布団に潜るが、どうにもソワソワして眠れない。スマホでYouTubeを見ているうちに外が明るくなってきて、これは睡眠アンチパターンだなと確信した。

dotfilesを読み返したり、積読にあった本を読んだりしていたところ、心拍数が上がったような感じになって、徹夜明けのコンディションになった。このまま一日を始めるのは過酷だと悟ったので布団に入って目をつぶった。

5分おきに、二度寝しては悪夢を見て目を覚ます、を繰り返していた。夢から覚めたかと思ったら、実家で寝ていたり、あるいは知らないログハウスの中で寝ていたりしていて、展開が早すぎる。途中、間違いないこれは夢だ!!! って思いながら夢の中を過ごしていた気がする。

ぎりぎりまで二度寝を繰り返していたら心拍数は下がった。

section無しのINIファイルをPythonのconfigparserで読み書きしたい

Pythonの標準ライブラリであるconfigparserを使うと、INIファイル っぽいもの を読み書きすることができます。が、セクションより先に key=value が来ると configparser.MissingSectionHeaderError 例外が送出されパースに失敗します。

読み出すときはINIファイルの先頭にダミーのセクションをくっつける、ということができそうだけど、書き出すときはどうしたらいいのか。実装を読んでみると、INIファイルを文字列やファイルから読み込むときは _read メソッドを、ファイルに書き出すときは _write_section メソッドを経由するように見えます。これらのメソッドをoverrideして、ダミーのセクションを付けたり外したりするとどうか、というアイデアで以下の実装を考えました。

実装

import configparser
import itertools

RESERVED_NO_SECTION = '___RESERVED_NO_SECTION'

# ref: https://github.com/python/cpython/blob/802ff7c0d339376a1b974e57d2caca898310de3d/Lib/configparser.py
class SectionlessConfigParser(configparser.ConfigParser):
    def _read(self, fp, fpname):
        chained_fp = itertools.chain([f'[{RESERVED_NO_SECTION}]'], fp)
        super()._read(chained_fp, fpname)

    def _write_section(self, fp, section_name, section_items, delimiter):
        # diff: treat section '___RESERVED_NO_SECTION' as no section
        if section_name != RESERVED_NO_SECTION:
            fp.write("[{}]\n".format(section_name))

        for key, value in section_items:
            value = self._interpolation.before_write(self, section_name, key,
                                                     value)
            if value is not None or not self._allow_no_value:
                value = delimiter + str(value).replace('\n', '\n\t')
            else:
                value = ""
            fp.write("{}{}\n".format(key, value))
        fp.write("\n")


ini_content = '''
key1=aaa

[section1]
key11=bbb
'''
config = SectionlessConfigParser()
config.read_string(ini_content)
print(dict(config[RESERVED_NO_SECTION]))
config.set(RESERVED_NO_SECTION, 'key1', 'poe~~~')

with open('out.ini', 'wt') as f:
    config.write(f)

with open('out.ini', 'rt') as f:
    print(f.read())

実行結果

たしかにセクションがないところも読み込めていそうに見えます。

% python3 read.py
{'key1': 'aaa'}
key1 = poe~~~

[section1]
key11 = bbb

感想

undocumentedなプライベートメソッドをoverrideしてるのでそのうち壊れそう!!!! configparser.ConfigParser クラスのコンストラクタの引数でうまく挙動を制御できないかと思ったけど、実装を読む限りセクション名は必須のようなのでこれぐらいしか思い付かなかったです。 なんかいい方法があったら教えてください。

あと、INIファイルの形式を規定する文章があったら教えてください*1

*1:ない気がしている

perlcriticのポリシー書いた

perlcriticとは

perlcriticとは、Perlのlinterです。JavaScriptで言うところのESLintみたいな感じで、Perlのソースコードを解析して、バグを招きかねない・ポリシーに合致しないコードがあれば警告をしてくれます。

perlcritic本体にさまざまなポリシーが含まれており、設定なしでも使うことができるようになっています。また、ESLintと同様に、特定のパターンに一致するコードがあれば警告するようなポリシーを自分で作ることができます。

作ったポリシー

Perl::Critic::Policy::ProhibitOrReturn

Perl::Critic::Policy::ProhibitOrReturn - Do not use `or return` - metacpan.org

以下のように ... or return でearly returnをする書き方をしたら怒る、というポリシーです。ぼくはこの書き方があまり好きではなく return ... unless ... のように書くべきだと思っています。perlcritic本体のコードでもよく使われているので、よくあるイディオムなのかもしれないけどどうにも受け入れがたい、ということでポリシーにしました。

sub func {
    my ($x) = @_;
    $x->{hoge} or return; # not ok

    ...;
}

Perl::Critic::Policy::BuiltinFunctions::ProhibitReturnOr というポリシーがありますが別物です。

Perl::Critic::Policy::ControlStructures::ProhibitReturnInDoBlock

Perl::Critic::Policy::ControlStructures::ProhibitReturnInDoBlock - Do not "return" in "do" block - metacpan.org

以下のように do ブロック内で return していたら怒る、というポリシーです。

sub func {
    my ($x) = @_;
    my $y = do {
        return unless $x; # not ok
        my $z = ...;

        ...;
    };

    ...;
}

変数宣言と同時に値を初期化したいけど、そのために一時変数を作りたくない、というときに do ブロックを使うことでスコープを汚さずに書くことができます。 このとき do ブロック内でearly returnしたくなって、ぼやっとしてるとこういう書き方をしてしまいます。このように書くと、 do ブロックではなく func サブルーチンからreturnしてしまい、混乱を招くことになります。early returnしなくなったら do ブロックの内部をサブルーチンに分けるべきでしょう。

困りごと

同じようなポリシーが既にないか探すのが難しい

metacpanでがんばって検索することになるけど、ポリシー単体で1つのdistributionになっていたり、ポリシーバンドルに含まれていたりするので、結局distributionを全部見ないと見落しがある気がします。

ポリシーバンドルについてはScrapboxにまとめてなんとかしようとしていますが、手でやってるのでこれも抜け漏れがありそう & 新しいdistributionが出たら探しに行かないといけない ということで結局難しい。metacpanがAPIを提供している*1ことを知ってから多少はましになったかもしれません。

ポリシーの名前空間をどうするかが難しい

  • Perl::Critic::Policy::ProhibitOrReturn のように、何をどうするか という名前にする
    • 単純明快
  • Perl::Critic::Policy::ControlStructures::ProhibitReturnInDoBlock のように、ポリシーの分類・何をどうするか という名前にする
    • 間違った分類名にしてしまうと気まずい
      • 迷ったらPrePANで見てもらうとよい?
      • すでに Perl::Critic::Policy::BuiltinFunctions::ProhibitReturnInDoBlock という名前にしたほうがよかったかもと思ってきている……
    • 分類名どうするか決めるのが難しいパターンがありそう
  • Perl::Critic::Policy::UTGWKK::Hoge のように、誰が作った・何をどうするか という名前にする??
    • ポリシーの分類も含められる
    • ぼくが作ったこういうポリシーです、という名前になってて、ちょっと主張が強い気がする
      • ポリシーのバンドルみたいなのを作るときには、どのバンドル由来か分かって便利そう

ポリシーのdistribution単位をどうするか

  • ポリシーごとに1 distributionにする
    • ポリシー単体でインストールできる
  • ポリシーバンドルを1 distributionにして、バンドル内にポリシーを追加していく
    • ポリシー単体でインストールできなさそう
      • バンドル内に相容れないポリシーがあったら都度無効化する必要がある
    • 最初からバンドルとして作っていく、という気持ちがない場合はこうしない気がする
  • ポリシーごとに1 distributionにして、ポリシーバンドルのdistributionを入れるとポリシーが複数入るかたちにする
    • 上2つの複合形
    • ポリシー単体で入れたい、というのとバンドルを入れたい、というのを両立できる

perlcriticやってる友だちを知らない

発表や意見交換をする機会がなくて、知見は集まってきたけど他に困ってる人いないのか・なにか知ってる人はいないのか などとくに分からない状態でやっています。

Perlアドベントカレンダー参加します

Perl Advent Calendar 2020 - Qiita に参加します。perlcriticネタがけっこう集まってきたのでひとつ書こうと思っていますが、別のネタが出てきたらそっちにシフトするかもしれません。

*1:なんとElasticsearchのクエリを直接投げることができる!!!!

二日酔いに苦しんで休日を1日潰す。二日酔いにならなければできたことがあるのではないか、ということを考えるが、いま二日酔いであるということは解決しないので、回復するのを待つしかない。

銭湯に向かう。外の冷たい空気を吸うと、部屋の酸素が足りていなかったのではと感じる。湯船に浸かり、サウナと水風呂を行き来するうちに、心は落ち着いていく。毎日湯船に浸かっていた時期と、シャワーだけ浴びている今とで、どれくらい調子が変わっているのだろうか。

次に引っ越すなら、風呂トイレ別の部屋にしたい。ユニットバスではシャワーを浴びるのが精一杯。サウナは行ける範囲にあればいいや。

f:id:utgwkk:20201114224901j:image

perlcriticに --top オプションを渡したとき、severityが指定されてなかったら1になる

最初に回避方法を書くと、 --top オプションと一緒にseverityを指定してやるとこの現象は回避できる。

これは実装を読むと実際そうなっているし、 perldoc perlcritic にもそう書いてある

普通にperlcriticを使っていたらハマらなかったと思うけど、sfodje.perlcritic というVSCode拡張機能を通してperlcriticを使おうとしてハマった。

--profile あるいは -p オプションがいずれも渡されてなかったら、拡張機能の設定項目にあるseverityをコマンドライン引数に渡す、という実装の条件式が間違っていたので修正した。すると、 --profile .perlcriticrc というオプションを渡していて、かつ .perlcriticrc には severity = 3 しか書いてないのに、severityが2以下の警告がどんどん出てくる。

出したPRの実装がまずかったのかと思ったが、どうやら .perlcriticrc は読み込まれていて、severityの設定が効いていないようである。sfodje.perlcriticの実装を読みに行くと --top オプションを渡していて、これが怪しいのではないかとアタリを付けたところ的中した。

2005年には --top オプションを指定することでseverity 1になるように実装されていて、すぐ後にseverityが指定されてないときだけseverity 1にするように修正された。つまり15年ぐらいこの挙動であるということが分かる。

記事の冒頭にも書いたが、 --top オプションと一緒にseverityを指定してやるとこの現象は回避できる。sfodje.perlcriticの設定として書くならこういう感じになる。

{
    "perlcritic.additionalArguments": [
        "--profile", ".perlcriticrc", "-3"
    ]
}

perlcriticとsfodje.perlcriticの両方の実装を眺めないと問題に気づけなかった。