Rule S3693: Exception constructors should not throw exceptions #629
Conversation
| @@ -0,0 +1,60 @@ | |||
| using System; | |||
There was a problem hiding this comment.
Please add a couple of tests:
- Throw without object initialization, e.g.
var a = new Exception(); throw a; - Rethrow, e.g.
throw;.
| classDeclaration.Members | ||
| .OfType<ConstructorDeclarationSyntax>() | ||
| .Select(ctor => ctor.DescendantNodes().OfType<ThrowStatementSyntax>()) | ||
| .Where(throwStatementsPerCtor => throwStatementsPerCtor.Any()) |
There was a problem hiding this comment.
[optional] Wouldn't creating a list and checking length be faster - as it avoids duplicating calls to OfType()? I'm genuinely curious. Feel free to skip it, in that case I'll add it to my todo list.
| .Select(ctor => ctor.DescendantNodes().OfType<ThrowStatementSyntax>()) | ||
| .Where(throwStatementsPerCtor => throwStatementsPerCtor.Any()) | ||
| .ToList() | ||
| .ForEach(throwStatementsPerCtor => |
There was a problem hiding this comment.
[optional] That's quite a long statement already, but actually reads quite well. I'd extract the bit in foreach to a separate method.
| { | ||
| public MyException4() | ||
| { | ||
| throw; // Noncompliant |
There was a problem hiding this comment.
is this a valid statement? what will be thrown here?
There was a problem hiding this comment.
I don't think this is a valid statement in this condition but I guess the idea was to be sure the analyzer doesn't crash.
| public sealed class ExceptionConstructorShouldNotThrow : SonarDiagnosticAnalyzer | ||
| { | ||
| internal const string DiagnosticId = "S3693"; | ||
| private const string MessageFormat = "Avoid throwing exception in this constructor."; |
There was a problem hiding this comment.
may be this should be Avoid throwing exceptions in this constructor. (e.g. plural exception)
|
rebase done |
Fix #584