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

[EC22] - check relevance of this rule #23

Closed
Mester766 opened this issue Apr 9, 2024 · 3 comments · Fixed by #24
Closed

[EC22] - check relevance of this rule #23

Mester766 opened this issue Apr 9, 2024 · 3 comments · Fixed by #24
Assignees
Labels
🗃️ rule rule improvment or rule development or bug ❔ question Further information is requested

Comments

@Mester766
Copy link

Hi, on my symfony project, we have many code smells coming from this rule and i wonder if she is relevant to php.

For example, we have issues coming from file_exists or filemtime which are definitely not basic operations.

On the other hand, calling a function which only do a basic mathematic operation is clearly a code smell.

So I wonder if there is a relevance for this rule or if it could be enhanced so it does not match on non basic operations.

Thanks for your reply,

@glalloue glalloue added the ❔ question Further information is requested label Apr 12, 2024
@dedece35
Copy link
Member

hi @Mester766 , thank you for issue.
clearly, we are missing PHP experts, and Symphony experts.
Could you give some screenshots to prove it, please ?
You can also open a PR with improvments to change rule implémentation if you have some ideas. But maybe, you only want to delete this rule for php ... ?!
thank you.
regards.
David.

@Mester766
Copy link
Author

Hi @dedece35,
As you can see here, some complex operations are recognized as basic ones :
image

I will try some time to make my own rule and make a proposition through PR but i think that for now this rule makes more fake positive than real ones.

Thank you,

@dedece35
Copy link
Member

dedece35 commented May 2, 2024

Hi @Mester766 ,

I'm ok with, I think the discussion on the same rule for Java is also quite relevant for PHP : see green-code-initiative/creedengo-rules-specifications#148
I'm going to mark this rule as deprecated to prove the relevance with measurement.
This rule will be deleted on next minor version of plugin.

@dedece35 dedece35 self-assigned this May 2, 2024
@dedece35 dedece35 added 🗃️ rule rule improvment or rule development or bug 🔥 in progress 🔥 labels May 2, 2024
@dedece35 dedece35 linked a pull request May 2, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🗃️ rule rule improvment or rule development or bug ❔ question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants