Fix invalid usages of ArgumentNullException #4629
Conversation
GitExtUtils/Linq/LinqExtensions.cs
Outdated
| if (key == null) | ||
| { | ||
| var ex = new ArgumentNullException("KeySelector produced a key that is null. See exception data for source."); | ||
| var ex = new Exception("KeySelector produced a key that is null. See exception data for source."); |
There was a problem hiding this comment.
Two usages here are not validating arguments. They're validating the result of keySelector. Opted for Exception as that's what a lot of existing code does. Could be InvalidOperationException, but I'm indifferent.
There was a problem hiding this comment.
Specifically, the first argument to ArgumentNullException is supposed to be a parameter name. Given there's no parameter here, it's not possible to use that exception type here.
There was a problem hiding this comment.
💡 You could use ArgumentException here since keySelector does not behavior in a manner consistent with the requirements of this method.
| @@ -98,7 +98,7 @@ private bool HaveValidCommitMsgHook([NotNull] IGitModule gitModule, bool force) | |||
| { | |||
| if (gitModule == null) | |||
| { | |||
| throw new ArgumentNullException("gitDirectory"); | |||
| throw new ArgumentNullException(nameof(gitModule)); | |||
There was a problem hiding this comment.
Previous code likely an artefact of refactoring. New code shouldn't have that problem in future.
Codecov Report
@@ Coverage Diff @@
## master #4629 +/- ##
=======================================
Coverage 26.35% 26.35%
=======================================
Files 503 503
Lines 41424 41424
Branches 5997 5997
=======================================
Hits 10919 10919
Misses 30016 30016
Partials 489 489
Continue to review full report at Codecov.
|
drewnoakes
force-pushed
the
argument-exceptions
branch
from
f2b6df1
to
f6e5af0
Compare
|
Replaced |
For one occurrence only, intentional? |
drewnoakes
force-pushed
the
argument-exceptions
branch
from
f6e5af0
to
539037c
Compare
|
@gerhardol thanks, fixed in 539037c. |
Pretty simple change. Will annotate the diff with descriptions.