-
Notifications
You must be signed in to change notification settings - Fork 419
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/script supersede lower versions #1103
Feature/script supersede lower versions #1103
Conversation
src/OmniSharp.Script/ScriptHelper.cs
Outdated
var referencesSupersedeLowerVersionsProperty = typeof(CompilationOptions).GetProperty(ReferencesSupersedeLowerVersionsProperty, BindingFlags.Instance | BindingFlags.NonPublic); | ||
if (referencesSupersedeLowerVersionsProperty != null) | ||
{ | ||
referencesSupersedeLowerVersionsProperty?.SetValue(compilationOptions, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the ?.
needed after checking for null above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically the if could be removed couldn't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are both right 😀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Conflicting assembly versions in scripting are typically resolved via identity comparer, which we already use.
However, scripting scenarios in Roslyn additionally set a special flag in the ReferenceManager to enable additional tie-breaking behavior in case of conflicting references. This flag is propagated from the compilation options, and is always implicitly injected for script compilations.
To mirror 1:1 the runtime behavior of scripts, we should ensure the same flag is set at tooling level (at the moment we have it set to to☹️ ).
false
which can lead to edge case false-positive intellisense errors). Unfortunately the flag is internal so it's set with reflection (along with a few other things we do with reflection in script tooling