VB quickfix "insert missing cast" improvements

[ljw1004]

I'm putting two related bug reports in this issue.

First: the quickfix "Insert Missing Cast" for BC30512 suggests here to insert CType(expr,String). It should instead be suggesting the nicer CStr(expr).

[edit: retrying this with VS2015 RTM, I can still repro the first issue, but can't repro the second...]

Second: if we switch to Option Strict Custom, the compiler is correct to give warning BC42016 instead of error BC30512. Unfortunately the only quickfix offered for BC42016 is #Disable Warning. But it should first offer the same "Insert Missing Cast" quickfix as it does for BC30512.

[Pilchie]

@ljw1004: Can you be more specific about what compilation settings you had for the second case?

[ljw1004]

Now in VS2015 RTM I can no longer repro the second case.

[jrmoreno1]

Just came looking for this, it's really annoying as it is.

[CyrusNajmabadi]

@jrmoreno1 This is a 'help wanted' issue. We would happily take a PR here if you're interested :)

[jrmoreno1]

@CyrusNajmabadi : I was thinking about it...

[paul1956]

@jrmoreno1 If you are not going to get to it I will try it. Do you know where the code is for this fix?

[jrmoreno1]

@paul1956 : I was planning on looking into it either this weekend or next. I believe it is happening in :
\repos\roslyn\src\Features\VisualBasic\Portable\CodeFixes\AddExplicitCast\VisualBasicAddExplicitCastCodeFixProvider.vb, but that is a quick look, I wouldn't bet on it being the right spot.

[paul1956]

@jrmoreno1 I wanted to look into #44516 and though it might to related code or at least a related test. I can't know which test covers this Visual Basic Casts.

[CyrusNajmabadi]

The code is in AbstractAddExplicitCastCodeFixProvider and subclasses: https://github.com/dotnet/roslyn/blob/master/src/Features/Core/Portable/CodeFixes/AddExplicitCast/AbstractAddExplicitCastCodeFixProvider.cs

[CyrusNajmabadi]

I can't know which test covers this

Tests for this are in: AddExplicitCastTests.vb and AddExplicitCastTests.cs

[paul1956]

@jrmoreno1 I will leave this one to you and look at "CInt Redundant Cast incorrect for Math functions and constants" #44516

[jrmoreno1]

@paul1956 : the problem is simple, the extension method GetPredefinedCastKeyword in ITypeSymbolExtensions doesn't include a check for SpecialType.System_String or return SyntaxKind.CStrKeyword. I'll have to see what if anything breaks when I change it, which is hard because the test take so long on my machine.

[jrmoreno1]

@ljw1004, @CyrusNajmabadi : I don't understand what should be different about Option Strict Custom. I don't know where BC42016 is being generated in that case or where I would find a test to see if my pull request #45139 does fix it.

[CyrusNajmabadi]

Sorry, I don't know what this if in reference to.

[jrmoreno1]

@CyrusNajmabadi: the second point for this issue was:

Second: if we switch to Option Strict Custom, the compiler is correct to give warning BC42016 instead of error BC30512. Unfortunately the only quickfix offered for BC42016 is #Disable Warning. But it should first offer the same "Insert Missing Cast" quickfix as it does for BC30512.

There’s an update saying it couldn’t be reproduced in 2015RTM, I’m trying to figure out if it is still an issue and how to test for it if it is.

[CyrusNajmabadi]

You don't need to worry about 'option strict custom'. It's not really relevant for this scenario. It's more about if the individual warnings are elevated to errors or not.

[CyrusNajmabadi]

Closing out as this works now: