This project is read-only.
1
Vote

Alternative to fading when fix simplifies but does not remove a snippet of code?

description

This might be already fixed with the previous changes to the dimming behavior, which I am unable to verify myself.

Blank console app, add references to PresentationCore and WindowsBase.
using System.Collections.Generic;
using System.Linq;
using System.Windows.Media;
using System.Windows.Media.Imaging;

namespace ConsoleApplication1
{
    class Program
    {
        static IList<int> foo;
        static void Main(string[] args)
        {
            foo.Max();
            WriteableBitmap.Create(1, 1, 96, 96, PixelFormats.Bgr32, null, null, 0);
        }
    }
}
The WriteableBitmap class name is dimmed as unnecessary (seems to be triggered by the LINQ call).

comments

AdamSpeight2008 wrote Apr 30, 2014 at 7:30 PM

Could it be dimmed because you're not assigning the results of them to anything?

JanKucera wrote Apr 30, 2014 at 8:07 PM

No, the method can perform a task I might want to do without caring about the return value.

Moreover, the point of dimming as I was told is the mark unnecessary code. You should be able to delete anything dimmed and still maintain the functionality. Regardless of whether the method does anything or returns anything, you simply can't delete the dimmed code. Not only from the functionality perspective but because it would simply result in invalid syntax.

srivatsn wrote May 5, 2014 at 10:40 PM

Jon, can you please take a look?

jmarolf wrote May 6, 2014 at 10:34 PM

This is an interesting case, thanks for bringing this to our attention!

I think the reason the fade is showing is correct, but we may need to re-think the UI. Currently faded members represent things that can be "simplified" but not necessarily "deleted." However, the most common case by far is just "this text is unnecessary" so I can see where the confusion comes from. In this situation, however, the compiler is actually pointing out something rather interesting.

the Create method on WriteableBitmap.Create is actually defined in its parent class BitmapSource so the fade is to signify a type simplification from WriteableBitmap to BitmapSource. If you were to place your cursor on this line an hit hit Ctrl+. or click your mouse on the light-bulb, you would see an option to change WriteableBitmap to BitmapSource.

I actually like this because if I had the code below:
var bitMap = WriteableBitmap.Create(1, 1, 96, 96, PixelFormats.Bgr32, null, null, 0);
I might expect, at first glance, to get a WriteableBitmap from the create method, but Create actually returns a BitmapSource object.

So this is what's actually happening:
BitmapSource bitMap = WriteableBitmap.Create(1, 1, 96, 96, PixelFormats.Bgr32, null, null, 0);
because we're just calling the static method defined in BitmapSource.

the simplification to the following has the same meaning as before:
BitmapSource.Create(1, 1, 96, 96, PixelFormats.Bgr32, null, null, 0);
but shows explicitly what is being called.

JanKucera wrote May 6, 2014 at 11:10 PM

Interesting, indeed. However, there was a point with the LINQ thing. I admit it is not as easy as in the suggested repro, but I believe I had a case in which the WriteableBitmap was not dimmed which is what confused me in the first place. I will try to come with that more carefully again.

By the way, the light-bulb failed here again for me to explain the issue as I expect the smart tag to be local to the place where it belongs to. Having light-bulb far left does not really suggest any relation to the issue, while the small expansion rectangle directly below the token is definitely something I would check.

jmarolf wrote May 7, 2014 at 2:19 AM

If the code was unable to compile I would expect the text to not dim, but if you can find some steps where adding linq extension methods confuses the simplifier that would be a welcome bug.

regarding the light-bulb: There is still some back and forth regarding the UI here, I'll pass your feedback along to the team, any other thoughts are also welcome.

regarding the bug: If you can get a repo of the linq issue I'll get started on that, otherwise I say we switch this bug to being about changing the UI experience so faded code always means "you can delete this" and we have some other UI mechanism to inform the user that a type can be simplified.

AlexTurnMSFT wrote Jul 2, 2014 at 12:42 AM

As Jon suggested, I've changed this item to be specifically about finding some alternative UI affordance that can indicate when a given span can have a more canonical representation, but it will not be entirely removed.

AlexTurnMSFT wrote Jul 2, 2014 at 12:43 AM

Oops, I'd accidentally resolved it. Fixed now :)

JanKucera wrote Jul 26, 2014 at 10:57 AM

Okay, I would like to withdraw my complain on this one. I have just hit a bug caused by the point rasied by jmarolf: http://referencesource.microsoft.com/#PresentationFramework/src/Framework/System/Windows/Markup/XamlReader.cs#77

Go and warn people. :-)