-
Notifications
You must be signed in to change notification settings - Fork 280
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Issues 236 & 238 #239
Issues 236 & 238 #239
Conversation
…risonsTests) Modify analyzer to handle only the case where both operands are bool. Update unit tests accordingly.
…etric) Modify so that analyzer and codefix also work when the operands are reversed. Update unit tests to cover those cases.
|
||
if (!(right.Value is bool)) return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is made unnecessary by previous check - we already know that it's a bool
@thomaslevesque Your update is throwing an NullReferenceException when analyzing Corefx: https://ci.appveyor.com/project/code-cracker/code-cracker/build/1.0.0.341 |
// Only handle the case where both operands of type bool; other cases involve | ||
// too much complexity to be able to deliver an accurate diagnostic confidently. | ||
var leftType = context.SemanticModel.GetTypeInfo(comparison.Left).Type; | ||
var rightType = context.SemanticModel.GetTypeInfo(comparison.Right).Type; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe leftType
or rightType
are null and then it is throwing the nullref.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know how it's possible... If the type is unknown, TypeInfo.Type
returns a IErrorTypeSymbol
, not null. But perhaps it's GetTypeInfo
that returns null (EDIT: actually it's not possible, since TypeInfo is a struct)... I'll investigate
I'm unable to reproduce the error on my machine... AnalyzeCoreFx.ps1 runs without errors, and the log contains no mention of SimplifyRedundantBooleanComparisonsAnalyzer. However I noticed this in corefx.log (several times):
So it looks like the analysis doesn't even run... Any idea what could cause this? |
OK, I was using the wrong MSBuild version... got the error now |
You were right, there is a case where leftType or rightType can be null: when one of the operands is the literal |
Does it correctly not report in the cases where the condition is incomplete? |
@AdamSpeight2008, good point. It correctly ignores that case; I just added a test to make sure. |
Ok, it is fixed now: https://ci.appveyor.com/project/code-cracker/code-cracker/build/1.0.0.342 |
No description provided.