Skip to content

Add Style/OpenStructUse cop

Matthias Käppler requested to merge open-struct-use-cop into master

Description of the proposal

gitlab-org/gitlab#343421 (closed)

NOTE: There are 2 competing proposals, this one, and https://gitlab.com/gitlab-org/gitlab-styles/-/merge_requests/91. Please vote only once using 👍/👎 on either MR. Assuming one MR gets enough votes, the one with more 👍 wins.

This is a stricter version of https://gitlab.com/gitlab-org/gitlab-styles/-/merge_requests/91 which flags any use of OpenStruct whatsoever as it is discouraged to be used as of Ruby 3.0.

This would subsume the existing Performance/OpenStruct cop that was enabled outside of tests, which I have therefore disabled.

Note that this will catch many more uses: I see 117 uses of OpenStruct in gitlab currently, all but 9 of them in specs:

grep -R 'OpenStruct' app/ lib/ spec/ 

app/finders/snippets_finder.rb:    @params = OpenStruct.new(params)
app/serializers/README.md:behaves like an `OpenStruct` object, with the difference that it will raise
app/helpers/application_settings_helper.rb:    Gitlab.config.repositories.storages.keys.each_with_object(OpenStruct.new) do |storage, weights|
lib/mattermost/session.rb:      @request ||= OpenStruct.new(parameters: params)
lib/gitlab/fake_application_settings.rb:    # Mimic behavior of OpenStruct, which absorbs any calls into undefined
lib/gitlab/testing/request_inspector_middleware.rb:        request = OpenStruct.new(
lib/gitlab/git/diff_collection.rb:        OpenStruct.new(limits)
lib/gitlab/import_export/after_export_strategies/base_after_export_strategy.rb:          @options = OpenStruct.new(attributes)
lib/api/wikis.rb:            present OpenStruct.new(result[:result]), with: Entities::WikiAttachment
spec/workers/packages/maven/metadata/sync_worker_spec.rb:    OpenStruct.new(
spec/lib/gitlab/relative_positioning/range_spec.rb:  item_a = OpenStruct.new(relative_position: 100, object: :x, positioned?: true)
spec/lib/gitlab/relative_positioning/range_spec.rb:  item_b = OpenStruct.new(relative_position: 200, object: :y, positioned?: true)
spec/lib/gitlab/relative_positioning/range_spec.rb:    item_c = OpenStruct.new(relative_position: 150, object: :z, positioned?: true)
spec/lib/gitlab/relative_positioning/range_spec.rb:    item_d = OpenStruct.new(relative_position: 050, object: :w, positioned?: true)
spec/lib/gitlab/relative_positioning/range_spec.rb:    item_e = OpenStruct.new(relative_position: 250, object: :r, positioned?: true)
spec/lib/gitlab/relative_positioning/range_spec.rb:    item_f = OpenStruct.new(positioned?: false)
spec/lib/gitlab/relative_positioning/range_spec.rb:    item_ax = OpenStruct.new(relative_position: 100, object: :not_x, positioned?: true)
spec/lib/gitlab/relative_positioning/range_spec.rb:    item_bx = OpenStruct.new(relative_position: 200, object: :not_y, positioned?: true)
spec/lib/gitlab/grape_logging/loggers/queue_duration_logger_spec.rb:      let(:mock_request) { OpenStruct.new(env: {}) }
spec/lib/gitlab/grape_logging/loggers/queue_duration_logger_spec.rb:        OpenStruct.new(
spec/lib/gitlab/grape_logging/loggers/exception_logger_spec.rb:  let(:mock_request) { OpenStruct.new(env: {}) }
spec/lib/gitlab/grape_logging/loggers/exception_logger_spec.rb:        OpenStruct.new(
spec/lib/gitlab/grape_logging/loggers/perf_logger_spec.rb:  let(:mock_request) { OpenStruct.new(env: {}) }
spec/lib/gitlab/database/migrations/runner_spec.rb:        migration_runs << OpenStruct.new(dir: dir, version_to_migrate: version_to_migrate)
spec/lib/gitlab/graphql/pagination/keyset/connection_generic_keyset_spec.rb:  let(:context) { GraphQL::Query::Context.new(query: OpenStruct.new(schema: schema), values: nil, object: nil) }
spec/lib/gitlab/graphql/pagination/keyset/connection_spec.rb:  let(:context) { GraphQL::Query::Context.new(query: OpenStruct.new(schema: schema), values: nil, object: nil) }
spec/lib/gitlab/quick_actions/dsl_spec.rb:      expect(dynamic_description_def.to_h(OpenStruct.new(noteable: 'issue'))[:description]).to eq('A dynamic description for ISSUE')
spec/lib/gitlab/quick_actions/dsl_spec.rb:      expect(dynamic_description_def.execute_message(OpenStruct.new(noteable: 'issue'), 'arg')).to eq('A dynamic execution message for ISSUE passing arg')
spec/lib/gitlab/quick_actions/command_definition_spec.rb:    let(:opts) { OpenStruct.new(go: false) }
spec/lib/gitlab/quick_actions/command_definition_spec.rb:    let(:context) { OpenStruct.new(run: false, commands_executed_count: nil) }
spec/lib/gitlab/gitaly_client/diff_stitcher_spec.rb:      diff_1 = OpenStruct.new(
spec/lib/gitlab/gitaly_client/diff_stitcher_spec.rb:      diff_2 = OpenStruct.new(
spec/lib/gitlab/gitaly_client/diff_stitcher_spec.rb:      diff_3 = OpenStruct.new(
spec/lib/gitlab/gitaly_client/diff_stitcher_spec.rb:      msg_1 = OpenStruct.new(diff_1.to_h.except(:patch))
spec/lib/gitlab/gitaly_client/diff_stitcher_spec.rb:      msg_2 = OpenStruct.new(diff_2.to_h.except(:patch))
spec/lib/gitlab/gitaly_client/diff_stitcher_spec.rb:      msg_3 = OpenStruct.new(raw_patch_data: diff_2.patch[101..-1], end_of_patch: true)
spec/lib/gitlab/gitaly_client/diff_stitcher_spec.rb:      msg_4 = OpenStruct.new(diff_3.to_h.except(:patch))
spec/lib/gitlab/gitaly_client/blobs_stitcher_spec.rb:        OpenStruct.new(oid: 'abcdef1', path: 'path/to/file', size: 1642, revision: 'f00ba7', mode: 0100644, data: "first-line\n"),
spec/lib/gitlab/gitaly_client/blobs_stitcher_spec.rb:        OpenStruct.new(oid: '', data: 'second-line'),
spec/lib/gitlab/gitaly_client/blobs_stitcher_spec.rb:        OpenStruct.new(oid: '', data: '', revision: 'f00ba7', path: 'path/to/non-existent/file'),
spec/lib/gitlab/gitaly_client/blobs_stitcher_spec.rb:        OpenStruct.new(oid: 'abcdef2', path: 'path/to/another-file', size: 2461, revision: 'f00ba8', mode: 0100644, data: "GIF87a\x90\x01".b)
spec/lib/gitlab/usage_data_spec.rb:        OpenStruct.new(name: 'google_oauth2'),
spec/lib/gitlab/usage_data_spec.rb:        OpenStruct.new(name: 'ldapmain'),
spec/lib/gitlab/usage_data_spec.rb:        OpenStruct.new(name: 'group_saml')
spec/lib/gitlab/auth/o_auth/provider_spec.rb:        provider = OpenStruct.new(
spec/lib/gitlab/auth/o_auth/provider_spec.rb:          expect(subject).to be_a(OpenStruct)
spec/lib/gitlab/auth/o_auth/provider_spec.rb:      let(:provider) { OpenStruct.new({ 'name' => name, 'label' => label }) }
spec/lib/gitlab/auth/o_auth/provider_spec.rb:      let(:provider) { OpenStruct.new({ 'name' => name } ) }
spec/lib/gitlab/cleanup/orphan_lfs_file_references_spec.rb:      .and_return(oids.map { |oid| OpenStruct.new(lfs_oid: oid) })
spec/lib/gitlab/legacy_github_import/project_creator_spec.rb:    OpenStruct.new(
spec/presenters/packages/nuget/search_results_presenter_spec.rb:  let_it_be(:search_results) { OpenStruct.new(total_count: 3, results: [package_a, packages_b, packages_c].flatten) }
spec/support/helpers/repo_helpers.rb:    OpenStruct.new(
spec/support/helpers/repo_helpers.rb:    OpenStruct.new(
spec/support/helpers/repo_helpers.rb:    OpenStruct.new(
spec/support/helpers/repo_helpers.rb:    OpenStruct.new(
spec/support/helpers/repo_helpers.rb:    OpenStruct.new(
spec/support/helpers/repo_helpers.rb:    OpenStruct.new(
spec/support/helpers/import_spec_helper.rb:    provider = OpenStruct.new(
spec/support/helpers/login_helpers.rb:    OpenStruct.new(name: 'saml', label: 'saml', args: {
spec/support/shared_examples/controllers/githubish_import_controller_shared_examples.rb:  let(:repo) { OpenStruct.new(login: 'vim', full_name: 'asd/vim', name: 'vim', owner: { login: 'owner' }) }
spec/support/shared_examples/controllers/githubish_import_controller_shared_examples.rb:  let(:org) { OpenStruct.new(login: 'company') }
spec/support/shared_examples/controllers/githubish_import_controller_shared_examples.rb:  let(:org_repo) { OpenStruct.new(login: 'company', full_name: 'company/repo', name: 'repo', owner: { login: 'owner' }) }
spec/support/shared_examples/controllers/githubish_import_controller_shared_examples.rb:    stub_client(repos: [repo, org_repo], orgs: [org], org_repos: [org_repo], each_page: [OpenStruct.new(objects: [repo, org_repo])].to_enum)
spec/support/shared_examples/controllers/githubish_import_controller_shared_examples.rb:    let(:repo_2) { OpenStruct.new(login: 'emacs', full_name: 'asd/emacs', name: 'emacs', owner: { login: 'owner' }) }
spec/support/shared_examples/controllers/githubish_import_controller_shared_examples.rb:      allow(client).to receive(:each_page).and_return([OpenStruct.new(objects: repos)].to_enum)
spec/support/shared_examples/controllers/githubish_import_controller_shared_examples.rb:        allow(client).to receive(:each_page).and_return([OpenStruct.new(objects: repos)].to_enum)
spec/support/shared_examples/controllers/githubish_import_controller_shared_examples.rb:  let(:provider_user) { OpenStruct.new(login: provider_username) }
spec/support/shared_examples/controllers/githubish_import_controller_shared_examples.rb:    OpenStruct.new(
spec/support/shared_examples/controllers/githubish_import_controller_shared_examples.rb:      owner: OpenStruct.new(login: provider_username)
spec/support/shared_examples/controllers/githubish_import_controller_shared_examples.rb:      provider_repo.owner = OpenStruct.new(login: other_username)
spec/support/shared_examples/requests/api/nuget_packages_shared_examples.rb:    let(:target) { OpenStruct.new(id: id) }
spec/support/shared_examples/requests/api/nuget_packages_shared_examples.rb:    let(:target) { OpenStruct.new(id: 1234567890) }
spec/support/shared_examples/requests/api/pypi_packages_shared_examples.rb:    let(:project) { OpenStruct.new(id: 1234567890) }
spec/support/shared_examples/requests/api/pypi_packages_shared_examples.rb:    let(:group) { OpenStruct.new(id: 1234567890) }
spec/factories/go_module_versions.rb:    params { OpenStruct.new(mod: mod, type: type, commit: commit, name: name, semver: semver, ref: ref) }
spec/factories/go_module_versions.rb:      params { OpenStruct.new(mod: mod, type: :ref, commit: commit, semver: name, ref: ref) }
spec/factories/wiki_pages.rb:      page { OpenStruct.new(url_path: title) }
spec/finders/template_finder_spec.rb:    subject(:result) { described_class.new(type, project, params).template_names.values.flatten.map { |el| OpenStruct.new(el) } }
spec/graphql/mutations/merge_requests/create_spec.rb:      query: OpenStruct.new(schema: nil),
spec/graphql/mutations/merge_requests/accept_spec.rb:      query: OpenStruct.new(schema: GitlabSchema),
spec/graphql/mutations/commits/create_spec.rb:      query: OpenStruct.new(schema: nil),
spec/graphql/mutations/clusters/agent_tokens/create_spec.rb:      query: OpenStruct.new(schema: nil),
spec/graphql/mutations/clusters/agents/delete_spec.rb:      query: OpenStruct.new(schema: nil),
spec/graphql/mutations/clusters/agents/create_spec.rb:      query: OpenStruct.new(schema: nil),
spec/graphql/mutations/branches/create_spec.rb:      query: OpenStruct.new(schema: nil),
spec/graphql/types/range_input_type_spec.rb:      query: OpenStruct.new(schema: nil),
spec/support_specs/helpers/stub_feature_flags_spec.rb:          ['string', 1, 1.0, OpenStruct.new]
spec/tooling/rspec_flaky/flaky_example_spec.rb:  let(:example) { OpenStruct.new(example_attrs) }
spec/initializers/doorkeeper_spec.rb:      allow(controller).to receive(:request).and_return(OpenStruct.new(fullpath: '/return-path'))
spec/helpers/application_settings_helper_spec.rb:      expect(helper.storage_weights).to eq(OpenStruct.new(
spec/helpers/profiles_helper_spec.rb:      it { expect(helper.user_status_set_to_busy?(OpenStruct.new(availability: availability))).to eq(result) }
spec/helpers/profiles_helper_spec.rb:      it { expect(helper.show_status_emoji?(OpenStruct.new(message: message, emoji: emoji))).to eq(result) }
spec/helpers/profiles_helper_spec.rb:    provider = OpenStruct.new(
spec/controllers/concerns/import_url_params_spec.rb:    controller = OpenStruct.new(params: params).extend(described_class)
spec/controllers/admin/clusters_controller_spec.rb:            OpenStruct.new(
spec/controllers/groups/clusters_controller_spec.rb:          OpenStruct.new(
spec/controllers/import/gitlab_controller_spec.rb:      @repo = OpenStruct.new(id: 1, path: 'vim', path_with_namespace: 'asd/vim', web_url: 'https://gitlab.com/asd/vim')
spec/controllers/import/fogbugz_controller_spec.rb:      @repo = OpenStruct.new(id: 'demo', name: 'vim')
spec/controllers/projects/clusters_controller_spec.rb:          OpenStruct.new(
spec/requests/api/import_github_spec.rb:    let(:provider_user) { OpenStruct.new(login: provider_username) }
spec/requests/api/import_github_spec.rb:      OpenStruct.new(
spec/requests/api/import_github_spec.rb:        owner: OpenStruct.new(login: provider_username)
spec/requests/api/graphql/mutations/design_management/delete_spec.rb:      let(:designs) { %w/foo bar baz/.map { |fn| OpenStruct.new(filename: fn) } }
spec/requests/api/internal/base_spec.rb:        pull(OpenStruct.new(id: 0), project)
spec/features/projects/clusters_spec.rb:          OpenStruct.new(
spec/features/projects/clusters/gcp_spec.rb:            OpenStruct.new(
spec/dependencies/omniauth_saml_spec.rb:  let(:settings) { OpenStruct.new({ soft: false, idp_cert_fingerprint: 'something' }) }
spec/services/system_note_service_spec.rb:          allow(instance).to receive(:comments).and_return([OpenStruct.new(body: message)])
spec/services/packages/nuget/metadata_extraction_service_spec.rb:      let(:package_file) { OpenStruct.new(id: 555) }
spec/services/packages/nuget/metadata_extraction_service_spec.rb:        allow_any_instance_of(Zip::File).to receive(:glob).and_return([OpenStruct.new(size: 6.megabytes)])
spec/services/projects/import_service_spec.rb:      provider = OpenStruct.new(
spec/models/design_management/design_at_version_spec.rb:      expect(dav).not_to eq(OpenStruct.new(id: dav.id))
spec/models/design_management/design_action_spec.rb:    let(:design) { OpenStruct.new(full_path: path) }
spec/models/design_management/design_action_spec.rb:    let(:design) { OpenStruct.new(issue_id: issue_id) }
spec/models/user_spec.rb:    let(:request) { OpenStruct.new(remote_ip: "127.0.0.1") }
spec/bin/feature_flag_spec.rb:      let(:options) { OpenStruct.new(name: 'foo', type: :development) }
spec/bin/feature_flag_spec.rb:      let(:options) { OpenStruct.new(name: 'foo', type: :development) }

It would take some time to migrate all those over.

Check-list

  • Mention this proposal in the relevant Slack channels (e.g. #development, #backend, #frontend)
  • [-] If there is a choice to make between two potential styles, set up an emoji vote in the MR:
    • CHOICE_A: 🅰
    • CHOICE_B: 🅱
    • Vote yourself for both choices so that people know these are the choices
  • The MR doesn't have significant objections, and is getting a majority of 👍 vs 👎 (remember that we don't need to reach a consensus)
  • [-] (If applicable) One style is getting a majority of vote (compared to the other choice)
  • [-] (If applicable) Update the MR with the chosen style
  • Follow the review process as usual

/cc @gitlab-org/maintainers/rails-backend

Edited by Peter Leitzen

Merge request reports