Repository navigation
Conversation
This branch has not been deployed
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
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.
Stacked on #928.
Adds a
[workspace.lints]table and opts every member crate into it ([lints] workspace = true, also in thejust new-cratetemplate), and turns on the panic lints forpgls_lsp, the crate that handles client input first.Workspace lints
clippy::todo,clippy::unimplementedandclippy::dbg_macroare now denied everywhere (dbg!stays allowed in tests, as before). The existing occurrences are fixed, not allowed:MemoryFileSystem::resolve_configurationwas atodo!(), so a configuration thatextendsa package name (not a path) panicked on the memory file system, which the WASM build uses. It returnsResolveError::NotFoundnow, which is reported as "can't resolve" like an unresolvable package on disk.unimplemented!()for array and object literals and for boolean schemas. It now prints[a, b],{ "k": v },unknownandnever. The generated bindings don't change.generate_tool_configurationreturns an error forassistsand the transformation category instead of panicking.unreachable!()got a message.pgls_lsp: no
unwrap,expector indexingpgls_lsp/src/lib.rswarns onclippy::unwrap_used,clippy::expect_usedandclippy::indexing_slicing, and CI's-D warningsturns them into errors. Tests are exempt (allow-*-in-testsinclippy.toml). On top of #928, which already removed theparams.arguments[0]indexing, there were 21 findings (19unwrap, 2expect):std::synclock unwraps.pgls_lspusesparking_lotlocks now, which can't be poisoned. Before, a request handler that panicked while holding the documents lock (panic_to_lsp_errorcatches the panic) poisoned the lock, and every later request on that session panicked too.get_cursor_positionand completionsexpected/unwrapped the client capabilities. They useSession::position_encoding(), which falls back to UTF-16 like the other handlers.diagnostic.advices(..)errors are returned through?. Serialising theexecuteStatementarguments drops the action on failure. The unusedprint_markup(two unwraps) is removed.#[expect(..., reason = "...")]: theu32offset inLineIndex::new(documents over 4 GiB aren't supported anywhere) and theConfigurationStatusatomic, which only ever stores aConfigurationStatus as u8.Counts for the next crate
pgls_workspace(non-test code) has 76 findings: 48unwrap, 5expect, 16 indexing and 7 slicing. About 34 of the unwraps are lock unwraps, whichparking_lotwould remove the same way.matcher/pattern.rs(20, the glob matcher's index arithmetic) andserver.rs(16) have most of the rest. For reference, the same measurement over the other crates found 178 inpgls_configuration(mostly generated code), 89 inpgls_pretty_print, 70 inpgls_catalogand 39 inpgls_treesitter.Risks
Open PRs that add
std::synclocks with.unwrap()topgls_lsp(#895, #908) will fail clippy once this is merged. The fix is to useparking_lot::{Mutex, RwLock}and drop the.unwrap().client_capabilities()stays because #908, #911 and #919 use it.Tests:
cargo clippy --workspace --all-targets --all-featuresis clean,pgls_lspandpgls_fstests pass, andjust gen-lintleaves the generated files unchanged.Follow-ups: #895, #908 and #936 now use
parking_lotlocks. Only #881's ownprojectsandworkspace_folderslocks remain to be switched when the two meet.