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

fix(lsp): correctly infer file source from un-named files #1413

Merged
merged 2 commits into from
Jan 3, 2024

Conversation

ematipico
Copy link
Member

Summary

Closes #1394

Newly created files weren't correctly processed when calling the analyzer from the LSP. The reason why this wasn't working anymore is because:

  • Some time ago we changed the JavaScript analyzer to accept the JsFileSource, which tells what kind of JavaScript file we are processing (JS, JSX, TS, etc.). The analyser changed to default to zero diagnostics in case we couldn't compute the JsFileSource from the file path. This broke the LSP when the LSP analyzed untitled files.
  • We didn't have a test to check these cases

Untitled files are saved and updated by using the actions textDocument/didOpen and textDocument/didChange, that's where we keep track of their language.

Now when pulling diagnostics and computing JsFileSource, we:

  • try to compute it from the file path;
  • if we fail, we compute it from the language hint;
  • if we fail, we return 0 diagnostics

Test Plan

Added a new test

Copy link

netlify bot commented Jan 3, 2024

Deploy Preview for biomejs ready!

Name Link
🔨 Latest commit 18a27ae
🔍 Latest deploy log https://app.netlify.com/sites/biomejs/deploys/659540a8ca912a000871874f
😎 Deploy Preview https://deploy-preview-1413--biomejs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 98 (🔴 down 2 from production)
Accessibility: 97 (no change from production)
Best Practices: 100 (no change from production)
SEO: 93 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@github-actions github-actions bot added A-Project Area: project A-LSP Area: language server protocol A-Website Area: website A-Changelog Area: changelog labels Jan 3, 2024
Comment on lines +272 to +285
let file_source = match params.parse.file_source(params.path) {
Ok(file_source) => file_source,
Err(_) => {
if let Some(file_source) = params.language.as_js_file_source() {
file_source
} else {
return LintResults {
errors: 0,
diagnostics: vec![],
skipped_diagnostics: 0,
};
}
}
};
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the new change and the bulk of the logic.

@ematipico ematipico requested review from a team January 3, 2024 11:11
@ematipico ematipico merged commit d869a33 into main Jan 3, 2024
20 checks passed
@ematipico ematipico deleted the fix/pull-diagnostics-from-new-files branch January 3, 2024 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Changelog Area: changelog A-LSP Area: language server protocol A-Project Area: project A-Website Area: website
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 Format the content of an in-memory editor in VS Code without configuration
1 participant