Skip to content

Do not break completion for unsupported FS providers #5536

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

Merged
merged 1 commit into from
May 7, 2025

Conversation

fbricon
Copy link
Contributor

@fbricon fbricon commented May 6, 2025

Description

vscode-java uses a custom URI scheme, jdt, to represent binary class files. However it only provides a TextDocumentProvider so jdt:// URIS can be opened as documents. Because vscode-java doesn't (yet) provide a FileSystem provider, vscode.workspace.fs.readFile calls throw an ugly exception:

ENOPRO: No file system provider found for resource 'jdt://contents/java.base/java.lang/String.class?%3Ddemogorgon%2F%5C%2FUsers%5C%2Ffbricon%5C%2F.sdkman%5C%2Fcandidates%5C%2Fjava%5C%2F21.0.2-tem%5C%2Flib%5C%2Fjrt-fs.jar%60java.base%3D%2Fjavadoc_location%3D%2Fhttps%3A%5C%2F%5C%2Fdocs.oracle.com%5C%2Fen%5C%2Fjava%5C%2Fjavase%5C%2F21%5C%2Fdocs%5C%2Fapi%5C%2F%3D%2F%3D%2Fmaven.pomderived%3D%2Ftrue%3D%2F%3Cjava.lang%28String.class'

Continue ends up handling these URIs most likely after performing some LSP calls that reach the Java language server.

The fix should be 2-fold:

Basically, the idea is to centralize all vscode.workspace.fs.readFile calls in one place, and intercept/ignore such ENOPRO errors, then ignore known problematic schemes in subsequent calls.
Caveat: Similar calls to vscode.workspace.fs.stat and vscode.workspace.fs.readDirectory might be affected as well but I haven't seen it in the wild, so I didn't do anything there. [done]

Fixes #4903,
Fixes #3795,
Fixes #5226

Checklist

  • I've read the contributing guide
  • [] The relevant docs, if any, have been updated or created
  • [] The relevant tests, if any, have been updated or created

Testing instructions

Open a Java project and try to complete in a java file referencing external classes.

@fbricon fbricon requested a review from a team as a code owner May 6, 2025 10:35
@fbricon fbricon requested review from Patrick-Erichsen and removed request for a team May 6, 2025 10:35
Copy link

netlify bot commented May 6, 2025

Deploy Preview for continuedev canceled.

Name Link
🔨 Latest commit 4d1a33f
🔍 Latest deploy log https://app.netlify.com/sites/continuedev/deploys/681b3b9484d02e00082dc637

Copy link
Collaborator

@Patrick-Erichsen Patrick-Erichsen left a comment

Choose a reason for hiding this comment

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

Wow, this is a great find! I saw #5226 but didn't have a good hunch as to what the issue was since I wasn't aware of TextDocumentProvider vs FileSystemProvider.

Great implementation as well! The dynamic set is a smart solution.

Caveat: Similar calls to vscode.workspace.fs.stat and vscode.workspace.fs.readDirectory might be affected as well but I haven't seen it in the wild, so I didn't do anything there.

This feels quite likely. What are your thoughts on the following:

  • Add similar wrapper methods in extensions/vscode/src/util/ideUtils.ts
  • Don't implement, just wrap, but add a code comment indicating that we likely need to do something similar to ideUtils.readFile

There aren't many instances of either stat or readDirectory so this seems worth the effort.

Happy to merge as-is however, and I can do that myself in a follow up PR.

@fbricon
Copy link
Contributor Author

fbricon commented May 6, 2025

Ok, I'll add the other wrappers

@fbricon
Copy link
Contributor Author

fbricon commented May 7, 2025

@Patrick-Erichsen added stat and readDirectory wrapper functions

Signed-off-by: Fred Bricon <fbricon@gmail.com>
@Patrick-Erichsen Patrick-Erichsen merged commit 0261d47 into continuedev:main May 7, 2025
32 checks passed
@github-project-automation github-project-automation bot moved this from Todo to Done in Issues and PRs May 7, 2025
@Patrick-Erichsen
Copy link
Collaborator

Excellent, appreciate the extra work adding stat and readDirectory. Thanks for the great PR!

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label May 8, 2025
@fbricon
Copy link
Contributor Author

fbricon commented May 8, 2025

@Patrick-Erichsen thanks for merging. I initially messed up the syntax for automatically closing multiple issues, can you please manually close #3795 and #5226

@KeroZhai
Copy link

KeroZhai commented May 9, 2025

@Patrick-Erichsen thanks for merging. I initially messed up the syntax for automatically closing multiple issues, can you please manually close #3795 and #5226

Thanks for your great work! I think I can close #5226 myself once released (and checked).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:L This PR changes 100-499 lines, ignoring generated files.
Projects
Status: Done
3 participants