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

Code action to apply DeMorgan's law #1243

Open
DougGregor opened this issue May 8, 2024 · 7 comments · May be fixed by #1552
Open

Code action to apply DeMorgan's law #1243

DougGregor opened this issue May 8, 2024 · 7 comments · May be fixed by #1552
Labels
code action Code action provided by LSP enhancement New feature or request

Comments

@DougGregor
Copy link
Member

Description

Sometimes in code we end up with something like:

if !x && y != z { ... }

and it would be cleaner to apply DeMorgan's law to make this:

if !(x || y == z) { ... }

We should have a code action to do this correctly, which requires dealing with inverting conditions and dealing with operator precedence. The SwiftOperators module of swift-syntax can help here.

@DougGregor DougGregor added the enhancement New feature or request label May 8, 2024
@ahoppen ahoppen added the code action Code action provided by LSP label May 8, 2024
@ahoppen
Copy link
Member

ahoppen commented May 13, 2024

Synced to Apple’s issue tracker as rdar://128016621

@AppAppWorks
Copy link
Contributor

Could this code action extend to cover bitwise operators as well?

@ahoppen
Copy link
Member

ahoppen commented Jul 1, 2024

Yes, that would make sense.

@AppAppWorks
Copy link
Contributor

@ahoppen as you mentioned in #1539, being overly restrictive is more of a problem than being too permissive. However for DeMorgan's law, the situation is a bit different. We may identify multiple candidate expressions as we walk up the syntax tree, e.g. !(a != b || !(c && d)), which I believe isn't a terribly uncommon form of expression.

I can think of two possible ways to deal with the ambiguity,

  1. pick the nearest or the farthermost candidate expression;
  2. provide a code action for each candidate expression encountered as we ascend to the root expression.

@ahoppen
Copy link
Member

ahoppen commented Jul 1, 2024

It’s probably one of these situations where we need some reference implementation and then use it to decide what feels best. But I think starting with the outermost expression makes sense as a start.

@AppAppWorks
Copy link
Contributor

Shall we support generalised DeMorgan's in addition to dual DeMorgan's?
e.g. !a && !b && !c -> !(a || b || c)

@ahoppen
Copy link
Member

ahoppen commented Jul 2, 2024

I would start with one direction. The dual would be a nice follow-up after that.

@AppAppWorks AppAppWorks linked a pull request Jul 3, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code action Code action provided by LSP enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants