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

Compiler version VS Code IDE warning is challenging to take action on #657

Closed
annelo-msft opened this issue Jun 23, 2022 · 11 comments
Closed
Labels
bug Something isn't working
Milestone

Comments

@annelo-msft
Copy link
Member

I am getting this error in the VS Code IDE and unable to parse the text to figure out what action to take.

image

Per @xirzec, if the language service defaulted to using the workspace instead of the global cadl, the error would be unneeded.

@ghost ghost added the Needs Triage label Jun 23, 2022
@bterlson
Copy link
Member

At the very least the error should probably tell you what to configure (i.e. mention workspace settings) as it could be referring to cadl-project.yml, and it should also probably not suggest using an absolute path for that configuration key but something rooted at $WorkspaceRoot, so you can just copy/paste the suggestion in everything but weird cases.

@allenjzhang allenjzhang added bug Something isn't working and removed Needs Triage labels Jun 23, 2022
@allenjzhang allenjzhang added this to the [2022] August milestone Jun 23, 2022
@nguerrera
Copy link
Contributor

nguerrera commented Jun 23, 2022

something rooted at $WorkspaceRoot

It's a little challenging to give a $WorkspaceRoot path since at present, we have no concept of workspace at the point in the compiler where we detect this, but we could plumb something through the compiler host.

Per @xirzec Jeff Fisher FTE, if the language service defaulted to using the workspace instead of the global cadl, the error would be unneeded.

If you open a folder that is the root of your cadl package/project in VS Code, it will prefer the local cadl over the global one without configuration. However, if @cadl-lang/compiler can't be resolved from the root workspace folder using node module resolution, say because you have a monorepo open, then we don't currently have any other smarts to try to find a local one. I think it would be nice if we could infer the location in more scenarios, but we would have to design the inference. Currently we have one cadl-server running for the whole workspace and if we can't find it off the root of the workspace, it's dicey to go hunting further down. I don't know if we get told which file caused the cadl-server to load first. If so, we could maybe use that .cadl file path, but it could result in weird hard-to-diagnose issues where behaviour depends on which file you load first.

Another angle: it is possible to have different files served by different, concurrent instances of the language server. There's a vscode-extensions sample that shows this.

@nguerrera
Copy link
Contributor

#594 added better error messages with pointer to documentation when VS or VS code can't find cadl-server at all. Perhaps we should do likewise here when it finds one but it doesn't match a local version.

@nguerrera
Copy link
Contributor

Another angle: it is possible to have different files served by different, concurrent instances of the language server. There's a vscode-extensions sample that shows this.

Just brainstorming, but maybe we can detect the mismatch diagnostic and automatically start a new server and retry. Could get compilcated.

@nguerrera
Copy link
Contributor

More brainstorming: Maybe if we can get the error to include a reasonable path relative to $WorkspaceRoot, then we could have a fixer for this diagnostic that goes and slaps it in to settings.json for you.

@annelo-msft
Copy link
Member Author

annelo-msft commented Jun 24, 2022

It would be helpful (at least to me in my current newbieness) to have it documented somewhere about what the workspace setup is expected to look like, in order for the tooling to work (if this is already written down, happy to accept a pointer). I'm just a bit confused about what's expected, and how to keep it all up to date.

The instructions I've been using are these:

It'd be great to understand the expectations around what's laid out where so I can stay in step with tooling, and in general the expectations of the Cadl team to aid communication. (And apologies, I'm coming to this without npm background, which probably makes it harder.)

@nguerrera
Copy link
Contributor

To recap, the intent was:

  • If you open the same folder that has your package.json file next to you cadl source, where you run npm install, then this error message should not appear. Things should just work. Generally, all of the tutorial/getting started content guides you to this simple case, which is why they don't cover the configuration of server path. We should treat cases where you have done this and got this error message as a bug. (*)

  • If you open a folder deeper than that, you will need to configure the compiler path in .vscode/settings.json.

    • For example, if the folder you open is /mycode/bigproject and the cadl project is /mycode/bigproject/somecadl. Then you should have the following:
{
  "cadl.cadl-server.path": "${workspaceRoot}/somecadl/node_modules/@cadl-lang/compiler"
}

Basically, we can find ${workspaceRoot}/node_modules/@cadl-lang/compiler automatically, but if you npm install the compiler deeper down in your workspace, you will get this error and need to tell us where to find it. This is documented in https://github.com/microsoft/cadl#installing-vs-code-extension but the documentation could be much clearer.

(*) It occurs to me now that one such bug might be if you open the folder in VS Code before you've run npm install at least once prior. Then it would only find the local cadl by itself if you restart VS Code, I think. If so, that wasn't intended, and we should fix it. @annelo-msft Could that be what you hit? Or do you genuinely have your cadl code and its package.json deeper down than the root of your VS Code workspace? Or if you have another scenario the cadl project is at the root of the workspace and this message shows up, repro steps would be helpful.

I think we should do all of the following to resolve this issue:

  • Find any cases where the cadl is at the root of the workspace and this error message still appears and fix them so that things just work.
  • Replace the absolute path in the error message with a ${workspaceRoot} relative path that you can cut and paste into settings.json.
  • Improve the documentation about configuring the server path. Explain why this is sometimes needed, and provide a realistic sample.
  • Link to the improved documentation from the error message.

And then optionally, for bonus points:

  • Look at making more cases than root-of-workspace-cadl just work along the lines of some of the brainstorming above.

@annelo-msft
Copy link
Member Author

annelo-msft commented Jun 24, 2022

This is super helpful, thank you so much @nguerrera! That sounds like a good plan to me!

Or do you genuinely have your cadl code and its package.json deeper down than the root of your VS Code workspace?

Yes, I was opening my VS Code project at the https://github.com/Azure/autorest.testserver/tree/cadl root so I could see multiple .cadl files together and compare them against each other. Now I understand why that was a problem. I think by this expectation, it was probably not technically a bug, but it might be good to address this scenario in the docs.

In the meantime, I've hardcoded the value in my .vscode/settings.json and can move forward. Thanks!

@nguerrera
Copy link
Contributor

I now think #784 is the simplest and cheapest path forward that could reduce this to a warning where the tooling would stil work most of the time, and reduce the situations where you see it all.

@timotheeguerin
Copy link
Member

I think now that #784 has been merged, additional improvement to the resolution logic can be moved to the backlog.

@timotheeguerin
Copy link
Member

I don't think we have heard much more confusion in the current state since the improvements. I'll close this and we can reopen a new one if it is still confusing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants