Skip to content
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

Razor "Go to definition" breaks when target is defined an SDK-style project #3010

Closed
onyxmaster opened this issue Nov 30, 2017 · 31 comments
Closed
Assignees
Labels
Area-External Owned by another feature area and not this repo. Likely be closed in lieu of issue against it. Feature-Language-Service Populating the Roslyn workspace with references, source files, analyzers, etc Parity-Legacy-Feature Missing features from the legacy project system.

Comments

@onyxmaster
Copy link

Consider the source at https://github.com/onyxmaster/RazorDefs. Open the solution in Visual Studio 2017 (tested with 15.5 Preview 5), then build the solution and open (with the same instance of VS) the index.cshtml in RazorDefs project.
When opened, try navigating to all three types definitions via F12/"Go To Definition". The first two types are handled properly. Unfortunately, the last one would use the fallback "metadata" decompilation mode.
The apparent reason is that the code is defined in the SDK-style project. The target framework of this project is unimportant (netstandard2.0 or net461 both lead to such problems), the debugging format also appears not to matter (at first I blamed portable PDBs).
This is mildly disappointing when one is gradually converting a pre-VS2017 solution projects to new format.

P.S. I understand that this at least partly a Visual Studio + ASP.NET issue, but I'm not sure where this should be sent, so I place it here. Please excuse me for failing to find a better place to report this.

@davkean
Copy link
Member

davkean commented Nov 30, 2017

@jasonmalinowski @BillHiebert @dpoeschl @Pilchie Where do we start with this, is this likely a Razor, Roslyn or project system issue?

@Pilchie
Copy link
Member

Pilchie commented Nov 30, 2017

My initial guess would be that the reference to the sdk project is not being added properly as a project to project reference from the .cshtml project.

This means that either the P2P reference api isn't being called in Roslyn, or the output path that is passed for netstandard reference that is passed doesn't match what Roslyn has internally.

I suspect this should start with @BillHiebert.

@jasonmalinowski
Copy link
Member

Did somebody grab a screenshot by chance to see what the metadata as source code is? Looking at the "assembly" header up top would give a good way to diagnose.

@onyxmaster
Copy link
Author

image

@Pilchie
Copy link
Member

Pilchie commented Dec 4, 2017

@BillHiebert - this looks like a razor tooling issue to me. Should this issue be moved somewhere?

@Pilchie Pilchie added the Area-External Owned by another feature area and not this repo. Likely be closed in lieu of issue against it. label Dec 4, 2017
@Pilchie
Copy link
Member

Pilchie commented Dec 5, 2017

Ping @BillHiebert?

@BillHiebert
Copy link
Contributor

BillHiebert commented Dec 5, 2017

Razor (and other web pages like aspx, master, etc) intellisense requires the project being referenced to implement the IVsHierarchy property VSHPROPID_IntellisenseUnknown. I don't think the managed project system implements it.

Note that I am referring to Razor intellisense in non-core projects which RazorDefs is.

@jasonmalinowski
Copy link
Member

@BillHiebert Why does it use that...?

@davkean
Copy link
Member

davkean commented Dec 6, 2017

Tag @rynowak. Can someone point me to the usage of VSHPROPID_IntellisenseUnknown so that I can understand how Razar uses it?

@BillHiebert
Copy link
Contributor

@davkean - it s not razor per se, but the intellisense infrastructure in the older project systems (websites and WAPs). Sync up with me and I can point you to code which uses it.

@Pilchie
Copy link
Member

Pilchie commented Dec 13, 2017

Is there an internal thread on this that I'm not aware of?

@Pilchie
Copy link
Member

Pilchie commented Jan 3, 2018

Ping @davkean @rynowak @BillHiebert ?

@rynowak
Copy link
Member

rynowak commented Jan 3, 2018

I don't have any context on this, @BillHiebert is probably the only one who can help.

@Pilchie Pilchie added this to the 15.7 milestone Jan 8, 2018
@Pilchie Pilchie added Feature-Language-Service Populating the Roslyn workspace with references, source files, analyzers, etc Area-New-Project-System labels Jan 8, 2018
@Pilchie Pilchie modified the milestones: 15.7, 15.8 Apr 4, 2018
@lt-gerjan
Copy link

I have filed this under #3558 but it seems the same issue.
This is targetted 15.8, do you guys have confidence that this will/can be solved by then? Whats the rough estimate for 15.8?

Sorry for pushing but this one is kinda dealbreaker for me to move to .NET Standard libs :(

@davkean
Copy link
Member

davkean commented May 22, 2018

@rynowak/@BillHiebert Would this part of the rewrite of the Razor <-> Roslyn layer? Or would it be separate?

@Pilchie
Copy link
Member

Pilchie commented May 22, 2018

I assume that would only fix the case where it's ASP.NET Core and razor pages on the source side, not when it's a traditional webforms or MVC app referencing a .NET Standard Library.

@lt-gerjan
Copy link

lt-gerjan commented May 22, 2018

@davkean @Pilchie Yes, in my case it is a WebForms website. So that's slightly different than the OP/TS

@rynowak
Copy link
Member

rynowak commented May 22, 2018

Can someone point me to the usage of VSHPROPID_IntellisenseUnknown so that I can understand how Razar uses it?

Sorry, I must have missed this ping. I don't have any familiarity with this. @BillHiebert is your guy.

@davkean davkean modified the milestones: 15.8, 16.0 May 22, 2018
@davkean davkean added the Parity-Legacy-Feature Missing features from the legacy project system. label May 22, 2018
@davkean
Copy link
Member

davkean commented May 22, 2018

At this point, we don't have an intention of returning anything for VSHPROPID_IntellisenseUnknown - language service hookup is much different in new project system versus legacy. I suspect we'll run into the same issue when we move over to IWorkspaceProjectContext for legacy as well.

We should figure out why the automatic conversion from binary reference to P2P isn't kicking in here, and change either side to make this work. This is in 15.8 but cannot see how we make this fit.

@wanton7
Copy link

wanton7 commented Nov 18, 2018

Any chance instead putting man hours to fixing this, just add new csproj format support for ASP.NET MVC 5 projects instead?

@NickCraver
Copy link
Member

As mentioned in #2670 (comment), this is a huge pain point for migrations. I'm suffering this in Opserver because it's just me and I can pay the price to help advance this whole thing. Or I thought, if in retrospect I knew this kind of stuff would be punted for so long I never would have touched the new project system for anything referenced by ASP.NET MVC 5. It's not worth it. And I won't put others through it.

When it comes to a whole team of developers migrating Stack Overflow (and anyone working on it, not just those actively migrating), I can't ask developers to suffer this. It's a huge productivity loss and frustration. And it's a regression. When you want to go see source, you can't. You're back to find all references or find in files. Code Lens is useless (I don't believe this is hyperbole, if it doesn't find things, you can't trust it at all): you can't tell what's referenced, what's not, what's safe to remove, etc. It makes porting a nightmare and incredibly more risky.

When we need to port an ASP.NET application to ASP.NET Core, we first extract all code possible into another project to minimize the duration, impact, and risk of the ASP.NET move itself. This way we can be building as much code as possible not only on net4* but also netstandard or netcoreapp - whatever the final target is. But we can't do that. We have to choose between doing that and a working IDE. This means one of the two reasons for moving all that code and ensuring it's good to go on the net target frameworks is moot. We have to big bang it at the end anyway.

This is one of the biggest pain points I've hit as part of #2670 (meant to be an issue covering all the pain points). Please consider fixing this portion of it first. It prevents us even using the new project system during a migration - we basically have to pretend it doesn't exist if referenced anywhere in the chain.

@lt-gerjan
Copy link

lt-gerjan commented Dec 6, 2018

To support @NickCraver 's statement: I have a large software project (180+ projects, mixed .NET and .NET Core) and I really want to move to the new MsBuild SDK/Project System, but this single issue is stopping me from doing it.
It's just unacceptable for me that our developers lose F12, Code lens etc.

Porting everything to .NET core would be a 1-year+ task.

PS1: Yes I know I can only +1 Nicks reaction, but I think a comment would give more "weight"
PS2: Please do not forget WebForms, see #3558

@onyxmaster
Copy link
Author

We're in situation not unlike Nick's one. We've got a 200+-project solution that is partially new-style netstandard2.0, partially new-style net472 and has two old-style v4.7.2 web application projects. I guess our developer team is much smaller, but still we have the same problems. In fact for me, this is second-worst "feature" I encounter when working on this project (the first one is VS being "slow", but that's another story).

Can we live with it? Sure, in most cases F12+Ctrl-T or just Ctrl-T work fine, since we mostly use this feature for exploring existing service APIs (and my team is good at naming things). But it feels like I'm fighting against the tools I have to use.

@lt-gerjan
Copy link

@davkean @Pilchie Is it possible for you to say when this will be fixed? Or is it hard to say? (I know its saying milestone 16 but that one has no date)
I am at a point where I want to start converting but I'm still hesitating

@Pilchie
Copy link
Member

Pilchie commented Jan 7, 2019

@BillHiebert and @rynowak - is this something that we could take another look at given that other features like CodeLens and FAR have been updated to work here?

cc @jjmew

@NickCraver
Copy link
Member

Is there any update on this? As stated above it's a major issue for migrations and we're about to eat a lot of pain experiencing it.

@jjmew
Copy link
Contributor

jjmew commented Feb 8, 2019

The fix will be available in VS 2019 Preview 4.

@jjmew jjmew closed this as completed Feb 8, 2019
@davkean davkean modified the milestones: 16.0, 16.0 Preview 4 Feb 8, 2019
@onyxmaster
Copy link
Author

Installed the RC, it works now, I can't thank you enough.

@davkean
Copy link
Member

davkean commented Feb 28, 2019

@onyxmaster Great to hear! //cc @BillHiebert

@NickCraver
Copy link
Member

@davkean It's also working well for us (and a big help at this critical part of our migration). Thanks for getting this in!

@davkean
Copy link
Member

davkean commented Mar 4, 2019

@NickCraver Thanks for following up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-External Owned by another feature area and not this repo. Likely be closed in lieu of issue against it. Feature-Language-Service Populating the Roslyn workspace with references, source files, analyzers, etc Parity-Legacy-Feature Missing features from the legacy project system.
Projects
None yet
Development

No branches or pull requests

10 participants