VB -> C# handle > operators for strings

[v1nce]

It would be awesome if code converter could handle [string] >[string] for us

Input code

mystring > ""

Erroneous output

mystring > ""

Expected output

String.compare(mystring , "") > 0

Details

• Product in use:VS extension
• Version in use: 7.2.0.0

[GrahamTheCoder]

Thanks for raising this. Sounds very sensible, though we'd need to carefully check the expected output (by decompiling a vb dll using it) since VB may use it's own special method with subtly different behaviour. Let me know if you've already checked this, I've never used the string greater than operator myself so I don't know. The change would probably need to be made around a method called something like VisitBinaryExpression in case someone wants to tackle this before I get around to it.

…

On Wed, 23 Oct 2019 at 17:59, v1nce ***@***.***> wrote: It would be awesome if code converter could handle [string] >[string] for us Input code mystring > "" Erroneous output mystring > "" Expected output String.compare(mystring , "") > 0 Details - Product in use:VS extension - Version in use: 7.2.0.0 — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#396?email_source=notifications&email_token=AATAA4QJJI6ITLRCJFOMASDQQB7HVA5CNFSM4JEEUZTKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HT4FO3A>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AATAA4UJ6X4GXTMBZGQNDGDQQB7HVANCNFSM4JEEUZTA> .

[v1nce]

I don't use them myself, this is legacy code.

It looks like it's more complicated than I expected.

The "right" thing to do (for strings) could be to use

using Microsoft.VisualBasic.CompilerServices;
Operators.CompareString("Peter", "Davis", %comparisonOption%)

With comparisonOption being an "external" var defined in the *.vbproj (see image attached)

In VB comparisons operators are also valid between string, int, float...
https://docs.microsoft.com/en-us/dotnet/visual-basic/language-reference/operators/comparison-operators?view=netframework-4.8

https://docs.microsoft.com/en-us/dotnet/visual-basic/language-reference/statements/option-compare-statement#when-an-option-compare-statement-is-not-present

[GrahamTheCoder]

Yep that sounds more like what I'd expect. Shouldn't be too tricky to use that compilation option, it's already used for string equality comparison (the per file overrides of options are currently ignored, there's a to do in the code for that). The class dealing with string equality might be good place to look when fixing this actually, see https://github.com/icsharpcode/CodeConverter/blob/master/ICSharpCode.CodeConverter/CSharp/VisualBasicEqualityComparison.cs

[GrahamTheCoder]

This is a handy place to start for checking the implementation: https://github.com/dotnet/roslyn/blob/master/src/Compilers/VisualBasic/Portable/Binding/Binder_Operators.vb#L130
I'm hoping the symbol/operation API will allow me to detect whether an operator is user defined without doing the heavy lifting myself.

The comment just above the linked line makes a good point about a possible improvement to VisitBinaryExpression to avoid stackoverflows (I've seen at least one before from this effect):

Some tools, such as ASP .NET, generate expressions containing thousands
of string concatenations. For this reason, for string concatenations,
avoid the usual recursion along the left side of the parse. Also, attempt
to flatten whole sequences of string literal concatenations to avoid
allocating space for intermediate results.

[GrahamTheCoder]

The implementation that binds to the VB methods is here:
https://github.com/dotnet/roslyn/blob/master/src/Compilers/VisualBasic/Portable/Lowering/LocalRewriter/LocalRewriter_BinaryOperators.vb#L233-L464

Note that it doesn't rewrite the method if it's within an expression lambda (as we've already got wrong for equality - see #316)