Skip to content

Ruby: flow through getters/setters #9287

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

Merged
merged 3 commits into from
May 27, 2022

Conversation

aibaars
Copy link
Contributor

@aibaars aibaars commented May 24, 2022

This pull request adds support for data flow through getter/setter methods. It assumes that setter methods like field= write to an instance variable named @field. This is to handle cases where the body of the setter method is not visible to the analysis. While this is an approximation, I'd expect very few cases of false flow. If my assumption turns out to be wrong we should make the analysis more precise.

@aibaars aibaars requested a review from a team as a code owner May 24, 2022 12:05
@github-actions github-actions bot added the Ruby label May 24, 2022
@aibaars aibaars force-pushed the instance-variable-flow-2 branch from 4418c8d to 05d8452 Compare May 24, 2022 12:06
@aibaars aibaars added the no-change-note-required This PR does not need a change note label May 24, 2022
Copy link
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

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

LGTM. Let's see what DCA says.

@aibaars aibaars force-pushed the instance-variable-flow-2 branch from 05d8452 to a6508e7 Compare May 24, 2022 12:54
@aibaars aibaars force-pushed the instance-variable-flow-2 branch from c1e5464 to 033df76 Compare May 25, 2022 14:01
Copy link
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

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

LGTM. Let's wait for DCA before merging.

@aibaars aibaars merged commit e3ef258 into github:main May 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-change-note-required This PR does not need a change note Ruby
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants