Review: correctness of VS visualizers

[RussKie]

As it was discovered in https://github.com/dotnet/corefx/issues/38130 a number of type visualizers are defined in VS repo instead of near types.

We need to review correctness of definitions for WinForms types:

// System.Windows.Forms
[assembly: DebuggerDisplay(@"\{ExecutablePath = {executablePath}}", Target = typeof(WinForms::Application))]
[assembly: DebuggerDisplay(@"\{Text = {Text}}", Target = typeof(WinForms::Button))]
[assembly: DebuggerDisplay(@"\{Text = {Text} CheckState = {checkState}}", Target = typeof(WinForms::CheckBox))]
[assembly: DebuggerDisplay(@"\{SelectedItem = {Text}}", Target = typeof(WinForms::CheckedListBox))]
[assembly: DebuggerDisplay(@"\{Text = {Text}}", Target = typeof(WinForms::DataGrid))]
[assembly: DebuggerDisplay(@"\{Type = {type} Column = {col} Row = {row}}", Target = typeof(WinForms::DataGrid.HitTestInfo))]
[assembly: DebuggerDisplay(@"\{HeaderText = {headerName}}", Target = typeof(WinForms::DataGridColumnStyle))]
[assembly: DebuggerDisplay(@"\{Text = {Text}}", Target = typeof(WinForms::DataGridTextBox))]
[assembly: DebuggerDisplay(@"\{HeaderText = {headerName}}", Target = typeof(WinForms::DataGridTextBoxColumn))]
[assembly: DebuggerDisplay(@"\{Font = {font} Color = {color}}", Target = typeof(WinForms::FontDialog))]
[assembly: DebuggerDisplay(@"\{Value = {value} Min = {minimum} Max = {maximum}}", Target = typeof(WinForms::HScrollBar))]
[assembly: DebuggerDisplay(@"\{InvalidRect = {invalidRect}}", Target = typeof(WinForms::InvalidateEventArgs))]
[assembly: DebuggerDisplay(@"\{Index = {index}}", Target = typeof(WinForms::ItemChangedEventArgs))]
[assembly: DebuggerDisplay(@"\{Index = {index} NewValue = {newValue} CurrentValue = {currentValue}}", Target = typeof(WinForms::ItemCheckEventArgs))]
[assembly: DebuggerDisplay(@"\{KeyData = {keyData}}", Target = typeof(WinForms::KeyEventArgs))]
[assembly: DebuggerDisplay(@"\{KeyChar = {keyChar}}", Target = typeof(WinForms::KeyPressEventArgs))]
[assembly: DebuggerDisplay(@"\{LinkText = {linkText}}", Target = typeof(WinForms::LinkClickedEventArgs))]
[assembly: DebuggerDisplay(@"\{SelectedItem = {Text}}", Target = typeof(WinForms::ListBox))]
[assembly: DebuggerDisplay(@"\{Text = {Text}}", Target= typeof(WinForms::ListViewItem))]
[assembly: DebuggerDisplay(@"\{X = {x} Y = {y} Button = {button}}", Target = typeof(WinForms::MouseEventArgs))]
[assembly: DebuggerDisplay(@"\{Value = {currentValue} Min = {minimum} Max = {maximum}}", Target = typeof(WinForms::NumericUpDown))]
[assembly: DebuggerDisplay(@"\{ClipRectangle = {clipRect}}", Target = typeof(WinForms::PaintEventArgs))]
[assembly: DebuggerDisplay(@"\{Value = {value} Min = {minimum} Max = {maximum}}", Target = typeof(WinForms::ProgressBar))]
[assembly: DebuggerDisplay(@"\{Text = {Text} Checked = {isChecked}}", Target = typeof(WinForms::RadioButton))]
[assembly: DebuggerDisplay(@"\{Text = {Text}}", Target = typeof(WinForms::RichTextBox))]
[assembly: DebuggerDisplay(@"\{Bounds = {bounds} WorkingArea = {workingArea} Primary = {primary} DeviceName = {deviceName}}", Target = typeof(WinForms::Screen))]
[assembly: DebuggerDisplay(@"\{Start = {start} End = {end}}", Target = typeof(WinForms::SelectionRange))]
[assembly: DebuggerDisplay(@"\{SplitPosition = {splitSize} MinExtra = {minExtra} MinSize = {minSize}}", Target = typeof(WinForms::Splitter))]
[assembly: DebuggerDisplay(@"\{SplitX = {splitX} SplitY = {splitY}}", Target = typeof(WinForms::SplitterEventArgs))]
[assembly: DebuggerDisplay(@"\{Text = {Text}}", Target = typeof(WinForms::TextBox))]
[assembly: DebuggerDisplay(@"\{Interval = {interval}}", Target = typeof(WinForms::Timer))]
[assembly: DebuggerDisplay(@"\{Value = {Value} Min = {minimum} Max = {maximum}}", Target = typeof(WinForms::TrackBar))]
[assembly: DebuggerDisplay(@"\{Text = {text}}", Target = typeof(WinForms::TreeNode))]
[assembly: DebuggerDisplay(@"\{Value = {value} Min = {minimum} Max = {maximum}}", Target = typeof(WinForms::VScrollBar))]

https://devdiv.visualstudio.com/DevDiv/_git/VS?path=%2Fsrc%2Fcsharp%2Fautoexp%2Fautoexp.cs&version=GBmaster&line=58&lineStyle=plain&lineEnd=91&lineStartColumn=1&lineEndColumn=122

For any changes required liaise with @r-ramesh.

[weltkante]

Any reason why .NET Core doesn't bring their own visualizers in the assembly the type is defined in? Would help with future refactorings running out of sync again.

I wonder what takes priority if visualizers are defined both in .NET Core assemblies and in the VS extension. If the defining assembly takes precedence then you could just add the attributes in .NET Core and leave the VS extension in place for Desktop Framework.

[edit] apparently the VS extension has priority over diagnostic attributes in the assembly, see discussion in dotnet/corefx#38130

[RussKie]

It certainly would help and solve the issue.
I believe it is under investigation by the relevant team to determine whether this would in fact work.

[RussKie]

FWIW, the VS priority isn't changing anytime soon.

[RussKie]

@r-ramesh / @ericstj I've gone through our definitions and it looks like a few got outdated and need changing as follows:

// System.Windows.Forms
[assembly: DebuggerDisplay(@"\{ExecutablePath = {ExecutablePath}}", Target = typeof(WinForms::Application))]
[assembly: DebuggerDisplay(@"\{Text = {Text}}", Target = typeof(WinForms::Button))]
[assembly: DebuggerDisplay(@"\{Text = {Text} CheckState = {CheckState}}", Target = typeof(WinForms::CheckBox))]
[assembly: DebuggerDisplay(@"\{SelectedItem = {Text}}", Target = typeof(WinForms::CheckedListBox))]
[assembly: DebuggerDisplay(@"\{Text = {Text}}", Target = typeof(WinForms::DataGrid))]
[assembly: DebuggerDisplay(@"\{Type = {Type} Column = {Column} Row = {Row}}", Target = typeof(WinForms::DataGrid.HitTestInfo))]
[assembly: DebuggerDisplay(@"\{HeaderText = {HeaderText}}", Target = typeof(WinForms::DataGridColumnStyle))]
[assembly: DebuggerDisplay(@"\{Text = {Text}}", Target = typeof(WinForms::DataGridTextBox))]
[assembly: DebuggerDisplay(@"\{HeaderText = {HeaderText}}", Target = typeof(WinForms::DataGridTextBoxColumn))]
[assembly: DebuggerDisplay(@"\{Font = {Font} Color = {Color}}", Target = typeof(WinForms::FontDialog))]
[assembly: DebuggerDisplay(@"\{Value = {Value} Min = {Minimum} Max = {Maximum}}", Target = typeof(WinForms::HScrollBar))]
[assembly: DebuggerDisplay(@"\{InvalidRect = {InvalidRect}}", Target = typeof(WinForms::InvalidateEventArgs))]
[assembly: DebuggerDisplay(@"\{Index = {Index}}", Target = typeof(WinForms::ItemChangedEventArgs))]
[assembly: DebuggerDisplay(@"\{Index = {Index} NewValue = {NewValue} CurrentValue = {CurrentValue}}", Target = typeof(WinForms::ItemCheckEventArgs))]
[assembly: DebuggerDisplay(@"\{KeyData = {KeyData}}", Target = typeof(WinForms::KeyEventArgs))]
[assembly: DebuggerDisplay(@"\{KeyChar = {KeyChar}}", Target = typeof(WinForms::KeyPressEventArgs))]
[assembly: DebuggerDisplay(@"\{LinkText = {LinkText}}", Target = typeof(WinForms::LinkClickedEventArgs))]
[assembly: DebuggerDisplay(@"\{SelectedItem = {Text}}", Target = typeof(WinForms::ListBox))]
[assembly: DebuggerDisplay(@"\{Text = {Text}}", Target= typeof(WinForms::ListViewItem))]
[assembly: DebuggerDisplay(@"\{X = {X} Y = {Y} Button = {Button}}", Target = typeof(WinForms::MouseEventArgs))]
[assembly: DebuggerDisplay(@"\{Value = {Value} Min = {Minimum} Max = {Maximum}}", Target = typeof(WinForms::NumericUpDown))]
[assembly: DebuggerDisplay(@"\{ClipRectangle = {ClipRectangle}}", Target = typeof(WinForms::PaintEventArgs))]
[assembly: DebuggerDisplay(@"\{Value = {Value} Min = {Minimum} Max = {Maximum}}", Target = typeof(WinForms::ProgressBar))]
[assembly: DebuggerDisplay(@"\{Text = {Text} Checked = {Checked}}", Target = typeof(WinForms::RadioButton))]
[assembly: DebuggerDisplay(@"\{Text = {Text}}", Target = typeof(WinForms::RichTextBox))]
[assembly: DebuggerDisplay(@"\{Bounds = {Bounds} WorkingArea = {WorkingArea} Primary = {Primary} DeviceName = {DeviceName}}", Target = typeof(WinForms::Screen))]
[assembly: DebuggerDisplay(@"\{Start = {Start} End = {End}}", Target = typeof(WinForms::SelectionRange))]
[assembly: DebuggerDisplay(@"\{SplitPosition = {SplitPosition} MinExtra = {MinExtra} MinSize = {MinSize}}", Target = typeof(WinForms::Splitter))]
[assembly: DebuggerDisplay(@"\{SplitX = {SplitX} SplitY = {SplitY}}", Target = typeof(WinForms::SplitterEventArgs))]
[assembly: DebuggerDisplay(@"\{Text = {Text}}", Target = typeof(WinForms::TextBox))]
[assembly: DebuggerDisplay(@"\{Interval = {Interval}}", Target = typeof(WinForms::Timer))]
[assembly: DebuggerDisplay(@"\{Value = {Value} Min = {Minimum} Max = {Maximum}}", Target = typeof(WinForms::TrackBar))]
[assembly: DebuggerDisplay(@"\{Text = {Text}}", Target = typeof(WinForms::TreeNode))]
[assembly: DebuggerDisplay(@"\{Value = {Value} Min = {Minimum} Max = {Maximum}}", Target = typeof(WinForms::VScrollBar))]

