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.

Sign up for GitHub

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

Jump to bottom

Create rule S7116: The first element of an array should not be accessed implicitly CPP-5674 #4377

Merged
merged 17 commits into from
Nov 12, 2024
Merged

Create rule S7116: The first element of an array should not be accessed implicitly CPP-5674 #4377

merged 17 commits into from
Nov 12, 2024

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Oct 7, 2024 •
edited by loic-joly-sonarsource
Loading

CPP-5674

You can preview this rule here (updated a few minutes after each push).

Review

A dedicated reviewer checked the rule description successfully for:

  • logical errors and incorrect information
  • information gaps and missing content
  • text style and tone
  • PR summary and labels follow the guidelines

@github-actions github-actions bot added the cfamily C / C++ / Objective-C label Oct 7, 2024
@tomasz-kaminski-sonarsource tomasz-kaminski-sonarsource changed the title Create rule S7116 Create rule S7116: First array element should not be accessed with arrow Oct 7, 2024
@tomasz-kaminski-sonarsource tomasz-kaminski-sonarsource changed the title Create rule S7116: First array element should not be accessed with arrow Create rule S7116: First array element should not be accessed using arrow Oct 7, 2024
@tomasz-kaminski-sonarsource tomasz-kaminski-sonarsource changed the title Create rule S7116: First array element should not be accessed using arrow Create rule S7116: First array element should be accessed using subscript operator Oct 7, 2024
@tomasz-kaminski-sonarsource tomasz-kaminski-sonarsource changed the title Create rule S7116: First array element should be accessed using subscript operator CPP-5674 Create rule S7116: First array element should be accessed using subscript operator Oct 7, 2024
@tomasz-kaminski-sonarsource tomasz-kaminski-sonarsource marked this pull request as ready for review October 8, 2024 12:09
@frederic-tingaud-sonarsource frederic-tingaud-sonarsource changed the title CPP-5674 Create rule S7116: First array element should be accessed using subscript operator Create rule S7116: First array element should be accessed using subscript operator CPP-5674 Oct 9, 2024
loic-joly-sonarsource
loic-joly-sonarsource requested changes Oct 9, 2024
View reviewed changes
Copy link
Contributor

@loic-joly-sonarsource loic-joly-sonarsource left a comment

Choose a reason for hiding this comment

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

Mostly suggesting changes in the wording.

@tomasz-kaminski-sonarsource tomasz-kaminski-sonarsource changed the title Create rule S7116: First array element should be accessed using subscript operator CPP-5674 Create rule S7116: The first element of an array should not be accessed implicitly CPP-5674 Oct 9, 2024
loic-joly-sonarsource
loic-joly-sonarsource requested changes Oct 9, 2024
View reviewed changes
rules/S7116/cfamily/rule.adoc Outdated
==== Compliant solution

If the access to the first element was intentional, you can use subscript explicitly:
[source,cpp,diff-id=1,diff-type=compliant]
Copy link
Contributor

Choose a reason for hiding this comment

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

Yet in the next example, you highlight a solution that does change the meaning of the code... (I also considered removing it there, but that solution really seemed more meaningful than the other, while here all suggestions look as likely to me)

Maybe we can wait and see how it looks in the product, I'm afraid it will not look good.

loic-joly-sonarsource
loic-joly-sonarsource approved these changes Oct 9, 2024
View reviewed changes
Copy link
Contributor

@loic-joly-sonarsource loic-joly-sonarsource left a comment

Choose a reason for hiding this comment

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

I still don't fully agree on the issue of diffs, but it's not large enough to block this PR.

@tomasz-kaminski-sonarsource
Copy link
Contributor

Added the comment to PR: https://sonarsource.atlassian.net/browse/CPP-5674?focusedCommentId=671412, to check how RSPEC look in product.

@sonarqube-next SonarQube-Next
Copy link

Quality Gate passed Quality Gate passed for 'rspec-tools'

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

@sonarqube-next SonarQube-Next
Copy link

Quality Gate passed Quality Gate passed for 'rspec-frontend'

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

@sonarqube-next SonarQube-Next
Copy link

Quality Gate passed Quality Gate passed for 'rspec-frontend'

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

@sonarqube-next SonarQube-Next
Copy link

Quality Gate passed Quality Gate passed for 'rspec-tools'

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

@tomasz-kaminski-sonarsource tomasz-kaminski-sonarsource merged commit b2e18a8 into master Nov 12, 2024
9 of 10 checks passed
@tomasz-kaminski-sonarsource tomasz-kaminski-sonarsource deleted the rule/add-RSPEC-S7116 branch November 12, 2024 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomasz-kaminski-sonarsource tomasz-kaminski-sonarsource tomasz-kaminski-sonarsource left review comments

@loic-joly-sonarsource loic-joly-sonarsource loic-joly-sonarsource approved these changes

Assignees

@tomasz-kaminski-sonarsource tomasz-kaminski-sonarsource

Labels
cfamily C / C++ / Objective-C
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tomasz-kaminski-sonarsource @loic-joly-sonarsource