Drop the inert unnecessary-assert disable from the typecheck config - #4
Merged
Merged
Conversation
…onfig The key was carried over from the config proposed in the #3 review on the assumption it was load-bearing on lua-language-server 3.18.2-dev. It is not: the review confirmed 11 problems either way there, with no such diagnostic emitted, and 3.19.0 agrees. A disabled diagnostic that never fires reads as a known exception to anyone changing this file later, so it is worse than nothing. Verified: `make typecheck` 11 problems in 3 files, unchanged; 15/15 modules; `make format-check` clean.
derek-miller
approved these changes
Aug 8, 2026
derek-miller
added a commit
that referenced
this pull request
Aug 8, 2026
The paragraph above the typecheck target named the two categories this change resolves, type-narrowing and deliberate-bad-argument findings, and deferred the decision this change makes. Left alone it reads as a current known exception to anyone editing the file later, which is the same defect class #4 removed.
derek-miller
added a commit
that referenced
this pull request
Aug 8, 2026
… narrowing (#5) * Gate typecheck in check, fix the AESWord annotation and the SRP group narrowing lua-crypto was the one library where typecheck was not part of check, so this class of defect had no CI gate here at all. Two real annotation defects fall out once it runs: aes_gcm.lua create_aes_word() returns a 4-element word but annotated its body @type AESState, which is [AESWord, AESWord, AESWord, AESWord]. Same copy-paste as the one fixed in lua-bthome-ble aes_ccm.lua. Clearing it takes the repo from 11 findings to 6. srp.lua read group.g three times across a type(group.g) branch. Narrowing applies to locals, not table fields, so neither arm was narrowed and both bignum.from_number and bignum.from_hex saw the full string|integer union. Reading it into a local once fixes both. Same root cause as the bit64 test.expected hoist in lua-bitn. The remaining four are deliberate negative tests that pass nil or an integer to assert the function rejects it, so the wrong type is the point. Those get disable-next-line rather than a signature change. The config carries the full bypass set the other four libraries now use, including runtime.plugin, which a grep of the diagnostics paths cannot surface: check_worker.lua does require "plugin", so an OnSetText returning an empty edit blanks every file and the check passes having analysed nothing. diagnostics.disable keeps its existing unnecessary-assert entry. CI installs lua-language-server 3.19.0 before Check, matching the other four. Full test suite passes, including the SRP change: AES, AES-GCM, ChaCha20, Poly1305, SHA-256/512, BLAKE2, HKDF, SRP, bignum, X25519, X448, Ed25519 and the OpenSSL gating. * Correct the Makefile comment that still deferred gating typecheck The paragraph above the typecheck target named the two categories this change resolves, type-narrowing and deliberate-bad-argument findings, and deferred the decision this change makes. Left alone it reads as a current known exception to anyone editing the file later, which is the same defect class #4 removed.
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.
Follow-up to your approval on #3: you asked for the
unnecessary-assertdisable to be dropped, and #3 auto-merged before I could act on it.I had kept the key verbatim from your proposed config on the assumption it was load-bearing on 3.18.2-dev. Your review confirmed it is not: 11 problems either way there, with no such diagnostic emitted, and 3.19.0 agrees.
Worth removing rather than leaving: a disabled diagnostic that never fires reads as a known exception to whoever next edits this file.
Verification
On lua-language-server 3.19.0, against
mainat821ae5c:make typecheck: 11 problems in 3 files (aes_gcm5,ed255193,srp3), unchanged./run_tests.sh: 15/15 modulesmake format-check: cleanNot verified locally:
make lint, same luacheck 1.2.0 / Lua 5.5 environment problem as before. CI covers it, and this changes no Lua.