Skip to content

Add GetLanguageService overload for binary compatibility#27557

Merged
sharwell merged 1 commit intodotnet:dev15.8-preview3from
sharwell:add-overload
Jun 7, 2018
Merged

Add GetLanguageService overload for binary compatibility#27557
sharwell merged 1 commit intodotnet:dev15.8-preview3from
sharwell:add-overload

Conversation

@sharwell
Copy link
Copy Markdown
Contributor

@sharwell sharwell commented Jun 7, 2018

Failed TypeScript build:

https://devdiv.visualstudio.com/DevDiv/_git/VS/pullrequest/126057?_a=overview

Customer scenario

Internal product build failed.

Bugs this fixes

Here is the internal insertion PR that failed:
https://devdiv.visualstudio.com/DevDiv/_git/VS/pullrequest/126057?_a=overview

Workarounds, if any

N/A

Risk

Low. Document derives from TextDocument, so the pair of overloads will not result in ambiguities at the call site.

Performance impact

No observable impact.

Is this a regression from a previous update?

Yes.

Root cause analysis

We have a large number of InternalsVisibleTo (IVT) attributes applied with no validation running during the pull request builds. There is no reasonable expectation that this type of failure will be avoided, so we get to fix it when things fail during insertions.

How was the bug found?

Insertion build.

Test documentation updated?

N/A

@sharwell sharwell added this to the 15.8 milestone Jun 7, 2018
@sharwell sharwell requested a review from a team as a code owner June 7, 2018 04:02
@jinujoseph
Copy link
Copy Markdown
Contributor

Approved to merge for 15.8.Preview3 (To unblock insertion)

@sharwell sharwell merged commit a679e3d into dotnet:dev15.8-preview3 Jun 7, 2018
@sharwell sharwell deleted the add-overload branch June 7, 2018 11:55
public static TLanguageService GetLanguageService<TLanguageService>(this TextDocument document) where TLanguageService : class, ILanguageService
=> document?.Project?.LanguageServices?.GetService<TLanguageService>();

// ⚠ Verify IVTs do not use this method before removing it.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I both love the emoji and fear for what it will break.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've used it in a few other cases and it seems to work great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants