=== undefinedと=== nullを禁止したい

[Hiroshiba]

内容

タイトルの通りです。

厳密等価===は有用ですが、undefinedやnullを右辺に持ってくる時、左辺がundefinedなのかnullなのかを意識する必要があります。
左辺がどちらになりえるかを往々にして間違えることがあり、気づくことが難しいです。
実際に間違っていることもありました。

• 厳密にはundefinedではなくnullだったのを修正 #1485

この問題の解決としていろんな方法があるかもしれませんが、1つの方法としてundefinedやnullとの厳密等価比較を禁止する手がありそうです。

ESLintに詳しい方は解決しやすいと思います。

Pros 良くなる点

気づきにくいバグを防ぐことができる

Cons 悪くなる点

ESLintの設定が難しい

実現方法

試してみたのですがなかなかうまくいきませんでした。こんな感じのeslint.jsonに記載しました。

    // undefinedとnullの厳密等価比較を禁止する
    "no-restricted-syntax": [
      "error",
      {
        selector:
          "BinaryExpression[operator='==='][left.type!='Literal'][right.type='Identifier'][right.name='undefined']",
        message: "Do not use '=== undefined'. Use '== undefined' instead.",
      },
      {
        selector:
          "BinaryExpression[operator='==='][left.type!='Literal'][right.type='Identifier'][right.name='null']",
        message: "Do not use '=== null'. Use '== null' instead.",
      },
    ],

以下は全部エラーになって欲しい例ですが、この場合いくつかのパターンが素通りしてしまいました。

      const fuga = "null";
      const a = fuga === undefined;

      const b = "fuga" === undefined;

      const button = { text: null };
      const c = button.text === null;

      const d = "fuga" === null;

ESLintで禁止できそうですが、もしやり方をご存知の方がいたらアドバイスいただければ･･･。

VOICEVOXのバージョン

0.?.0

OSの種類/ディストリ/バージョン

• Windows
• macOS
• Linux

その他

nullとの絶対比較を絶対に使ってはいけないとは思っておらず、使わないといけない時はdisableにして利用すれば良いと思っています。

あと話は↓と近いかも。

• undefinedかnullの片方を使用禁止にする #1035

[Hiroshiba]

=== undefinedや=== nullが使われていたらワーニングが出るようになりました！（thx @Tksn07 !!!）
（修正箇所がかなり大量に及ぶので一旦ワーニングという方針です）

全てのワーニングを直してエラーが出るようにできればこの課題は解決かなと思います！

[Tksn07]

@Hiroshiba マージありがとうございます！
引き続き、このワーニングを直すのは私の方で進めさせていただこうと思います 🙏

そこで1つ、このワーニングのPRの切る粒度について相談したいです 👀

修正必要ファイルを洗い出したところ

• build
  • splitNsisArchive.js
• src/
  • background.ts
  • components/
    • AccentPhrase.vue
    • AudioCell.vue
    • udioDetail.vue
    • AudioInfo.vue
    • CharacterOrderDialog.vue
    • CharacterPortrait.vue
    • DefaultStyleListDialog.vue
    • DefaultStyleSelectDialog.vue
    • Dialog.ts
    • EngineManageDialog.vue
    • FileNamePatternDialog.vue
    • HeaderBarCustomDialog.vue
    • MenuBar.vue
    • MenuItem.vue
  • background/
    • engineManager.ts
    • portManager.ts
    • configMigration014.ts
  • • vvppManager.ts
  • browser/
    • fileImpl.ts
    • sandbox.ts
    • storeImpl.ts
  • helpers/
    • SelectionHelperForQInput.ts
    • map.ts
    • previewSliderHelper.ts
  • infrastructures
    • EngineConnector.ts
  • store
    • audio.ts
    • audioPlayer.ts
    • dictionary.ts
    • engine.ts
    • index.ts
    • preset.ts
    • project.ts
    • proxy.ts
    • setting.ts
    • utility.ts
  • types/
    • result.ts
  • views/
    • EditorHome.vue
