search

dotnet / roslyn-sdk

Roslyn-SDK templates and Syntax Visualizer
MIT License
498 stars 254 forks source link

Add Roslyn diagnostic comparison code #1019

Open 333fred opened 1 year ago

333fred commented 1 year ago

Roslyn has a set of diagnostic comparison helpers that give a significantly better experience verifying error messages, as it compares a large number of things simultaneously (number of errors, right messages, right arguments, right locations, all for all errors/warnings), whereas the test SDK compares these things one a time, and gives significantly worse diffs when doing so (if the number of errors is different, for example, there is no diff on which errors are new and which were expected). The comparison code from Roslyn should be ported to the testing SDK to enable all users to benefit.

sharwell commented 1 year ago

I agree with providing better messages, but strongly disagree with removing the current comparison code. The comparison code used by roslyn-sdk does a depth-first search to find a least-cost deviation from matching expected and actual diagnostics in situations where the expected diagnostics only partially specify the data contained in the actual diagnostics.

Fortunately #988 should make it easier to alter the presentation of diffs without interfering the matching/alignment code.

333fred commented 1 year ago

The comparison code used by roslyn-sdk does a depth-first search to find a least-cost deviation from matching expected and actual diagnostics in situations where the expected diagnostics only partially specify the data contained in the actual diagnostics.

I'm not sure how this is different from the roslyn verification code, and my experience using both frameworks strongly recommends using the one in the compiler.

sharwell commented 1 year ago

I have some ideas about how to improve the reporting. If you have any examples of specific scenarios where the current behavior was a problem we can discuss whether or not these ideas would have simplified your workflow.

333fred commented 1 year ago

The main thing roslyn gives me is a very good diff of all diagnostics at once. It correctly identifies which diagnostics are currently specified, which ones are missing, and which ones are extra, and does an excellent job showing me this information. The current framework does not do this, and is particularly bad if the current diagnostics are the same length (ie, same number of diagnostics expected and actual), but multiple diagnostics have incorrect data (as verification stops after the first incorrect diagnostic).