Custom raw exceptions list for "Generic exceptions should never be thrown" rule (squid:S00112)

[nem...@gmail.com]

Hi all,

Code such as:

    try {

      return IoUtil.toByteArrayAndClose(new BufferedInputStream(in));

    } catch (IOException e) {

      throw new RuntimeException(e);

    }

violates "Generic exceptions should never be thrown" rule (squid:S00112). However, in our case, we are happy with such code, as long as it is RuntimeException.

I checked the source of RawExceptionCheck and I see that the exception list is hard-coded:

Set<String> RAW_EXCEPTIONS = ImmutableSet.of("java.lang.Throwable", "java.lang.Error", "java.lang.Exception", "java.lang.RuntimeException");

Could you make the exception list configurable? Or at least make RuntimeException optional?

I wouldn't want to write my own plugin, just for that.

Thanks,

Neeme

[Michael Gumowski]

Hello Neeme,

Thank you for your feedback, however we are not going to make the list configurable, nor make RuntimeException optional.

If throwing a RuntimeException may be valid in your case, it is not enough to make it optional. Using generic exceptions or errors is a bad pattern as it won't be possible to identify your application-related exceptions from the system-generated exceptions.

Why throwing a custom and business-related exception extending RuntimeException is not a possible solution in your case?

Regards,

Michael GUMOWSKI | SonarSource

Software Developer @ Language Team
http://sonarsource.com

--
You received this message because you are subscribed to the Google Groups "SonarQube" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sonarqube+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/ffdb922f-ae89-4b51-9256-a7e9cf5c2933%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[nem...@gmail.com]

> Using generic exceptions or errors is a bad pattern as it won't be possible to identify your application-related exceptions from the system-generated exceptions.

I can relate your comment to the following code: throw new RuntimeException("IO error");

Yes, that is bad and should be avoided.

However, in case of "throw new RuntimeException(e)", we are basically just converting a checked exception to an unchecked one. Yes, we are aware that the calling code will not be able distinguish these exceptions from the system-generated exceptions - and we are ok with it. We do not need to distinguish them. When we need to distinguish, of course we will introduce a specific Exception subclass.

Yes, we could introduce a MyException extends RuntimeException and throw that everywhere where we are currently throwing RuntimeException. But this is just plain wrong -- introducing a new exception just to make Sonar happy. And we would need to put this new exception in some dependency and make sure all our modules have access to that -- it is just too much effort for no gain.

As a general comment: the decision of how people write code is not yours to make. If a team working on a project has agreed upon a certain set of coding patterns then the toolset should support those patterns not work against them (within certain reasonable limits, of course). In most cases, Sonar adapts itself quite nicely. And there are plenty of rules to choose from. However, in this particular case, I agree to disagree.

I'm a firm believer of sensible defaults. And in case the defaults are not enough, they should be configurable.

As writing a custom plugin is too much effort for us (compared to the gain we get from this rule), we have just disabled this rule. This will reduce the usefulness of Sonar for us, but so be it -- life is not perfect.

Thank you for the response.

Neeme