Use Assumes.Present instead of Debug.Assert

[sharwell]

Assumes.Present produces a more informative exception and message on failure.

Closes #34329

Ask Mode template not completed

Customer scenario

What does the customer do to get into this situation, and why do we think this
is common enough to address for this release. (Granted, sometimes this will be
obvious "Open project, VS crashes" but in general, I need to understand how
common a scenario is)

Bugs this fixes

(either VSO or GitHub links)

Workarounds, if any

Also, why we think they are insufficient for RC vs. RC2, RC3, or RTW

Risk

This is generally a measure our how central the affected code is to adjacent
scenarios and thus how likely your fix is to destabilize a broader area of code

Performance impact

(with a brief justification for that assessment (e.g. "Low perf impact because no extra allocations/no complexity changes" vs. "Low")

Is this a regression from a previous update?

Root cause analysis

How did we miss it? What tests are we adding to guard against it in the future?

How was the bug found?

(E.g. customer reported it vs. ad hoc testing)

Test documentation updated?

If this is a new non-compiler feature or a significant improvement to an existing feature, update https://github.com/dotnet/roslyn/wiki/Manual-Testing once you know which release it is targeting.

[jasonmalinowski]

@sharwell How do we want to reconcile this with Contract.ThrowIfNull() that we probably also use in a few places?

[sharwell]

@jasonmalinowski Assumes.Present seems to be the preferred call for services in VS. It's part of vs-validation which means we could take a dependency on it in Workspaces and update Contract.ThrowIfNull to call it, but I'm not sure what that would buy us.

I was repeatedly hitting these assertions under an unknown deployment configuration and the resulting modal dialogs were causing problems for some tests.

[jasonmalinowski]

@sharwell: so guidance going forward would be Assumes.Present for services in VS layer, and for everything else do as you have previously done? Want to do a pass to see if there is a bunch of other ones we can clean up too?

[sharwell]

We currently reference Assumes in the Editor Features layer. Should it be used there and up? It's very similar to Contract.ThrowIfNull, but is intended to provide a more relevant message in the case of singletons of a specific type (aka "services").

[sharwell]

The assertion test issue was resolved by #33600. @jasonmalinowski do you have any interest in us pursuing unification of these checks?

[jasonmalinowski]

@sharwell If you want to merge this PR I'm fine with it...I'm not sure I'd send time fixing up other places though. I'd also expect that with C# nullable coming we'd have some new GetService() method anyways that returns explicitly non-null which would moot this pattern.