fix(cli): narrow parse_subnet_option/1 spec to the value it returns - #85
Conversation
Dialyzer runs with :underspecs and reported contract_supertype: the spec
said {atom(), map()} while the function only ever returns an
:edns_client_subnet tagged tuple whose map has four known keys.
Declare the shape as a type instead. The spec now says what the caller
can rely on, and a change to the returned map is reported rather than
absorbed by map().
No behaviour change; the existing parse_subnet_option/1 tests cover the
IPv4 and IPv6 paths unchanged.
Resolves #46
Claude-Session: https://claude.ai/code/session_01YAjSJR67tWTvYDJLcQThbQ
The pattern was a bare `tdig`, which matches any path component with that name -- including the lib/tdig/ source directory. A file added there was ignored and did not appear in `git status`, so it could be lost silently. Existing files were unaffected because ignore rules do not apply to tracked paths, which is why this went unnoticed. Anchor it to the root so it covers only the escript binary. Claude-Session: https://claude.ai/code/session_01YAjSJR67tWTvYDJLcQThbQ
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| %{ | ||
| family: 1 | 2, | ||
| client_subnet: :inet.ip_address(), | ||
| source_prefix: integer(), |
There was a problem hiding this comment.
ℹ️ [LOW] source_prefix: integer() について確認です。RFC 7871 のプレフィックス長は 0〜128 の非負値であり、実際に負値になることはないはずです。より success typing に近づけるなら non_neg_integer()（あるいは実装が返し得る具体的な範囲）にした方が、:underspecs 観点でも意図が明確になります。実装が本当に任意の integer を返し得るのであれば現状で問題ありませんが、範囲を絞れるなら検討をおすすめします。
There was a problem hiding this comment.
「実装が本当に任意の integer を返し得るのであれば現状で問題ありません」という条件のほうが成立していました。実測です。
192.0.2.1/24: source_prefix: 24
192.0.2.1/-5: source_prefix: -5 ← 負値が通る
192.0.2.1/999: source_prefix: 32 ← 上限だけは効く
2001:db8::1/-1: source_prefix: -1
prefix = String.to_integer(prefix_str) は "-5" を素直に -5 にし、min(prefix, 32) は上限しか抑えないためです。RFC 7871 上は 0〜128 ですが、この実装はその範囲を検証していません。
したがって non_neg_integer() にすると spec が実装より狭くなり、実際に返る値を排除する嘘の契約になります（Dialyzer の success typing も integer() です）。ここは integer() のまま据え置きます。
ただし指摘の背後にある「負の prefix は本来受け付けるべきでない」という点は妥当で、これは spec ではなく入力検証の欠落です。値域外を弾くのかクランプするのかは挙動の変更を伴うため、本 PR（Dialyzer の解消）とは分けて #86 に起票しました。
| # The escript binary, at the repository root only. Unanchored, this pattern | ||
| # also matches the lib/tdig/ directory, and a file added there is ignored | ||
| # without ever showing up in `git status`. | ||
| /tdig |
There was a problem hiding this comment.
✨ [POSITIVE] tdig から /tdig への修正は、lib/tdig/ ディレクトリ配下の新規ファイルが暗黙に無視される問題を的確に解消しており、コメントで理由も明記されている点が非常に良いです。
1 participant
概要
main で失敗し続けていた Dialyzer を解消する。あわせて、作業中に踏んだ
.gitignoreのパターン漏れを直す。Resolves #46
issue #46 の実態
issue 本文は PLT の絶対パスエラー（
/Users/toshi/...を CI が探しに行く）を報告しているが、これは issue の元になった commitf4693cc自身（PR #45）が修正済みだった。しかし Dialyzer は現在の main でも別の理由で失敗し続けている。なぜ気づけなかったか
再利用ワークフローの Dialyzer 実行ステップには
continue-on-error: trueが付いている。つまりci / Dialyzer Analysis ...: successは Dialyzer が通った証拠にならない。 失敗しても job は success で、issue が立つだけである。最新 main (56d90d7) のステップ単位の結果がそれを示している。
さらに issue 作成には重複防止があり、open な Dialyzer issue が既にあると新規作成をスキップする。#46 が開いたままだったため、以後の失敗は一切表に出ていなかった。
変更内容
1. spec を実態に合わせて狭める
parse_subnet_option/1は常に:edns_client_subnetタグ付きタプルを返し、その map のキーは 4 つとも既知である。{atom(), map()}は広すぎるため、:underspecsがcontract_supertypeを報告していた。形を
@type edns_client_subnetとして宣言した。呼び出し側が何に依存してよいかが spec に書かれ、返す map が変わったときにmap()に吸収されず報告されるようになる。挙動は変えていない。IPv4 / IPv6 両経路の既存テストはそのまま通る。
2.
.gitignoreの escript パターンをルートに固定commit しようとしたところ
git add lib/tdig/cli.exが次の警告を出した。29 行目が裸の
tdigで、これは同名のパス構成要素すべてにマッチする。escript バイナリのつもりがlib/tdig/ディレクトリにも当たっていた。追跡済みファイルには ignore 規則が適用されないので既存ファイルは無事だったが、実測すると新規ファイルは消える。/tdigに固定した。修正後はlib/tdig/配下が??として見え、ルートのバイナリは従来どおり ignore される。検証
補足
Dialyzer は push 時のみ動く（
github.event_name == 'push'）ため、この PR の CI では走らない。main に入った後の run でClose Dialyzer issues on successステップが実行され #46 が閉じられることを確認するのが最終確認になる。https://claude.ai/code/session_01YAjSJR67tWTvYDJLcQThbQ