Skip to content

Conversation

saul
Copy link
Contributor

@saul saul commented Mar 30, 2017

Project references

image

Assembly references

image

Remaining work

  • Localise text
  • Copy 'metadata' references from other projects in the solution if we can't find any matching projects
  • Fix assertion failure when adding the reference
  • Removing references are not reflected in the Roslyn workspace until solution reload

@saul
Copy link
Contributor Author

saul commented Mar 30, 2017

I've opened this PR to get some help/pick the brains of others. The problem is that ProjectSysten.OAReferences.AddProject is being called with Library1, i.e. it's attempting to add a reference to Library1 from Library1.

We must be reporting our projects to Roslyn incorrectly, as the names are correct on the Roslyn end (as shown in the screenshot). ApplyProjectReferenceAdded is the first in the call-stack before we enter the F# project system code.

I have a feeling that we're calling projectInfoManager.UpdateProjectInfo with the correct project GUID but the incorrect site. My very preliminary debugging indicated that we're passing in the correct data, though.

@vasily-kirichenko
Copy link
Contributor

@saul sorry for offtopic, I cannot find the place where you started the work to enable F# <-> C# Go to Definition. Have you suspended or given up on it? We'd like to join the effort if you are stuck or something.

@saul
Copy link
Contributor Author

saul commented Mar 31, 2017

@vasily-kirichenko it's very WIP state on my desktop (I'm only on my laptop for a few days). I'll push what I've got onto a branch on Sunday night/Monday. It's certainly not suspended/given up on :)

@saul
Copy link
Contributor Author

saul commented Apr 3, 2017

8228545 raised a bit of a doozy in our Roslyn workspace setup code. When we setup the project references, we were passing in the dependent project as the hierarchy for the dependency. How this hasn't caused more issues is beyond me!

@saul
Copy link
Contributor Author

saul commented Apr 3, 2017

Added support for metadata references:

image

@saul saul changed the title [WIP] Added "You must add a reference to '<assembly>'" code fixer Added "You must add a reference to '<assembly>'" code fixer Apr 3, 2017
@saul
Copy link
Contributor Author

saul commented Apr 3, 2017

This is now ready to merge 👍

@saul
Copy link
Contributor Author

saul commented Apr 3, 2017

I've screwed my rebase up again... I really should read up on how to do this properly.

@cloudRoutine
Copy link
Contributor

just checkout a new branch off of master and cherry pick the important commits and you'll be back to status quo in a few mins

@saul
Copy link
Contributor Author

saul commented Apr 15, 2017

@dotnet-bot test this please

@cartermp
Copy link
Contributor

Ugh "The value, namespace, type or module 'CommonRoslynHelpers' is not defined." makes no sense

@dotnet-bot test this please

@saul
Copy link
Contributor Author

saul commented Apr 15, 2017

@cartermp it's because I'm an idiot - fixed

@cartermp
Copy link
Contributor

Looks like there's a merge conflict now. Unrelated to the build error, though.

@saul
Copy link
Contributor Author

saul commented Apr 16, 2017

It's green!!

| Some metadataRef ->
let codefix =
createCodeFix(
sprintf "Add an assembly reference to '%s'" metadataRef.Display, // TODO: localise
Copy link
Contributor

Choose a reason for hiding this comment

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

can you localize this before I pull it.

| Some refProject ->
let codefix =
createCodeFix(
sprintf "Add a project reference to '%s'" refProject.Name, // TODO: localise
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@saul
Copy link
Contributor Author

saul commented Apr 17, 2017

@KevinRansom it is localised - see the latest commits

@saul
Copy link
Contributor Author

saul commented Apr 18, 2017

@dotnet-bot test this please

@saul
Copy link
Contributor Author

saul commented Apr 19, 2017

@KevinRansom you did a dodgy merge - fixed and merged master again. Hopefully this will be the final time -.-

@saul
Copy link
Contributor Author

saul commented Apr 20, 2017

@cartermp @KevinRansom can this be merged?

@KevinRansom
Copy link
Contributor

looking at it right now.

@KevinRansom
Copy link
Contributor

Thank you for this, it looks great.,

@KevinRansom KevinRansom merged commit 4c31706 into dotnet:master Apr 20, 2017
nosami pushed a commit to xamarin/visualfsharp that referenced this pull request Jan 26, 2022
)

* Added "You must add a reference to 'foo'" code fixer

* Preliminary work to enable metadata reference code fixing

* Fix SetupProjectFile passing in the wrong IVsHierarchy to CreateProjectContext

* Localise code fix strings, add support for metadata (assembly) references

* Fix removing references not being reflected in the Roslyn workspace

* Added "You must add a reference to 'foo'" code fixer

* Preliminary work to enable metadata reference code fixing

* Rebased onto master

* Localise code fix strings, add support for metadata (assembly) references

* Fix removing references not being reflected in the Roslyn workspace

* Fix dodgy merge

* Fix reference to CommonRoslynHelpers

* Removed intellisense strings accidentally added in merge

* Ignore case when comparing assembly names

* Fix dodgy merge
@psfinaki
Copy link
Contributor

Hey @saul, I know it's been 6 years, but maybe do you remember how to repro this? I would like to add some tests for this code fix but, whatever I am doing, I am either getting FS0039 or the the assembly is discovered and added automatically. So I wonder if this is still relevant.

@saul
Copy link
Contributor Author

saul commented Sep 25, 2023

From memory, I think this would repro when you had a reference (on .NET Framework, before SDK-style projects judging by the year) onto Library2, which in turn referenced Library3 and exposed it through its public API (e.g. method parameters).

I think nowadays assembly references are rare, and in the age of SDK-style projects, project references are transitive. This may be quite a difficult scenario to stumble upon now.

@psfinaki
Copy link
Contributor

Ohhhhhh this thing. Okay yeah, my brain has already purged the memories of those days and those project files :D
Yeah, I downloaded VS2017 and verified in on that scenario - this still works.

Thanks @saul for such a prompt response - you saved me a lot of time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants