rubberduck-vba / Rubberduck3

COM add-in for the VBIDE
GNU General Public License v3.0
89 stars 17 forks source link

Pre-compilation pass to capture alternate definitions #51

Open retailcoder opened 1 year ago

retailcoder commented 1 year ago

Rubberduck needs a way to access the "dead code" tokens from the pre-compilation parse pass.

That way we could access the different possible declarations of a given definition (RD got 'em backwards I think), and thus support the LSP "definition" (or "declaration"?) requests.

Rubberduck 2.x assumed to be working with compilable code at all times, and would bail otherwise.

Rubberduck 3.0 reports syntax errors instead, so we get to analyze error nodes and still work out the identifier names here:

#If True Then
Sub DoSomething(ByVal Value As Long)
#Else
Sub DoSomething(ByVal Value As Variant)
#End If
   '...
End Sub

There might be a way to recover these nodes as some ConditionalCompilationDeadCodeContext, or to otherwise have a way to suppress the conditional compilation related "syntax errors" - so they don't consistently appear with red squiggles in the Rubberduck Editor. The error handler already filters some specific syntax errors to issue specific error messages, it's wouldn't be impossible to achieve the desired result via syntax errors.

Having this information also allows the LSP server to mark that code as "unreachable" for formatting purposes, so the editor can (should) render it dimmed/fainted. It also gives us multiple navigable definitions for that DoSomething procedure and Value parameter`. Only one of these definitions is ever "live", so identifier resolution and the semantic layer wouldn't necessarily need major tweaks to support this, I think.

MDoerner commented 1 year ago

What exactly is the problem here? On one side, there is the notion of alternative definitions and on the other side the issue of confusion of the parser due to syntax errors from conditional compilation? What exactly is the problem with the old approach from Rubberduck 2.x for the second issue?

Let me shortly remind you of how this works in Rubberduck 2.x. First there is a pre-compilation parse, which assumes that the precompiler syntax is valid. (I really do not know what to make of user-code with illegal precompiler directives.) Then we figure out which parts are dead. More precisely, we figure out which tokens are either in a dead block of are precompiler directives. For these, we set the hidden attribute on the token stream we feed to the actual parser. In RD2, that parser always expects valid code, but we should just as well be able to use the RD3 parser at that point.

Personally, I think trying to figure out all possible declarations based on all possible pre-compiler arguments might be a very complicated undertaking. So, I think, just showing everything based on the current pre-compiler arguments should be sufficient.

retailcoder commented 1 year ago

I don't think we need to know what code is live in every possible combination of precompiler constant values - we still only need to know about the live code (and thus mark the rest as "dead"). What I'm wondering is if there's a way to somehow still be able to parse this dead code and determine if it contains alternate definitions for stuff in the live code; having this information, we would still only resolve identifier references off the live code, but then refactor/rename would have access to these dead-code alternative definitions, so they're actual symbols to the LSP server - alternatively we could enhance refactor/rename to "search comments and string literals" to perform these dead-code renames without the symbol information, and not implement the definitions LSP endpoints.