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

[EC34][Java][5R] - Rules "try..catch.." #128

Closed
wants to merge 3 commits into from

Conversation

Silicoman
Copy link
Contributor

Issue #101

Pour partage avec autre équipe

@sonarcloud
Copy link

sonarcloud bot commented Apr 5, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 4 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@Silicoman Silicoman changed the title Ec34 Java - Rules "try..catch.." [EC34][Java][5R] - Rules "try..catch.." Apr 6, 2023
@wokier
Copy link

wokier commented Apr 6, 2023

This rule will create too many warning
other way using existing rule: green-code-initiative/creedengo-common#15

@Silicoman
Copy link
Contributor Author

@Ella-dee , be inform about your work. :) As we discuss on team meeting, it's difficult without details rules. It will generate too many warnings false-positive without more restricted implementation.

@MP-Aubay MP-Aubay added 🗃️ rule rule improvment or rule development or bug java 🏆 challenge2023 🏆 Work done during the ecoCode Challenge 2023 labels Apr 7, 2023
@utarwyn
Copy link
Member

utarwyn commented Apr 14, 2023

Based on previous comments and recent work made on custom tags, is this PR still relevant?

I am also afraid of the number of warnings that this rule will cause: we are clearly on a language-based rule and not a platform one. And I am not sure that the impact is that significant compared to the constraints it will bring. What do you think of this?

@github-actions
Copy link

This PR has been automatically marked as stale because it has no activity for 30 days.
Please add a comment if you want to keep the issue open. Thank you for your contributions!

@dedece35
Copy link
Member

dedece35 commented Sep 22, 2023

waiting for discussion inside core-team with issue #173

@dedece35
Copy link
Member

@glalloue
Copy link
Contributor

glalloue commented Oct 6, 2023

This rule test only "try" existence and does not give a lot of "intelligence" in the test.
It will return a lot of warnings not really related to eco-conception problems

We suggest to delete this rule and create more specific rules related to try catch (example : add try catch when you create a document to return an error if document does not exists, but prefer test if the document exists before)

Seen with @dedece35 and @jhertout

@dedece35
Copy link
Member

dedece35 commented Oct 6, 2023

TODOs for me :

Copy link

This PR has been automatically marked as stale because it has no activity for 30 days.
Please add a comment if you want to keep the issue open. Thank you for your contributions!

@github-actions github-actions bot added the stale label Nov 29, 2023
@dedece35 dedece35 removed the stale label Dec 4, 2023
@dedece35 dedece35 linked an issue Dec 4, 2023 that may be closed by this pull request
@dedece35 dedece35 marked this pull request as draft December 4, 2023 21:21
@dedece35
Copy link
Member

After all discussions on this EC34 rule for Java , python and PHP, here are conclusions :

  • Python : like described above, EC34 was deprecated and transformed to EC35 with more specific checks
  • PHP : same as Python, check above
  • Java : neither EC34 rule isn't applicable nor EC35. For this one, file open process must be try-catch even if an exist control of file is done before try

thus, sorry @Silicoman, but I have to refused this PR for Java and close the referenced issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏗️ refactoring refactoring for best practices 🗃️ rule rule improvment or rule development or bug 🏆 challenge2023 🏆 Work done during the ecoCode Challenge 2023 java
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[EC34] [Java] Using try...catch...finally calls
7 participants