Add numeric parameter checks refactoring #78748
Conversation
dotnet-policy-service
bot
added
Community
| src\Workspaces\SharedUtilitiesAndExtensions\Workspace\VisualBasic\VisualBasicWorkspaceExtensions.projitems*{57ca988d-f010-4bf2-9a2e-07d6dcd2ff2c}*SharedItemsImports = 5 | ||
| src\Analyzers\CSharp\Tests\CSharpAnalyzers.UnitTests.projitems*{58969243-7f59-4236-93d0-c93b81f569b3}*SharedItemsImports = 13 | ||
| src\Dependencies\Collections\Microsoft.CodeAnalysis.Collections.projitems*{5f8d2414-064a-4b3a-9b42-8e2a04246be5}*SharedItemsImports = 5 | ||
| src\Dependencies\Contracts\Microsoft.CodeAnalysis.Contracts.projitems*{5f8d2414-064a-4b3a-9b42-8e2a04246be5}*SharedItemsImports = 5 |
There was a problem hiding this comment.
As soon as I edit something in VS it adds this. So maybe just keep it?
| [InlineData("ThrowIfLessThanOrEqual(num, 3)")] | ||
| [InlineData("ThrowIfEqual(num, 15)")] | ||
| [InlineData("ThrowIfNotEqual(num, 10)")] | ||
| [InlineData("ThrowIfZero(num)")] |
There was a problem hiding this comment.
Although we have refactorrings only for ThrowIfNegative and ThrowIfNegativeOrZero as the most common ones we recognize other numeric checks as well since adding our own if one of this is present doesn't make much sense
| { | ||
| private const string s_isPrefix = "Is"; | ||
| private const string s_throwIfPrefix = "ThrowIf"; | ||
| private const string IsPrefix = "Is"; |
There was a problem hiding this comment.
VS was constantly yelling at me for naming style violations. In this case I agreed (s_ is for static readonly and pascal case is for consts)
| var throwIfNullMethod = argumentNullExceptionType | ||
| .GetMembers(s_throwIfNullName) | ||
| .OfType<IMethodSymbol>() | ||
| .FirstOrDefault(m => m.Parameters is [{ Type.SpecialType: SpecialType.System_Object }, ..]); |
There was a problem hiding this comment.
No need to double-filter, allocate additional IEnumerable and so on
| { | ||
| var text = methodName switch | ||
| { | ||
| nameof(string.IsNullOrEmpty) => new LocalizableResourceString(nameof(FeaturesResources._0_cannot_be_null_or_empty), FeaturesResources.ResourceManager, typeof(FeaturesResources)).ToString(), |
There was a problem hiding this comment.
I don't understand the move with creating a localizable string and then immideatelly invoking ToString on it. This should be the same as just accessing the resource directly. Am I correct?
...atures/Core/Portable/InitializeParameter/AbstractAddParameterCheckCodeRefactoringProvider.cs
Show resolved
Hide resolved
...atures/Core/Portable/InitializeParameter/AbstractAddParameterCheckCodeRefactoringProvider.cs
Show resolved
Hide resolved
|
Thanks! |
3 participants
Closes: #37653