Return to Revisions

As Matt has provided an alternative solution I'll focus more on improving your current approach (well that was covered too but there are some extra things)

' Check if a name is valid: it may be "declared" in Excel using LET().
Public Function NameIsValid(name As String) As Boolean

Using Let is an implementation detail (as Matt demonstrated with an alternative implementation), and not something the caller really needs to worry about. Instead focus on input/output. You could use a RubberDuck description annotation here which means, once synced, this comment magically turns into a description in the VBA object explorer.

 '@Description("Returns True if candidate is a valid name for an Excel named reference, False if not or the check fails")

    ' Invalidate names that are empty or too long.
    If name = Empty Or VBA.Len(name) > 255 Then

This comment is unnecessary as we can see what happens on the next line - at least qualify what too long means, e.g.

'Excel names have a maximum length

to explain why you perform the check. Or better yet get rid of 255 and use a named constant:

Const MAX_NAME_LENGTH As Long = 255
If name = vbNullString Or VBA.Len(name) > MAX_NAME_LENGTH Then
    NameIsValid = False

No comment required. Generally, you want to eliminate comments that explain what the code does where possible because if your code changes the comment becomes out of sync.

While we're here, NameIsValid = False is actually unnecessary because False is the default value of a boolean function. I would prefer to write Exit Function here, which has the side effect of allowing you to reduce the nesting of your code:

If name = Empty Or VBA.Len(name) > 255 Then Exit Function
If ( _
    name = "C" Or name = "c" Or _
    name = "R" Or name = "r" _
) Then Exit Function
    
If name <> Application.WorksheetFunction.Clean(VBA.Trim(name)) Then Exit Function
If ( _
    VBA.InStr(1, name, "(") Or _
    VBA.InStr(1, name, ",") Or _
    VBA.InStr(1, name, ";") Or _
    VBA.InStr(1, name, ")") _
) Then Exit Function

' Get the result of formulaically declaring a name with LET() in Excel.
Dim eval As Variant
eval = Application.Evaluate("= LET(" & name & ", 0, 0)")

' Check if the declaration erred due to invalid nomenclature.
NameIsValid = IsError(eval)

' Invalidate names with external whitespace (or double spaces internally),
' which are invalid in names and yet could mesh syntactically with
    ' formulaic calls to LET() in Excel.

This is a great comment, explains a non-obvious bit of code.

name = "C" Or name = "c" Or _
name = "R" Or name = "r"

VBA.InStr(1, name, "(") Or _
VBA.InStr(1, name, ",") Or _
VBA.InStr(1, name, ";") Or _
VBA.InStr(1, name, ")") _

These can both be syntactically simplified with the Like operator:

If name Like "[cCrR]" Then Exit Function 'catches single character reserved names
If name Like "*[(,;)]*" Then Exit Function 'catches at least one occurence of invalid character

although I'm not sure what that would do for speed (see below). The InStr functions are definitely your slow point (as long as the evaluate branch is never reached); not only does checking require iterating over every character until the correct one is found, VBA does not short circuit operators so always performs all 4 checks even if the first one fails. Split onto 4 lines if performance is key. In any language worth its salt the Like operator should be fast and compiled - but as Matt says VBA's string operators and functions are notoriously slow.

Update: I just checked, swapping to Like is ~25% faster than with InStr. However the call to Evaluate swallows about 80% of the execution time so changes to the earlier stage aren't too impactful (i.e that's 25% improvement if the name is invalid, but only 25*20 = 5% improvement if the name is potentially valid and has to be evaluated).

FWIW, Matt's approach is about 30x slower (worst case no cache hits) - however the point is very important that for < 1000 checks (on my CPU) it's still under a second so really performance only matters for bigger numbers

The approach in that attached xlsm appears to be about 5-10x faster than your code so perhaps that approach is just better for performance (although a complete mess in terms of amount of code that it hardly seems worth it, Matt's version is IMO easiest to follow and guaranteed to be correct).

Questions

1. Am I missing any subtle (or obvious) edge cases in my design? It is risky and rarely best practice to Evaluate() a literal String as code. Furthermore, I am wary of assuming that I have innovated a (somewhat trivial) solution that escaped the meticulous author of excel-names.

I wouldn't be too wary about a novel solution; LET was introduced in the last couple of years, that workbook was last updated in 2017. Also I was interested in that security concern; indeed IIUC it is possible to execute cmd.exe from Evaluate using direct data exchange, or send sensitive data to an attacker's web server with the HYPERLINK function or something similar. However in the context of the LET function, Excel if expecting a name not a formula so shouldn't evaluate things and I think you've managed to exclude the "injection characters" otherwise of course ",0,0) & MALICIOUSCODE() & CONCAT(0" would trip things up. I think leave an explicit comment about the security concern is important to avoid this protection being removed at a later date.

2. Should I distinguish granularly between error types, and only invalidate the name for specific reasons?

Yes IMO you should only surpress errors you expect, other errors you rethrow to the user (in case this is consumed by VBA) or return as an error if this is a UDF:

'@Description("Returns True if candidate is a valid name for an Excel named reference, False if not, and an error if the check fails")
Public Function NameIsValid(name As String) As Variant
    '...
    
    If IsError(eval) Then
        ' Granularly distinguish between specific errors.
        If ( _
            eval = CVErr(xlErrName) Or _
            eval = CVErr(xlErrValue) _
        ) Then
            NameIsValid = False
        Else
            NameIsValid = eval 'return the unknown error
        End If
    Else
        NameIsValid = True
    End If
End Function