コンテンツへスキップ

リファクタで関数スコープに閉じ込めた名前を、別の関数から参照し続けていた話 — Python の遅延評価と AST 未定義名検出テスト

ブラウザ自動更新の処理が、ある日から「プラグイン更新は完了したけれど、その直後の残余チェックで NameError: name 'plugin_form_selectors' is not defined で死ぬ」という症状を出すようになりました。

リファクタ自体は v1.6.1 で入ったものでしたが、エラーが顕在化したのはずっと後。コードを読めば一目で原因はわかるのに、リリースから数回のラウンドを越えても誰も踏まなかった ── Python の遅延評価がリファクタの取り残しを隠していたわけです。今回はこの NameError の正体と、再発を AST 静的解析テストで構造的に防いだ話をまとめます。

何が起きていたか — スコープの境界線を越えた参照

browser_utils.py には更新フローを束ねる run_browser_update_flow() という関数と、プラグインだけ更新する browser_update_remaining_plugins() という関数があります。プラグイン更新フォームのセレクタ候補リスト plugin_form_selectors は、もともと run_browser_update_flow() のローカル変数として定義されていました。

v1.6.1 のリファクタで「プラグイン更新の本体を別関数に切り出そう」となった時、browser_update_remaining_plugins() を新設して、plugin_form_selectors の定義もそちらに移しました

# v1.6.1 リファクタ後
def browser_update_remaining_plugins(page, site, update_url):
    plugin_form_selectors = [        # ← ここで定義
        '#update-plugins-table-wrap form',
        'form[name="upgrade-plugins"]',
        'form[action*="do-plugin-upgrade"]',
        '.plugins-php form',
    ]
    # ... 更新処理 ...


def run_browser_update_flow(site, page):
    # ... プラグイン更新の呼び出し ...
    browser_update_remaining_plugins(page, site, update_url)

    # ★ 更新後の「残余チェック」で旧名を参照してしまっていた
    for selector in plugin_form_selectors:   # NameError
        if page.locator(selector).count() > 0:
            pending_browser.append(...)

プラグイン更新が本当に全部完了したか、最後にもう一度フォームの残存を確認する」という残余チェックのループが、run_browser_update_flow() 側にも残っていました。リファクタ時にこのループも一緒に切り出すべきだったのか、それとも参照先を別経路に変えるべきだったのか、その判断が抜けていました。結果、関数スコープの外から、関数スコープ内のローカル変数を呼ぼうとするコードが書かれた、というシンプルな事故です。

なぜリリースから数回越えても顕在化しなかったか — Python の遅延評価

Python の NameError は、コードが import される時ではなく、その行が実行された時に初めて発生します。for selector in plugin_form_selectors:plugin_form_selectors という名前は、関数定義時にはバインドされている必要がなく、関数が呼ばれてその for 行に制御が移った瞬間に解決を試みる、という仕組みです。

つまり、

  • アプリ起動: 問題なし(誰もこの関数を呼んでいない)
  • SSH 経由でプラグイン更新が成功するサイト: 問題なし(ブラウザ自動更新の経路に入らない)
  • ブラウザ自動更新が走るサイト: そのループ行に到達して初めて NameError

ブラウザ自動更新を必要とするサイトでしか踏まないので、SSH 経由のサイトだけで運用していると気づかず、テストでもカバーされていなかった、というのが顕在化の遅さの正体でした。

モジュール定数に昇格して両方から参照する

修正の方向は単純で、「両方の関数から見える場所に置けばいい」。モジュールレベルの定数に格上げします。

# モジュール先頭
PLUGIN_FORM_SELECTORS = [
    '#update-plugins-table-wrap form',
    'form[name="upgrade-plugins"]',
    'form[action*="do-plugin-upgrade"]',
    '.plugins-php form',
]


def browser_update_remaining_plugins(page, site, update_url):
    plugin_form_selectors = PLUGIN_FORM_SELECTORS    # 既存ローカル名で受ける
    # ... 更新処理 ...


def run_browser_update_flow(site, page):
    # ... プラグイン更新の呼び出し ...

    # ★ 残余チェックも同じ定数を参照
    for selector in PLUGIN_FORM_SELECTORS:
        if page.locator(selector).count() > 0:
            pending_browser.append(...)

ローカル変数名 plugin_form_selectors をそのまま残したのは、関数内のコードに最小限の差分しか入れないため。呼び出し元コードの可読性は変えず、参照先を 1 箇所に集約する、という方針です。

なぜ pyflakes / ruff で見つからなかったか

「こんなの lint で出るはずでは?」という疑問は妥当です。実際、pyflakes や ruff の F821(undefined name)チェックは未定義名を静的解析で検出します。ただ、罠はもう少し細かいところにあります。

問題の plugin_form_selectors という名前は、関数 browser_update_remaining_plugins() の中ではローカル変数として定義されていたので、Python の静的解析からすると「プロジェクト内のどこかで定義されている名前」として認識されます。run_browser_update_flow() の中で同じ名前を参照する行を見たとき、それが別関数のローカル変数だとは判別しきれない、というのが pyflakes の限界です。

正確に言うと、pyflakes は「現在のスコープから見て未定義」を検出するのではなく、「どこにも定義がない」を検出します。同名のローカル変数が別関数にあると、見逃される。

これを補うのが、スコープを意識した自前の AST 静的解析テストです。

AST 未定義名検出テスト — test_browser_undefined_names.py

採用したのは、browser_utils.py の全関数を AST で歩いて、各関数の中で参照されている名前が、その関数のスコープから到達可能かを実際に解決するテストです。概念図はこんな形:

import ast

def test_no_undefined_names_in_browser_utils():
    """browser_utils.py の関数内で参照される全名前が、
    モジュール先頭の定義・関数引数・関数ローカル定義のいずれかで
    解決可能であることを検証する。"""

    module = ast.parse(BROWSER_UTILS_PY.read_text())
    module_names = collect_module_level_definitions(module)
    builtin_names = set(dir(builtins))

    failures = []
    for func in walk_functions(module):
        # 関数の引数 + 関数内 Assign で定義される名前
        local_names = collect_local_definitions(func)
        scope = module_names | local_names | builtin_names

        for name_node in walk_names(func, ctx=ast.Load):
            if name_node.id not in scope:
                failures.append(
                    f"{func.name}:{name_node.lineno} undefined: {name_node.id}"
                )

    assert not failures, "\n".join(failures)

このテストは 「関数ローカル変数は他の関数から参照できない」というスコープのルールを正しく適用するので、pyflakes の限界をきれいに補完します。今回の plugin_form_selectors のような「別関数のローカル変数を誤って参照」する事故が、CI で機械的に落ちる構造になりました。

実装上は import 文・* from import・条件付き定義(if hasattr(...) else ...)等を真面目に扱うので、上の擬似コードよりは複雑ですが、「関数の境界で名前解決を独立にやり直す」という核は同じです。

学び — リファクタの「取り残し」を機械で塞ぐ

このラウンドから取り出せる原則は次の 3 つでした。

  1. 関数のローカル変数を外に出すリファクタは、参照箇所の grep をセットで — 関数内の変数を切り出すリファクタは、その変数を別関数から参照していないかを必ず確認する。シンプルだが、リファクタ時の気の緩みで踏みやすい。1 箇所を機械的に検索するだけで防げる
  2. Python の遅延評価は、リファクタの取り残しを長期間隠すNameError はその行が実行された時に初めて出るので、実行頻度の低い経路ほど顕在化が遅れる。テストカバレッジが低い経路では特に。これは Python の言語特性で、避けられない
  3. pyflakes / ruff の F821 では取れない「スコープ越境参照」は、自前 AST テストで補う — 標準 lint は「どこにも定義がない」は取るが「現在のスコープから到達できない」は取らない。スコープを意識した AST 静的解析テストを 1 つ書いておくと、リファクタで取り残された参照が CI で必ず落ちるようになる

今回の修正は、.first を癖にする運用と AST テストで Playwright strict mode 違反を機械検出した話 と同じく、「人間の注意力ではなく、機械の規律でコードベースを守る」設計の引き出しを増やすラウンドでした。WP 保守ツールのように長く使うコードは、こういう小さな AST テストを 1 つずつ積んでいくと、リファクタの心理的コストが下がる実感があります。