Add/Remove Line Numbers

[PaulWalkerUK]

Would it be possible for Rubberduck to have the ability to add/remove line numbers from procedures/modules/projects?

Having line numbers can be useful for error reporting, but it's harder editing while they're there and they make a mess of source control. It's useful therefore to be able to add/remove them en masse at a procedure, module and project level.

[retailcoder]

Removing line numbers should be incredibly easy to implement; simply walk the parse tree(s) for the specified scope, and send the token index of every line number label to the module's rewriter, replacing the tokens with empty strings - and then run the indenter, just in case.

I'm partial about adding line numbers though. Not that it's very hard, just that... They're not useful for error reporting, they're a PITA to maintain, and they're only supported for backward-compatibility that goes all the way to Commodore-BASIC 2.0 code from 1982.

I'm absolutely ok with writing a tool for stripping line numbers from VBA code, but I think I'd prefer leaving it to vbWatchDog and MZ-Tools to add them... but I won't reject a PR that introduces the feature.

[retailcoder]

Clarifying about how line numbers aren't useful for error reporting:

If a procedure is doing so many things that only a line number can help knowing where the error occurred, then the procedure is poorly implemented, very likely is doing way too many things and thus has too many reasons to fail and is impossible to name meaningfully so even its name doesn't say what the procedure does.

The solution isn't line numbers, it's refactoring.

Erl function (which means to return the line number of the last error) is undocumented. If an error occurs on a non-numbered line, Erl will return whatever the last encountered line number was. It doesn't account for supported edge cases either:

Sub test()

  On Error GoTo &HFFFFFFFE
&HFFFFFFFD Debug.Print 1 / 0

&HFFFFFFFE:
  Debug.Print Erl() 'Prints 65533

End Sub

Line numbers are contrary to everything Rubberduck is trying to promote: clear, well-factored, object-oriented, self-documenting, maintainable code. Structured error handling doesn't need line numbers, line numbers were needed when BASIC didn't even have procedure scopes. Line numbers were needed when omitting them ran the command in immediate mode rather than including it in the program's listing.

Line numbers are a relic of the past, that have no business is modern VBA code, whatever the reason is.

</rant>

[retailcoder]

Other perfectly legal things that break Erl:

Sub foo()
            On Error Resume Next
12345678    Err.Raise 1
            Debug.Print Erl   '24910
End Sub

Sub foo()
On Error Resume Next
-1                   Err.Raise 1
-2          Debug.Print Erl   '65535
End Sub

[retailcoder]

:warning: A Declaration instance is created for every line number label; if Declaration.References.Any is true, the line number is actually referred to by some live code, and therefore shouldn't be removed (it should be a line label instead!)

[MDoerner]

I think removing line numbers is slightly more complicated than stated above, simply because we cannot remove all line numbers. We can only remove line numbers safely that are not referenced; otherwise, the code does not compile anymore.

I am not too sure that we are already correctly resolving all references to line numbers. In this regard the implicit GoTo in an one-line if statement comes to my mind.

Public Sub Whatever()
    If False Then Else 3
    Debug.Print "You will never see this."
    Exit Sub
3 foo: Debug.Print "You will see this."
End Sub

[retailcoder]

@MDoerner ha, nice catch! we actually pick up that 3 as a line label declaration (notice the VBE inserts an explicit GoTo instruction there):

So indeed, it's a bit more complicated than just walking the parse trees with a chainsaw... but I'm pretty sure we do have all the information.

[PaulWalkerUK]

Thanks for the information, I didn't know any of that!

I started using line numbers after reading Error Handling and Debugging Tips and Techniques for Microsoft Access, VBA, and Visual Basic 6 (VB6), and when I found MZ-Tools could add/remove them easily, I just assumed they were a good thing. I agree about them being a PITA to maintain and work with, hence having a tool that can easily add/remove them is nice.

No worries if it's not something you want to add (it's well beyond my ability at this stage to implement it anyway!), so feel free to close if you like :smile:

[retailcoder]

@PaulWalkerUK Oh, but it's something I'd want to remove :smiley:

[PaulWalkerUK]

In that case, go for it! (and I'll try to stop relying on unsupported crutches and write cleaner code - at least now I've got Rubberduck to help me!) 😄 Thanks,

[PaulWalkerUK]

Don't suppose you have any links/quick pointers on how best to do error handling in something that's being used by "end users" rather than just the person who's developed it?

[retailcoder]

@PaulWalkerUK we've put up a lot of content on Documentation.SO (which is being phased out - currently read-only): https://stackoverflow.com/documentation/vba/3211/error-handling

Feel free to give Rubberduck a star if you like!

[ThunderFrame]

@PaulWalkerUK A lot of the FMSInc content was written a long time ago, when IDE features and common best-practices were different. Even some of Chip Pearson's content, which is widely regarded, by some, as being the gold-standard, is somewhat out of date, and no longer the best content.

[PaulWalkerUK]

Thanks for all the useful information (and ⭐️ added (and well deserved) - forgotten I hadn't already done it 😄). Guess I need to find new sources of information (Chip Pearson had been another that I'd liked and used a lot)......

[Stevefb]

@retailcoder FWIW totally agree that 'needing' line numbers is a good indication of when a sub or function is doing too much... Sadly there are a lot of subs and functions that are doing too much. I'm sure there is a lot of code out there (not just what I'm working on) that people would like to refactor and I for 1 think RD could continue to help / educate people (I'm a convert) and improve code by allowing them to add line numbers but pointing to a better way - because for these imperfect subs that do do too much line numbers are helpful.

I have started to experience that when subs & functions become more purposeful and smaller that the location of the error becomes obvious... and yes line numbers are not needed. But while I strive to eliminate my reliance on line numbers for this project, it would be fair to say that I do rely on them for the time being... but... I also have MZ for that.

So if RD were to support the addition of line numbers it would be along the lines of 'adding line numbers so we can get rid of them'... Just my 2c

[retailcoder]

@Stevefb I would much rather RD gets its Extract Method refactoring back, instead of adding line numbers. With Extract Method implemented and working flawlessly, there's simply no excuse anymore.

[Stevefb]

@retailcoder yup, there's that. Some of the subs in my project were so ridiculously big that I think my only option would have been to use both tools together. I do appreciate where you're coming from - also very thankful for RD :) Cheers,

[bclothier]

Found this old issue via SO link. The issue seems to be more about adding a command and the subsequent discussion is that we'd rather remove it. However, that would make for a terrible UX because if you provide a command to remove line numbers, people will understandably wonder where is the command to do the opposite.

Here's a better UX -- this should be an inspection, with a quickfix to remove the line number.

For the cases where we might cause code to break (e.g. a line label as a target of jump), we can at least warn the user that they may need to revise and proceed anyway if they so wish.

[retailcoder]

@bclothier don't we already have a "label not used" inspection? it should be trivial to make it work with line numbers that aren't referenced. Only thing is.. it would spawn an awful lot of results in a large project that uses line numbers everywhere.

[bclothier]

I don't think that's the best use of that particular inspection. Besides, we'd want the "Gratuitous Use of Line Number" inspection to run per-procedure scope, not per-line scope, right?

[retailcoder]

That does make sense. No more than one warning per scope for unwarranted line numbering, and the quickfix would leave used/referenced line numbers alone (while another inspection would recommend turning them to line labels instead)