Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SonarCloudにBugだと言われているCDicMgrの問題に対処する #1606

Merged
merged 1 commit into from
Mar 27, 2021

Conversation

berryzplus
Copy link
Contributor

@berryzplus berryzplus commented Mar 26, 2021

PR の目的

SonarCloudの警告を減らすことが目的です。

カテゴリ

  • リファクタリング

PR の背景

SonarCloudで以下のBugレベル警告が検出されています。

Explicitly define the missing copy constructor, move constructor, copy assignment operator and move assignment operator so that they will not be implicitly provided.
https://sonarcloud.io/project/issues?id=sakura-editor_sakura&issues=AWnxcyQMO9B58BDk-C3x&open=AWnxcyQMO9B58BDk-C3x

超訳:コピーコンストラクタとムーブコンストラクタとコピー演算子とムーブ演算子が、コンパイラにより意図しない形で自動生成されることを防ぐため、これらを明示的に定義してください。

これと似たようなBugsレベル警告が他に79件あります。 👈 詳細は #1605 参照。

PR のメリット

SonarCloudの警告が、たぶん減ります。

PR のデメリット (トレードオフとかあれば)

この修正でSonarColudの警告が減るかどうか分かりません。(本当に Fault Positive な可能性があります。)
CDicMgrクラスのデストラクタに処理を入れたくなったときに、枠を書き直す手間が増えます。

仕様・動作説明

アプリの機能や仕様とは関係ない変更です。

PR の影響範囲

影響はないと考えられます。

テスト内容

対策したい警告は、普通に作成したレポジトリでは検出されないため、テスト不能です。

関連 issue, PR

#1605
#1504

参考資料

必要のないデストラクタを実装すべきでない(らしい) 👈 Rule-of-Zero
デストラクタを実装するならコピーとムーブのコンストラクタ&演算子を定義して5つセットにする必要があります。
@usagisita usagisita changed the title SonarCloudにBugだと言われているCEolの問題に対処する SonarCloudにBugだと言われているCDicMgrの問題に対処する Mar 26, 2021
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@AppVeyorBot
Copy link

Copy link
Member

@kengoide kengoide left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@berryzplus
Copy link
Contributor Author

あ、すんません。タイトル間違ってました。
CEolも似たような問題が発生しているのを対処する必要があります。
今回の対象はCDicMgrです。

@berryzplus
Copy link
Contributor Author

レビューありがとうございます。
このPRはマージしてSonarCloudを走らせてみて初めて効果を確認できる特殊な変更なので、とっととマージしてしまいます。

@berryzplus berryzplus merged commit d65f986 into sakura-editor:master Mar 27, 2021
@berryzplus berryzplus deleted the feature/fix_CDicMgr branch March 27, 2021 01:55
@berryzplus
Copy link
Contributor Author

PRの目的が達成できたことを確認しました。

https://sonarcloud.io/project/issues?id=sakura-editor_sakura&issues=AWnxcyQMO9B58BDk-C3x&open=AWnxcyQMO9B58BDk-C3x
👆Close(Fixed)になっている。

しかし、Bugsの総数は 263 ⇒ 264 になったみたいです。数え間違ったかしら・・・。

@beru beru added the refactoring リファクタリング 【ChangeLog除外】 label Mar 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring リファクタリング 【ChangeLog除外】
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants