Repository navigation
lua-language-server type rules, a strict mode, flow narrowing and FiveM data updates - #20
Open
loaf-scripts wants to merge 155 commits into
Open
loaf-scripts wants to merge 155 commits into
loaf-scripts wants to merge 155 commits into
Conversation
`assert(v)` raises an error for a `nil` or `false` `v`, so the `v` it returns is neither, as lua-language-server reads it: `assert(load(code))` is a `function`, not a `function?`. The value takes the type of the argument itself, as binding the generic would widen the `false` of a `string|false` to a `boolean`.
… them The checks that only count declared types left a call of a generic function unknown, so with strict, `takes(first(names))` passed a `string?` to a `string` parameter unreported for `fun(list: V[]): V?` and a `names` declared as `string[]`, while the same value from a `---@return string?` was reported. The generics are now bound from the declared types of the arguments, as TypeScript binds them; one that no declared type binds stays unknown. lua-language-server reports these calls too, also without strict.
…othing after initialized The workspace pass only ran once a message arrived after `initialized`, so a client that registers no file watchers, and so has no request of the server to answer, got no diagnostics until it opened a file. The server now waits a moment for the first message, and reports the files nobody has open at once when none arrives, so a request that arrives just after indexing is still answered before that pass.
…s opened The workspace pass reports a closed file under the URI the index spells for it, and leaves an open file alone. When the editor opened it under another spelling, as `%5Blb%5D` for `[lb]`, the list under the index's spelling was never cleared, and the editor showed both.
… its function A function literal after `return` took no type from the `@return fun(ped: number): number` of the function around it, nor from a `---@type fun(): fun(...)` declared for that function, and no `---@param` above the `return` applied. It now takes both, as lua-language-server types it from the `@return`, and what it returns is checked against the declared `fun(...)`.
…ent declares `X = X or function(appearance) end` took neither the `---@type fun(...)` nor the `---@param` lines above the statement, nor the `@field` it implements. A function on either side of `or`, or on the right side of `and`, now takes them, as TypeScript types the sides of `||`. A table given that way, as `Config = Config or {}`, is left alone, as it is filled in later.
With `local gender = isMale and "male" or "female"`, `byGender[gender]` was unknown, as the local is a `string`. A key that is a literal, `a and "x" or "y"`, or a local without `---@type` that is never assigned again and holds such literals now reads the fields they name, as lua-language-server reads them. The local itself stays a `string`.
After `if cam ~= activeCam then return end`, a `number?` `cam` stayed `number?` although `activeCam` is a `number`. A comparison with a local now narrows by the type of that local where the comparison reads it, as TypeScript narrows by equality: one that is never `nil` or `false` rules those out, and one typed as a single literal narrows as that literal does.
Inside a function with `---@Generic T: table`, `pairs(t)` for a `t: T` gave `string` keys, as `T` was read as a class without fields, and unknown values. A name that is no class now tells nothing about either, as a plain `table` does not.
`tonumber(x)` was a `number?` even when `x` is a number. It now gives the number it is given, and `nil` only for another value or with a base.
Natives such as `GetActivePlayers`, `GetGamePool` and `GetPedDecorations` returned a plain `table`. `RETURN_TYPES` now gives the LuaCATS type of each table a native returns, as the runtime serializes it: `Player[]`, `Entity[]`, `[Hash, Hash][]`, the command objects of `GetRegisteredCommands`, and `any` for the stored value `GetStateBagValue` gives. It is applied where the data is read, so regenerating `natives.tsv` keeps it.
…no-unknown An unknown value was reported on the statement whose own `---@type` types it, as `---@type EsxSkinConfig` above `local config = ESX.GetConfig()`, which is how code gives such a value a type. And `---@PARAM x unknown`, `---@type unknown` and the loop variables over a `{ [unknown]: unknown }` were reported as having no type, although the type is written out, as TypeScript takes it.
The rules that only the language server runs, such as `param-type-mismatch` and `need-check-nil`, could not run outside an editor. `qbx-lua-ls --check [<dir>...]` reports what the editor reports for the files nobody has open, with `qbxlint.toml`, as `qbx-lint --format compact` writes its lines, and fails on an error or a warning. `--strict`, `--library`, `--rule` and `--max-warnings` set what the editor settings would. The server's workspace pass and the check share their code.
`list[#list + 1] = "text"` was not checked for a `number[]`: only the fields of classes were. An entry stored in an array, map, tuple or shape that an annotation declares now takes its type, as lua-language-server checks it, and a table constructor stored there is checked against it. A table that a constructor builds declares nothing about what code adds to it. As in lua-language-server, a value stored under a key in brackets may be `nil`, which removes the entry, while one stored in a named field may not; with `strict`, neither may, as TypeScript reads it.
…list `table.insert(list, "text")` passed for a `number[]` list, as the two-argument form took any value. It now takes a `T` for a `T[]` list, and `param-type-mismatch` checks a parameter typed with a generic of the function alone against what the annotated types of the other arguments decide for it, as TypeScript checks `push<T>(list: T[], value: T)`. A table constructor passed for it is checked against that type, and a list of no declared entries still takes any value.
The types of locals, and of the values that origins, assignments, sets of origins and linked call statements give, were cached even when the depth that inference follows cut their lookup short. A long chain of locals read from each other, as a function that reads `t`, `err`, `ax` and `bx` in turn, left the local where the cut fell unknown for good, and which one that was depended on what was read first, so `no-unknown` reported a different local from run to run while hovers showed its type. Such a result is now found again from a shallower point instead.
Inside the function that declares it, a generic such as the `T` of `---@Generic T` was read as a class, which counts as a table, so `if type(value) ~= "table" then return value end` ruled out every value of `T` and left it unknown. A name that names no class or alias may hold any value, so the guards leave it as it is, as TypeScript keeps a `T`, and narrow the rest of a union.
`---@cast coords Cell | Coords` for a `Coords` was reported, as each type the cast lists was compared with the declared type alone. A cast to a type that takes every value of the declared one, a union that lists it, a parent class or `T?` for a `T`, now passes, as TypeScript takes `x as A | B` for a `B`, although lua-language-server reports it. A cast to an unrelated type is still reported.
A field set through a local that a table constructor declares at the top of a file belonged to the table, while one set through such a local inside a function was missing from it, so `strict` reported one and not the other. Without `strict`, a local inside a function now owns the table it builds as one at the top of the file does, and what code sets through it is a field of the table, in hovers and completion too. With `strict`, as TypeScript reads it, a table that a constructor with fields builds has those alone, at the top of a file as inside a function, so `inject-field` and `undefined-field` report the fields set through its local later, while an empty `{}` still takes any field.
A global function took the signatures of one definition, the nearest, so a `---@meta` file that declared `OnAction` again with an `@overload` for `keyPressed` hid the `jobUpdated` overload of the code, and the handler passed for it took `any`. As lua-language-server reads it, a function now has the signatures of every definition the code sees, for globals and for the members of one table or class alike: calls pick from them, the handlers they pass take their parameters from them, and hover and signature help list them. A signature that takes the same parameters on the same side as one the function has is left out.
…iles `---@extend OnAction (server) fun(action: "jobUpdated", handler: fun(source: number, job: Job))` adds a signature to a global function, a field such as `Utils.notify`, or a method such as `Player:on`, from any file, so the modules and users of a library can type the actions and events they add without declaring the function again. It counts as an `@overload` for the code that would see a definition in its file, on the side its attribute names: calls pick it, handlers take its types, argument checks accept it, and `missing-parameter` and `redundant-parameter` count it beside the definitions, never instead of them, so a native keeps its checks. The files that read the function are indexed again when the line changes. A line whose signature no function takes, such as one with a misspelled name or a method only a parent class defines, is reported as `undefined-doc-name`.
…f its statement Below `---@Generic T`, `---@PARAM value T` and `---@return T`, the parameters of `local wrap = function(value) return value end` and of `return function(message) ... end` took the types of the doc comment, but the function value itself took none, so a call of it returned `T` rather than the type of its argument, as `assigned("x")` and `CreateScriptErrorHandler("x")("y")` did. The function now takes the doc comment of the `local` statement, assignment or `return` it is the value of, as a `local function` and a global assignment already did, so its generics are bound where it is called, as lua-language-server binds them.
The stub declared one signature with an optional handler that returned `EventHandlerData?`, so with `strict` storing the handle of `RegisterNetEvent(name, handler)` in a table of handles was reported. The runtime returns the handle whenever a handler is given: the declared signature now takes one and returns `EventHandlerData`, a call without one returns `nil`, and one whose handler may be `nil` keeps `EventHandlerData?`. The type of a function passed to a call no longer counts as unknown when the signatures that fit the call type its argument alike apart from `nil`, as `handler` and `handler?` do, so the handler is still checked as an event handler.
`undefined-doc-name` read only the `---` doc lines of a file, so `--[[@as rgerg]]` named a type nothing declares without a finding, and the cast then passed every check, as a type of no known kind does. Inline casts are now read too, as lua-language-server reads them. The generics that any doc comment of the file declares may be named there.
…be nil With `strict`, `states[id] = nil` removed an entry that only an index types, while `states[id] = isInside or nil`, which sets or removes it, was reported as `boolean?`. A value that may be `nil` is now accepted wherever `nil` is, in strict mode only: default mode keeps lua-language-server's reading, where a named key of a map, as `lists.a`, keeps its `nil`. Declared named fields and tuple positions still reject it. Messages also showed the table a local holds as an empty type, as in `Cannot assign `|nil``: a union now leaves such a table out only beside a type that names it and writes parts that read alike once, and messages show a list that a local holds as `integer[]`.
After `local playerName = playerSource and GetPlayerName(playerSource)`, `if playerName then` narrowed `playerName` alone, so `playerSource` stayed `number?` and its use in the guarded code was reported with `strict`. As TypeScript reads an aliased condition, a guard on a local that is never assigned again now also tells what the value it is declared with does, through at most five such locals, about the locals in it that are never assigned again either, so they hold there what they held where it was declared. Fields read in the value are not narrowed through it, since other code may set them in between. A side of `or` that is never true, as the `nil` of `name and value or nil`, also leaves what the other side tells, as does a side of `and` that is never false, where nothing was known before.
…tional in fivem/event-missing-arguments
`TriggerClientEvent('qb-clothes:loadSkin', source, true)` was reported for a handler documented as `---@PARAM _ boolean`, `---@PARAM model? string|number` and `---@PARAM data? QbClothingSkin|string`, as the rule counted every parameter of the handler. A trigger now has to pass the parameters up to the last one that the doc comment above the registration requires, as `missing-parameter` counts them: one marked `?` or typed to take `nil` may be left out. A handler without such a comment still needs every parameter, and the message says how many the handler requires.
… local by its type
In `type(max) == "function" and max(playerPed, character) or max`, the `or` side read `max` as a `number|fun(...)`, so storing the value in a `{ [string]: number }` was reported, as lua-language-server reports it. Only `nil` and `false` are false in Lua, so when `max(...)` gives a `number`, the `and` is only false when `max` is no function. Where a false `and` or a true `or` tells about a local through one side only, the other side now counts by its type there: when that type has no value of the truth the side needs, as a number, a string, a table, a function or a class is never false and `nil` never true, the facts of the first side hold. A side whose type may be either, as a `number?`, a `boolean` or `unknown`, still tells nothing, and so does a comparison or a `not`. Both modes narrow this way.
A parameter typed `{ [K]: V }` left `K` and `V` unbound, while the same parameter written `table<K, V>` bound them from the argument, so with `---@PARAM tbl { [K]: V }` the callback of `Find(names, function(name, id) ... end)` took unknown parameters and the call returned an unknown value. A brace table whose only entry is one `[K]: V` index now binds its generics as `table<K, V>` does, and a parameter that mentions a generic in such an index is left to the generics, as one that names it in a field already was.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
This brings the type checks of qbx-lua-ls close to lua-language-server 3.19.1. It also adds an opt-in strict mode that reports what TypeScript's strict mode would. And it fixes the indexing and inference gaps that showed up while comparing the two on real FiveM resources.
It is 155 commits. Each is one change with its own tests and docs, so it can be reviewed commit by commit, and the release notes read from the commit messages.
New rules
Most new rules default to
warning, so projects will see new findings after upgrading. The exceptions aretrailing-space,redundant-returnandunused-vararg, which default tohint, and the missing-doc rules, which areoff. Rules that need types run in qbx-lua-ls, and throughqbx-lua-ls --checkoutside an editor.param-type-mismatch,need-check-nil,cast-type-mismatch,cast-local-typeinject-field,redundant-return-value,discard-returns,invisibleclose-non-object,circle-doc-class,unknown-operator,unknown-cast-variableredundant-parameter,count-down-loop,duplicate-set-fieldnewline-call,newfield-call,ambiguity-1,global-in-nil-envredundant-return,unused-vararg,trailing-spaceundefined-doc-param,duplicate-doc-alias,duplicate-doc-field,duplicate-doc-param,doc-field-no-class,unknown-diag-codemissing-global-doc,missing-local-export-docandincomplete-signature-docExisting rules
missing-fields: checks tables typed as a union of classes and shapes, or as a shape. It also checks the tables that arrays, maps and tuples hold. A(partial)class requires only its own fields, and---@metafiles are skipped.undefined-fieldandinject-field: report fields read from, or set on, values whose type lacks them.assign-type-mismatchandreturn-type-mismatch: check each type of a union, the entries stored in declared tables, whattable.insertadds, and typed locals that nothing assigns.unreachable-code,deprecated,unused-local,unused-function,redefined-local,missing-parameter: cover the cases lua-language-server reports and these missed.native-wrong-side: reports names of one side used in shared scripts, and server-only Lua libraries used on the client.no-unknown: reports values of unknown type stored where a type is declared.fivem/event-missing-arguments: leaves out the handler parameters that its doc comment marks optional.lowercase-global,implicit-globalandunused-argument: leave---@metastubs alone.Strict mode
strict = trueinqbxlint.tomlturns it on. It also works per folder through[[overrides]], or through the editor'sdiagnostics.strict. The type rules then also report what TypeScript's strict mode does, such asn + 1for anumber?, or astring?passed for astring.It is off by default. Without it, the rules aim to report what lua-language-server 3.19.1 reports on the same code. The index, hover and completion don't depend on it.
Inference
type()andmath.type()checks, and comparisons with literals and with other locals;elseifchains, and locals that are assigned again;---@castlines;if okafterlocal ok = x and f(x);and/orsides whose type is never false.setmetatableand__index;@operatorlines;{ [K]: V }binding generics astable<K, V>does.---@extend Name fun(...)adds signatures from other files.@return,or/andside or statement declares.assert,tonumberand...Treturns are typed, and so are calls of functions that return nothing.Language server
---|values in completion and hover.qbx-lua-ls --check [<dir>...]reports what the editor reports, from the command line.sqltype and theqbx/sqlFunctionsrequest, so editors can highlight queries.FiveM data
feat(natives): update the natives and return their pointers as the Lua wrappers do, so that data diff can be reviewed apart. Natives return their pointers after the result, as the Lua wrappers do. The functions they call back and the tables they return are typed.cfx_events.luadeclares the events FiveM itself triggers. They're used for completion, hover, handler parameter types andfivem/event-wrong-side.CreateThread,json.decode,msgpack.unpack,table.*,RegisterNetEvent,load,string.findandos.date('*t'). The server-only Lua libraries are split into their own stub.LuaCATS
---@extend;async fun(...);---|lines that continue@param,@return,@typeand@field;[K]?: V.selfin doc types resolves to the class of the function's table.docs/rules.md,docs/reference.mdand the language server's README and protocol docs describe each change.The editor side is in the qbx-editor PR Qbox-project/qbx-editor#7: the strict setting, hover levels, SQL highlighting and
---@extendhighlighting. Its CI builds this repository'smain, so this PR should merge first.Checks
cargo fmt --all --checkcargo clippy --workspace --all-targets --locked -- -D warningscargo test --workspace --lockedcargo test --locked -p qbx_fivem_data --features docscargo build --release --locked -p qbx_lua_lsandcargo build --profile lint-release --locked -p qbx_lint.crates/qbx_lua_ls/testsandcrates/qbx_lua_analysis/tests.🤖 Generated with Claude Code