• tests/
  • e2e/browser/複数選択/選択.spec.ts

のファイルたちが修正が必要みたいです。
修正のやり方としては色々あると考えていて、特に現実的なのは

• 1つのPRで修正する
• 1ファイル1PRで修正する
• 機能単位で修正する

だと思っています。
個人的には、エラーが吐かれているメソッドを機能単位でPRを作成し、動作確認しながら修正していくやり方が最も確実で安全だと考えていますが、レビュワーにとってレビューしやすいやり方が一番だと思うので @Hiroshiba さんに意見いただきたいです🙏🙏🙏

[sevenc-nanashi]

参考までに：過去に、全てのVueコンポーネントを<script setup>にした時がありました（ #1065 ）
この時は「機能単位で修正する」に近いやり方でした。

[Hiroshiba]

まとめと段取り提案ありがとうございます！！
いろんなやり方と考え方があると思いますが、仰るとおり機能単位でPR頂けるとやりやすそうです！
１回で全部は流石に確認が大変そうなのと、動作確認を１回ずつで済ましやすいのと、他のコミッターがコンフリクト解消する回数を抑えられそうなので。

差分どれくらいの数か不明ですが、PR回数3~5回くらいが良い塩梅かな？となんとなく思ってます。
動作確認もしていただけるのであればとても嬉しいです！！
ただまあマルチエンジンまわりとかは実行チェックだいぶ大変そうなので、そういうとこは確認なしでも全然OKです！（そのためのレビュー）

[Hiroshiba]

ずっとGithub actionsの結果としてwarningが表示され続けている状況なので、そろそろ解決できると嬉しいです！

@Tksn07 お久しぶりです！！　もしよかったら取り組んでみませんか？？
数十個ずつくらい一気に進めてしまって、あとはなんとなく動作チェックを行い、レビュー時に問題がありそうかどうかを検査する形で良いのかなと思ってます！！
一番問題なのが現状放置されている状態で、それよりはもしかしてもしかしたらバグがあるかもしれないけれども改善する方が大事かなと･･･！
（そもそもバグが起こるのが相当珍しいと思います！）

[Tksn07]

@Hiroshiba お久しぶりです！コメントありがとうございます！！
期間が空いてしまい申し訳ございません🙇‍♂️
是非着手させていただきます！！一気に改修は難しいので少しづつ着手させていただきます🙇‍♀️🙇‍♀️🙇‍♀️

[Hiroshiba]

おお、ぜひぜひ！！
とりあえず1ファイルだけとかでも良いと思います！！

[cm-ayf]

#1747 や #1752 でかなり抹消されたのですが，実は.vueのv-if=""で比較されている場所で考慮漏れが発生しておりまして……
https://github.com/VOICEVOX/voicevox/blob/main/src/components/help/LibraryPolicy.vue#L7
https://github.com/VOICEVOX/voicevox/blob/main/src/components/HeaderBar.vue#L5

[Hiroshiba]

@cm-ayf あ！！！　ご報告ありがとうございます、とても助かります！！！

せっかくなのでプルリクエストを作ってみるのはいかがでしょうか 👀
（コントリビューターリストに追記されます。）

難しそうであればこちらで変更させていただきます！！

[cm-ayf]

これらを変更するのは難しくないのですが，難しいのはESLintがこれらを見逃さないようにすることですね……

[Hiroshiba]

あー･･･なるほどです。
流石にvueの中まではまだ見てくれない感じなんですね･･･！

[cm-ayf]

.vueの中を見るには，例えばeslint-plugin-vueの内部向けに存在するwrapRuleModuleなどを使う必要がありそうです．

[Hiroshiba]

なるほどです！！
なぜvueの中でeslintがある程度働けてるのか疑問だったのですが、eslint-plugin-vueが頑張ってくれてたんですね。

できれば.vueのtemplateの中まで見れるととても嬉しいですね･････！！！

[cm-ayf]

別issueにしましょうか．→ #1758