Skip to content

Commit 6bcf2ca

Browse files
harehareharehare
authored
Merge pull request #1445 from harehare/feat/lsp-type-error-filter
🐛 feat(lsp): filter type errors to only those originating from the current source
2 parents b862dbf + 93adb97 commit 6bcf2ca

File tree

7 files changed

+94
-74
lines changed

7 files changed

+94
-74
lines changed

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/mq-check/src/builtin.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -442,7 +442,8 @@ fn register_array(ctx: &mut InferenceContext) {
442442
let a = ctx.fresh_var();
443443
register_unary(ctx, "array", Type::Var(a), Type::array(Type::Var(a)));
444444

445-
// range: (number, number) -> [number], (number, number, number) -> [number]
445+
// range: (number) -> [number], (number, number) -> [number], (number, number, number) -> [number]
446+
register_unary(ctx, "range", Type::Number, Type::array(Type::Number));
446447
register_binary(ctx, "range", Type::Number, Type::Number, Type::array(Type::Number));
447448
register_ternary(
448449
ctx,

‎crates/mq-lsp/Cargo.toml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,17 @@ clap = {workspace = true, features = ["derive"]}
1717
dashmap = {workspace = true}
1818
itertools = {workspace = true}
1919
miette = {workspace = true, features = ["fancy"]}
20+
mq-check = {workspace = true}
2021
mq-formatter = {workspace = true}
2122
mq-hir = {workspace = true}
2223
mq-lang = {workspace = true, features = ["ast-json", "sync", "file-io"]}
2324
mq-markdown = {workspace = true}
24-
mq-check = {workspace = true}
2525
serde_json = {workspace = true}
2626
tokio = {workspace = true, features = ["macros", "io-std", "rt-multi-thread"]}
2727
tokio-macros = {workspace = true}
2828
tower-lsp-server = {workspace = true}
2929
url = {workspace = true}
30+
rustc-hash = {workspace = true}
3031

3132
[[bin]]
3233
name = "mq-lsp"

‎crates/mq-lsp/src/completions.rs‎

Lines changed: 39 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -16,71 +16,68 @@ pub fn response(
1616
) -> Option<CompletionResponse> {
1717
match source_map.get_by_left(&url.to_string()) {
1818
Some(source_id) => {
19-
let scope_id = hir
20-
.read()
21-
.unwrap()
19+
let hir_guard = hir.read().unwrap();
20+
21+
let scope_id = hir_guard
2222
.find_scope_in_position(
2323
*source_id,
2424
mq_lang::Position::new(position.line + 1, (position.character + 1) as usize),
2525
)
2626
.map(|(scope_id, _)| scope_id)
27-
.unwrap_or_else(|| hir.read().unwrap().find_scope_by_source(source_id));
27+
.unwrap_or_else(|| hir_guard.find_scope_by_source(source_id));
2828

2929
let module_completion = if position.character >= 3 {
3030
let before_pos =
3131
mq_lang::Position::new(position.line + 1, (position.character.saturating_sub(3)) as usize);
3232

33-
hir.read()
34-
.unwrap()
35-
.find_symbol_in_position(*source_id, before_pos)
36-
.and_then(|(_, symbol)| {
37-
if matches!(symbol.kind, mq_hir::SymbolKind::QualifiedAccess) {
38-
let module_name = symbol.value.as_ref()?;
39-
40-
let hir_guard = hir.read().unwrap();
41-
for (_, mod_symbol) in hir_guard.symbols() {
42-
if mod_symbol.is_module()
43-
&& mod_symbol.value.as_ref() == Some(module_name)
44-
&& let mq_hir::SymbolKind::Module(module_source_id) = mod_symbol.kind
45-
{
46-
return Some(hir_guard.find_symbols_in_module(module_source_id));
47-
} else if mod_symbol.is_import()
48-
&& mod_symbol.value.as_ref() == Some(module_name)
49-
&& let mq_hir::SymbolKind::Import(module_source_id) = mod_symbol.kind
50-
{
51-
return Some(hir_guard.find_symbols_in_module(module_source_id));
52-
}
53-
}
54-
None
55-
} else if symbol.is_module() {
56-
// Direct module reference
57-
if let mq_hir::SymbolKind::Module(module_source_id) = symbol.kind {
58-
Some(hir.read().unwrap().find_symbols_in_module(module_source_id))
59-
} else {
60-
None
33+
let found = hir_guard.find_symbol_in_position(*source_id, before_pos);
34+
found.and_then(|(_, symbol)| {
35+
if matches!(symbol.kind, mq_hir::SymbolKind::QualifiedAccess) {
36+
let module_name = symbol.value.as_ref()?;
37+
38+
for (_, mod_symbol) in hir_guard.symbols() {
39+
if mod_symbol.is_module()
40+
&& mod_symbol.value.as_ref() == Some(module_name)
41+
&& let mq_hir::SymbolKind::Module(module_source_id) = mod_symbol.kind
42+
{
43+
return Some(hir_guard.find_symbols_in_module(module_source_id));
44+
} else if mod_symbol.is_import()
45+
&& mod_symbol.value.as_ref() == Some(module_name)
46+
&& let mq_hir::SymbolKind::Import(module_source_id) = mod_symbol.kind
47+
{
48+
return Some(hir_guard.find_symbols_in_module(module_source_id));
6149
}
50+
}
51+
None
52+
} else if symbol.is_module() {
53+
// Direct module reference
54+
if let mq_hir::SymbolKind::Module(module_source_id) = symbol.kind {
55+
Some(hir_guard.find_symbols_in_module(module_source_id))
6256
} else {
6357
None
6458
}
65-
})
59+
} else {
60+
None
61+
}
62+
})
6663
} else {
6764
None
6865
};
6966

7067
let symbols = if let Some(module_symbols) = module_completion {
7168
module_symbols
7269
} else {
73-
let hir_guard = hir.read().unwrap();
74-
75-
itertools::concat(vec![
76-
hir_guard.find_symbols_in_scope(scope_id),
77-
hir_guard.find_symbols_in_source(hir_guard.builtin.source_id),
78-
])
79-
.into_iter()
80-
.unique_by(|s| s.value.clone())
81-
.collect::<Vec<_>>()
70+
let scope_symbols = hir_guard.find_symbols_in_scope(scope_id);
71+
let builtin_symbols = hir_guard.find_symbols_in_source(hir_guard.builtin.source_id);
72+
scope_symbols
73+
.into_iter()
74+
.chain(builtin_symbols)
75+
.unique_by(|s| s.value.clone())
76+
.collect::<Vec<_>>()
8277
};
8378

79+
drop(hir_guard);
80+
8481
Some(CompletionResponse::Array(
8582
symbols
8683
.iter()

‎crates/mq-lsp/src/hover.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ pub fn response(
1010
type_env: Option<mq_check::TypeEnv>,
1111
position: Position,
1212
) -> Option<Hover> {
13-
let source = hir.write().unwrap().source_by_url(&url);
13+
let source = hir.read().unwrap().source_by_url(&url);
1414

1515
if let Some(source) = source {
1616
if let Some((symbol_id, symbol)) = hir.read().unwrap().find_symbol_in_position(

‎crates/mq-lsp/src/semantic_tokens.rs‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,15 @@ pub fn response(hir: Arc<RwLock<mq_hir::Hir>>, url: Url) -> Vec<SemanticToken> {
88
let mut pre_line = 0;
99
let mut pre_start = 0;
1010

11-
let source_id = hir.read().unwrap().source_by_url(&url);
11+
let hir_guard = hir.read().unwrap();
12+
let source_id = hir_guard.source_by_url(&url);
1213
let symbols = source_id
13-
.map(|source_id| hir.read().unwrap().find_symbols_in_source(source_id))
14+
.map(|source_id| hir_guard.find_symbols_in_source(source_id))
1415
.unwrap_or_default();
1516

1617
let mut semantic_tokens = Vec::with_capacity(symbols.len());
1718

18-
for symbol in symbols
19-
.into_iter()
20-
.sorted_by_key(|symbol| symbol.source.text_range)
21-
.collect::<Vec<_>>()
22-
{
19+
for symbol in symbols.into_iter().sorted_by_key(|symbol| symbol.source.text_range) {
2320
for (range, _) in &symbol.doc {
2421
let line = range.start.line - 1_u32;
2522
let start = (range.start.column - 2) as u32;
@@ -102,9 +99,7 @@ pub fn response(hir: Arc<RwLock<mq_hir::Hir>>, url: Url) -> Vec<SemanticToken> {
10299
pre_line = line;
103100
pre_start = start;
104101

105-
let hir_guard = hir.read().unwrap();
106102
let is_builtin = hir_guard.is_builtin_symbol(&symbol);
107-
drop(hir_guard);
108103

109104
let mut modifiers_bitset = 0;
110105
if is_builtin {

‎crates/mq-lsp/src/server.rs‎

Lines changed: 45 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use std::sync::{Arc, RwLock};
44

55
use bimap::BiMap;
66
use dashmap::DashMap;
7+
use rustc_hash::FxHashSet;
78
use tower_lsp_server::ls_types::DocumentRangeFormattingParams;
89
use url::Url;
910

@@ -200,14 +201,12 @@ impl LanguageServer for Backend {
200201
&self,
201202
params: ls_types::DocumentFormattingParams,
202203
) -> jsonrpc::Result<Option<Vec<ls_types::TextEdit>>> {
203-
if !self
204+
if self
204205
.error_map
205206
.get(&params.text_document.uri.to_string())
206207
.unwrap()
207208
.iter()
208-
.filter(|e| matches!(e, LspError::SyntaxError(_)))
209-
.collect::<Vec<_>>()
210-
.is_empty()
209+
.any(|e| matches!(e, LspError::SyntaxError(_)))
211210
{
212211
return Ok(None);
213212
}
@@ -360,15 +359,40 @@ impl Backend {
360359
.collect::<Vec<_>>();
361360

362361
if errors.is_empty() && self.config.enable_type_checking {
362+
let hir_guard = self.hir.read().unwrap();
363363
let mut checker = mq_check::TypeChecker::with_options(self.config.type_checker_options);
364-
let type_errors = checker.check(&self.hir.read().unwrap());
364+
let type_errors = checker.check(&hir_guard);
365+
366+
// Build a set of (line, column) start positions from the current source's symbols
367+
// so that type errors originating from other sources (e.g., pre-loaded modules)
368+
// are not incorrectly attributed to this file.
369+
let source_locations: FxHashSet<(u32, usize)> = hir_guard
370+
.symbols_for_source(source_id)
371+
.filter_map(|(_, symbol)| {
372+
symbol
373+
.source
374+
.text_range
375+
.as_ref()
376+
.map(|r| (r.start.line, r.start.column))
377+
})
378+
.collect();
379+
365380
self.type_env_map
366381
.insert(uri_string.clone(), checker.symbol_types().clone());
367-
errors.extend(type_errors.into_iter().map(LspError::TypeError));
382+
errors.extend(
383+
type_errors
384+
.into_iter()
385+
.filter(|e| {
386+
e.location()
387+
.map(|(line, col)| source_locations.contains(&(line, col)))
388+
.unwrap_or(false)
389+
})
390+
.map(LspError::TypeError),
391+
);
368392
}
369393

370394
self.source_map.write().unwrap().insert(uri_string.clone(), source_id);
371-
self.text_map.insert(uri_string.clone(), text.to_string().into());
395+
self.text_map.insert(uri_string.clone(), text.into());
372396
self.error_map.insert(uri_string, errors);
373397
}
374398

@@ -381,28 +405,29 @@ impl Backend {
381405

382406
// Add parsing errors if they exist
383407
if let Some(errors) = file_errors {
384-
let errors: Vec<ls_types::Diagnostic> = (*errors).iter().map(Into::into).collect::<Vec<_>>();
385-
diagnostics.extend(errors);
408+
diagnostics.extend(errors.iter().map(Into::into));
386409
}
387410

388411
{
389412
let source_map_guard = self.source_map.read().unwrap();
390413
if let Some(source_id) = source_map_guard.get_by_left(&uri_string) {
391414
let hir_guard = self.hir.read().unwrap();
392415

393-
// Build a map of text_range -> bool for this file's symbols for O(1) lookup
394-
let mut range_map = std::collections::HashMap::new();
395-
for (_, symbol) in hir_guard.symbols() {
396-
if symbol.source.source_id == Some(*source_id)
397-
&& let Some(ref text_range) = symbol.source.text_range
398-
{
399-
range_map.insert(text_range, true);
400-
}
401-
}
416+
// Build a set of text_ranges for this file's symbols for O(1) lookup
417+
let range_set: FxHashSet<mq_lang::Range> = hir_guard
418+
.symbols()
419+
.filter_map(|(_, symbol)| {
420+
if symbol.source.source_id == Some(*source_id) {
421+
symbol.source.text_range
422+
} else {
423+
None
424+
}
425+
})
426+
.collect();
402427

403428
// Filter HIR errors to only include ones from this specific source
404429
diagnostics.extend(hir_guard.error_ranges().into_iter().filter_map(|(message, item)| {
405-
if range_map.contains_key(&item) {
430+
if range_set.contains(&item) {
406431
Some(ls_types::Diagnostic::new_simple(
407432
ls_types::Range::new(
408433
ls_types::Position {
@@ -448,7 +473,7 @@ impl Backend {
448473

449474
// Add HIR warnings (including unreachable code warnings)
450475
diagnostics.extend(hir_guard.warning_ranges().into_iter().filter_map(|(message, item)| {
451-
if range_map.contains_key(&item) {
476+
if range_set.contains(&item) {
452477
let mut diagnostic = ls_types::Diagnostic::new_simple(
453478
ls_types::Range::new(
454479
ls_types::Position {

0 commit comments

Comments
 (0)