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に「範囲外アクセスの可能性がある」と言われている箇所を対処する #1567

Merged

Conversation

berryzplus
Copy link
Contributor

@berryzplus berryzplus commented Mar 5, 2021

PR の目的

SonarCloudにBugだと言われている箇所を対処します。

カテゴリ

  • リファクタリング

PR の背景

#1504 の対応を少し進めたいので作ってみました。

PR のメリット

SonarCloudにBugだと言われている警告が2つ減ります。

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

単体テストでカバーできない範囲の修正なのでカバレッジが下がります。

仕様・動作説明

配列に対して添え字でアクセスするコードがBugだと言われているようなので、
添え字を使わないように同等の処理を実装することにより対策します。

PR の影響範囲

バックアップ機能のバックアップファイル名生成に影響する変更です。

テスト内容

デメリットに記載した通り、とくにテストは行っていません。

関連 issue, PR

#1504

参考資料

@AppVeyorBot
Copy link

@AppVeyorBot
Copy link

@berryzplus berryzplus marked this pull request as ready for review March 5, 2021 18:23
@berryzplus berryzplus marked this pull request as draft March 6, 2021 12:48
@berryzplus
Copy link
Contributor Author

PRをバラして出し直そうとしたところ、同ファイル内で検出されたBugsがスキャン結果にあがってしまって紛らわしくなったのでこのまま続行します。

@sonarcloud
Copy link

sonarcloud bot commented Mar 7, 2021

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

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@berryzplus berryzplus marked this pull request as ready for review March 7, 2021 06:10
@AppVeyorBot
Copy link

@berryzplus
Copy link
Contributor Author

レビューありがとうございます。マージしちゃいます。

@berryzplus berryzplus merged commit 8a22482 into sakura-editor:master Mar 7, 2021
@berryzplus berryzplus deleted the feature/fix_out_of_bounds branch March 7, 2021 14:49
@beru beru added the refactoring リファクタリング 【ChangeLog除外】 label Mar 21, 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