Skip to content

Enable rubocop rule to disallow OpenStruct usage#11397

Merged
mitchellhenke merged 3 commits intomainfrom
mitchellhenke/add-open-struct-rubocop
Oct 25, 2024
Merged

Enable rubocop rule to disallow OpenStruct usage#11397
mitchellhenke merged 3 commits intomainfrom
mitchellhenke/add-open-struct-rubocop

Conversation

@mitchellhenke
Copy link
Contributor

🛠 Summary of changes

OpenStruct usage is discouraged and Ruby 3.3 emits performance warnings for its use. This PR removes all of our usage as well as updates yard who removed usage in the most recent release. I have also enabled a Rubocop rule that will fail if we use it in the future.

changelog: Internal, Performance, Enable rubocop rule to disallow OpenStruct usage

def proof(applicant)
aamva_applicant =
Aamva::Applicant.from_proofer_applicant(OpenStruct.new(applicant))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the scariest removal, but in looking through the usage, it doesn't seem like we call methods on it at all, and only use hash key access.

https://github.com/18F/identity-idp/blob/ca1ce25/app/services/proofing/aamva/applicant.rb#L30-L43

# controller's params. As this is a model spec, we have to fake
# the params object.
fake_params = ActionController::Parameters.new(
user: OpenStruct.new(id: 'fake_user_id'),
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like we could add an exception to this rule for specs if we wanted?

Copy link
Contributor

Choose a reason for hiding this comment

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

I kinda feel like most of these examples are improved (made more realistic) by not using structs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that was my thought as well. We also have double() which is not quite the same as OpenStruct, but appropriate enough for testing cases where we need something that responds to a given method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also meant to mention that I'm much less concerned about using it in specs though, yeah.

# controller's params. As this is a model spec, we have to fake
# the params object.
fake_params = ActionController::Parameters.new(
user: OpenStruct.new(id: 'fake_user_id'),
Copy link
Contributor

Choose a reason for hiding this comment

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

I kinda feel like most of these examples are improved (made more realistic) by not using structs?

Mitchell Henke and others added 2 commits October 25, 2024 08:39
…c.rb

Co-authored-by: Andrew Duthie <1779930+aduth@users.noreply.github.com>
@mitchellhenke mitchellhenke merged commit 774fa2d into main Oct 25, 2024
@mitchellhenke mitchellhenke deleted the mitchellhenke/add-open-struct-rubocop branch October 25, 2024 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants