From 5908e54747a5fcecbc2bfd9682b4861ae3f5c88d Mon Sep 17 00:00:00 2001 From: Loaf <60279520+loaf-scripts@users.noreply.github.com> Date: Wed, 30 Sep 2026 18:26:02 +0200 Subject: [PATCH 01/12] feat(completion): suggest the values a declared type lists A value whose type lists what it can be, such as `---@alias State "busy"|"working"|"ready"`, got no suggestions outside a call: nothing opened after `local state = ` under a `---@type State`, and Ctrl+Space there returned nothing. The values are now offered where such a value starts on its line: - after the `=` of a `local` or an assignment, from the `---@type` above it or from what the assigned local or class field is declared as, - after the `=` of a field in a table typed as a class, from its `@field`, - after `return`, from the `@return` of the function, - after `==` and `~=`, from what the other side is declared as. Strings are written in quotes, and integers such as those of `1|2|3` as they are. A `boolean` lists `true` and `false`, and a type that lists values also lists the `nil` it allows, last, so `State?` ends with `nil` while `string?` lists nothing. Where a function is stored or returned, snippet clients get the function literal that arguments already get, with the parameters the `fun(...)` type names. Inside a string in one of those places, or on either side of the comparison, the string values replace the string's contents. A typed word filters the values, beside the names in scope. A local typed by `---@type` or `@param` keeps that type where it is assigned again. Types inferred from assigned values list nothing, since `local mode = 'dev'` tells what is stored today, and nothing is offered in front of a value that is already written. `=` and space are completion trigger characters for these items only: a request triggered by one of them returns nothing else, so no other space opens a list. Call arguments list integers, booleans and `nil` the same way. A typed `(` or `,` still opens the list only for a parameter that lists a string or an integer; the `true` and `false` of the many `boolean` parameters are listed on request or once a word is typed. Call snippets keep their quotes for parameters that list strings. --- crates/qbx_lua_ls/README.md | 12 +- crates/qbx_lua_ls/docs/protocol.md | 29 +- crates/qbx_lua_ls/src/features/comparisons.rs | 45 ++- crates/qbx_lua_ls/src/features/completion.rs | 273 +++++++++++++----- crates/qbx_lua_ls/src/features/expected.rs | 175 +++++++++++ crates/qbx_lua_ls/src/features/mod.rs | 1 + crates/qbx_lua_ls/src/infer.rs | 30 +- crates/qbx_lua_ls/src/server.rs | 2 + crates/qbx_lua_ls/tests/lsp.rs | 237 +++++++++++++++ 9 files changed, 707 insertions(+), 97 deletions(-) create mode 100644 crates/qbx_lua_ls/src/features/expected.rs diff --git a/crates/qbx_lua_ls/README.md b/crates/qbx_lua_ls/README.md index 34260b2..05b6d59 100644 --- a/crates/qbx_lua_ls/README.md +++ b/crates/qbx_lua_ls/README.md @@ -20,10 +20,14 @@ Available features depend on the editor's LSP client. - Completion and hover for Lua symbols, FiveM natives, exports, events and callbacks. - Call snippets for functions that take a callback, with the function literal written out. -- The string values a parameter lists, through aliases and `@overload`s, as soon as its argument - starts or inside its string, and a function literal with the matching parameters where an - argument takes one. Call snippets stop in the quotes of those values, and end after a value - that picks an overload, so the handler written next matches it. +- The values a parameter lists, through aliases and `@overload`s, as soon as its argument starts: + strings, integers, `true` and `false`, and `nil` beside them. Strings are also offered inside + the argument's string, and a function literal with the matching parameters where an argument + takes one. Call snippets stop in the quotes of listed strings, and end after a value that picks + an overload, so the handler written next matches it. +- The same values where a value of such a type, like `"busy"|"ready"`, `1|2|3` or `boolean`, is + assigned, set as a field of a class, returned or compared with `==` or `~=`, and the function + literal where a function is assigned or returned. - `---@` continuation on Enter inside LuaCATS annotation blocks, through on-type formatting. - Reference hovers for literal control IDs in PAD natives and ped configuration flags in `SetPedConfigFlag` / `GetPedConfigFlag`, using bundled Cfx documentation. diff --git a/crates/qbx_lua_ls/docs/protocol.md b/crates/qbx_lua_ls/docs/protocol.md index dfc3173..fa4a642 100644 --- a/crates/qbx_lua_ls/docs/protocol.md +++ b/crates/qbx_lua_ls/docs/protocol.md @@ -95,15 +95,30 @@ command, titled `Suggest callback names` or `Suggest values`, so they are sugges insertion. Other clients get the snippet without the command. Where an argument starts, right after the `(` or `,` in front of it, completion offers what it can -be. Values that its parameter lists are offered quoted, with the value as filter text. Snippet -clients also get a function literal labelled like `function(source)`, with the filter text -`function`, when the argument takes a function. Values come from every signature that the -arguments before it fit, and the function's parameters from the signature they fit best, so +be. String values that its parameter lists are offered quoted, with the value as filter text. +Integers are offered as they are, and so are `true` and `false`, which a `boolean` lists too. A +type that lists values also lists the `nil` it allows, last; a type such as `string?` lists +nothing. Snippet clients also get a function literal labelled like `function(source)`, with the +filter text `function`, when the argument takes a function. Values come from every signature that +the arguments before it fit, and the function's parameters from the signature they fit best, so `OnAction("playerUnloaded", ` picks the `@overload` that lists `"playerUnloaded"`. Signatures that fit equally well each get an item. `(` and `,` are completion trigger characters for these items -only: a request whose `context.triggerCharacter` is `(` or `,` returns nothing else, and returns -`null` where the argument lists no values and takes no function. Inside a string argument, the -same values replace the string's contents. +only: a request whose `context.triggerCharacter` is `(` or `,` returns nothing else, leaves out +the values of a parameter that lists no string or integer, as the many `boolean` parameters do, +and returns `null` where nothing is left and the argument takes no function. Inside a string +argument, the string values replace the string's contents. + +Where another value starts, right after an `=`, `==`, `~=` or `return` on its line, completion +offers the values that its type lists in the same way, and snippet clients get the function +literal where a function is stored or returned, though not where one is compared. The type is the +`---@type` above the `local` or assignment, what the assigned local or class field is declared as, +the `@field` that a table typed as a class sets, the `@return` of the function, or what the other +side of the comparison is declared as. A local typed by `---@type` or `@param` keeps that type +where it is assigned again; types inferred from assigned values list nothing. `=` and space are +completion trigger characters for these items only: a request whose `context.triggerCharacter` is +`=` or a space returns nothing else, and returns `null` where no type lists values or takes a +function. Inside a string in one of those places, or on either side of the comparison, the string +values replace the string's contents. The server requests file watches only when `workspace.didChangeWatchedFiles.dynamicRegistration` is `true`. It watches Lua, lint config, diff --git a/crates/qbx_lua_ls/src/features/comparisons.rs b/crates/qbx_lua_ls/src/features/comparisons.rs index 5d723d3..a49cad9 100644 --- a/crates/qbx_lua_ls/src/features/comparisons.rs +++ b/crates/qbx_lua_ls/src/features/comparisons.rs @@ -10,7 +10,7 @@ //! it is deliberate. use qbx_fivem_data::native; -use qbx_lua_analysis::scope::{LocalId, Resolved}; +use qbx_lua_analysis::scope::{Local, LocalId, Resolved}; use qbx_lua_syntax::ast::*; use qbx_lua_syntax::visit::{self, Visitor}; use qbx_lua_syntax::Span; @@ -25,19 +25,41 @@ const MAX_DEPTH: u32 = 16; /// Each `==` and `~=` of `chunk` whose sides share no value, with a message naming both types and /// the answer the comparison always gives. pub fn impossible_comparisons(infer: &Infer, chunk: &Chunk) -> Vec<(Span, String)> { - let mut finder = Finder { infer, classes: Classes::new(infer), out: Vec::new() }; + let mut finder = Finder { declared: Declared::new(infer), out: Vec::new() }; finder.visit_block(&chunk.block); finder.out } struct Finder<'a, 'b> { + declared: Declared<'a, 'b>, + out: Vec<(Span, String)>, +} + +/// Reads the types that expressions are declared to have. +pub struct Declared<'a, 'b> { infer: &'a Infer<'b>, classes: Classes<'a, 'b>, - out: Vec<(Span, String)>, + /// Whether a local that is assigned again keeps the type its annotation gives it. + keeps_annotations: bool, } -impl Finder<'_, '_> { +impl<'a, 'b> Declared<'a, 'b> { + pub fn new(infer: &'a Infer<'b>) -> Self { + Self { infer, classes: Classes::new(infer), keeps_annotations: false } + } + + /// Takes the `---@type` or `@param` of a local at its word, also where the local is assigned + /// again. Completion reads the values to offer for it from there, which no diagnostic can + /// rely on. + pub fn keeping_annotations(infer: &'a Infer<'b>) -> Self { + Self { keeps_annotations: true, ..Self::new(infer) } + } + /// The type of `expr` as far as it is declared, and `unknown` beyond that. + pub fn of(&self, expr: &Expr) -> Type { + self.declared(expr, 0) + } + fn declared(&self, expr: &Expr, depth: u32) -> Type { if depth > MAX_DEPTH { return Type::Unknown; @@ -85,7 +107,7 @@ impl Finder<'_, '_> { /// declaration is `unknown`, since its declared type may not be what it holds. fn local(&self, id: LocalId, offset: u32, depth: u32) -> Type { let local = self.infer.ctx.resolution.local(id); - if local.refs.iter().any(|r| r.write) { + if local.refs.iter().any(|r| r.write) && !(self.keeps_annotations && self.is_annotated(local)) { return Type::Unknown; } let ty = match self.infer.ctx.decl(local.decl.start) { @@ -101,6 +123,15 @@ impl Finder<'_, '_> { self.infer.narrowed(id, offset, ty) } + /// Whether a `---@type` above its declaration or a `@param` line types the local. + fn is_annotated(&self, local: &Local) -> bool { + match self.infer.ctx.decl(local.decl.start) { + Some(Decl::Local { stmt, .. }) => self.infer.ctx.doc_at(stmt.span.start).ty.is_some(), + Some(Decl::Param { .. }) => has_param_line(self.infer, local), + _ => false, + } + } + /// What the `local` statement `stmt` gives the name at `index`: its `---@type`, or the declared /// type of the value it takes. A literal written out is widened, as `local mode = 'dev'` is a /// setting to change. @@ -190,8 +221,8 @@ impl Finder<'_, '_> { impl<'c> Visitor<'c> for Finder<'_, '_> { fn visit_expr(&mut self, expr: &'c Expr) { if let ExprKind::Binary { op: op @ (BinOp::Eq | BinOp::Ne), lhs, rhs, .. } = &expr.kind { - let (left, right) = (self.declared(lhs, 0), self.declared(rhs, 0)); - if self.classes.never_equal(&left, &right) { + let (left, right) = (self.declared.of(lhs), self.declared.of(rhs)); + if self.declared.classes.never_equal(&left, &right) { let answer = if *op == BinOp::Eq { "false" } else { "true" }; self.out.push((expr.span, format!("Comparing `{left}` with `{right}` is always {answer}"))); } diff --git a/crates/qbx_lua_ls/src/features/completion.rs b/crates/qbx_lua_ls/src/features/completion.rs index 88163e8..2985e47 100644 --- a/crates/qbx_lua_ls/src/features/completion.rs +++ b/crates/qbx_lua_ls/src/features/completion.rs @@ -15,6 +15,7 @@ use rustc_hash::{FxHashMap, FxHashSet}; use serde_json::json; use super::class_tables::{class_table_at, named_field, Classes}; +use super::expected::{expected_type, Place}; use super::hover::{event_handler_signature, event_string_context}; use super::{lua_block, markdown, with_infer}; use crate::callback_wrappers::{families, takes_function, Wrapper}; @@ -289,7 +290,9 @@ fn call_snippet_item( let signatures = infer.open_call_signatures(fun, &[], via_colon, options.at); let listed: Vec = (0..fun.params.len()) .map(|i| { - let literals = argument_literals(infer, &signatures, i, via_colon, ""); + // The stop of a listed value is written between quotes, which only strings take. + let mut literals = argument_literals(infer, &signatures, i, via_colon, ""); + literals.retain(|literal| literal.string().is_some()); match literals.iter().any(|literal| !literal.taken_alone_by.is_empty()) { true => Listed::Deciding, false if literals.is_empty() => Listed::Nothing, @@ -443,7 +446,16 @@ pub fn completion( if comment.is_some() || in_string.is_some() { return None; } - let items = with_infer(ws, doc, |infer| argument_items(infer, doc, offset, before, snippets, quote)); + let items = with_infer(ws, doc, |infer| argument_items(infer, doc, offset, before, snippets, quote, true)); + return (!items.is_empty()).then(|| respond(items, false)); + } + // A typed `=` or space only asks for the values that what follows it can be, so that no other + // space opens a list. + if matches!(trigger_character, Some("=" | " ")) { + if comment.is_some() || in_string.is_some() { + return None; + } + let items = value_items(ws, doc, offset, before, snippets, quote); return (!items.is_empty()).then(|| respond(items, false)); } @@ -494,21 +506,29 @@ pub fn completion( return Some(respond(fields, false)); } } - let mut items = with_infer(ws, doc, |infer| argument_items(infer, doc, offset, before, snippets, quote)); + let mut items = with_infer(ws, doc, |infer| argument_items(infer, doc, offset, before, snippets, quote, false)); + items.extend(value_items(ws, doc, offset, before, snippets, quote)); if prefix.is_empty() { return (!items.is_empty()).then(|| respond(items, false)); } - let (scope, incomplete) = + let (mut scope, incomplete) = with_infer(ws, doc, |infer| scope_items(ws, infer, doc, offset, prefix, snippet_quote, call_snippets)); + // `true`, `false` and `nil` are listed once, as the values they are. + let is_keyword = |item: &CompletionItem| item.kind == Some(CompletionItemKind::KEYWORD); + scope.retain(|name| !is_keyword(name) || !items.iter().any(|value| is_keyword(value) && value.label == name.label)); items.extend(scope); Some(respond(items, incomplete)) } /// What the argument that the cursor starts can be, right after the `(` or `,` in front of it: the -/// string values its parameter lists, quoted, and a function literal when it takes one, such as -/// `function(${1:source})\n\t$0\nend` after `OnAction("playerUnloaded", `. Both come from the +/// values its parameter lists, the strings quoted, and a function literal when it takes one, such +/// as `function(${1:source})\n\t$0\nend` after `OnAction("playerUnloaded", `. Both come from the /// signatures that fit the arguments before it, the function from those that fit them best, with one /// item for each list of parameters they give it. Function literals are snippets. +/// +/// `typed`: the `(` or `,` was just typed. That lists the values of a parameter that names strings +/// or integers, and not the `true` and `false` that so many parameters take, which are left to a +/// request for them. fn argument_items( infer: &Infer, doc: &Document, @@ -516,6 +536,7 @@ fn argument_items( before: &str, snippets: bool, quote: char, + typed: bool, ) -> Vec { let prefix = identifier_prefix(before); let head = before[..before.len() - prefix.len()].trim_end(); @@ -544,17 +565,12 @@ fn argument_items( let space = if before[..before.len() - prefix.len()].ends_with(',') { " " } else { "" }; let signatures = infer.open_call_signatures(&fun, args, via_method, site.base.span.start); let name = callee_name(site.base, site.method); - let mut items: Vec = argument_literals(infer, &signatures, argument, via_method, &name) - .into_iter() - .enumerate() - .map(|(i, literal)| { - let quoted = format!("{quote}{}{quote}", escaped_string(&literal.value, quote)); - let mut out = literal.item(i, "ed); - out.filter_text = Some(literal.value.to_string()); - out.insert_text = Some(format!("{space}{quoted}")); - out - }) - .collect(); + let mut literals = argument_literals(infer, &signatures, argument, via_method, &name); + if typed && !literals.iter().any(|literal| matches!(literal.value, Type::StringLit(_) | Type::IntLit(_))) { + literals.clear(); + } + let mut items: Vec = + literals.iter().enumerate().map(|(i, literal)| literal.written_item(i, quote, space)).collect(); if !snippets { return items; } @@ -563,33 +579,96 @@ fn argument_items( let Some(param) = best.then(|| param_for_argument(&signature, argument, via_method)).flatten() else { continue; }; - if !takes_function(¶m.ty) { - continue; - } - let names = param.ty.as_fun().map(|callback| { - let names: Vec<&str> = callback.params.iter().map(|p| p.name.as_str()).collect(); - names.join(", ") - }); - let stop = match names.as_deref() { - Some("") => String::new(), - Some(names) => format!("${{1:{names}}}"), - None => "$1".to_string(), - }; - let body = format!("{space}function({stop})\n\t$0\nend"); - if !seen.insert(body.clone()) { - continue; + let Some(out) = function_literal_item(¶m.ty, ¶m.to_string(), space) else { continue }; + if seen.insert(out.insert_text.clone()) { + items.push(out); } - let label = format!("function({})", names.unwrap_or_default()); - let mut out = snippet_item(&label, &body, ¶m.to_string()); - out.kind = Some(CompletionItemKind::FUNCTION); - // Ahead of the `function name()` declaration snippet too. - out.sort_text = Some(format!(".{label}")); - out.filter_text = Some("function".to_string()); - items.push(out); } items } +/// A function literal for a value of type `ty` that takes a function: `function(source) ... end` +/// with the parameters `ty` names, written after `space`. `description` is shown beside it. +fn function_literal_item(ty: &Type, description: &str, space: &str) -> Option { + if !takes_function(ty) { + return None; + } + let names = ty.as_fun().map(|callback| { + let names: Vec<&str> = callback.params.iter().map(|p| p.name.as_str()).collect(); + names.join(", ") + }); + let stop = match names.as_deref() { + Some("") => String::new(), + Some(names) => format!("${{1:{names}}}"), + None => "$1".to_string(), + }; + let body = format!("{space}function({stop})\n\t$0\nend"); + let label = format!("function({})", names.unwrap_or_default()); + let mut out = snippet_item(&label, &body, description); + out.kind = Some(CompletionItemKind::FUNCTION); + // Ahead of the `function name()` declaration snippet too. + out.sort_text = Some(format!(".{label}")); + out.filter_text = Some("function".to_string()); + Some(out) +} + +/// What the value that the cursor starts can be, right after the `=`, `==`, `~=` or `return` in +/// front of it: the values that its type lists, the strings quoted, such as `"busy"` and `"ready"` +/// after `local state = ` under a `---@type "busy"|"ready"`, and a function literal where a +/// function is stored or returned. The type is the one [`expected_type`] finds. Function literals +/// are snippets. +fn value_items( + ws: &Workspace, + doc: &Document, + offset: u32, + before: &str, + snippets: bool, + quote: char, +) -> Vec { + let prefix = identifier_prefix(before); + let typed = &before[..before.len() - prefix.len()]; + let head = typed.trim_end(); + let line_end = doc.text[offset as usize..].find('\n').map_or(doc.text.len(), |i| offset as usize + i); + let rest = doc.text[offset as usize..line_end].trim_start_matches(|c: char| c.is_ascii_alphanumeric() || c == '_'); + if !takes_value(head) || !leaves_value_open(rest) { + return Vec::new(); + } + let at = offset - (before.len() - head.len()) as u32; + let space = if typed.len() == head.len() { " " } else { "" }; + with_infer(ws, doc, |infer| { + let Some(expected) = expected_type(infer, &doc.chunk, Place::After(at)) else { return Vec::new() }; + let values = listed_values(infer, &expected.ty); + let mut items: Vec = + values.iter().enumerate().map(|(i, value)| value.written_item(i, quote, space)).collect(); + if snippets && !expected.compared { + let ty = infer.resolve_alias(&expected.ty.without_nil()); + items.extend(function_literal_item(&ty, &expected.ty.to_string(), space)); + } + items + }) +} + +/// Whether `head`, the text in front of a value, ends in what takes one: `=`, `==`, `~=` or `return`. +fn takes_value(head: &str) -> bool { + match head.strip_suffix("return") { + Some(before) => !before.ends_with(|c: char| c.is_ascii_alphanumeric() || c == '_'), + None => head.ends_with('=') && !head.ends_with("<=") && !head.ends_with(">="), + } +} + +/// Whether `rest`, the text after the cursor on its line, has no value that an inserted one would +/// end up in front of: it is empty, a comment, or what closes or follows a value, like the `then` +/// of `if state == then`. +fn leaves_value_open(rest: &str) -> bool { + let rest = rest.trim_start(); + let word = rest.trim_start_matches(|c: char| c.is_ascii_alphanumeric() || c == '_'); + let word = &rest[..rest.len() - word.len()]; + rest.is_empty() + || rest.starts_with([')', '}', ']', ',', ';']) + || rest.starts_with("--") + || matches!(word, "then" | "do" | "and" | "or" | "end" | "else" | "elseif" | "until") +} + /// The quote that inserted strings use: the formatter's `quote_style`, or else the one that most /// strings of the document use, and `'` when it has none or there is no document. pub fn quote_of(ws: &Workspace, doc: Option<&Document>) -> char { @@ -716,17 +795,28 @@ fn string_items(ws: &Workspace, doc: &Document, offset: u32, token_index: usize) } let located = locate(&doc.chunk, offset); - let Some((_, Some((call, arg_index)))) = located.string else { return Vec::new() }; - let path = match &call.kind { - ExprKind::Call { callee, .. } => callee.dotted_path(), - _ => None, - }; + let Some((string, argument)) = located.string else { return Vec::new() }; // Event names and literal values contain punctuation. Give clients the whole string content so a // `:` retrigger keeps filtering from the opening quote and accepting does not duplicate it. let token = &tokens[token_index]; let content = string_content_span(token.span, &doc.text).unwrap_or_else(|| Span::new(token.span.start + 1, token.span.end)); let range = doc.range(content); + let quote = if token.span.text(&doc.text).starts_with('"') { '"' } else { '\'' }; + // A string that is no argument lists the values that its place takes. + let Some((call, arg_index)) = argument else { + return with_infer(ws, doc, |infer| { + let Some(expected) = expected_type(infer, &doc.chunk, Place::String(string.span)) else { + return Vec::new(); + }; + let values = listed_values(infer, &expected.ty); + values.iter().enumerate().filter_map(|(i, value)| value.content_item(i, range, quote)).collect() + }); + }; + let path = match &call.kind { + ExprKind::Call { callee, .. } => callee.dotted_path(), + _ => None, + }; let context = with_infer(ws, doc, |infer| event_string_context(infer, Some((call, arg_index)))); if let Some(context) = context { if !context.active { @@ -782,7 +872,6 @@ fn string_items(ws: &Workspace, doc: &Document, offset: u32, token_index: usize) }; return candidates(target_side.is_some()); } - let quote = if token.span.text(&doc.text).starts_with('"') { '"' } else { '\'' }; let literals = || with_infer(ws, doc, |infer| literal_items(infer, call, arg_index, range, quote)); let Some(path) = path else { return literals() }; let path = path.as_str(); @@ -824,21 +913,36 @@ fn string_items(ws: &Workspace, doc: &Document, offset: u32, token_index: usize) literals() } -/// A string value that an argument can take. -struct ArgumentLiteral { - value: SmolStr, - /// The type of the first parameter that lists it, such as the alias `Actions`. +/// A value that an argument, or a value written elsewhere, can take. +struct ListedValue { + /// A string, integer or boolean literal, or `nil`. + value: Type, + /// The type that lists it, such as the alias `Actions`: for an argument, that of the first + /// parameter that does. ty: Type, /// The signatures that take it alone, which a call passing it picks. taken_alone_by: Vec, } -impl ArgumentLiteral { +impl ListedValue { + /// The value when it is a string. + fn string(&self) -> Option<&SmolStr> { + match &self.value { + Type::StringLit(value) => Some(value), + _ => None, + } + } + /// The item for the `index`th value, in the order the values are declared. fn item(&self, index: usize, label: &str) -> CompletionItem { - let mut out = item(label, CompletionItemKind::ENUM_MEMBER, 0); + // `true`, `false` and `nil` look the way they do among the names in scope. + let kind = match self.value { + Type::BooleanLit(_) | Type::Nil => CompletionItemKind::KEYWORD, + _ => CompletionItemKind::ENUM_MEMBER, + }; + let mut out = item(label, kind, 0); out.sort_text = Some(format!("{index:04}")); - if !matches!(self.ty, Type::StringLit(_)) { + if !self.ty.is_literal() { out.detail = Some(self.ty.to_string()); } if !self.taken_alone_by.is_empty() { @@ -847,33 +951,67 @@ impl ArgumentLiteral { } out } + + /// The item that writes the value, a string in quotes, after `space`, where no quote is typed + /// yet. + fn written_item(&self, index: usize, quote: char, space: &str) -> CompletionItem { + let (written, filter) = match self.string() { + Some(value) => (format!("{quote}{}{quote}", escaped_string(value, quote)), value.to_string()), + None => (self.value.to_string(), self.value.to_string()), + }; + let mut out = self.item(index, &written); + out.filter_text = Some(filter); + out.insert_text = Some(format!("{space}{written}")); + out + } + + /// The item that replaces the contents of the string at `range` with the value, when the value + /// is a string. + fn content_item(&self, index: usize, range: Range, quote: char) -> Option { + let value = self.string()?; + let mut out = self.item(index, value); + let value = escaped_string(value, quote); + if range.start.line == range.end.line { + out.text_edit = Some(TextEdit { range, new_text: value }.into()); + } else { + out.insert_text = Some(value); + } + Some(out) + } +} + +/// The values that `ty` lists, in the order they are declared. +fn listed_values(infer: &Infer, ty: &Type) -> Vec { + let values = infer.listed_literals(ty).into_iter(); + values.map(|value| ListedValue { value, ty: ty.clone(), taken_alone_by: Vec::new() }).collect() } -/// The string values that the parameter of the argument at `arg_index` lists in `signatures`, in the -/// order they are declared: the `"playerLoaded"` of `action: "playerLoaded"|"playerUnloaded"|string` -/// or of an alias, and the `"keyPressed"` of an `@overload` that takes `action: "keyPressed"`. The -/// signatures that take a value alone are shown under `name`. +/// The values that the parameter of the argument at `arg_index` lists in `signatures`, in the +/// order they are declared, with the `nil` that any of them allows last: the `"playerLoaded"` of +/// `action: "playerLoaded"|"playerUnloaded"|string` or of an alias, and the `"keyPressed"` of an +/// `@overload` that takes `action: "keyPressed"`. The signatures that take a value alone are shown +/// under `name`. fn argument_literals( infer: &Infer, signatures: &[(Arc, bool)], arg_index: usize, via_method: bool, name: &str, -) -> Vec { - let mut literals: Vec = Vec::new(); +) -> Vec { + let mut literals: Vec = Vec::new(); for (signature, _) in signatures { let Some(param) = param_for_argument(signature, arg_index, via_method) else { continue }; let pinned = infer.pinned_literal(¶m.ty); - for value in infer.string_literals(¶m.ty) { + for value in infer.listed_literals(¶m.ty) { let index = match literals.iter().position(|known| known.value == value) { Some(index) => index, None => { let ty = param.ty.clone(); - literals.push(ArgumentLiteral { value: value.clone(), ty, taken_alone_by: Vec::new() }); + literals.push(ListedValue { value: value.clone(), ty, taken_alone_by: Vec::new() }); literals.len() - 1 } }; - if pinned.as_ref().is_some_and(|pinned| *pinned == Type::StringLit(value.clone())) { + if pinned.as_ref() == Some(&value) { literals[index].taken_alone_by.push(signature.signature(name)); } } @@ -892,18 +1030,9 @@ fn literal_items(infer: &Infer, call: &Expr, arg_index: usize, range: Range, quo let via_method = method.is_some(); let signatures = infer.open_call_signatures(&fun, &args[..arg_index.min(args.len())], via_method, base.span.start); argument_literals(infer, &signatures, arg_index, via_method, &callee_name(base, method)) - .into_iter() + .iter() .enumerate() - .map(|(i, literal)| { - let mut out = literal.item(i, &literal.value); - let value = escaped_string(&literal.value, quote); - if range.start.line == range.end.line { - out.text_edit = Some(TextEdit { range, new_text: value }.into()); - } else { - out.insert_text = Some(value); - } - out - }) + .filter_map(|(i, literal)| literal.content_item(i, range, quote)) .collect() } diff --git a/crates/qbx_lua_ls/src/features/expected.rs b/crates/qbx_lua_ls/src/features/expected.rs new file mode 100644 index 0000000..7d4c50f --- /dev/null +++ b/crates/qbx_lua_ls/src/features/expected.rs @@ -0,0 +1,175 @@ +//! The type that a value has to be where it is written, which completion offers the values of: +//! the `---@type` above the `local` or assignment that stores it, the declared type of the local +//! or class field it is assigned to, the `@field` it sets in a class-typed table, the `@return` of +//! the function that returns it, and the declared type of what `==` or `~=` compares it with. + +use qbx_lua_syntax::ast::*; +use qbx_lua_syntax::visit::{self, Visitor}; +use qbx_lua_syntax::Span; +use rustc_hash::FxHashMap; + +use super::class_tables::{class_tables, entries, Classes, Key}; +use super::comparisons::Declared; +use crate::infer::{documented_functions, Infer}; +use crate::types::Type; + +/// Where a value is written. +#[derive(Clone, Copy)] +pub enum Place { + /// Right after the `=`, `==`, `~=` or `return` that ends at this offset, where the value is + /// still to be written or only starts. + After(u32), + /// The string literal with this span. + String(Span), +} + +/// What a value has to be where it is written. +pub struct ExpectedValue { + pub ty: Type, + /// The value is compared with one of this type rather than stored as one, so any value the + /// type lists can be written, but not a new value of the type such as a function. + pub compared: bool, +} + +impl ExpectedValue { + fn stored(ty: Type) -> Self { + Self { ty, compared: false } + } +} + +/// The type that the value at `place` has to be, when something declares it. +pub fn expected_type(infer: &Infer, chunk: &Chunk, place: Place) -> Option { + let mut finder = Finder { + infer, + declared: Declared::keeping_annotations(infer), + place, + function_returns: Vec::new(), + documented: FxHashMap::default(), + found: None, + }; + finder.visit_block(&chunk.block); + finder.found +} + +struct Finder<'a, 'b> { + infer: &'a Infer<'b>, + declared: Declared<'a, 'b>, + place: Place, + /// The `@return` types of the functions around the statement being visited, innermost last. + function_returns: Vec>, + /// The `@return` types of the functions whose doc comment was visited, by the start of their + /// parameter list. + documented: FxHashMap>, + found: Option, +} + +impl Finder<'_, '_> { + /// An offset that everything around the value contains. + fn at(&self) -> u32 { + match self.place { + Place::After(at) => at, + Place::String(span) => span.start, + } + } + + /// Whether `expr` is the value at the place. What follows an `=` that has no value yet is + /// parsed as its value, also from the next line. + fn is_value(&self, expr: &Expr) -> bool { + match self.place { + Place::After(at) => { + let between = self.infer.ctx.source.get(at as usize..expr.span.start as usize); + between.is_some_and(|between| between.trim().is_empty()) + } + Place::String(span) => expr.span == span, + } + } + + /// The type of the `@field` that `key` sets in `table`, when the table is typed as a class. + fn field(&self, table: &Expr, key: &Key) -> Option { + let tables = class_tables(self.infer, self.infer.ctx.chunk); + let found = tables.into_iter().find(|found| found.table.span == table.span)?; + Classes::new(self.infer).field_type(&found.class, found.from, key).map(|(ty, _)| ty) + } +} + +impl<'c> Visitor<'c> for Finder<'_, '_> { + fn visit_stmt(&mut self, stmt: &'c Stmt) { + if self.found.is_some() || !stmt.span.contains_inclusive(self.at()) { + return; + } + let functions = documented_functions(stmt); + if !functions.is_empty() { + let doc = self.infer.ctx.doc_at(stmt.span.start); + let returns: Vec = doc.returns.iter().map(|r| r.ty.clone()).collect(); + for func in functions { + self.documented.insert(func.params_span.start, returns.clone()); + } + } + match &stmt.kind { + StmtKind::Local { exprs, .. } if exprs.iter().any(|expr| self.is_value(expr)) => { + let doc = self.infer.ctx.doc_at(stmt.span.start); + // `---@class Name` above a table declares the class rather than an instance of it. + if doc.classes.is_empty() { + self.found = doc.ty.clone().map(ExpectedValue::stored); + } + } + StmtKind::Assign { targets, exprs } => { + if let Some(index) = exprs.iter().position(|expr| self.is_value(expr)) { + let doc = self.infer.ctx.doc_at(stmt.span.start); + let ty = doc.ty.clone().or_else(|| targets.get(index).map(|target| self.declared.of(target))); + self.found = ty.map(ExpectedValue::stored); + } + } + StmtKind::Return(exprs) => { + let index = match self.place { + Place::After(at) => (stmt.span.start + "return".len() as u32 == at).then_some(0), + Place::String(_) => exprs.iter().position(|expr| self.is_value(expr)), + }; + if let Some(index) = index { + let returns = self.function_returns.last(); + self.found = returns.and_then(|returns| returns.get(index)).cloned().map(ExpectedValue::stored); + } + } + _ => {} + } + if self.found.is_none() { + visit::walk_stmt(self, stmt); + } + } + + fn visit_func_body(&mut self, func: &'c FuncBody) { + let returns = self.documented.remove(&func.params_span.start).unwrap_or_default(); + self.function_returns.push(returns); + visit::walk_func_body(self, func); + self.function_returns.pop(); + } + + fn visit_expr(&mut self, expr: &'c Expr) { + if self.found.is_some() || !expr.span.contains_inclusive(self.at()) { + return; + } + match &expr.kind { + ExprKind::Binary { op: BinOp::Eq | BinOp::Ne, op_span, lhs, rhs } => { + let other = match self.place { + Place::After(at) => (op_span.end == at).then_some(lhs), + Place::String(_) if self.is_value(rhs) => Some(lhs), + Place::String(_) => self.is_value(lhs).then_some(rhs), + }; + if let Some(other) = other { + self.found = Some(ExpectedValue { ty: self.declared.of(other), compared: true }); + } + } + ExprKind::Table(fields) => { + let mut entries = entries(self.infer, fields).into_iter(); + let entry = entries.find(|(_, _, value)| value.is_some_and(|value| self.is_value(value))); + if let Some((key, ..)) = entry { + self.found = self.field(expr, &key).map(ExpectedValue::stored); + } + } + _ => {} + } + if self.found.is_none() { + visit::walk_expr(self, expr); + } + } +} diff --git a/crates/qbx_lua_ls/src/features/mod.rs b/crates/qbx_lua_ls/src/features/mod.rs index 91dd6fd..c91f5ce 100644 --- a/crates/qbx_lua_ls/src/features/mod.rs +++ b/crates/qbx_lua_ls/src/features/mod.rs @@ -8,6 +8,7 @@ pub mod definition; pub mod diagnostics; pub mod doc_names; pub mod event_call; +pub mod expected; pub mod folding; pub mod hover; pub mod inlay; diff --git a/crates/qbx_lua_ls/src/infer.rs b/crates/qbx_lua_ls/src/infer.rs index 5c194cc..18f0a86 100644 --- a/crates/qbx_lua_ls/src/infer.rs +++ b/crates/qbx_lua_ls/src/infer.rs @@ -1260,26 +1260,42 @@ impl<'a> Infer<'a> { fits.into_iter().filter(|(_, fit)| *fit != Fit::No).map(|(signature, fit)| (signature, fit == best)).collect() } - /// The string literals a type lists, through aliases and unions: `"a"` and `"b"` of `"a"|"b"|string`. - pub fn string_literals(&self, ty: &Type) -> Vec { + /// The values a type lists, through aliases and unions: `"a"` and `"b"` of `"a"|"b"|string`, `1` + /// and `2` of `1|2`, and `true` and `false` of `boolean`. A type that lists any also lists the + /// `nil` it allows, last; alone, `nil` tells that a value may be missing, not what it can be. + pub fn listed_literals(&self, ty: &Type) -> Vec { let mut out = Vec::new(); - self.collect_string_literals(ty, 0, &mut out); + self.collect_listed_literals(ty, 0, &mut out); + let listed = out.len(); + out.retain(|value| !matches!(value, Type::Nil)); + if listed > out.len() && !out.is_empty() { + out.push(Type::Nil); + } out } - fn collect_string_literals(&self, ty: &Type, depth: u32, out: &mut Vec) { + fn collect_listed_literals(&self, ty: &Type, depth: u32, out: &mut Vec) { if depth > 8 { return; } match ty { - Type::StringLit(value) if !out.contains(value) => out.push(value.clone()), + Type::StringLit(_) | Type::IntLit(_) | Type::BooleanLit(_) | Type::Nil if !out.contains(ty) => { + out.push(ty.clone()) + } + Type::Boolean => { + for value in [Type::BooleanLit(true), Type::BooleanLit(false)] { + if !out.contains(&value) { + out.push(value); + } + } + } Type::Named(..) => match self.resolve_alias(ty) { Type::Named(..) => {} - resolved => self.collect_string_literals(&resolved, depth + 1, out), + resolved => self.collect_listed_literals(&resolved, depth + 1, out), }, Type::Union(types) => { for part in types { - self.collect_string_literals(part, depth + 1, out); + self.collect_listed_literals(part, depth + 1, out); } } _ => {} diff --git a/crates/qbx_lua_ls/src/server.rs b/crates/qbx_lua_ls/src/server.rs index 2e10755..2808427 100644 --- a/crates/qbx_lua_ls/src/server.rs +++ b/crates/qbx_lua_ls/src/server.rs @@ -108,6 +108,8 @@ pub fn capabilities() -> ServerCapabilities { "{".into(), "(".into(), ",".into(), + "=".into(), + " ".into(), ]), ..CompletionOptions::default() }), diff --git a/crates/qbx_lua_ls/tests/lsp.rs b/crates/qbx_lua_ls/tests/lsp.rs index 306f154..723e622 100644 --- a/crates/qbx_lua_ls/tests/lsp.rs +++ b/crates/qbx_lua_ls/tests/lsp.rs @@ -4853,6 +4853,243 @@ function Emitter:on(event, mode) end assert_eq!(loaded.map(|item| &item["filterText"]), Some(&json!("playerLoaded")), "{result}"); } +#[test] +fn values_offer_what_their_declared_type_lists() { + let mut client = Client::start(fixture_root()); + let defs = "\ +---@alias State \"busy\" | \"working\" | \"ready\" +---@alias Level 1 | 2 | 3 +---@alias Volume \"mute\" | 0 | 100 + +---@class Machine +---@field state State +---@field mode \"auto\"|\"manual\" +---@field fallback? State +---@field label string +---@field level Level +---@field enabled boolean +---@field onStop? fun(reason: string) + +---@alias Handler fun(id: integer, name: string) + +---@param level Level +---@param volume? Volume +function SetLevel(level, volume) end + +---@param enabled boolean +---@param mode? \"auto\"|\"manual\"|false +function SetEnabled(enabled, mode) end +"; + client.open_with("myresource/shared/config.lua", defs); + // `|` marks the cursor; `trigger` is the character typed to ask for completions. Items come back + // in the order their `sortText` lists them, as label and inserted text. + let mut values = |typed: &str, trigger: Option<&str>| -> Option> { + let (line, column) = pos(typed, "|", 0); + client.open_with(CLIENT, &typed.replace('|', "")); + let mut params = client.position_params(CLIENT, line, column); + if let Some(trigger) = trigger { + params["context"] = json!({ "triggerKind": 2, "triggerCharacter": trigger }); + } + let result = client.request("textDocument/completion", params); + let mut items = result["items"].as_array()?.clone(); + items.sort_by_key(|item| item["sortText"].as_str().unwrap_or_default().to_string()); + Some( + items + .iter() + // The listed values are numbered, and a function literal sorts ahead of them. + .filter(|item| { + let sort = item["sortText"].as_str().unwrap_or_default(); + (sort.len() == 4 && sort.bytes().all(|b| b.is_ascii_digit())) || sort.starts_with(".function(") + }) + .map(|item| { + let label = item["label"].as_str().unwrap().to_string(); + let text = item["insertText"].as_str().or(item["textEdit"]["newText"].as_str()); + (label.clone(), text.map_or(label, str::to_string)) + }) + .collect(), + ) + }; + let quoted = |values: &[&str], space: &str| -> Option> { + Some(values.iter().map(|value| (format!("'{value}'"), format!("{space}'{value}'"))).collect()) + }; + let plain = |values: &[&str]| -> Option> { + Some(values.iter().map(|value| (value.to_string(), value.to_string())).collect()) + }; + let pairs = |pairs: &[(&str, &str)]| -> Option> { + Some(pairs.iter().map(|(label, text)| (label.to_string(), text.to_string())).collect()) + }; + let states = ["busy", "working", "ready"]; + let state = "---@type State\nlocal state = 'busy'\n"; + let machine = "---@type Machine\nlocal machine = GetMachine()\n"; + + // A `---@type` above a `local` lists the values when the `=` is typed, with a space, again when + // the space is, and when asked. + assert_eq!(values("---@type State\nlocal state =|", Some("=")), quoted(&states, " ")); + assert_eq!(values("---@type State\nlocal state = |", Some(" ")), quoted(&states, "")); + assert_eq!(values("---@type State\nlocal state = |", None), quoted(&states, "")); + // Strings take the quote most of the document's strings use. + assert_eq!( + values("local label = \"x\"\n---@type Machine\nlocal machine = { mode = | }", None), + Some(vec![ + ("\"auto\"".to_string(), "\"auto\"".to_string()), + ("\"manual\"".to_string(), "\"manual\"".to_string()) + ]) + ); + // An assignment takes what its target is declared as: a local that is assigned again keeps + // its annotation, and a field has the type of its `@field`. + assert_eq!(values(&format!("{state}state = |"), Some(" ")), quoted(&states, "")); + assert_eq!(values(&format!("{state}if state == 'busy' then\n\tstate = |\nend"), None), quoted(&states, "")); + assert_eq!(values(&format!("{state}state = 'ready'\nif state == |"), None), quoted(&states, "")); + assert_eq!(values(&format!("{machine}machine.state = |"), Some(" ")), quoted(&states, "")); + assert_eq!(values(&format!("{machine}machine.mode =|"), Some("=")), quoted(&["auto", "manual"], " ")); + assert_eq!(values("---@type State\nCurrent = |", None), quoted(&states, "")); + // A table typed as a class takes them for the field being set. + assert_eq!(values("---@type Machine\nlocal machine = { state = | }", Some(" ")), quoted(&states, "")); + assert_eq!( + values("---@type Machine\nlocal machine = {\n\tstate = 'busy',\n\tmode = |\n}", None), + quoted(&["auto", "manual"], "") + ); + // A `return` takes the `@return` of its function. + let returns = "---@return State\nlocal function current()\n\treturn |\nend"; + assert_eq!(values(returns, Some(" ")), quoted(&states, "")); + // A comparison takes what its other side is declared as. + let handle = "---@param state State\nlocal function handle(state)\n\t"; + assert_eq!(values(&format!("{handle}if state == | then\n\tend\nend"), Some(" ")), quoted(&states, "")); + assert_eq!(values(&format!("{handle}if state ~=| then\n\tend\nend"), Some("=")), quoted(&states, " ")); + assert_eq!(values(&format!("{handle}return state == |\nend"), None), quoted(&states, "")); + assert_eq!( + values(&format!("{handle}if state == 'busy' then\n\telseif state == | then\n\tend\nend"), None), + quoted(&states, "") + ); + assert_eq!(values(&format!("{machine}if machine.state == |"), Some(" ")), quoted(&states, "")); + assert_eq!(values(&format!("{state}if ready and state == |"), None), quoted(&states, "")); + // The statement on the next line is no value of the `=` that is still without one. + assert_eq!(values(&format!("{state}state = |\nprint(state)"), None), quoted(&states, "")); + // Inside a string, the values replace its contents. + assert_eq!(values("---@type State\nlocal state = '|'", None), plain(&states)); + assert_eq!(values("---@type State\nlocal state = '|", Some("'")), plain(&states)); + assert_eq!(values(&format!("{state}state = \"bu|\""), None), plain(&states)); + assert_eq!(values("---@type Machine\nlocal machine = { state = '|' }", None), plain(&states)); + assert_eq!(values(&format!("{handle}if state == '|' then\n\tend\nend"), None), plain(&states)); + assert_eq!(values(&format!("{handle}if 'busy|' ~= state then\n\tend\nend"), None), plain(&states)); + assert_eq!(values("---@return State\nlocal function current()\n\treturn '|'\nend", None), plain(&states)); + + // Integers are written as they are, in the order the type lists them beside its strings, and + // stay out of a string. + let levels = plain(&["1", "2", "3"]); + assert_eq!(values("---@type Level\nlocal level = |", Some(" ")), levels); + assert_eq!(values("---@type Level\nlocal level =|", Some("=")), pairs(&[("1", " 1"), ("2", " 2"), ("3", " 3")])); + assert_eq!(values(&format!("{machine}machine.level = |"), None), levels); + assert_eq!(values(&format!("{machine}if machine.level ~= | then\nend"), Some(" ")), levels); + assert_eq!(values("---@return Level\nlocal function current()\n\treturn |\nend", None), levels); + let volumes = pairs(&[("'mute'", "'mute'"), ("0", "0"), ("100", "100")]); + assert_eq!(values("---@type Volume\nlocal volume = |", None), volumes); + assert_eq!(values("---@type Volume\nlocal volume = '|'", None), plain(&["mute"])); + assert_eq!(values("---@type Level\nlocal level = '|'", None), Some(Vec::new())); + // So are those of an argument. + assert_eq!(values("SetLevel(|)", Some("(")), levels); + assert_eq!(values("SetLevel(1,|)", Some(",")), pairs(&[("'mute'", " 'mute'"), ("0", " 0"), ("100", " 100")])); + assert_eq!(values("SetLevel('|')", None), Some(Vec::new())); + assert_eq!(values("SetLevel(1, '|')", None), plain(&["mute"])); + + // A `boolean` lists `true` and `false`, and a type that lists values also lists the `nil` it + // allows, last. + let booleans = plain(&["true", "false"]); + assert_eq!(values("---@type boolean\nlocal enabled = |", Some(" ")), booleans); + assert_eq!( + values("---@type boolean\nlocal enabled =|", Some("=")), + pairs(&[("true", " true"), ("false", " false")]) + ); + assert_eq!(values("---@type boolean?\nlocal enabled = |", None), plain(&["true", "false", "nil"])); + assert_eq!(values(&format!("{machine}machine.enabled = |"), Some(" ")), booleans); + assert_eq!(values(&format!("{machine}if machine.enabled == | then\nend"), Some(" ")), booleans); + assert_eq!(values("---@return boolean\nlocal function ok()\n\treturn |\nend", Some(" ")), booleans); + let optional_states = + pairs(&[("'busy'", "'busy'"), ("'working'", "'working'"), ("'ready'", "'ready'"), ("nil", "nil")]); + assert_eq!(values("---@type State?\nlocal state = |", Some(" ")), optional_states); + assert_eq!( + values("---@param state? State\nlocal function handle(state)\n\tif state ~= | then\n\tend\nend", None), + optional_states + ); + assert_eq!(values(&format!("{machine}machine.fallback = |"), None), optional_states); + assert_eq!(values("---@type boolean\nlocal enabled = '|'", None), Some(Vec::new())); + // A typed `(` or `,` opens the list for strings and integers, not for booleans alone, which + // are listed when asked for. + assert_eq!(values("SetEnabled(|)", Some("(")), None); + assert_eq!(values("SetEnabled(|)", None), booleans); + assert_eq!( + values("SetEnabled(true,|)", Some(",")), + pairs(&[("'auto'", " 'auto'"), ("'manual'", " 'manual'"), ("false", " false")]) + ); + + // A value that is stored or returned as a function gets a function literal with the + // parameters its type names, but one that is compared with a function does not. + let on_stop = pairs(&[("function(reason)", "function(${1:reason})\n\t$0\nend")]); + assert_eq!(values("---@type Machine\nlocal machine = { onStop = | }", Some(" ")), on_stop); + assert_eq!(values(&format!("{machine}machine.onStop = |"), None), on_stop); + assert_eq!( + values("---@type Handler\nlocal handler =|", Some("=")), + pairs(&[("function(id, name)", " function(${1:id, name})\n\t$0\nend")]) + ); + assert_eq!( + values("---@return fun()\nlocal function make()\n\treturn |\nend", Some(" ")), + pairs(&[("function()", "function()\n\t$0\nend")]) + ); + assert_eq!(values("---@type function\nlocal run = |", None), pairs(&[("function()", "function($1)\n\t$0\nend")])); + assert_eq!(values(&format!("{machine}if machine.onStop == | then\nend"), Some(" ")), None); + + // A typed `=` or space asks for nothing else, and for nothing where no type lists values. + for (typed, trigger) in [ + ("local state = |", " "), + ("local state =|", "="), + ("local |", " "), + ("print(1, |)", " "), + ("---@type string\nlocal label = |", " "), + // `nil` alone tells that a value may be missing, not what it can be. + ("---@type string?\nlocal label = |", " "), + ("---@type Machine\nlocal machine = { label = | }", " "), + ("---@type State\nlocal state = 'busy'\nlocal ready = state >= |", " "), + ("---@type State\nlocal state = 'busy'\nlocal ready = state >=|", "="), + ("---@type State\nlocal state = 'busy' -- state = |", " "), + ("---@type State\nlocal state = 'a = |'", " "), + // The value after the cursor is already written. + ("---@type State\nlocal state = | 'busy'", " "), + // What the code assigns tells what a local holds today, not what it may. + ("local mode = 'dev'\nmode = |", " "), + ] { + let found = values(typed, Some(trigger)); + assert!(found.is_none(), "{typed}: {found:?}"); + } + + // A typed word filters them by the value, beside the names in scope. + let (line, column) = pos("local bucket = 1\n---@type State\nlocal state = bu|", "|", 0); + client.open_with(CLIENT, "local bucket = 1\n---@type State\nlocal state = bu"); + let result = client.request("textDocument/completion", client.position_params(CLIENT, line, column)); + let items = result["items"].as_array().unwrap(); + let busy = items.iter().find(|item| item["label"] == "'busy'"); + assert_eq!(busy.map(|item| &item["filterText"]), Some(&json!("busy")), "{result}"); + assert_eq!(busy.map(|item| &item["detail"]), Some(&json!("State")), "{result}"); + assert!(items.iter().any(|item| item["label"] == "bucket"), "{result}"); + // `true` is listed once, as the value, and not again as a keyword. + let (line, column) = pos("---@type boolean\nlocal enabled = t|", "|", 0); + client.open_with(CLIENT, "---@type boolean\nlocal enabled = t"); + let result = client.request("textDocument/completion", client.position_params(CLIENT, line, column)); + let listed: Vec<&Value> = + result["items"].as_array().unwrap().iter().filter(|item| item["label"] == "true").collect(); + assert_eq!(listed.len(), 1, "{result}"); + assert_eq!(listed[0]["sortText"], "0000", "{result}"); + assert_eq!(listed[0]["detail"], "boolean", "{result}"); + + // A call snippet writes quotes only around a stop whose parameter lists strings. + let (line, column) = pos("SetLev|", "|", 0); + client.open_with(CLIENT, "SetLev"); + let result = client.request("textDocument/completion", client.position_params(CLIENT, line, column)); + let items = result["items"].as_array().unwrap(); + let snippets: Vec<&Value> = + items.iter().filter(|item| item["label"] == "SetLevel" && item["insertTextFormat"] == 2).collect(); + assert!(snippets.is_empty(), "{snippets:?}"); +} + #[test] fn call_snippets_leave_listed_values_to_the_list() { let defs = "\ From d370f3e022c9b7b489e1209a6580c3a1b394c93b Mon Sep 17 00:00:00 2001 From: Loaf <60279520+loaf-scripts@users.noreply.github.com> Date: Wed, 30 Sep 2026 18:51:25 +0200 Subject: [PATCH 02/12] fix(luacats): give each name of a `---@type` list its own type A `---@type` line that lists several types, as `---@type boolean, string?` does above `local ok, err = pcall(...)`, was read up to its first comma, and that one type was given to every name of the statement: `err` was a `boolean`. The line now types the names one by one, and a name past the end of the list keeps the type inferred from its value. One type still covers every name. Hovers, the values completion offers, the class a table constructor is checked against and `impossible-comparison` follow the type of the name at hand. --- .../qbx_lua_ls/src/features/class_tables.rs | 14 ++++++---- crates/qbx_lua_ls/src/features/comparisons.rs | 4 +-- crates/qbx_lua_ls/src/features/expected.rs | 15 ++++++----- crates/qbx_lua_ls/src/infer.rs | 2 +- crates/qbx_luacats/src/luacats.rs | 26 ++++++++++++++++++- 5 files changed, 46 insertions(+), 15 deletions(-) diff --git a/crates/qbx_lua_ls/src/features/class_tables.rs b/crates/qbx_lua_ls/src/features/class_tables.rs index 71c7162..745eb15 100644 --- a/crates/qbx_lua_ls/src/features/class_tables.rs +++ b/crates/qbx_lua_ls/src/features/class_tables.rs @@ -726,17 +726,21 @@ impl<'c> Visitor<'c> for Finder<'_, '_, 'c> { StmtKind::Local { exprs, .. } if exprs.iter().any(is_table) => { let doc = self.classes.infer.ctx.doc_at(stmt.span.start); // `---@class Name` above a table declares the class rather than an instance of it. - if let (true, Some(ty)) = (doc.classes.is_empty(), &doc.ty) { - for expr in exprs { - self.top_table(ty, expr); + if doc.classes.is_empty() { + for (index, expr) in exprs.iter().enumerate() { + if let Some(ty) = doc.type_at(index) { + self.top_table(ty, expr); + } } } } StmtKind::Assign { targets, exprs } if exprs.iter().any(is_table) => { let doc = self.classes.infer.ctx.doc_at(stmt.span.start); if doc.classes.is_empty() { - for (target, expr) in targets.iter().zip(exprs).filter(|(_, expr)| is_table(expr)) { - let expected = doc.ty.clone().unwrap_or_else(|| self.classes.infer.expr(target)); + let values = targets.iter().zip(exprs).enumerate(); + for (index, (target, expr)) in values.filter(|(_, (_, expr))| is_table(expr)) { + let declared = doc.type_at(index).cloned(); + let expected = declared.unwrap_or_else(|| self.classes.infer.expr(target)); self.top_table(&expected, expr); } } diff --git a/crates/qbx_lua_ls/src/features/comparisons.rs b/crates/qbx_lua_ls/src/features/comparisons.rs index a49cad9..ee0a890 100644 --- a/crates/qbx_lua_ls/src/features/comparisons.rs +++ b/crates/qbx_lua_ls/src/features/comparisons.rs @@ -126,7 +126,7 @@ impl<'a, 'b> Declared<'a, 'b> { /// Whether a `---@type` above its declaration or a `@param` line types the local. fn is_annotated(&self, local: &Local) -> bool { match self.infer.ctx.decl(local.decl.start) { - Some(Decl::Local { stmt, .. }) => self.infer.ctx.doc_at(stmt.span.start).ty.is_some(), + Some(Decl::Local { stmt, index }) => self.infer.ctx.doc_at(stmt.span.start).type_at(*index).is_some(), Some(Decl::Param { .. }) => has_param_line(self.infer, local), _ => false, } @@ -138,7 +138,7 @@ impl<'a, 'b> Declared<'a, 'b> { fn local_value(&self, id: LocalId, stmt: &Stmt, index: usize, depth: u32) -> Type { let StmtKind::Local { exprs, in_unpack: false, .. } = &stmt.kind else { return Type::Unknown }; let doc = self.infer.ctx.doc_at(stmt.span.start); - if doc.ty.is_some() || !doc.classes.is_empty() { + if doc.type_at(index).is_some() || !doc.classes.is_empty() { return self.infer.local_type(id); } match exprs.get(index) { diff --git a/crates/qbx_lua_ls/src/features/expected.rs b/crates/qbx_lua_ls/src/features/expected.rs index 7d4c50f..2954796 100644 --- a/crates/qbx_lua_ls/src/features/expected.rs +++ b/crates/qbx_lua_ls/src/features/expected.rs @@ -106,17 +106,20 @@ impl<'c> Visitor<'c> for Finder<'_, '_> { } } match &stmt.kind { - StmtKind::Local { exprs, .. } if exprs.iter().any(|expr| self.is_value(expr)) => { - let doc = self.infer.ctx.doc_at(stmt.span.start); - // `---@class Name` above a table declares the class rather than an instance of it. - if doc.classes.is_empty() { - self.found = doc.ty.clone().map(ExpectedValue::stored); + StmtKind::Local { exprs, .. } => { + if let Some(index) = exprs.iter().position(|expr| self.is_value(expr)) { + let doc = self.infer.ctx.doc_at(stmt.span.start); + // `---@class Name` above a table declares the class rather than an instance of it. + if doc.classes.is_empty() { + self.found = doc.type_at(index).cloned().map(ExpectedValue::stored); + } } } StmtKind::Assign { targets, exprs } => { if let Some(index) = exprs.iter().position(|expr| self.is_value(expr)) { let doc = self.infer.ctx.doc_at(stmt.span.start); - let ty = doc.ty.clone().or_else(|| targets.get(index).map(|target| self.declared.of(target))); + let declared = doc.type_at(index).cloned(); + let ty = declared.or_else(|| targets.get(index).map(|target| self.declared.of(target))); self.found = ty.map(ExpectedValue::stored); } } diff --git a/crates/qbx_lua_ls/src/infer.rs b/crates/qbx_lua_ls/src/infer.rs index 18f0a86..70f99d4 100644 --- a/crates/qbx_lua_ls/src/infer.rs +++ b/crates/qbx_lua_ls/src/infer.rs @@ -617,7 +617,7 @@ impl<'a> Infer<'a> { if let Some(class) = doc.classes.last() { return Type::Named(class.name.clone(), Vec::new()); } - if let Some(ty) = &doc.ty { + if let Some(ty) = doc.type_at(index) { return ty.clone(); } if *in_unpack { diff --git a/crates/qbx_luacats/src/luacats.rs b/crates/qbx_luacats/src/luacats.rs index cc9b592..13d0a64 100644 --- a/crates/qbx_luacats/src/luacats.rs +++ b/crates/qbx_luacats/src/luacats.rs @@ -80,6 +80,9 @@ pub struct DocGroup { /// position holds across them. pub return_sets: Vec>, pub ty: Option, + /// The types a `@type` line lists after its first, as `---@type string, number` does for the + /// second name of the statement below. + pub ty_rest: Vec, pub enum_name: Option, pub enum_keys: bool, pub enum_side: Option, @@ -97,6 +100,16 @@ impl DocGroup { !self.params.is_empty() || !self.returns.is_empty() || !self.overloads.is_empty() } + /// The `@type` of the name at `index` of the statement below. A line that lists several types + /// gives each name its own, and one type covers every name. + pub fn type_at(&self, index: usize) -> Option<&Type> { + if index == 0 || self.ty_rest.is_empty() { + self.ty.as_ref() + } else { + self.ty_rest.get(index - 1) + } + } + /// Builds the function type from `@param`/`@return`, keeping the order of the real parameters. pub fn fun_type(&self, param_names: &[SmolStr], has_vararg: bool, is_method: bool) -> FunType { let mut params: Vec = param_names @@ -362,7 +375,11 @@ pub fn parse_doc_lines(lines: &[&str]) -> DocGroup { } } } - "type" => group.ty = Some(TypeParser::new(rest).parse()), + "type" => { + let mut types = TypeParser::new(rest).parse_list().into_iter(); + group.ty = types.next(); + group.ty_rest = types.collect(); + } "generic" => { let names = rest.split(',').filter_map(|g| g.trim().split([':', ' ']).next()).filter(|g| !g.is_empty()); group.generics.extend(names.map(SmolStr::new)); @@ -817,7 +834,14 @@ mod tests { fn type_and_generics() { let doc = parse("---@generic T: table, K\n---@type table"); assert_eq!(doc.generics, ["T", "K"]); + assert_eq!(doc.type_at(1).unwrap().to_string(), "table", "one type covers every name"); assert_eq!(doc.ty.unwrap().to_string(), "table"); + + let doc = parse("---@type boolean, table | string"); + let at = |index| doc.type_at(index).map(Type::to_string); + assert_eq!(at(0).as_deref(), Some("boolean")); + assert_eq!(at(1).as_deref(), Some("table|string")); + assert_eq!(at(2), None, "a list types no more names than it has types"); } #[test] From fa6cd6015e9ccace3f35292a5a1b20b164b2469c Mon Sep 17 00:00:00 2001 From: Loaf <60279520+loaf-scripts@users.noreply.github.com> Date: Wed, 30 Sep 2026 18:51:57 +0200 Subject: [PATCH 03/12] feat(rules): check the values stored in typed variables `assign-type-mismatch` only covered the fields of class tables, so a variable took any value whatever its annotation said: ---@alias State 1 | 2 | 3 | false | true ---@type State local state = "test" state = "abc" Both values are now reported, as "Cannot assign `"test"` to `state` of type `State`". The rule checks: - the values of a `local` or an assignment with a `---@type` above it, - later assignments to a local declared with `---@type`, - later assignments to a parameter documented with `@param`. A mismatch is what it is for `return-type-mismatch`: a different kind of value, or a literal the type does not list. Values whose type is not known pass. A call at the end of the values gives each name the value it returns at that position. A local is compared with the type it is declared with, not with what the guards around the assignment narrow it to, so `name = "x"` inside `if not name then` passes for a `string?`. `= nil` needs a type that allows it, while a `local` declared without a value is not reported. Locals without an annotation take any value, since their type is inferred from what they hold, and so do globals in assignments that have no `---@type` above them. A value that a class field rejects is reported once, for the field. --- crates/qbx_lua_analysis/src/rules.rs | 2 +- crates/qbx_lua_ls/README.md | 3 +- crates/qbx_lua_ls/src/features/assignments.rs | 100 ++++++++++++++++++ .../qbx_lua_ls/src/features/class_tables.rs | 14 +++ crates/qbx_lua_ls/src/features/diagnostics.rs | 5 +- crates/qbx_lua_ls/src/features/mod.rs | 1 + crates/qbx_lua_ls/src/features/returns.rs | 17 +-- crates/qbx_lua_ls/tests/lsp.rs | 88 +++++++++++++++ docs/reference.md | 27 +++++ docs/rules.md | 2 +- 10 files changed, 239 insertions(+), 20 deletions(-) create mode 100644 crates/qbx_lua_ls/src/features/assignments.rs diff --git a/crates/qbx_lua_analysis/src/rules.rs b/crates/qbx_lua_analysis/src/rules.rs index 0bedd61..0a18ba7 100644 --- a/crates/qbx_lua_analysis/src/rules.rs +++ b/crates/qbx_lua_analysis/src/rules.rs @@ -139,7 +139,7 @@ pub static RULES: &[Rule] = &[ rule(MISSING_PARAMETER, Correctness, WARN, false, "A function is called without an argument for a parameter its LuaCATS annotations require."), rule(UNDEFINED_DOC_NAME, Correctness, WARN, false, "A LuaCATS annotation names a type that no @class, @alias or @enum declares for the file's side (language server only)."), rule(MISSING_FIELDS, Correctness, WARN, false, "A table constructor typed as a LuaCATS class leaves out required fields (language server only)."), - rule(ASSIGN_TYPE_MISMATCH, Correctness, WARN, false, "A table constructor or assignment sets a field of a LuaCATS class to a value of the wrong type (language server only)."), + rule(ASSIGN_TYPE_MISMATCH, Correctness, WARN, false, "A table constructor or assignment stores a value of the wrong type in a field of a LuaCATS class, or in a variable typed with @type or @param (language server only)."), rule(UNDECLARED_FIELD, Correctness, WARN, false, "A field or key that a strict LuaCATS class does not declare is set or read (language server only)."), rule(RETURN_TYPE_MISMATCH, Correctness, WARN, false, "A function returns a value of a different type than its @return annotation declares (language server only)."), rule(MISSING_RETURN, Correctness, WARN, false, "A function with a required @return value can end, or return, without it (language server only)."), diff --git a/crates/qbx_lua_ls/README.md b/crates/qbx_lua_ls/README.md index 05b6d59..9087bf6 100644 --- a/crates/qbx_lua_ls/README.md +++ b/crates/qbx_lua_ls/README.md @@ -42,7 +42,8 @@ Available features depend on the editor's LSP client. [sets of values](#sets-of-returned-values) such as `false | (string, string)`. - Diagnostics and quick fixes with resource and client/server context, plus LuaCATS type checks: `missing-fields` and `assign-type-mismatch` for tables and assignments that leave out required - fields of their class or store a value of the wrong type, `undeclared-field` for fields and keys + fields of their class or store a value of the wrong type, in a field or in a variable typed with + `---@type` or `@param`, `undeclared-field` for fields and keys a `---@class (strict)` does not declare, and `return-type-mismatch` and `missing-return` for functions, including callback handlers, that do not return what their `@return` declares. `impossible-comparison` reports an `==` or `~=` between values whose declared types share no diff --git a/crates/qbx_lua_ls/src/features/assignments.rs b/crates/qbx_lua_ls/src/features/assignments.rs new file mode 100644 index 0000000..e09f3bb --- /dev/null +++ b/crates/qbx_lua_ls/src/features/assignments.rs @@ -0,0 +1,100 @@ +//! `assign-type-mismatch` for variables: a `local` or an assignment with a `---@type` above it has +//! to store values of that type, and so does a later assignment to a local declared with +//! `---@type` or to a parameter documented with `@param`. As for `@return`, a value of a different +//! kind, or a literal the type does not list, is a mismatch. `class_tables` checks the fields of +//! class tables. + +use qbx_lua_analysis::scope::{LocalId, Resolved}; +use qbx_lua_syntax::ast::*; +use qbx_lua_syntax::visit::{self, Visitor}; +use qbx_lua_syntax::Span; + +use super::class_tables::{mismatched_fields, Classes}; +use super::unknown_types::has_param_line; +use crate::infer::{Decl, Infer}; +use crate::types::Type; + +/// Each value stored in a class field or a variable whose type does not take it, with the message +/// naming both types. A value that its field rejects is reported for the field alone. +pub fn mismatched_assignments(infer: &Infer, chunk: &Chunk) -> Vec<(Span, String)> { + let mut out = mismatched_fields(infer, chunk); + let mut finder = Finder { infer, classes: Classes::new(infer), out: Vec::new() }; + finder.visit_block(&chunk.block); + finder.out.retain(|(span, _)| !out.iter().any(|(reported, _)| reported == span)); + out.append(&mut finder.out); + out +} + +struct Finder<'a, 'b> { + infer: &'a Infer<'b>, + classes: Classes<'a, 'b>, + out: Vec<(Span, String)>, +} + +impl Finder<'_, '_> { + /// The `---@type` above `stmt` for the name at `index`. `---@class Name` above a table + /// declares the class rather than a value of it. + fn stmt_type(&self, stmt: &Stmt, index: usize) -> Option { + let doc = self.infer.ctx.doc_at(stmt.span.start); + doc.type_at(index).filter(|_| doc.classes.is_empty()).cloned() + } + + /// The type a local is declared with: the `---@type` above its `local` statement, or its + /// `@param` line. What the guards around an assignment tell about the local does not limit + /// what it may be given. + fn local_type(&self, id: LocalId) -> Option { + let ctx = self.infer.ctx; + let local = ctx.resolution.local(id); + match ctx.decl(local.decl.start)? { + Decl::Local { stmt, index } => self.stmt_type(stmt, *index), + Decl::Param { .. } if has_param_line(self.infer, local) => Some(self.infer.local_type(id)), + _ => None, + } + } + + /// Reports each of `exprs` that the declared type of the target it is stored in does not take. + /// A target is where its name is written, with its type when one is declared. + fn check(&mut self, targets: &[(Span, Option)], exprs: &[Expr]) { + if targets.iter().all(|(_, ty)| ty.is_none()) { + return; + } + let from = self.classes.file(); + for ((name, expected), (given, span)) in targets.iter().zip(self.classes.values(exprs)) { + let Some(expected) = expected else { continue }; + if self.classes.rejects(expected, from, &given) { + let shown = if self.classes.literal_mismatch(expected, from, &given) { given } else { given.widen() }; + let name = name.text(self.infer.ctx.source); + self.out.push((span, format!("Cannot assign `{shown}` to `{name}` of type `{expected}`"))); + } + } + } +} + +impl<'c> Visitor<'c> for Finder<'_, '_> { + fn visit_stmt(&mut self, stmt: &'c Stmt) { + match &stmt.kind { + StmtKind::Local { names, exprs, in_unpack: false } => { + let declared = |(index, name): (usize, &AttribName)| (name.name.span, self.stmt_type(stmt, index)); + let targets: Vec<_> = names.iter().enumerate().map(declared).collect(); + self.check(&targets, exprs); + } + StmtKind::Assign { targets, exprs } => { + // The `---@type` above the assignment, or else the type its target is declared with. + let declared = |(index, target): (usize, &Expr)| { + let ty = self.stmt_type(stmt, index).or_else(|| match &target.kind { + ExprKind::Name(name) => match self.infer.ctx.resolution.resolve_at(name.span.start) { + Some(Resolved::Local(id)) => self.local_type(id), + _ => None, + }, + _ => None, + }); + (target.span, ty) + }; + let targets: Vec<_> = targets.iter().enumerate().map(declared).collect(); + self.check(&targets, exprs); + } + _ => {} + } + visit::walk_stmt(self, stmt); + } +} diff --git a/crates/qbx_lua_ls/src/features/class_tables.rs b/crates/qbx_lua_ls/src/features/class_tables.rs index 745eb15..1966806 100644 --- a/crates/qbx_lua_ls/src/features/class_tables.rs +++ b/crates/qbx_lua_ls/src/features/class_tables.rs @@ -360,6 +360,20 @@ impl<'a, 'b> Classes<'a, 'b> { self.infer.expr(expr) } + /// The values that `exprs` give a `return` or an assignment, with where each is written. A call + /// at the end gives every value it returns. + pub fn values(&self, exprs: &[Expr]) -> Vec<(Type, Span)> { + let mut values = Vec::new(); + for (i, expr) in exprs.iter().enumerate() { + if i + 1 == exprs.len() && expr.is_call() { + values.extend(self.infer.expr_multi(expr).into_iter().map(|ty| (ty, expr.span))); + } else { + values.push((self.value_type(expr), expr.span)); + } + } + values + } + /// Whether a field of type `ty` may be left out: `string?`, `string|nil`, `any` or no type. pub fn admits_nil(&self, ty: &Type, from: FileId) -> bool { self.admits_nil_at(ty, from, 0) diff --git a/crates/qbx_lua_ls/src/features/diagnostics.rs b/crates/qbx_lua_ls/src/features/diagnostics.rs index fa98a47..3902657 100644 --- a/crates/qbx_lua_ls/src/features/diagnostics.rs +++ b/crates/qbx_lua_ls/src/features/diagnostics.rs @@ -14,8 +14,9 @@ use qbx_lua_analysis::{ use qbx_lua_syntax::Span; use serde::{Deserialize, Serialize}; +use super::assignments::mismatched_assignments; use super::callback_payloads::missing_payloads; -use super::class_tables::{mismatched_fields, missing_fields}; +use super::class_tables::missing_fields; use super::comparisons::impossible_comparisons; use super::doc_names::undefined_doc_names; use super::returns::{mismatched_returns, missing_returns}; @@ -59,7 +60,7 @@ fn type_diagnostics(ws: &Workspace, doc: &Document, config: &FileConfig) -> Vec< undefined_doc_names(&ws.index, &doc.text, &doc.chunk, side) }), (MISSING_FIELDS, |ws, doc| with_infer(ws, doc, |infer| missing_fields(infer, &doc.chunk))), - (ASSIGN_TYPE_MISMATCH, |ws, doc| with_infer(ws, doc, |infer| mismatched_fields(infer, &doc.chunk))), + (ASSIGN_TYPE_MISMATCH, |ws, doc| with_infer(ws, doc, |infer| mismatched_assignments(infer, &doc.chunk))), (UNDECLARED_FIELD, |ws, doc| { // Without a strict class there is nothing to find, so no assignment needs its type inferred. if !ws.lint_config.strict_classes && !ws.index.has_strict_class() { diff --git a/crates/qbx_lua_ls/src/features/mod.rs b/crates/qbx_lua_ls/src/features/mod.rs index c91f5ce..d0c0016 100644 --- a/crates/qbx_lua_ls/src/features/mod.rs +++ b/crates/qbx_lua_ls/src/features/mod.rs @@ -1,3 +1,4 @@ +pub mod assignments; pub mod assistant; pub mod callback_payloads; pub mod class_tables; diff --git a/crates/qbx_lua_ls/src/features/returns.rs b/crates/qbx_lua_ls/src/features/returns.rs index f112e3e..f1c76c8 100644 --- a/crates/qbx_lua_ls/src/features/returns.rs +++ b/crates/qbx_lua_ls/src/features/returns.rs @@ -24,7 +24,7 @@ pub fn mismatched_returns(infer: &Infer, chunk: &Chunk) -> Vec<(Span, String)> { let mut out = Vec::new(); for function in documented(infer, chunk) { for (_, exprs) in return_stmts(&function.func.body) { - let values = returned_values(infer, &classes, exprs); + let values = classes.values(exprs); out.extend(mismatches(&classes, function.closest(&classes, exprs, &values), &values)); } } @@ -74,7 +74,7 @@ pub fn missing_returns(infer: &Infer, chunk: &Chunk) -> Vec<(Span, String)> { let required = match sets.is_empty() { true => required, false => { - let values = returned_values(infer, &classes, exprs); + let values = classes.values(exprs); required_values(&classes, function.closest(&classes, exprs, &values)) } }; @@ -126,19 +126,6 @@ fn expected_at(returns: &[Type], index: usize) -> Option<&Type> { } } -/// The values one `return` passes with where each is written, the last spread when it is a call. -fn returned_values(infer: &Infer, classes: &Classes, exprs: &[Expr]) -> Vec<(Type, Span)> { - let mut values = Vec::new(); - for (i, expr) in exprs.iter().enumerate() { - if i + 1 == exprs.len() && expr.is_call() { - values.extend(infer.expr_multi(expr).into_iter().map(|ty| (ty, expr.span))); - } else { - values.push((classes.value_type(expr), expr.span)); - } - } - values -} - /// A function documented with `@return`. struct Documented<'c> { func: &'c FuncBody, diff --git a/crates/qbx_lua_ls/tests/lsp.rs b/crates/qbx_lua_ls/tests/lsp.rs index 723e622..f8c0e7f 100644 --- a/crates/qbx_lua_ls/tests/lsp.rs +++ b/crates/qbx_lua_ls/tests/lsp.rs @@ -2803,6 +2803,94 @@ for i = 1, 2 do print(tuple[i]) end ); } +#[test] +fn values_assigned_to_typed_variables_are_checked() { + let mut client = Client::start(fixture_root()); + let text = "\ +---@alias Test.State 1 | 2 | 3 | false | true + +---@type Test.State +local state = 'test' +state = 'abc' +state = 2 +state = 4 +state = nil + +---@type Test.State? +local maybe = nil +if not maybe then maybe = 1 end +maybe = {} + +---@type string +local unset +unset = 'later' + +---@type 'a'|'b' +local letter = 'c' +letter = state + +---@class Test.Holder +---@field name string +local Holder = {} +Holder = nil + +---@type Test.Holder +local holder = { name = 'a' } +---@type number +holder.name = true +---@type string +holder.label = 5 +---@type string +TestLabel = false + +---@return string, integer +local function pair() return 'a', 1 end +---@type string +local first, second = pair() +---@type string, integer? +local name, count, extra = pair() +name, count, extra = 1, 'two', 3 + +---@param id number +---@param label? string +local function show(id, label, other) + id = tostring(id) + label = label or 'none' + label = nil + other = 1 +end + +local plain = 1 +plain = 'text' +"; + client.open_with(CLIENT, text); + let finding = |needle: &str, message: &str| { + ("assign-type-mismatch".to_string(), pos(text, needle, 0).0 as u64, message.to_string()) + }; + assert_eq!( + findings(&mut client, CLIENT, &["assign-type-mismatch"]), + [ + finding("local state", "Cannot assign `\"test\"` to `state` of type `Test.State`"), + finding("state = 'abc'", "Cannot assign `\"abc\"` to `state` of type `Test.State`"), + finding("state = 4", "Cannot assign `4` to `state` of type `Test.State`"), + finding("state = nil", "Cannot assign `nil` to `state` of type `Test.State`"), + finding("maybe = {}", "Cannot assign `table` to `maybe` of type `Test.State?`"), + finding("local letter", "Cannot assign `\"c\"` to `letter` of type `\"a\"|\"b\"`"), + // The `@field` decides what a class field takes, and reports it once. + finding("holder.name = true", "Cannot assign `boolean` to field `name` of type `string`"), + finding("holder.label = 5", "Cannot assign `integer` to `holder.label` of type `string`"), + finding("TestLabel = false", "Cannot assign `boolean` to `TestLabel` of type `string`"), + finding("second = pair()", "Cannot assign `integer` to `second` of type `string`"), + // A list of types gives each name its own, and the names after it none. + finding("name, count, extra = 1", "Cannot assign `integer` to `name` of type `string`"), + finding("name, count, extra = 1", "Cannot assign `string` to `count` of type `integer?`"), + finding("id = tostring(id)", "Cannot assign `string` to `id` of type `number`"), + ], + "values the type takes, a guarded local, a local without a value, a reassigned local as the value, \ + the table a `---@class` declares, optional and undocumented parameters and untyped locals pass" + ); +} + #[test] fn literal_keyed_class_fields_type_their_own_keys() { let mut client = Client::start(fixture_root()); diff --git a/docs/reference.md b/docs/reference.md index 59ae8e7..b7ce033 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -365,6 +365,33 @@ assigned again after its declaration, since its declared type may not be what it applies to the values `assign-type-mismatch` checks, where `nil` is a value like any other: `abc.field = nil` needs a field type that allows it, such as `string?` or `string|nil`. +## Typed variables + +`assign-type-mismatch` also covers variables whose type is declared. A `local` or an assignment +with a `---@type` above it has to store values of that type, and so does every later assignment to +a local declared with `---@type` or to a parameter documented with `@param`: + +```lua +---@alias State 1 | 2 | 3 | false | true + +---@type State +local state = "test" -- Cannot assign `"test"` to `state` of type `State` + +state = "abc" -- Cannot assign `"abc"` to `state` of type `State` +state = nil -- Cannot assign `nil` to `state` of type `State` +``` + +The check is that of `return-type-mismatch`: a different kind of value, or a literal the type does +not list. A local is compared with the type it is declared with, not with what the guards around +the assignment narrow it to, so `name = "x"` inside `if not name then` passes for a `string?`. A +`local` declared without a value is not reported, while `= nil` needs a type that allows it. A call +at the end of the values gives each name the value it returns at that position. + +A `---@type` line that lists several types gives each name of the statement its own, as +`---@type boolean, string?` does above `local ok, err = pcall(...)`; one type covers every name. +Locals without a `---@type`, whose type is inferred from what they hold, take any value, and so do +globals in assignments that have no `---@type` above them. + ## Impossible comparisons `impossible-comparison` reports an `==` or `~=` whose two sides can never be equal, so that the diff --git a/docs/rules.md b/docs/rules.md index 32d20eb..0e02d74 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -31,7 +31,7 @@ set any rule to `off`, `hint`, `info`, `warning`, or `error`. | `missing-parameter` | warning | A call leaves out an argument that the function's LuaCATS annotations require. qbx-lua-ls also reports `---@callback` wrapper calls whose payload leaves out a value the registered handler requires. | | `undefined-doc-name` | warning | A LuaCATS annotation names a type that no `@class`, `@alias` or `@enum` declares, or that only `(server)` or `(client)` declarations of the other side declare. Reported by qbx-lua-ls only. | | `missing-fields` | warning | A table constructor typed as a `---@class`, including one returned for an `@return` type, leaves out required fields. Reported by qbx-lua-ls only. | -| `assign-type-mismatch` | warning | A table constructor or an assignment stores a value that the `@field` type, or the value type of an index such as `[string] number`, does not take, such as a number for a `string` field. Clearing a field with `nil` needs a type that allows it, such as `string?` or `string|nil`. Reported by qbx-lua-ls only. | +| `assign-type-mismatch` | warning | A table constructor or an assignment stores a value that the `@field` type, or the value type of an index such as `[string] number`, does not take, such as a number for a `string` field. Clearing a field with `nil` needs a type that allows it, such as `string?` or `string|nil`. The same goes for a value stored in a variable typed with `---@type` or `@param`, such as `"test"` for `---@type number`. Reported by qbx-lua-ls only. | | `undeclared-field` | warning | A field or key that a strict `---@class`, marked `(strict)` or `(exact)` or made strict by `strict_classes`, does not declare is set in a table constructor or assignment, or read. Reported by qbx-lua-ls only. | | `return-type-mismatch` | warning | A function returns a value that its `@return` type does not take, such as a number for `string`. Reported by qbx-lua-ls only. | | `missing-return` | warning | A function with a required `@return` value can reach its end without returning, or has a `return` with fewer values than required. Reported by qbx-lua-ls only. | From 03bb03a48601a06820fb5a3e653e6b3d008e1348 Mon Sep 17 00:00:00 2001 From: ChatDisabled <44729807+ChatDisabled@users.noreply.github.com> Date: Thu, 1 Oct 2026 03:59:02 +0200 Subject: [PATCH 04/12] fix(luacats): give a single `@type` to the first name only --- crates/qbx_lua_ls/tests/lsp.rs | 7 +++++-- crates/qbx_luacats/src/luacats.rs | 12 ++++++------ docs/reference.md | 8 +++++--- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/crates/qbx_lua_ls/tests/lsp.rs b/crates/qbx_lua_ls/tests/lsp.rs index f8c0e7f..0fe6df5 100644 --- a/crates/qbx_lua_ls/tests/lsp.rs +++ b/crates/qbx_lua_ls/tests/lsp.rs @@ -2880,8 +2880,8 @@ plain = 'text' finding("holder.name = true", "Cannot assign `boolean` to field `name` of type `string`"), finding("holder.label = 5", "Cannot assign `integer` to `holder.label` of type `string`"), finding("TestLabel = false", "Cannot assign `boolean` to `TestLabel` of type `string`"), - finding("second = pair()", "Cannot assign `integer` to `second` of type `string`"), - // A list of types gives each name its own, and the names after it none. + // A list of types gives each name its own, and the names after it none, as one type + // gives every name after the first. finding("name, count, extra = 1", "Cannot assign `integer` to `name` of type `string`"), finding("name, count, extra = 1", "Cannot assign `string` to `count` of type `integer?`"), finding("id = tostring(id)", "Cannot assign `string` to `id` of type `number`"), @@ -2889,6 +2889,9 @@ plain = 'text' "values the type takes, a guarded local, a local without a value, a reassigned local as the value, \ the table a `---@class` declares, optional and undocumented parameters and untyped locals pass" ); + let (line, column) = pos(text, "second = pair()", 0); + let hover = client.hover_text(CLIENT, line, column); + assert!(hover.contains("local second: integer"), "{hover}"); } #[test] diff --git a/crates/qbx_luacats/src/luacats.rs b/crates/qbx_luacats/src/luacats.rs index 13d0a64..f131a2e 100644 --- a/crates/qbx_luacats/src/luacats.rs +++ b/crates/qbx_luacats/src/luacats.rs @@ -101,12 +101,11 @@ impl DocGroup { } /// The `@type` of the name at `index` of the statement below. A line that lists several types - /// gives each name its own, and one type covers every name. + /// gives each name its own, and one type only the first name, as LuaLS binds it. pub fn type_at(&self, index: usize) -> Option<&Type> { - if index == 0 || self.ty_rest.is_empty() { - self.ty.as_ref() - } else { - self.ty_rest.get(index - 1) + match index { + 0 => self.ty.as_ref(), + _ => self.ty_rest.get(index - 1), } } @@ -834,7 +833,8 @@ mod tests { fn type_and_generics() { let doc = parse("---@generic T: table, K\n---@type table"); assert_eq!(doc.generics, ["T", "K"]); - assert_eq!(doc.type_at(1).unwrap().to_string(), "table", "one type covers every name"); + assert_eq!(doc.type_at(0).unwrap().to_string(), "table"); + assert_eq!(doc.type_at(1), None, "one type is the first name's alone"); assert_eq!(doc.ty.unwrap().to_string(), "table"); let doc = parse("---@type boolean, table | string"); diff --git a/docs/reference.md b/docs/reference.md index b7ce033..a075729 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -388,9 +388,11 @@ the assignment narrow it to, so `name = "x"` inside `if not name then` passes fo at the end of the values gives each name the value it returns at that position. A `---@type` line that lists several types gives each name of the statement its own, as -`---@type boolean, string?` does above `local ok, err = pcall(...)`; one type covers every name. -Locals without a `---@type`, whose type is inferred from what they hold, take any value, and so do -globals in assignments that have no `---@type` above them. +`---@type boolean, string?` does above `local ok, err = pcall(...)`. A single type is the first +name's alone, as lua-language-server binds it, so `---@type boolean` above +`local onScreen, x, y = GetScreenCoordFromWorldCoord(...)` types only `onScreen`. Names without a +type of their own, and other locals whose type is inferred from what they hold, take any value, +and so do globals in assignments that have no `---@type` above them. ## Impossible comparisons From 66af0ea216ba935b6ee127ad679d894eef72068d Mon Sep 17 00:00:00 2001 From: ChatDisabled <44729807+ChatDisabled@users.noreply.github.com> Date: Thu, 1 Oct 2026 03:59:49 +0200 Subject: [PATCH 05/12] fix(ls): type each global of an assignment from its `@type` list --- crates/qbx_lua_ls/src/indexer.rs | 22 ++++++++++++---------- crates/qbx_lua_ls/tests/lsp.rs | 7 +++++++ 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/crates/qbx_lua_ls/src/indexer.rs b/crates/qbx_lua_ls/src/indexer.rs index 0013570..8bc3315 100644 --- a/crates/qbx_lua_ls/src/indexer.rs +++ b/crates/qbx_lua_ls/src/indexer.rs @@ -244,11 +244,13 @@ impl<'a> Indexer<'a> { } /// The symbol for `name = value`, registering nested table fields under `nested_owner`. + /// `position` is where `name` is among the targets of its statement, which picks its `@type`. fn value_symbol( &mut self, name: &Name, value: Option<&Expr>, doc_anchor: u32, + position: usize, nested_owner: &str, table_depth: u32, ) -> Symbol { @@ -260,7 +262,7 @@ impl<'a> Indexer<'a> { } kind = SymbolKind::Table; Type::Named(class.name.clone(), Vec::new()) - } else if let Some(ty) = &doc.ty { + } else if let Some(ty) = doc.type_at(position) { ty.clone() } else { match value.map(|v| (&v.kind, v)) { @@ -343,7 +345,7 @@ impl<'a> Indexer<'a> { TableField::Keyed { .. } | TableField::Positional(_) | TableField::SetMember(_) => continue, }; let nested = format!("{owner}.{}", name.text); - let mut symbol = self.value_symbol(&name, Some(value), name.span.start, &nested, depth); + let mut symbol = self.value_symbol(&name, Some(value), name.span.start, 0, &nested, depth); if symbol.kind == SymbolKind::Variable { symbol.kind = SymbolKind::Field; } @@ -409,7 +411,7 @@ impl<'a> Indexer<'a> { } StmtKind::Assign { targets, exprs } => { for (i, target) in targets.iter().enumerate() { - self.assignment(stmt, target, exprs.get(i)); + self.assignment(stmt, target, i, exprs.get(i)); self.expr(target); } exprs.iter().for_each(|e| self.expr(e)); @@ -488,29 +490,29 @@ impl<'a> Indexer<'a> { self.push_member(owner, symbol, injected); } - fn assignment(&mut self, stmt: &Stmt, target: &Expr, value: Option<&Expr>) { + fn assignment(&mut self, stmt: &Stmt, target: &Expr, position: usize, value: Option<&Expr>) { match &target.kind { ExprKind::Name(name) if self.is_global(name) => { - let symbol = self.value_symbol(name, value, stmt.span.start, &name.text.clone(), 0); + let symbol = self.value_symbol(name, value, stmt.span.start, position, &name.text.clone(), 0); self.out.globals.push(symbol); } ExprKind::Field { base, name, .. } if !name.is_missing() => { - self.member_assignment(stmt, base, name, value); + self.member_assignment(stmt, base, name, position, value); } ExprKind::Index { base, index, .. } => { if let Some(text) = index.as_string() { let name = Name { text: text.clone(), span: index.span }; - self.member_assignment(stmt, base, &name, value); + self.member_assignment(stmt, base, &name, position, value); } } _ => {} } } - fn member_assignment(&mut self, stmt: &Stmt, base: &Expr, name: &Name, value: Option<&Expr>) { + fn member_assignment(&mut self, stmt: &Stmt, base: &Expr, name: &Name, position: usize, value: Option<&Expr>) { if let ExprKind::Name(root) = &base.kind { if self.is_global(root) && matches!(root.text.as_str(), "_ENV" | "_G") { - let symbol = self.value_symbol(name, value, stmt.span.start, &name.text.clone(), 0); + let symbol = self.value_symbol(name, value, stmt.span.start, position, &name.text.clone(), 0); self.out.globals.push(symbol); return; } @@ -536,7 +538,7 @@ impl<'a> Indexer<'a> { }; let class_table = matches!(&base.kind, ExprKind::Name(root) if self.is_class_table(root)); let nested = format!("{owner}.{}", name.text); - let mut symbol = self.value_symbol(name, value, stmt.span.start, &nested, 1); + let mut symbol = self.value_symbol(name, value, stmt.span.start, position, &nested, 1); if symbol.kind == SymbolKind::Variable { symbol.kind = SymbolKind::Field; } diff --git a/crates/qbx_lua_ls/tests/lsp.rs b/crates/qbx_lua_ls/tests/lsp.rs index 0fe6df5..ed16e35 100644 --- a/crates/qbx_lua_ls/tests/lsp.rs +++ b/crates/qbx_lua_ls/tests/lsp.rs @@ -2842,6 +2842,10 @@ holder.name = true holder.label = 5 ---@type string TestLabel = false +---@type string, integer +TestName, TestCount = 'a', 1 +---@type integer +local counted = TestCount ---@return string, integer local function pair() return 'a', 1 end @@ -2892,6 +2896,9 @@ plain = 'text' let (line, column) = pos(text, "second = pair()", 0); let hover = client.hover_text(CLIENT, line, column); assert!(hover.contains("local second: integer"), "{hover}"); + let (line, column) = pos(text, "TestCount =", 0); + let hover = client.hover_text(CLIENT, line, column); + assert!(hover.contains("TestCount: integer"), "{hover}"); } #[test] From 5a5737d110e25bca5bd2f9638f0e96c4c342281f Mon Sep 17 00:00:00 2001 From: ChatDisabled <44729807+ChatDisabled@users.noreply.github.com> Date: Thu, 1 Oct 2026 04:00:53 +0200 Subject: [PATCH 06/12] fix(rules): let the statement under a `@type` set its names to nil --- crates/qbx_lua_ls/src/features/assignments.rs | 34 ++++++++++++++----- crates/qbx_lua_ls/tests/lsp.rs | 10 ++++++ docs/reference.md | 6 ++-- docs/rules.md | 2 +- 4 files changed, 40 insertions(+), 12 deletions(-) diff --git a/crates/qbx_lua_ls/src/features/assignments.rs b/crates/qbx_lua_ls/src/features/assignments.rs index e09f3bb..9fc4c54 100644 --- a/crates/qbx_lua_ls/src/features/assignments.rs +++ b/crates/qbx_lua_ls/src/features/assignments.rs @@ -53,42 +53,58 @@ impl Finder<'_, '_> { } /// Reports each of `exprs` that the declared type of the target it is stored in does not take. - /// A target is where its name is written, with its type when one is declared. - fn check(&mut self, targets: &[(Span, Option)], exprs: &[Expr]) { - if targets.iter().all(|(_, ty)| ty.is_none()) { + fn check(&mut self, targets: &[Target], exprs: &[Expr]) { + if targets.iter().all(|target| target.ty.is_none()) { return; } let from = self.classes.file(); - for ((name, expected), (given, span)) in targets.iter().zip(self.classes.values(exprs)) { - let Some(expected) = expected else { continue }; + for (index, (target, (given, span))) in targets.iter().zip(self.classes.values(exprs)).enumerate() { + let Some(expected) = &target.ty else { continue }; + // LuaLS lets the statement whose `---@type` declares a name give it `nil`. + if target.annotated && exprs.get(index).is_some_and(|expr| matches!(expr.kind, ExprKind::Nil)) { + continue; + } if self.classes.rejects(expected, from, &given) { let shown = if self.classes.literal_mismatch(expected, from, &given) { given } else { given.widen() }; - let name = name.text(self.infer.ctx.source); + let name = target.name.text(self.infer.ctx.source); self.out.push((span, format!("Cannot assign `{shown}` to `{name}` of type `{expected}`"))); } } } } +/// Where a value is stored: the name it is written to, the type declared for it, and whether the +/// `---@type` of the statement that stores it declares that type. +struct Target { + name: Span, + ty: Option, + annotated: bool, +} + impl<'c> Visitor<'c> for Finder<'_, '_> { fn visit_stmt(&mut self, stmt: &'c Stmt) { match &stmt.kind { StmtKind::Local { names, exprs, in_unpack: false } => { - let declared = |(index, name): (usize, &AttribName)| (name.name.span, self.stmt_type(stmt, index)); + let declared = |(index, name): (usize, &AttribName)| { + let ty = self.stmt_type(stmt, index); + Target { name: name.name.span, annotated: ty.is_some(), ty } + }; let targets: Vec<_> = names.iter().enumerate().map(declared).collect(); self.check(&targets, exprs); } StmtKind::Assign { targets, exprs } => { // The `---@type` above the assignment, or else the type its target is declared with. let declared = |(index, target): (usize, &Expr)| { - let ty = self.stmt_type(stmt, index).or_else(|| match &target.kind { + let own = self.stmt_type(stmt, index); + let annotated = own.is_some(); + let ty = own.or_else(|| match &target.kind { ExprKind::Name(name) => match self.infer.ctx.resolution.resolve_at(name.span.start) { Some(Resolved::Local(id)) => self.local_type(id), _ => None, }, _ => None, }); - (target.span, ty) + Target { name: target.span, ty, annotated } }; let targets: Vec<_> = targets.iter().enumerate().map(declared).collect(); self.check(&targets, exprs); diff --git a/crates/qbx_lua_ls/tests/lsp.rs b/crates/qbx_lua_ls/tests/lsp.rs index ed16e35..265dee1 100644 --- a/crates/qbx_lua_ls/tests/lsp.rs +++ b/crates/qbx_lua_ls/tests/lsp.rs @@ -2821,6 +2821,14 @@ local maybe = nil if not maybe then maybe = 1 end maybe = {} +---@type number +local handle = nil +if handle then handle = nil end +---@type string +TestZone = nil +---@type number, string +local width, caption = nil, nil + ---@type string local unset unset = 'later' @@ -2879,6 +2887,8 @@ plain = 'text' finding("state = 4", "Cannot assign `4` to `state` of type `Test.State`"), finding("state = nil", "Cannot assign `nil` to `state` of type `Test.State`"), finding("maybe = {}", "Cannot assign `table` to `maybe` of type `Test.State?`"), + // The statement whose `---@type` declares a name may give it `nil`, as in LuaLS. + finding("then handle = nil", "Cannot assign `nil` to `handle` of type `number`"), finding("local letter", "Cannot assign `\"c\"` to `letter` of type `\"a\"|\"b\"`"), // The `@field` decides what a class field takes, and reports it once. finding("holder.name = true", "Cannot assign `boolean` to field `name` of type `string`"), diff --git a/docs/reference.md b/docs/reference.md index a075729..2df3f3b 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -384,8 +384,10 @@ state = nil -- Cannot assign `nil` to `state` of type `State` The check is that of `return-type-mismatch`: a different kind of value, or a literal the type does not list. A local is compared with the type it is declared with, not with what the guards around the assignment narrow it to, so `name = "x"` inside `if not name then` passes for a `string?`. A -`local` declared without a value is not reported, while `= nil` needs a type that allows it. A call -at the end of the values gives each name the value it returns at that position. +`local` declared without a value is not reported, and neither is the `nil` that the statement under +a `---@type` gives the names it declares, as lua-language-server allows `---@type number` above +`Hp = nil`. A later `= nil` needs a type that allows it. A call at the end of the values gives each +name the value it returns at that position. A `---@type` line that lists several types gives each name of the statement its own, as `---@type boolean, string?` does above `local ok, err = pcall(...)`. A single type is the first diff --git a/docs/rules.md b/docs/rules.md index 0e02d74..0e2b593 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -31,7 +31,7 @@ set any rule to `off`, `hint`, `info`, `warning`, or `error`. | `missing-parameter` | warning | A call leaves out an argument that the function's LuaCATS annotations require. qbx-lua-ls also reports `---@callback` wrapper calls whose payload leaves out a value the registered handler requires. | | `undefined-doc-name` | warning | A LuaCATS annotation names a type that no `@class`, `@alias` or `@enum` declares, or that only `(server)` or `(client)` declarations of the other side declare. Reported by qbx-lua-ls only. | | `missing-fields` | warning | A table constructor typed as a `---@class`, including one returned for an `@return` type, leaves out required fields. Reported by qbx-lua-ls only. | -| `assign-type-mismatch` | warning | A table constructor or an assignment stores a value that the `@field` type, or the value type of an index such as `[string] number`, does not take, such as a number for a `string` field. Clearing a field with `nil` needs a type that allows it, such as `string?` or `string|nil`. The same goes for a value stored in a variable typed with `---@type` or `@param`, such as `"test"` for `---@type number`. Reported by qbx-lua-ls only. | +| `assign-type-mismatch` | warning | A table constructor or an assignment stores a value that the `@field` type, or the value type of an index such as `[string] number`, does not take, such as a number for a `string` field. Clearing a field with `nil` needs a type that allows it, such as `string?` or `string|nil`. The same goes for a value stored in a variable typed with `---@type` or `@param`, such as `"test"` for `---@type number`, except for the `nil` that the statement under a `---@type` gives the names it declares. Reported by qbx-lua-ls only. | | `undeclared-field` | warning | A field or key that a strict `---@class`, marked `(strict)` or `(exact)` or made strict by `strict_classes`, does not declare is set in a table constructor or assignment, or read. Reported by qbx-lua-ls only. | | `return-type-mismatch` | warning | A function returns a value that its `@return` type does not take, such as a number for `string`. Reported by qbx-lua-ls only. | | `missing-return` | warning | A function with a required `@return` value can reach its end without returning, or has a `return` with fewer values than required. Reported by qbx-lua-ls only. | From fcfd9d44b88bfc102667c065a8a7d97897a524e0 Mon Sep 17 00:00:00 2001 From: ChatDisabled <44729807+ChatDisabled@users.noreply.github.com> Date: Thu, 1 Oct 2026 04:01:50 +0200 Subject: [PATCH 07/12] fix(completion): offer no values in front of a word already written --- crates/qbx_lua_ls/docs/protocol.md | 4 +++- crates/qbx_lua_ls/src/features/completion.rs | 8 +++++--- crates/qbx_lua_ls/tests/lsp.rs | 8 ++++++++ 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/crates/qbx_lua_ls/docs/protocol.md b/crates/qbx_lua_ls/docs/protocol.md index fa4a642..af90dbf 100644 --- a/crates/qbx_lua_ls/docs/protocol.md +++ b/crates/qbx_lua_ls/docs/protocol.md @@ -114,7 +114,9 @@ literal where a function is stored or returned, though not where one is compared `---@type` above the `local` or assignment, what the assigned local or class field is declared as, the `@field` that a table typed as a class sets, the `@return` of the function, or what the other side of the comparison is declared as. A local typed by `---@type` or `@param` keeps that type -where it is assigned again; types inferred from assigned values list nothing. `=` and space are +where it is assigned again; types inferred from assigned values list nothing. Nothing is offered +in front of a value already written, such as the `other` of `local enabled = other` with the +cursor before it, or in a word the cursor is in. `=` and space are completion trigger characters for these items only: a request whose `context.triggerCharacter` is `=` or a space returns nothing else, and returns `null` where no type lists values or takes a function. Inside a string in one of those places, or on either side of the comparison, the string diff --git a/crates/qbx_lua_ls/src/features/completion.rs b/crates/qbx_lua_ls/src/features/completion.rs index 2985e47..4d8c8bc 100644 --- a/crates/qbx_lua_ls/src/features/completion.rs +++ b/crates/qbx_lua_ls/src/features/completion.rs @@ -629,8 +629,7 @@ fn value_items( let typed = &before[..before.len() - prefix.len()]; let head = typed.trim_end(); let line_end = doc.text[offset as usize..].find('\n').map_or(doc.text.len(), |i| offset as usize + i); - let rest = doc.text[offset as usize..line_end].trim_start_matches(|c: char| c.is_ascii_alphanumeric() || c == '_'); - if !takes_value(head) || !leaves_value_open(rest) { + if !takes_value(head) || !leaves_value_open(&doc.text[offset as usize..line_end]) { return Vec::new(); } let at = offset - (before.len() - head.len()) as u32; @@ -658,8 +657,11 @@ fn takes_value(head: &str) -> bool { /// Whether `rest`, the text after the cursor on its line, has no value that an inserted one would /// end up in front of: it is empty, a comment, or what closes or follows a value, like the `then` -/// of `if state == then`. +/// of `if state == then`. A word that starts right at the cursor is a value already written. fn leaves_value_open(rest: &str) -> bool { + if rest.starts_with(|c: char| c.is_ascii_alphanumeric() || c == '_') { + return false; + } let rest = rest.trim_start(); let word = rest.trim_start_matches(|c: char| c.is_ascii_alphanumeric() || c == '_'); let word = &rest[..rest.len() - word.len()]; diff --git a/crates/qbx_lua_ls/tests/lsp.rs b/crates/qbx_lua_ls/tests/lsp.rs index 265dee1..07048b2 100644 --- a/crates/qbx_lua_ls/tests/lsp.rs +++ b/crates/qbx_lua_ls/tests/lsp.rs @@ -5162,12 +5162,20 @@ function SetEnabled(enabled, mode) end ("---@type State\nlocal state = 'a = |'", " "), // The value after the cursor is already written. ("---@type State\nlocal state = | 'busy'", " "), + ("---@type boolean\nlocal enabled = |other", " "), + ("---@type boolean\nlocal enabled = |123", " "), + ("---@type boolean\nlocal enabled = |true", " "), + ("---@type State\nlocal state = 'busy'\nif state == |other then end", " "), // What the code assigns tells what a local holds today, not what it may. ("local mode = 'dev'\nmode = |", " "), ] { let found = values(typed, Some(trigger)); assert!(found.is_none(), "{typed}: {found:?}"); } + // Nor are they in front of the rest of a word, which a value would run into. + assert_eq!(values("---@type boolean\nlocal enabled = |other", None), None); + assert_eq!(values("---@type boolean\nlocal enabled = t|other", None), Some(Vec::new())); + assert_eq!(values("---@type boolean\nlocal enabled = t|", None), plain(&["true", "false"])); // A typed word filters them by the value, beside the names in scope. let (line, column) = pos("local bucket = 1\n---@type State\nlocal state = bu|", "|", 0); From 970ee5683cb85dff8a99e6a0e6e58bc7526dc580 Mon Sep 17 00:00:00 2001 From: ChatDisabled <44729807+ChatDisabled@users.noreply.github.com> Date: Thu, 1 Oct 2026 04:03:44 +0200 Subject: [PATCH 08/12] fix(completion): list values on a typed space only, as incomplete --- crates/qbx_lua_ls/README.md | 3 +- crates/qbx_lua_ls/docs/protocol.md | 12 ++++---- crates/qbx_lua_ls/src/features/completion.rs | 8 ++--- crates/qbx_lua_ls/src/server.rs | 8 ++++- crates/qbx_lua_ls/tests/lsp.rs | 31 +++++++++++--------- 5 files changed, 37 insertions(+), 25 deletions(-) diff --git a/crates/qbx_lua_ls/README.md b/crates/qbx_lua_ls/README.md index 9087bf6..452f387 100644 --- a/crates/qbx_lua_ls/README.md +++ b/crates/qbx_lua_ls/README.md @@ -27,7 +27,8 @@ Available features depend on the editor's LSP client. an overload, so the handler written next matches it. - The same values where a value of such a type, like `"busy"|"ready"`, `1|2|3` or `boolean`, is assigned, set as a field of a class, returned or compared with `==` or `~=`, and the function - literal where a function is assigned or returned. + literal where a function is assigned or returned. They open on the space typed after the `=`, + `==`, `~=` or `return`, beside the names in scope once a word is typed. - `---@` continuation on Enter inside LuaCATS annotation blocks, through on-type formatting. - Reference hovers for literal control IDs in PAD natives and ped configuration flags in `SetPedConfigFlag` / `GetPedConfigFlag`, using bundled Cfx documentation. diff --git a/crates/qbx_lua_ls/docs/protocol.md b/crates/qbx_lua_ls/docs/protocol.md index af90dbf..0af059e 100644 --- a/crates/qbx_lua_ls/docs/protocol.md +++ b/crates/qbx_lua_ls/docs/protocol.md @@ -116,11 +116,13 @@ the `@field` that a table typed as a class sets, the `@return` of the function, side of the comparison is declared as. A local typed by `---@type` or `@param` keeps that type where it is assigned again; types inferred from assigned values list nothing. Nothing is offered in front of a value already written, such as the `other` of `local enabled = other` with the -cursor before it, or in a word the cursor is in. `=` and space are -completion trigger characters for these items only: a request whose `context.triggerCharacter` is -`=` or a space returns nothing else, and returns `null` where no type lists values or takes a -function. Inside a string in one of those places, or on either side of the comparison, the string -values replace the string's contents. +cursor before it, or in a word the cursor is in. A space is a completion trigger character for +these items only: a request whose `context.triggerCharacter` is a space returns nothing else, +returns `null` where no type lists values or takes a function, and marks its list `isIncomplete`, +so that the client asks again once a word is typed and gets the names in scope beside the values. +`=` is no trigger character, so Enter right after a typed `=` still starts a new line. Inside a +string in one of those places, or on either side of the comparison, the string values replace the +string's contents. The server requests file watches only when `workspace.didChangeWatchedFiles.dynamicRegistration` is `true`. It watches Lua, lint config, diff --git a/crates/qbx_lua_ls/src/features/completion.rs b/crates/qbx_lua_ls/src/features/completion.rs index 4d8c8bc..4681f97 100644 --- a/crates/qbx_lua_ls/src/features/completion.rs +++ b/crates/qbx_lua_ls/src/features/completion.rs @@ -449,14 +449,14 @@ pub fn completion( let items = with_infer(ws, doc, |infer| argument_items(infer, doc, offset, before, snippets, quote, true)); return (!items.is_empty()).then(|| respond(items, false)); } - // A typed `=` or space only asks for the values that what follows it can be, so that no other - // space opens a list. - if matches!(trigger_character, Some("=" | " ")) { + // A typed space only asks for the values that what follows it can be, so that no other space + // opens a list. The list is incomplete, so a typed word asks again for the names in scope too. + if trigger_character == Some(" ") { if comment.is_some() || in_string.is_some() { return None; } let items = value_items(ws, doc, offset, before, snippets, quote); - return (!items.is_empty()).then(|| respond(items, false)); + return (!items.is_empty()).then(|| respond(items, true)); } if let Some(comment) = comment { diff --git a/crates/qbx_lua_ls/src/server.rs b/crates/qbx_lua_ls/src/server.rs index 2808427..6664c0f 100644 --- a/crates/qbx_lua_ls/src/server.rs +++ b/crates/qbx_lua_ls/src/server.rs @@ -108,7 +108,6 @@ pub fn capabilities() -> ServerCapabilities { "{".into(), "(".into(), ",".into(), - "=".into(), " ".into(), ]), ..CompletionOptions::default() @@ -899,4 +898,11 @@ mod tests { .count(); assert_eq!(published, 1); } + + #[test] + fn completion_triggers_on_a_space_and_not_on_an_equals_sign() { + let triggers = capabilities().completion_provider.and_then(|c| c.trigger_characters).unwrap_or_default(); + assert!(triggers.iter().any(|c| c == " "), "{triggers:?}"); + assert!(!triggers.iter().any(|c| c == "="), "Enter after a typed `=` makes a new line: {triggers:?}"); + } } diff --git a/crates/qbx_lua_ls/tests/lsp.rs b/crates/qbx_lua_ls/tests/lsp.rs index 07048b2..2a1485d 100644 --- a/crates/qbx_lua_ls/tests/lsp.rs +++ b/crates/qbx_lua_ls/tests/lsp.rs @@ -5030,9 +5030,9 @@ function SetEnabled(enabled, mode) end let state = "---@type State\nlocal state = 'busy'\n"; let machine = "---@type Machine\nlocal machine = GetMachine()\n"; - // A `---@type` above a `local` lists the values when the `=` is typed, with a space, again when - // the space is, and when asked. - assert_eq!(values("---@type State\nlocal state =|", Some("=")), quoted(&states, " ")); + // A `---@type` above a `local` lists the values when the space after the `=` is typed, and when + // asked, with a space when asked right after the `=`. + assert_eq!(values("---@type State\nlocal state =|", None), quoted(&states, " ")); assert_eq!(values("---@type State\nlocal state = |", Some(" ")), quoted(&states, "")); assert_eq!(values("---@type State\nlocal state = |", None), quoted(&states, "")); // Strings take the quote most of the document's strings use. @@ -5049,7 +5049,7 @@ function SetEnabled(enabled, mode) end assert_eq!(values(&format!("{state}if state == 'busy' then\n\tstate = |\nend"), None), quoted(&states, "")); assert_eq!(values(&format!("{state}state = 'ready'\nif state == |"), None), quoted(&states, "")); assert_eq!(values(&format!("{machine}machine.state = |"), Some(" ")), quoted(&states, "")); - assert_eq!(values(&format!("{machine}machine.mode =|"), Some("=")), quoted(&["auto", "manual"], " ")); + assert_eq!(values(&format!("{machine}machine.mode =|"), None), quoted(&["auto", "manual"], " ")); assert_eq!(values("---@type State\nCurrent = |", None), quoted(&states, "")); // A table typed as a class takes them for the field being set. assert_eq!(values("---@type Machine\nlocal machine = { state = | }", Some(" ")), quoted(&states, "")); @@ -5063,7 +5063,7 @@ function SetEnabled(enabled, mode) end // A comparison takes what its other side is declared as. let handle = "---@param state State\nlocal function handle(state)\n\t"; assert_eq!(values(&format!("{handle}if state == | then\n\tend\nend"), Some(" ")), quoted(&states, "")); - assert_eq!(values(&format!("{handle}if state ~=| then\n\tend\nend"), Some("=")), quoted(&states, " ")); + assert_eq!(values(&format!("{handle}if state ~=| then\n\tend\nend"), None), quoted(&states, " ")); assert_eq!(values(&format!("{handle}return state == |\nend"), None), quoted(&states, "")); assert_eq!( values(&format!("{handle}if state == 'busy' then\n\telseif state == | then\n\tend\nend"), None), @@ -5086,7 +5086,7 @@ function SetEnabled(enabled, mode) end // stay out of a string. let levels = plain(&["1", "2", "3"]); assert_eq!(values("---@type Level\nlocal level = |", Some(" ")), levels); - assert_eq!(values("---@type Level\nlocal level =|", Some("=")), pairs(&[("1", " 1"), ("2", " 2"), ("3", " 3")])); + assert_eq!(values("---@type Level\nlocal level =|", None), pairs(&[("1", " 1"), ("2", " 2"), ("3", " 3")])); assert_eq!(values(&format!("{machine}machine.level = |"), None), levels); assert_eq!(values(&format!("{machine}if machine.level ~= | then\nend"), Some(" ")), levels); assert_eq!(values("---@return Level\nlocal function current()\n\treturn |\nend", None), levels); @@ -5104,10 +5104,7 @@ function SetEnabled(enabled, mode) end // allows, last. let booleans = plain(&["true", "false"]); assert_eq!(values("---@type boolean\nlocal enabled = |", Some(" ")), booleans); - assert_eq!( - values("---@type boolean\nlocal enabled =|", Some("=")), - pairs(&[("true", " true"), ("false", " false")]) - ); + assert_eq!(values("---@type boolean\nlocal enabled =|", None), pairs(&[("true", " true"), ("false", " false")])); assert_eq!(values("---@type boolean?\nlocal enabled = |", None), plain(&["true", "false", "nil"])); assert_eq!(values(&format!("{machine}machine.enabled = |"), Some(" ")), booleans); assert_eq!(values(&format!("{machine}if machine.enabled == | then\nend"), Some(" ")), booleans); @@ -5136,7 +5133,7 @@ function SetEnabled(enabled, mode) end assert_eq!(values("---@type Machine\nlocal machine = { onStop = | }", Some(" ")), on_stop); assert_eq!(values(&format!("{machine}machine.onStop = |"), None), on_stop); assert_eq!( - values("---@type Handler\nlocal handler =|", Some("=")), + values("---@type Handler\nlocal handler =|", None), pairs(&[("function(id, name)", " function(${1:id, name})\n\t$0\nend")]) ); assert_eq!( @@ -5146,10 +5143,9 @@ function SetEnabled(enabled, mode) end assert_eq!(values("---@type function\nlocal run = |", None), pairs(&[("function()", "function($1)\n\t$0\nend")])); assert_eq!(values(&format!("{machine}if machine.onStop == | then\nend"), Some(" ")), None); - // A typed `=` or space asks for nothing else, and for nothing where no type lists values. + // A typed space asks for nothing else, and for nothing where no type lists values. for (typed, trigger) in [ ("local state = |", " "), - ("local state =|", "="), ("local |", " "), ("print(1, |)", " "), ("---@type string\nlocal label = |", " "), @@ -5157,7 +5153,6 @@ function SetEnabled(enabled, mode) end ("---@type string?\nlocal label = |", " "), ("---@type Machine\nlocal machine = { label = | }", " "), ("---@type State\nlocal state = 'busy'\nlocal ready = state >= |", " "), - ("---@type State\nlocal state = 'busy'\nlocal ready = state >=|", "="), ("---@type State\nlocal state = 'busy' -- state = |", " "), ("---@type State\nlocal state = 'a = |'", " "), // The value after the cursor is already written. @@ -5176,6 +5171,14 @@ function SetEnabled(enabled, mode) end assert_eq!(values("---@type boolean\nlocal enabled = |other", None), None); assert_eq!(values("---@type boolean\nlocal enabled = t|other", None), Some(Vec::new())); assert_eq!(values("---@type boolean\nlocal enabled = t|", None), plain(&["true", "false"])); + // What a typed space lists is incomplete, so the client asks again once a word is typed, and + // gets the names in scope beside the values. + let (line, column) = pos("---@type boolean\nlocal enabled = |", "|", 0); + client.open_with(CLIENT, "---@type boolean\nlocal enabled = "); + let mut params = client.position_params(CLIENT, line, column); + params["context"] = json!({ "triggerKind": 2, "triggerCharacter": " " }); + let result = client.request("textDocument/completion", params); + assert_eq!(result["isIncomplete"], json!(true), "{result}"); // A typed word filters them by the value, beside the names in scope. let (line, column) = pos("local bucket = 1\n---@type State\nlocal state = bu|", "|", 0); From c8bdbfeca7f12e7a2517448d4fc01efc781ca01c Mon Sep 17 00:00:00 2001 From: ChatDisabled <44729807+ChatDisabled@users.noreply.github.com> Date: Thu, 1 Oct 2026 04:05:41 +0200 Subject: [PATCH 09/12] fix(infer): leave calls of side-split global functions unknown --- crates/qbx_lua_ls/src/infer.rs | 46 +++++++++++++++++++++++++- crates/qbx_lua_ls/tests/lsp.rs | 59 ++++++++++++++++++++++++++++++++++ docs/reference.md | 5 +++ 3 files changed, 109 insertions(+), 1 deletion(-) diff --git a/crates/qbx_lua_ls/src/infer.rs b/crates/qbx_lua_ls/src/infer.rs index 70f99d4..752a53b 100644 --- a/crates/qbx_lua_ls/src/infer.rs +++ b/crates/qbx_lua_ls/src/infer.rs @@ -1095,8 +1095,15 @@ impl<'a> Infer<'a> { _ => {} } } - let Some((fun, _)) = self.callee_fun(base, method) else { return Default::default() }; let args = CallArgs::new(args); + let fun = match self.sided_definition(base, method, &args) { + Some(Some(fun)) => fun, + Some(None) => return Default::default(), + None => match self.callee_fun(base, method) { + Some((fun, _)) => fun, + None => return Default::default(), + }, + }; let fun = self.signature_for(&fun, &args, method.is_some(), base.span.start); // `AwaitServerCallback('name', ...)` returns what the handler of `name` returns. if let Some(wrapper) = Wrapper::of(&fun, method.is_some()).filter(|w| w.tag.role == CallbackRole::Await) { @@ -1119,6 +1126,43 @@ impl<'a> Infer<'a> { } } + /// The definition that a call of the global function `base` uses when client and server files + /// define it differently, as a `shared_script` sees two `GetJob`s: the one left for the side the + /// call runs on, if its arguments fit it. `Some(None)` when no single definition is left or the + /// arguments do not fit it, and `None` when the sides do not split the global. + fn sided_definition(&self, base: &Expr, method: Option<&Name>, args: &CallArgs) -> Option>> { + let (None, ExprKind::Name(name)) = (method, &base.kind) else { return None }; + if !matches!(self.ctx.resolution.resolve_at(name.span.start), Some(Resolved::Global(_))) { + return None; + } + let side_of = |file: FileId| self.index.file(file).and_then(|f| f.side); + let defined: Vec<(Option, &Arc)> = self + .index + .globals_named(&name.text, self.ctx.file) + .into_iter() + .filter_map(|(file, symbol)| Some((side_of(file), symbol.ty.as_fun()?))) + .collect(); + let defined_on = |side| defined.iter().any(|(on, _)| *on == Some(side)); + if !defined_on(Side::Client) || !defined_on(Side::Server) { + return None; + } + let side = self.side_at(base.span.start); + let mut left: Vec<&Arc> = Vec::new(); + for (on, fun) in defined { + if applies_on(on, side) && !left.contains(&fun) { + left.push(fun); + } + } + let fits = |fun: &Arc| { + let signature = self.signature_for(fun, args, false, base.span.start); + self.fit(&signature, args, false) != Fit::No + }; + Some(match left[..] { + [fun] if fits(fun) => Some(fun.clone()), + _ => None, + }) + } + /// Binds the generics of `fun` from the arguments of a call. Function literals go last, and only /// `with_callbacks`: their parameters are typed from what the other arguments bound, and their /// returns bind the rest, as `RV` and `RK` in `fun(value: V, key: K): RV, RK`. diff --git a/crates/qbx_lua_ls/tests/lsp.rs b/crates/qbx_lua_ls/tests/lsp.rs index 2a1485d..0da3b7f 100644 --- a/crates/qbx_lua_ls/tests/lsp.rs +++ b/crates/qbx_lua_ls/tests/lsp.rs @@ -2911,6 +2911,65 @@ plain = 'text' assert!(hover.contains("TestCount: integer"), "{hover}"); } +#[test] +fn shared_calls_of_a_function_each_side_defines_differently_return_unknown() { + let mut client = Client::start(fixture_root()); + client.open_with(CLIENT, "---@return string\nfunction TestGetJob()\n return 'police'\nend\n"); + client.open_with( + SERVER, + "---@param source number\n---@return { name: string }\nfunction TestGetJob(source)\n return { name = 'police' }\nend\n", + ); + let shared = "myresource/shared/config.lua"; + let text = "\ +local function check(source) + ---@type string? + local jobName + if source then + jobName = TestGetJob(source).name + else + jobName = TestGetJob() + end + return jobName +end + +---@return string +local function current() + return TestGetJob() +end + +---@return { name: string } +local function record() + return TestGetJob() +end + +---@type table +local job = TestGetJob(1) + +if IsDuplicityVersion() then + ---@type string + local onServer = TestGetJob(1) +else + ---@type table + local onClient = TestGetJob() + ---@type table + local unfit = TestGetJob(1) +end +"; + client.open_with(shared, text); + let finding = |needle: &str, message: &str| { + ("assign-type-mismatch".to_string(), pos(text, needle, 0).0 as u64, message.to_string()) + }; + assert_eq!( + findings(&mut client, shared, &["assign-type-mismatch", "return-type-mismatch"]), + [ + finding("local onServer", "Cannot assign `{ name: string }` to `onServer` of type `string`"), + finding("local onClient", "Cannot assign `string` to `onClient` of type `table`"), + ], + "code that runs on both sides calls neither definition, and one that its arguments do not fit \ + is none either" + ); +} + #[test] fn literal_keyed_class_fields_type_their_own_keys() { let mut client = Client::start(fixture_root()); diff --git a/docs/reference.md b/docs/reference.md index 2df3f3b..a149fe8 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -221,6 +221,11 @@ Recognized runtime guards narrow side checks within a file. Examples include An early return such as `if not IsDuplicityVersion() then return end` narrows the following code to the server. Event registrations inside these regions use that effective side. +Where client and server scripts define a global function differently, as two `GetJob`s, a call +from code that runs on both sides takes neither definition: what it returns is unknown and passes +every type check. Inside one of these guards the call takes that side's definition, as long as its +arguments fit it. + ## Function arguments `missing-parameter` compares calls with the LuaCATS annotations of the function they call. A From bd32df0e4201ecb7adff84f02a54abda5697dada Mon Sep 17 00:00:00 2001 From: ChatDisabled <44729807+ChatDisabled@users.noreply.github.com> Date: Thu, 1 Oct 2026 04:06:15 +0200 Subject: [PATCH 10/12] fix(infer): read no type from the `nil` a global is declared with --- crates/qbx_lua_ls/src/infer.rs | 3 ++- crates/qbx_lua_ls/tests/lsp.rs | 27 +++++++++++++++++++++++++++ docs/reference.md | 4 ++++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/crates/qbx_lua_ls/src/infer.rs b/crates/qbx_lua_ls/src/infer.rs index 752a53b..6618efb 100644 --- a/crates/qbx_lua_ls/src/infer.rs +++ b/crates/qbx_lua_ls/src/infer.rs @@ -539,10 +539,11 @@ impl<'a> Infer<'a> { return Type::Exports(None); } let symbols = self.index.globals_named(name, self.ctx.file); + // `Zone = nil` declares a global that some handler sets later, not what it holds. let known = symbols .iter() .map(|(_, s)| &s.ty) - .filter(|ty| !ty.is_unknown()) + .filter(|ty| !ty.is_unknown() && !matches!(ty, Type::Nil)) .max_by_key(|ty| (matches!(ty, Type::GlobalTable(_) | Type::Named(..)), ty.specificity())); if let Some(ty) = known.filter(|ty| !(matches!(ty, Type::Table) && self.index.has_members(name))) { // `local lib = {}` published with `_ENV.lib = lib` and then extended as `function lib.x()` diff --git a/crates/qbx_lua_ls/tests/lsp.rs b/crates/qbx_lua_ls/tests/lsp.rs index 0da3b7f..5e760ef 100644 --- a/crates/qbx_lua_ls/tests/lsp.rs +++ b/crates/qbx_lua_ls/tests/lsp.rs @@ -2911,6 +2911,33 @@ plain = 'text' assert!(hover.contains("TestCount: integer"), "{hover}"); } +#[test] +fn a_global_declared_as_nil_takes_any_value() { + let mut client = Client::start(fixture_root()); + let text = "\ +TestCurrentZone = nil + +RegisterNetEvent('test:enterZone', function(name) + TestCurrentZone = name +end) + +---@type string +local zone = TestCurrentZone + +---@param name string +local function enter(name) + name = TestCurrentZone +end + +---@return string +local function current() + return TestCurrentZone +end +"; + client.open_with(CLIENT, text); + assert_eq!(findings(&mut client, CLIENT, &["assign-type-mismatch", "return-type-mismatch"]), []); +} + #[test] fn shared_calls_of_a_function_each_side_defines_differently_return_unknown() { let mut client = Client::start(fixture_root()); diff --git a/docs/reference.md b/docs/reference.md index a149fe8..bee9880 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -401,6 +401,10 @@ name's alone, as lua-language-server binds it, so `---@type boolean` above type of their own, and other locals whose type is inferred from what they hold, take any value, and so do globals in assignments that have no `---@type` above them. +The value stored is what the code is known to give. A global's `nil` tells nothing: one declared as +`CurrentZone = nil` and set to a name by an event handler holds whatever the handler gives it, so +storing it anywhere passes. + ## Impossible comparisons `impossible-comparison` reports an `==` or `~=` whose two sides can never be equal, so that the From 544b75d91a8e92ed1c73b3ab6b78cea8db58d82b Mon Sep 17 00:00:00 2001 From: ChatDisabled <44729807+ChatDisabled@users.noreply.github.com> Date: Thu, 1 Oct 2026 04:07:43 +0200 Subject: [PATCH 11/12] feat(infer): honour `@as` casts written after a value --- crates/qbx_lua_ls/src/infer.rs | 34 +++++++++++++++++++++++++++---- crates/qbx_lua_ls/tests/lsp.rs | 37 ++++++++++++++++++++++++++++++++++ docs/reference.md | 5 ++++- 3 files changed, 71 insertions(+), 5 deletions(-) diff --git a/crates/qbx_lua_ls/src/infer.rs b/crates/qbx_lua_ls/src/infer.rs index 6618efb..8cbc5c8 100644 --- a/crates/qbx_lua_ls/src/infer.rs +++ b/crates/qbx_lua_ls/src/infer.rs @@ -8,14 +8,14 @@ use qbx_lua_analysis::env::leading_doc_lines; use qbx_lua_analysis::scope::{LocalId, LocalKind, Resolution, Resolved}; use qbx_lua_analysis::side_guard::SideRegions; use qbx_lua_syntax::ast::*; -use qbx_lua_syntax::{NumberValue, SmolStr}; +use qbx_lua_syntax::{CommentKind, NumberValue, SmolStr}; use rustc_hash::{FxHashMap, FxHashSet}; use crate::callback_wrappers::{self, Wrapper}; use crate::index::{FileId, Index, SymbolKind}; use crate::luacats::{applies_on, parse_doc_lines, DocGroup}; use crate::narrow::Guards; -use crate::types::{CallbackRole, FunType, Param, Shape, ShapeField, Type}; +use crate::types::{CallbackRole, FunType, Param, Shape, ShapeField, Type, TypeParser}; const MAX_DEPTH: u32 = 24; const MAX_SHAPE_FIELDS: usize = 96; @@ -399,11 +399,37 @@ impl<'a> Infer<'a> { } pub fn expr_multi(&self, expr: &Expr) -> Vec { - self.guarded(|| match &expr.kind { + let mut values = self.guarded(|| match &expr.kind { ExprKind::Call { callee, args, .. } => self.call(callee, None, args), ExprKind::MethodCall { base, method, args, .. } => self.call(base, Some(method), args), _ => vec![self.single(expr)], - }) + }); + if let Some(cast) = self.cast_after(expr.span.end) { + match values.first_mut() { + Some(first) => *first = cast, + None => values.push(cast), + } + } + values + } + + /// The type that a `--[[@as T]]` or `---@as T` comment casts the expression ending at `end` to, + /// when nothing but spaces stands between them. + fn cast_after(&self, end: u32) -> Option { + let comments = &self.ctx.chunk.comments; + let comment = comments.get(comments.partition_point(|c| c.span.start < end))?; + let between = self.ctx.source.get(end as usize..comment.span.start as usize)?; + if !between.bytes().all(|b| b == b' ' || b == b'\t') { + return None; + } + let content = comment.content.text(self.ctx.source); + let content = match comment.kind { + CommentKind::Long => content, + CommentKind::Line => content.strip_prefix('-')?, + CommentKind::CStyle => return None, + }; + let ty = content.trim_start().strip_prefix("@as")?; + ty.starts_with(char::is_whitespace).then(|| TypeParser::new(ty.trim()).parse()) } fn single(&self, expr: &Expr) -> Type { diff --git a/crates/qbx_lua_ls/tests/lsp.rs b/crates/qbx_lua_ls/tests/lsp.rs index 5e760ef..c8cafc4 100644 --- a/crates/qbx_lua_ls/tests/lsp.rs +++ b/crates/qbx_lua_ls/tests/lsp.rs @@ -2911,6 +2911,43 @@ plain = 'text' assert!(hover.contains("TestCount: integer"), "{hover}"); } +#[test] +fn as_casts_type_the_expression_right_before_them() { + let mut client = Client::start(fixture_root()); + let text = "\ +---@return integer, string +local function pair() return 1, 'a' end + +---@type string +local a = 5 --[[@as string]] +---@type string, string +local b, c = pair() --[[@as string]] +---@type string +local d = 5 ---@as string +---@type string +local e = 5 -- @as string +---@type string +local f = 5 +--[[@as string]] +local cast = 5 --[[@as string]] +"; + client.open_with(CLIENT, text); + let finding = |needle: &str, message: &str| { + ("assign-type-mismatch".to_string(), pos(text, needle, 0).0 as u64, message.to_string()) + }; + assert_eq!( + findings(&mut client, CLIENT, &["assign-type-mismatch"]), + [ + finding("local e", "Cannot assign `integer` to `e` of type `string`"), + finding("local f =", "Cannot assign `integer` to `f` of type `string`"), + ], + "a cast on the line of its value types the first value, and a plain comment or a line of its own casts nothing" + ); + let (line, column) = pos(text, "cast =", 0); + let hover = client.hover_text(CLIENT, line, column); + assert!(hover.contains("local cast: string"), "{hover}"); +} + #[test] fn a_global_declared_as_nil_takes_any_value() { let mut client = Client::start(fixture_root()); diff --git a/docs/reference.md b/docs/reference.md index bee9880..52870a9 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -403,7 +403,10 @@ and so do globals in assignments that have no `---@type` above them. The value stored is what the code is known to give. A global's `nil` tells nothing: one declared as `CurrentZone = nil` and set to a name by an event handler holds whatever the handler gives it, so -storing it anywhere passes. +storing it anywhere passes. A `--[[@as T]]` or `---@as T` right after a value, on its line, casts +it to `T` as in lua-language-server, which silences a check that the code knows better than: +`local count = GetValue() --[[@as integer]]` stores an `integer`. On a call, the cast types its +first value. ## Impossible comparisons From f92d839deb8be1dadaff6c62edfd0a001b124d4a Mon Sep 17 00:00:00 2001 From: ChatDisabled <44729807+ChatDisabled@users.noreply.github.com> Date: Thu, 1 Oct 2026 04:08:43 +0200 Subject: [PATCH 12/12] fix(completion): list `nil` after the values of every signature --- crates/qbx_lua_ls/docs/protocol.md | 3 ++- crates/qbx_lua_ls/src/features/completion.rs | 1 + crates/qbx_lua_ls/tests/lsp.rs | 9 +++++++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/crates/qbx_lua_ls/docs/protocol.md b/crates/qbx_lua_ls/docs/protocol.md index 0af059e..816c492 100644 --- a/crates/qbx_lua_ls/docs/protocol.md +++ b/crates/qbx_lua_ls/docs/protocol.md @@ -100,7 +100,8 @@ Integers are offered as they are, and so are `true` and `false`, which a `boolea type that lists values also lists the `nil` it allows, last; a type such as `string?` lists nothing. Snippet clients also get a function literal labelled like `function(source)`, with the filter text `function`, when the argument takes a function. Values come from every signature that -the arguments before it fit, and the function's parameters from the signature they fit best, so +the arguments before it fit, with `nil` after those of all of them, and the function's parameters +from the signature they fit best, so `OnAction("playerUnloaded", ` picks the `@overload` that lists `"playerUnloaded"`. Signatures that fit equally well each get an item. `(` and `,` are completion trigger characters for these items only: a request whose `context.triggerCharacter` is `(` or `,` returns nothing else, leaves out diff --git a/crates/qbx_lua_ls/src/features/completion.rs b/crates/qbx_lua_ls/src/features/completion.rs index 4681f97..b4fc03c 100644 --- a/crates/qbx_lua_ls/src/features/completion.rs +++ b/crates/qbx_lua_ls/src/features/completion.rs @@ -1018,6 +1018,7 @@ fn argument_literals( } } } + literals.sort_by_key(|literal| literal.value == Type::Nil); literals } diff --git a/crates/qbx_lua_ls/tests/lsp.rs b/crates/qbx_lua_ls/tests/lsp.rs index c8cafc4..798e7ae 100644 --- a/crates/qbx_lua_ls/tests/lsp.rs +++ b/crates/qbx_lua_ls/tests/lsp.rs @@ -5110,6 +5110,10 @@ function SetLevel(level, volume) end ---@param enabled boolean ---@param mode? \"auto\"|\"manual\"|false function SetEnabled(enabled, mode) end + +---@param mode \"auto\"|nil +---@overload fun(mode: \"manual\") +function SetMode(mode) end "; client.open_with("myresource/shared/config.lua", defs); // `|` marks the cursor; `trigger` is the character typed to ask for completions. Items come back @@ -5240,6 +5244,11 @@ function SetEnabled(enabled, mode) end optional_states ); assert_eq!(values(&format!("{machine}machine.fallback = |"), None), optional_states); + // Also after the values of other signatures. + assert_eq!( + values("SetMode(|)", Some("(")), + pairs(&[("'auto'", "'auto'"), ("'manual'", "'manual'"), ("nil", "nil")]) + ); assert_eq!(values("---@type boolean\nlocal enabled = '|'", None), Some(Vec::new())); // A typed `(` or `,` opens the list for strings and integers, not for booleans alone, which // are listed when asked for.