Add WithNonVoidReturnType #762
Conversation
| @@ -46,7 +46,6 @@ protected virtual CodeFixProvider GetBasicCodeFixProvider() | |||
| /// <param name="newSource">A class in the form of a string after the code fix was applied to it.</param> | |||
| /// <param name="codeFixIndex">Index determining which code fix to apply if there are multiple.</param> | |||
| /// <param name="allowNewCompilerDiagnostics">A boolean controlling whether or not the test will fail if the code fix introduces other warnings after being applied.</param> | |||
| [SuppressMessage("Microsoft.Design", "CA1026:DefaultParametersShouldNotBeUsed", Justification = "It's not intended to be a public API, so it doesn't matter")] | |||
There was a problem hiding this comment.
I disabled this warning in the ruleset for tests, since it only matters for public APIs
|
If you squash the first two commits, I presume it will keep @balazsbonis as the author? |
I'm almost certain it will, but I'll check before pushing |
|
It's academic in this case, since he's not GPG signing his commits, but if he was, that would remove the |
|
I don't think it's possible to rebase a GPG signed commit, even if there's no conflict, since the SHA1 would be different |
|
(I mean someone else's commit, of course; you can still rebase your own commit) |
thomaslevesque
force-pushed
the
withnonvoidreturntype
branch
2 times, most recently
from
a64c1ee
to
b38bf6f
Compare
thomaslevesque
force-pushed
the
withnonvoidreturntype
branch
from
b38bf6f
to
3b368a3
Compare
|
I made all the changes, including the squash. Thanks for the review! |
| this.configuration.WithNonVoidReturnType(); | ||
|
|
||
| // Assert | ||
| Assert.That(this.callRule.ApplicableToAllNonVoidReturnTypes, Is.EqualTo(true)); |
There was a problem hiding this comment.
Is this the only reason the property has a getter? I'd be inclined to drop it (and this test) in that case. Of course, that leaves a set-only property, but we could switch to a method.
On the other hand, the property as is does follow the pattern established by ApplicableToMembersWithReturnType, so if it stays as is, I'm okay.
There was a problem hiding this comment.
Well, it's also read in AnyCallCallRule... but yes, it's the only place outside the class where it's read. It doesn't really bother me, though; IMO, if we can set it, we should be able to get it too.
There was a problem hiding this comment.
I'm fine with it remaining. I'd care more if the type was public, but since it's internal.. meh.
|
Generally looks good to me. Thanks, @thomaslevesque! |
|
Thanks @thomaslevesque ! |
|
I remembered right after I went up to bed last night. Docs? |
|
Argh, I keep forgetting... |
|
I'll send a new PR soon |
Supersedes #674
I left @balazsbonis's commit intact for now so you can see what I changed; I'll squash the first two commits before merge.
Fixes #648