What's the best way to proceed with this?

[ericstj]

[assembly: DebuggerDisplay(@"{Type = {type} Column = {Column} Row = {Row}}", Target = typeof(WinForms::DataGrid.HitTestInfo))]

Looks like this one is still using private type, looks like that could be Type.

What's the best way to proceed with this?

Open a PR in VS. Here's @danmosemsft's PR as a template to follow: https://devdiv.visualstudio.com/DevDiv/_git/VS/pullrequest/185456?_a=overview

[ericstj]

/cc @r-ramesh

[RussKie]

PR raised: https://devdiv.visualstudio.com/DevDiv/_git/VS/pullrequest/186044?_a=overview

[merriemcgaw]

@Olina-Zhang could you verify that this doesn't change behavior in the Framework scenarios. @RussKie will provide you the PR build and scenario

[ericstj]

You can actually test by copy pasting that code into a an app, then making that app initialize locals for every type in the list. The app is preferred over autoexp. You can make your app cross-target for net472 and netcoreapp3.0 and examine local window in debugger.

This is how I tested @danmosemsft’s fix.

[RussKie]

You can actually test by copy pasting that code into a an app

Thanks for the suggestions.

I've created a demo project (https://github.com/RussKie/Test-VS-Visualizers), however it doesn't seem to be picking the changes if I uncomment the attributes:
https://github.com/RussKie/Test-VS-Visualizers/blob/master/Form1.cs#L16-L23

Do I need to subclass types?

[ericstj]

Hmm, wonder why I didn't see that, but I confirm the same as you. You can manually update autoexp per instructions here: https://docs.microsoft.com/en-us/visualstudio/debugger/using-the-debuggerdisplay-attribute?view=vs-2019

For me this was:

1. open elevated command prompt.
2. cd C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\Packages\Debugger\Visualizers\Original
3. notepad autoexp.cs
4. csc /t:library autoexp.cs

Alternatively you can just move the existing autoexp.dll and use your current app and it has a similar effect.

[RussKie]

Awesome, thanks. Renaming autoexp.dll worked!

@Olina-Zhang please use my demo project https://github.com/RussKie/Test-VS-Visualizers to test

[Olina-Zhang]

@RussKie, tested your VS PR build, Winforms framework scenarios look good, and no issue was found. And tested your provided demo project as ReadMe.md file.
Before renaming autoexp.dll on test machine, run app directly, just fontFamily and fontSize in fontDialog has visualizer issue in watch window:

After renaming autoexp.dll and uncomment DebuggerDisplayAttribute declarations to run app, no issue in watch window:

Is above expected result?

[RussKie]

Using the default VS visualisers you should see more errors:

Were you running it against the latest WinFroms version built locally?

[Olina-Zhang]

@RussKie, I used VS is from your fix PR VS, it should without any visualizers issue in test app. Using fixed PR build to test directly, the result should be same as renaming autoexp.dll + uncomment DebuggerDisplayAttribute declarations in code. Is it right? In addition, we tried to use non-fixed PR VS for testing directly, more errors found in watch window, then rename autoexp.dll + uncomment DebuggerDisplayAttribute declarations in code, errors disappear.

[RussKie]

Without my fix viewing WF types, you should see a number of errors (as per the screenshot above).
Rename autoexp.dll + uncomment DebuggerDisplayAttribute declarations in code - you should see all errors gone.

[Olina-Zhang]

Thanks for your confirmation, your fix is good.

[RussKie]

The VS PR merged.
https://devdiv.visualstudio.com/DevDiv/_git/VS/pullrequest/186044