swiftlang / sourcekit-lsp

Language Server Protocol implementation for Swift and C-based languages
Apache License 2.0
3.25k stars 271 forks source link

Code action to apply DeMorgan's law #1243

Open DougGregor opened 3 months ago

DougGregor commented 3 months ago

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.

ahoppen commented 3 months ago

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

AppAppWorks commented 2 months ago

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

ahoppen commented 2 months ago

Yes, that would make sense.

AppAppWorks commented 2 months ago

@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 commented 2 months ago

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 commented 2 months ago

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

ahoppen commented 2 months ago

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