Skip to content
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

Support BinaryOperation for Boolean expressions in SE #5479

Merged
merged 6 commits into from
Mar 17, 2022

Conversation

pavel-mikula-sonarsource
Copy link
Contributor

Fixes #5478

@pavel-mikula-sonarsource pavel-mikula-sonarsource changed the title Support BinaryOperatorKind in ShimLayer Support BinaryOperation for Boolean expressions in SE Mar 11, 2022
@pavel-mikula-sonarsource pavel-mikula-sonarsource force-pushed the Pavel/SE-ConditionalOperationState branch from 577ea5e to 6ca13af Compare March 11, 2022 15:14
@pavel-mikula-sonarsource pavel-mikula-sonarsource linked an issue Mar 14, 2022 that may be closed by this pull request
Base automatically changed from Pavel/SE-ConditionalOperationState to master March 14, 2022 11:31
@pavel-mikula-sonarsource pavel-mikula-sonarsource marked this pull request as ready for review March 14, 2022 12:21
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

Copy link
Contributor

@csaba-sagi-sonarsource csaba-sagi-sonarsource left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! We should also add support for unary operators. Negation is used a lot and would be nice to support. Most probably in a separate PR. Will you open an issue for it?

@pavel-mikula-sonarsource
Copy link
Contributor Author

Negation on booleans does not translate to unary operation, it is already supported by supporting the property CFG shape, see

[TestMethod]
public void Branching_TrueConstraintNegated_VisitsElseBranch()
{
const string code = @"
var value = true;
if (!value)
{
Tag(""If"");
}
else
{
Tag(""Else"");
}
Tag(""End"");";
SETestContext.CreateCS(code).Validator.ValidateTagOrder(
"Else",
"End");
}

[TestMethod]
public void Branching_FalseConstraintNegated_VisitsIfBranch()
{
const string code = @"
var value = false;
if (!value)
{
Tag(""If"");
}
else
{
Tag(""Else"");
}
Tag(""End"");";
SETestContext.CreateCS(code).Validator.ValidateTagOrder(
"If",
"End");
}

and complex case

[TestMethod]
public void Branching_BoolConstraints_ComplexCase()
{
const string code = @"
var isTrue = true;
var isFalse = false;
if (isTrue && isTrue && !isFalse)
{
if (isFalse || !isTrue)
{
Tag(""UnreachableIf"");
}
else if (isFalse)
{
Tag(""UnreachableElseIf"");
}
else
{
Tag(""Reachable"");
}
}
else
{
Tag(""UnreachableElse"");
}
Tag(""End"");";
SETestContext.CreateCS(code).Validator.ValidateTagOrder(
"Reachable",
"End");
}

@csaba-sagi-sonarsource
Copy link
Contributor

@pavel-mikula-sonarsource I did not provide an example, my bad...
I tried the following example, which did not work:

var isTrue = true;
var isFalse = false;

if (isTrue == !isFalse)
    Tag(""True"");
else
    Tag(""True Unreachable"");

@pavel-mikula-sonarsource
Copy link
Contributor Author

MMF-2563 will be a good place to do stuff like this

@pavel-mikula-sonarsource pavel-mikula-sonarsource merged commit 5f1b46e into master Mar 17, 2022
@pavel-mikula-sonarsource pavel-mikula-sonarsource deleted the Pavel/SE-BinaryOperation branch March 17, 2022 07:07
@Tim-Pohlmann
Copy link
Contributor

The example above works now. It got implemented during an earlier MMF.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support BinaryOperation for Boolean expressions in SE
3 participants