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

Style/HashEachMethods: Detect use of .each + _ variables #12370

Closed
splattael opened this issue Nov 8, 2023 · 0 comments · Fixed by #12398
Closed

Style/HashEachMethods: Detect use of .each + _ variables #12370

splattael opened this issue Nov 8, 2023 · 0 comments · Fixed by #12398

Comments

@splattael
Copy link
Contributor

Is your feature request related to a problem? Please describe.

Let Style/HashEachMethods flag code like:

# bad
hash.each do |key, _|
end
hash.each do |key, _value|
end

# bad
hash.each do |_, value|
end
hash.each do |_key, value|
end

# good
hash.each_key do |key|
end

hash.each_value do |value|
end

Describe the solution you'd like

The cop should suggest faster alternatives to each when used with underscore variables.

Describe alternatives you've considered

Alternatively, we could create a new cop rule but I feel it be a nice extension to Style/HashEachMethods.

Additional context

Potential impact on projects done via rg "each do \|(_(\w+)?, \w+|\w+, _(\w+)?)\|".

RuboCop

spec/rubocop/cop/lint/unused_block_argument_spec.rb:            hash.each do |key, _value|
spec/rubocop/cop/lint/unused_block_argument_spec.rb:            hash.each do |_key, value|
spec/rubocop/cop/lint/unused_block_argument_spec.rb:            hash.each do |_key, _value|
lib/rubocop/cop/style/bisected_attr_accessor.rb:          find_macros(class_node.body).each do |_visibility, macros|

GitLab

lib/feature/definition.rb:    TYPES.each do |type, _|
lib/tasks/gitlab/setup.rake:    Gitlab.config.repositories.storages.each do |name, _details|
lib/tasks/gitlab/db.rake:        Gitlab::Database.database_base_models.each do |_, model_class|
lib/gitlab/dependency_linker/podspec_json_linker.rb:        dependencies.each do |name, _|
lib/backup/database.rb:      base_models_for_backup.each do |database_name, _base_model|
lib/backup/manager.rb:      definitions.each do |_, definition|
lib/gitlab/language_data.rb:              YAML.load_file(Rails.root.join('vendor', 'languages.yml')).each do |_name, details|
lib/gitlab/repo_path.rb:      Gitlab::GlRepository.types.each do |_name, type|
lib/gitlab/database/load_balancing/configuration.rb:          config.service_discovery.each do |key, _|
lib/gitlab/seeder.rb:      Gitlab::Database.database_base_models.each do |_, model|
app/graphql/types/commit_signatures/verification_status_enum.rb:      ::CommitSignatures::GpgSignature.verification_statuses.each do |status, _|
app/services/packages/debian/process_package_file_service.rb:        file_metadata[:files].each do |_, entry|
app/models/concerns/bulk_insertable_associations.rb:    self.class.reflections.each do |_, reflection|
spec/lib/gitlab/database/migrations/reestablished_connection_stack_spec.rb:    Gitlab::Database.database_base_models_with_gitlab_shared.each do |db_config_name, _|
spec/lib/gitlab/database/load_balancing/setup_spec.rb:      models.each do |_, model|
spec/models/state_note_spec.rb:    ResourceStateEvent.states.each do |state, _value|
spec/models/ci/job_artifact_spec.rb:    described_class.file_types.each do |file_type, _|
spec/models/ci/job_artifact_spec.rb:    described_class.file_types.each do |file_type, _|
spec/models/ci/build_spec.rb:    Ci::JobArtifact.file_types.each do |type, _|
spec/requests/api/project_templates_spec.rb:    TemplateFinder::VENDORED_TEMPLATES.each do |template_type, _|
spec/support/shared_examples/lib/gitlab/database/reestablished_connection_stack_shared_examples.rb:    Gitlab::Database.database_base_models.each do |_, model_class|
spec/support/helpers/database/multiple_databases_helpers.rb:      Gitlab::Database.database_base_models.each do |_, base_model|
spec/support/helpers/repo_helpers.rb:    files.each do |filename, _content|
doc/development/database/efficient_in_operator_queries.md:records_by_id.each do |id, _|
spec/features/commits_spec.rb:      commits.chunk { |c| c.committed_date.in_time_zone.to_date }.each do |day, _daily_commits|
spec/config/settings_spec.rb:      described_class.repositories.storages.each do |_, storage|
qa/qa/specs/features/api/3_create/repository/project_archive_compare_spec.rb:        users.each do |_, user_info|
qa/qa/specs/features/ee/browser_ui/10_govern/change_vulnerability_status_spec.rb:          vulnerabilities.each do |name, _description|
qa/spec/specs/runner_spec.rb:          QA::Runtime::Env.supported_features.each do |tag, _|
qa/spec/specs/runner_spec.rb:          QA::Runtime::Env.supported_features.each do |tag, _|
tooling/danger/analytics_instrumentation.rb:          helper.changed_lines(file_name).each do |mod_line, _i|
ee/lib/gitlab/graphql/aggregations/epics/lazy_epic_aggregate.rb:            tree.each do |_, node|
ee/lib/remote_development/workspaces/create/post_flatten_devfile_validator.rb:          variables.each do |variable, _|
ee/lib/gitlab/code_owners/file.rb:        parsed_data.each do |_, section_entries|
ee/app/services/elastic/process_bookkeeping_service.rb:      specs.each do |spec, _|
ee/app/services/security/security_orchestration_policies/ci_configuration_service.rb:        ci_configuration.each do |_, job_configuration|
ee/spec/lib/gitlab/usage/metrics/instrumentations/count_security_scans_metric_spec.rb:      ::Security::Scan.scan_types.except('cluster_image_scanning').each do |name, _|
ee/spec/lib/gitlab/usage/metrics/instrumentations/count_secure_pipelines_metric_spec.rb:      ::Security::Scan.scan_types.except('cluster_image_scanning').each do |name, _|
ee/spec/lib/gitlab/usage/metrics/instrumentations/count_secure_pipelines_metric_spec.rb:    ::Security::Scan.scan_types.except('cluster_image_scanning').each do |name, _|
ee/spec/lib/ee/gitlab/elastic/helper_spec.rb:      @indices.each do |index_name, _|
ee/spec/models/ee/namespace_spec.rb:        limits.each do |_attribute, limit|
ee/spec/models/vulnerabilities/feedback_spec.rb:      described_class.categories.each do |category, _|
ee/spec/models/package_metadata/sync_configuration_spec.rb:    ::Enums::Sbom::PURL_TYPES.each do |purl_type, _|
ee/spec/requests/api/graphql/mutations/member_role/create_member_role_spec.rb:    MemberRole::ALL_CUSTOMIZABLE_PERMISSIONS.each do |permission, _options|
ee/spec/controllers/projects_controller_spec.rb:          params.each do |param, _value|
ee/spec/services/approval_rules/finalize_service_spec.rb:              expected_rules.each do |_key, hash|
ee/spec/services/approval_rules/finalize_service_spec.rb:              expected_rules.each do |_key, hash|
ee/spec/services/approval_rules/finalize_service_spec.rb:        ApprovalProjectRule.rule_types.except(:code_owner, :report_approver).each do |rule_type, _value|
ee/spec/services/package_metadata/sync_service_spec.rb:        ::Enums::PackageMetadata.purl_types.each do |purl_type, _|
koic added a commit to koic/rubocop that referenced this issue Nov 21, 2023
…k value

Fixes rubocop#12370.

This PR makes `Style/HashEachMethods` aware of unused block value.
And this PR suppresses the following new offense in this repository:

```console
$ bundle exec rubocop --only Style/HashEachMethods -A
(snip)

Offenses:

lib/rubocop/cop/style/bisected_attr_accessor.rb:36:11: C: [Corrected] Style/HashEachMethods:
Use each_value instead of each, and remove the unused _visibility block argument.
          find_macros(class_node.body).each do |_visibility, macros| ...
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1519 files inspected, 1 offense detected, 1 offense corrected
```
bbatsov pushed a commit that referenced this issue Nov 21, 2023
Fixes #12370.

This PR makes `Style/HashEachMethods` aware of unused block value.
And this PR suppresses the following new offense in this repository:

```console
$ bundle exec rubocop --only Style/HashEachMethods -A
(snip)

Offenses:

lib/rubocop/cop/style/bisected_attr_accessor.rb:36:11: C: [Corrected] Style/HashEachMethods:
Use each_value instead of each, and remove the unused _visibility block argument.
          find_macros(class_node.body).each do |_visibility, macros| ...
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1519 files inspected, 1 offense detected, 1 offense corrected
```
kachick added a commit to kachick/ruby-ulid that referenced this issue Dec 4, 2023
Personally I will not be happy with this cop, I intentionally specified _ variables
But no motivation to change it for now...

rubocop/rubocop#12370
kachick added a commit to kachick/ruby-ulid that referenced this issue Dec 4, 2023
This reverts commit c321e03.

It is not a Hash, false positive detection by rubocop

rubocop/rubocop#12370
kachick added a commit to kachick/ruby-ulid that referenced this issue Dec 4, 2023
* Bump the rubocop-dependencies group with 1 update

Updates the requirements on [rubocop](https://github.com/rubocop/rubocop) to permit the latest version.
- [Release notes](https://github.com/rubocop/rubocop/releases)
- [Changelog](https://github.com/rubocop/rubocop/blob/master/CHANGELOG.md)
- [Commits](rubocop/rubocop@v1.57.2...v1.58.0)

---
updated-dependencies:
- dependency-name: rubocop
  dependency-type: direct:development
  dependency-group: rubocop-dependencies
...

Signed-off-by: dependabot[bot] <support@github.com>

* `bundle exec rubocop -A`

Personally I will not be happy with this cop, I intentionally specified _ variables
But no motivation to change it for now...

rubocop/rubocop#12370

* Revert "`bundle exec rubocop -A`"

This reverts commit c321e03.

It is not a Hash, false positive detection by rubocop

rubocop/rubocop#12370

* Suppress wrong rubocop warnings with parentheses

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Kenichi Kamiya <kachick1@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants