Implemented SA1117 diagnostic + unit tests #1480
Conversation
|
I've been looking forward to this! 😎 |
|
Sorry for the delay, was busy with other issues |
|
No problem, so was I. 😄 |
|
I didn't spot any issues during my initial review. In addition, I ran it on Roslyn.sln to verify that it didn't throw any exceptions there and also looked through many reported violations to verify the results were as-expected. I'm hoping to get a second review as well. 😄 |
|
|
||
| private static readonly ImmutableArray<DiagnosticDescriptor> SupportedDiagnosticsValue = | ||
| ImmutableArray.Create(Descriptor); | ||
|
|
||
| /// <inheritdoc/> | ||
| public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics | ||
| public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => SupportedDiagnosticsValue; |
There was a problem hiding this comment.
You could also write
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } =
ImmutableArray.Create(Descriptor);and get rid of the SupportedDiagnosticsValue field.
|
I'm missing tests that validate the following valid cases are handled properly: public void Method1(
int arg1, int arg2, int arg3)
{
}
public void Method2(
int arg1,
int arg2,
int arg3)
{
}I also doubt that the first case is handled properly |
| @@ -301,7 +301,7 @@ | |||
| <value>The parameters to a C# method or indexer call or declaration are not all on the same line or each on a separate line.</value> | |||
| </data> | |||
| <data name="SA1117MessageFormat" xml:space="preserve"> | |||
| <value /> | |||
| <value>The parameters must all be placed on the same line or alternatively, each parameter must be placed on its own line.</value> | |||
There was a problem hiding this comment.
Actually, I think it reads better without "alternativelty" at all, since "each" is repeated.
|
👍 Looks good to me now |
Fixes #51