From 1bdb22c13972b3a3dc9cb4ef31fbf37db051dd1c Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Sun, 27 Apr 2025 11:44:55 +0100 Subject: [PATCH] [red-knot] Fix offset handling in playground for 2-code-point UTF16 characters (#17520) --- crates/red_knot_wasm/src/lib.rs | 125 ++++++++++++++++------ crates/red_knot_wasm/tests/api.rs | 10 +- crates/ruff_source_file/src/line_index.rs | 3 +- playground/knot/src/Playground.tsx | 4 +- 4 files changed, 101 insertions(+), 41 deletions(-) diff --git a/crates/red_knot_wasm/src/lib.rs b/crates/red_knot_wasm/src/lib.rs index 5f9333d028..918aaaf67a 100644 --- a/crates/red_knot_wasm/src/lib.rs +++ b/crates/red_knot_wasm/src/lib.rs @@ -19,7 +19,7 @@ use ruff_db::system::{ use ruff_db::Upcast; use ruff_notebook::Notebook; use ruff_python_formatter::formatted_file; -use ruff_source_file::{LineColumn, LineIndex, OneIndexed, PositionEncoding, SourceLocation}; +use ruff_source_file::{LineIndex, OneIndexed, SourceLocation}; use ruff_text_size::{Ranged, TextSize}; use wasm_bindgen::prelude::*; @@ -42,13 +42,18 @@ pub fn run() { #[wasm_bindgen] pub struct Workspace { db: ProjectDatabase, + position_encoding: PositionEncoding, system: WasmSystem, } #[wasm_bindgen] impl Workspace { #[wasm_bindgen(constructor)] - pub fn new(root: &str, options: JsValue) -> Result { + pub fn new( + root: &str, + position_encoding: PositionEncoding, + options: JsValue, + ) -> Result { let options = Options::deserialize_with( ValueSource::Cli, serde_wasm_bindgen::Deserializer::from(options), @@ -62,7 +67,11 @@ impl Workspace { let db = ProjectDatabase::new(project, system.clone()).map_err(into_error)?; - Ok(Self { db, system }) + Ok(Self { + db, + position_encoding, + system, + }) } #[wasm_bindgen(js_name = "updateOptions")] @@ -216,13 +225,18 @@ impl Workspace { let source = source_text(&self.db, file_id.file); let index = line_index(&self.db, file_id.file); - let offset = position.to_text_size(&source, &index)?; + let offset = position.to_text_size(&source, &index, self.position_encoding)?; let Some(targets) = goto_type_definition(&self.db, file_id.file, offset) else { return Ok(Vec::new()); }; - let source_range = Range::from_text_range(targets.file_range().range(), &index, &source); + let source_range = Range::from_text_range( + targets.file_range().range(), + &index, + &source, + self.position_encoding, + ); let links: Vec<_> = targets .into_iter() @@ -231,10 +245,12 @@ impl Workspace { full_range: Range::from_file_range( &self.db, FileRange::new(target.file(), target.full_range()), + self.position_encoding, ), selection_range: Some(Range::from_file_range( &self.db, FileRange::new(target.file(), target.focus_range()), + self.position_encoding, )), origin_selection_range: Some(source_range), }) @@ -248,13 +264,18 @@ impl Workspace { let source = source_text(&self.db, file_id.file); let index = line_index(&self.db, file_id.file); - let offset = position.to_text_size(&source, &index)?; + let offset = position.to_text_size(&source, &index, self.position_encoding)?; let Some(range_info) = hover(&self.db, file_id.file, offset) else { return Ok(None); }; - let source_range = Range::from_text_range(range_info.file_range().range(), &index, &source); + let source_range = Range::from_text_range( + range_info.file_range().range(), + &index, + &source, + self.position_encoding, + ); Ok(Some(Hover { markdown: range_info @@ -272,14 +293,19 @@ impl Workspace { let result = inlay_hints( &self.db, file_id.file, - range.to_text_range(&index, &source)?, + range.to_text_range(&index, &source, self.position_encoding)?, ); Ok(result .into_iter() .map(|hint| InlayHint { markdown: hint.display(&self.db).to_string(), - position: Position::from_text_size(hint.position, &index, &source), + position: Position::from_text_size( + hint.position, + &index, + &source, + self.position_encoding, + ), }) .collect()) } @@ -348,6 +374,7 @@ impl Diagnostic { Some(Range::from_file_range( &workspace.db, FileRange::new(span.file(), span.range()?), + workspace.position_encoding, )) }) } @@ -378,21 +405,31 @@ impl Range { } impl Range { - fn from_file_range(db: &dyn Db, file_range: FileRange) -> Self { + fn from_file_range( + db: &dyn Db, + file_range: FileRange, + position_encoding: PositionEncoding, + ) -> Self { let index = line_index(db.upcast(), file_range.file()); let source = source_text(db.upcast(), file_range.file()); - Self::from_text_range(file_range.range(), &index, &source) + Self::from_text_range(file_range.range(), &index, &source, position_encoding) } fn from_text_range( text_range: ruff_text_size::TextRange, line_index: &LineIndex, source: &str, + position_encoding: PositionEncoding, ) -> Self { Self { - start: Position::from_text_size(text_range.start(), line_index, source), - end: Position::from_text_size(text_range.end(), line_index, source), + start: Position::from_text_size( + text_range.start(), + line_index, + source, + position_encoding, + ), + end: Position::from_text_size(text_range.end(), line_index, source, position_encoding), } } @@ -400,23 +437,19 @@ impl Range { self, line_index: &LineIndex, source: &str, + position_encoding: PositionEncoding, ) -> Result { - let start = self.start.to_text_size(source, line_index)?; - let end = self.end.to_text_size(source, line_index)?; + let start = self + .start + .to_text_size(source, line_index, position_encoding)?; + let end = self + .end + .to_text_size(source, line_index, position_encoding)?; Ok(ruff_text_size::TextRange::new(start, end)) } } -impl From<(LineColumn, LineColumn)> for Range { - fn from((start, end): (LineColumn, LineColumn)) -> Self { - Self { - start: start.into(), - end: end.into(), - } - } -} - #[wasm_bindgen] #[derive(Copy, Clone, Eq, PartialEq, Debug)] pub struct Position { @@ -436,7 +469,12 @@ impl Position { } impl Position { - fn to_text_size(self, text: &str, index: &LineIndex) -> Result { + fn to_text_size( + self, + text: &str, + index: &LineIndex, + position_encoding: PositionEncoding, + ) -> Result { let text_size = index.offset( SourceLocation { line: OneIndexed::new(self.line).ok_or_else(|| { @@ -451,22 +489,22 @@ impl Position { })?, }, text, - PositionEncoding::Utf32, + position_encoding.into(), ); Ok(text_size) } - fn from_text_size(offset: TextSize, line_index: &LineIndex, source: &str) -> Self { - line_index.line_column(offset, source).into() - } -} - -impl From for Position { - fn from(location: LineColumn) -> Self { + fn from_text_size( + offset: TextSize, + line_index: &LineIndex, + source: &str, + position_encoding: PositionEncoding, + ) -> Self { + let location = line_index.source_location(offset, source, position_encoding.into()); Self { line: location.line.get(), - column: location.column.get(), + column: location.character_offset.get(), } } } @@ -506,6 +544,25 @@ impl From for TextRange { } } +#[derive(Default, Copy, Clone)] +#[wasm_bindgen] +pub enum PositionEncoding { + #[default] + Utf8, + Utf16, + Utf32, +} + +impl From for ruff_source_file::PositionEncoding { + fn from(value: PositionEncoding) -> Self { + match value { + PositionEncoding::Utf8 => Self::Utf8, + PositionEncoding::Utf16 => Self::Utf16, + PositionEncoding::Utf32 => Self::Utf32, + } + } +} + #[wasm_bindgen] pub struct LocationLink { /// The target file path diff --git a/crates/red_knot_wasm/tests/api.rs b/crates/red_knot_wasm/tests/api.rs index 65ba79b283..6b6802680b 100644 --- a/crates/red_knot_wasm/tests/api.rs +++ b/crates/red_knot_wasm/tests/api.rs @@ -1,12 +1,16 @@ #![cfg(target_arch = "wasm32")] -use red_knot_wasm::{Position, Workspace}; +use red_knot_wasm::{Position, PositionEncoding, Workspace}; use wasm_bindgen_test::wasm_bindgen_test; #[wasm_bindgen_test] fn check() { - let mut workspace = - Workspace::new("/", js_sys::JSON::parse("{}").unwrap()).expect("Workspace to be created"); + let mut workspace = Workspace::new( + "/", + PositionEncoding::Utf32, + js_sys::JSON::parse("{}").unwrap(), + ) + .expect("Workspace to be created"); workspace .open_file("test.py", "import random22\n") diff --git a/crates/ruff_source_file/src/line_index.rs b/crates/ruff_source_file/src/line_index.rs index 893baa7ef4..73807034c1 100644 --- a/crates/ruff_source_file/src/line_index.rs +++ b/crates/ruff_source_file/src/line_index.rs @@ -643,10 +643,9 @@ impl FromStr for OneIndexed { } } -#[derive(Default, Copy, Clone, Debug)] +#[derive(Copy, Clone, Debug)] pub enum PositionEncoding { /// Character offsets count the number of bytes from the start of the line. - #[default] Utf8, /// Character offsets count the number of UTF-16 code units from the start of the line. diff --git a/playground/knot/src/Playground.tsx b/playground/knot/src/Playground.tsx index c3440a55e6..971d5f9eac 100644 --- a/playground/knot/src/Playground.tsx +++ b/playground/knot/src/Playground.tsx @@ -10,7 +10,7 @@ import { useState, } from "react"; import { ErrorMessage, Header, setupMonaco, useTheme } from "shared"; -import { FileHandle, Workspace } from "red_knot_wasm"; +import { FileHandle, PositionEncoding, Workspace } from "red_knot_wasm"; import { persist, persistLocal, restore } from "./Editor/persist"; import { loader } from "@monaco-editor/react"; import knotSchema from "../../../knot.schema.json"; @@ -30,7 +30,7 @@ export default function Playground() { workspacePromiseRef.current = workspacePromise = startPlayground().then( (fetched) => { setVersion(fetched.version); - const workspace = new Workspace("/", {}); + const workspace = new Workspace("/", PositionEncoding.Utf16, {}); restoreWorkspace(workspace, fetched.workspace, dispatchFiles, setError); setWorkspace(workspace); return workspace;