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] [PHP] raises issues on try-catch blocks without the finally clause #173

Closed
jycr opened this issue Apr 12, 2023 · 6 comments
Closed
Assignees
Labels
🏗️ refactoring refactoring for best practices 🗃️ rule rule improvment or rule development or bug php

Comments

@jycr
Copy link
Contributor

jycr commented Apr 12, 2023

From @ganncamp (see: https://community.sonarsource.com/t/new-plugin-ecocode-requesting-inclusion-in-sonarqube-marketplace/85398/15):

ecocode-php:EC34’s title is “Avoid using try-catch-finally”, but it raises issues on try-catch blocks without the finally clause. The code sample clears it up, but it seems the rule is really “Avoid using try-catch” or perhaps “Avoid using try-catch and try-catch-finally”?

Ditto Python.

@dedece35 dedece35 added 🗃️ rule rule improvment or rule development or bug php 🏗️ refactoring refactoring for best practices __PRIO_HIGH__ labels Apr 13, 2023
@dedece35 dedece35 self-assigned this Apr 13, 2023
@dedece35
Copy link
Member

dedece35 commented Apr 13, 2023

Hi @jycr, (@glalloue, @mdubois81 , @jules-delecour-dav)

after investigation, for me this rule EC34 is not valid because :

  • the issue description isn't accurate
  • the rule only checks existence of try statement and then launch an issue
  • I think this rule should be applicable only for some cases (to be identified) : for example, use a test of existence or not of a file (or directory) instead of using a try catch based on FileNotFoundException

by the other hand, this rule is not present in 4th edition of 115 rules (see RULES.md).

to conclude, for me, this rule has to be refactored to do real good things.

@jycr
Copy link
Contributor Author

jycr commented Apr 14, 2023

If everyone agrees that the rule is currently not appropriate, why not just delete it?
We can work on it later (inspired by what has been developed by looking at the Git history).

@dedece35 dedece35 changed the title ecocode-php:EC34 - raises issues on try-catch blocks without the finally clause [EC34] [PHP] raises issues on try-catch blocks without the finally clause Aug 25, 2023
@dedece35
Copy link
Member

Hi @jycr,
the error message was modifier in 1.2.0 version to "Avoid using try-catch" instead of "Avoid using try-catch-finally" as asked for.
this rule is only implemented for PHP and Python plugin.

TO DISCUSS IN CORE TEAM :

  • do we keep this rule ? and make Java implementation also ?
  • do we delete this rule ?

@dedece35 dedece35 added python and removed 🏗️ refactoring refactoring for best practices labels Sep 22, 2023
@glalloue
Copy link
Contributor

glalloue commented Oct 6, 2023

OK to delete this rule (seen with Julien, Maxime, David)

@dedece35
Copy link
Member

dedece35 commented Oct 6, 2023

check TODOs on #128

@dedece35
Copy link
Member

close because the work is done in #128

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 php
Projects
None yet
Development

No branches or pull requests

3 participants