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

Suggest to reverse condition when 'Then' block is empty #874

Closed
retailcoder opened this issue Dec 16, 2015 · 11 comments
Closed

Suggest to reverse condition when 'Then' block is empty #874

retailcoder opened this issue Dec 16, 2015 · 11 comments
Assignees
Labels
difficulty-03-duck Involves more challenging problems and/or developing within and revising the internals API feature-inspections up-for-grabs Use this label in conjunction with a difficulty level label, e.g. difficulty-02-ducky
Milestone

Comments

@retailcoder
Copy link
Member

This code:

If foo = 42 Then Else DoSomething

Should be rewritten as:

If Not (foo = 42) Then DoSomething

Or better:

If foo <> 42 Then DoSomething

But reversing operators might not be trivial in all condition expressions.

Also applies to if blocks:

If foo  = 42 Then
Else
    DoSomething
End If

Should be rewritten as:

If Not (foo = 42) Then
    DoSomething
End If
@retailcoder retailcoder added this to the Version 2.0 milestone Dec 16, 2015
@Vogel612
Copy link
Member

This might be interesting with more complex if-conditions...

If foo < CONST and bar <> 545 or baz = 25 Then Else DoSomething

@Hosch250
Copy link
Member

I handled this for C# code in VSDiagnostics. I should be able to handle it for VBE code too.

@Hosch250 Hosch250 self-assigned this Dec 16, 2015
@retailcoder
Copy link
Member Author

@Hosch250 does VSD revert the operators, or simply sticks a ! in front of the existing condition? I don't mind either, but it sure makes a smarter ducky if we actually revert the operators (correctly accounting for operator precedence and parentheses... doesn't look simple)

@Hosch250
Copy link
Member

It actually reverts the operators:

!= becomes ==
== becomes !=
> becomes <=
<= becomes >
< becomes >=
>= becomes <
!(condition) becomes (condition)
(condition) becomes !(condition)
&& becomes ||
|| becomes &&

@retailcoder
Copy link
Member Author

Ok, so

<> becomes =
= becomes <>
> becomes <=
<= becomes >
< becomes >=
>= becomes <
Not {condition} becomes {condition}
{condition} becomes Not {condition}
And becomes Or
Or becomes And

What of XOr? Also, keep in mind that C# logical operators short-circuit; not VBA's.

@Hosch250
Copy link
Member

This will definitely be a mite trickier than VSD because Roslyn has that nifty tree defining precedence and stuff, but not impossible. It will be a little like parsing a mathematical expression, which I have done before. Also, I think XOr stays the same, but I can't say for sure.

@Vogel612
Copy link
Member

"A XOr B" is basically "(A and not B) or (not A and B)", inverting those is a little trickier...

@Hosch250
Copy link
Member

A And Not B inverted makes Not A Or B

@retailcoder retailcoder modified the milestones: v2.1, Version 2.0 Dec 24, 2015
@ThunderFrame
Copy link
Member

Need to consider reversing these too:

If obj Is Nothing Then
If Not obj Is Nothing

If IsArray(aData) Then
If Not IsArray(aData) Then

@ThunderFrame
Copy link
Member

Might need to apply the same functionality to Compile Directives.

@Hosch250
Copy link
Member

Other than the code to figure out which operator needs to be inverted (only the top-level one does), this is easily doable with the rewriters at this point. Basically, this would be a parse tree inspection that checked for empty if blocks with non-empty else blocks.

A similar inspection would be an if block that was empty and either no else or an empty else. If there was a non-empty Else If block, then it would just remove the if and the else before the next if and leave everything else.

@Hosch250 Hosch250 removed their assignment Apr 15, 2017
@retailcoder retailcoder added difficulty-03-duck Involves more challenging problems and/or developing within and revising the internals API up-for-grabs Use this label in conjunction with a difficulty level label, e.g. difficulty-02-ducky labels May 8, 2017
@Hosch250 Hosch250 self-assigned this May 8, 2017
Hosch250 added a commit that referenced this issue May 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty-03-duck Involves more challenging problems and/or developing within and revising the internals API feature-inspections up-for-grabs Use this label in conjunction with a difficulty level label, e.g. difficulty-02-ducky
Projects
None yet
Development

No branches or pull requests

4 participants