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

modify-by-receiver: PoC of what is not detected yet #1180

Closed

Conversation

ccoVeille
Copy link
Contributor

Earlier, I looked at the code in the PR

There was something that was unclear for me.
I wondered how the code would react to more complex cases than the one I saw in the tests.

Tonight, I added the tests, so we could discuss it.

I'm unsure if I detected things revive could report, or things that could lead to false-positive

@denisvmedia @chavacava @alexandear Please take a look and tell me.

Thanks

This follows up :

Please note I code the test with an increment, but I think it applies to everything modify-by-receiver currently detects

@ccoVeille
Copy link
Contributor Author

The tests are failing on purpose to show the potential issues we have with the rule now

@ccoVeille ccoVeille marked this pull request as draft December 8, 2024 21:43
@ccoVeille ccoVeille force-pushed the modify-by-receiver-poc branch from 917829d to e7ac41a Compare December 9, 2024 08:38
@denisvmedia
Copy link
Collaborator

I agree these should be detected. Do you have a PoC how to detect them?

@ccoVeille
Copy link
Contributor Author

ccoVeille commented Dec 9, 2024

Not yet, I'm unsure I will be able to work on them anytime soon. My AST is a bit rusty 😅

I wanted to gather feedbacks, first to see if I was wrong or not.

Could you confirm the one you would agree to report? Can you spot them one by one in the code changes provided.

I'm also interested by the one you agree to keep undetected.

When I will some feedbacks I think I will open an issue,because yes, anyone can work on fixing this.

@alexandear
Copy link
Contributor

Maybe it's better to create an issue and continue the discussion there? PRs are usually used for showing work, not for discussions.

@ccoVeille
Copy link
Contributor Author

ccoVeille commented Dec 9, 2024

In fact, I did a month ago

I have to confess I totally forgot it 😄

@chavacava just replied me about the complexity of such detection.

So let's continue on that issue

@ccoVeille ccoVeille closed this Dec 9, 2024
@ccoVeille ccoVeille deleted the modify-by-receiver-poc branch December 9, 2024 20:41
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