Skip to content

Commit 1bdb22c

Browse files
MichaReiserMichaReiser
authored
[red-knot] Fix offset handling in playground for 2-code-point UTF16 characters (astral-sh#17520)
1 parent 1c65e0a commit 1bdb22c

File tree

4 files changed

+101
-41
lines changed

4 files changed

+101
-41
lines changed

‎crates/red_knot_wasm/src/lib.rs‎

Lines changed: 91 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use ruff_db::system::{
1919
use ruff_db::Upcast;
2020
use ruff_notebook::Notebook;
2121
use ruff_python_formatter::formatted_file;
22-
use ruff_source_file::{LineColumn, LineIndex, OneIndexed, PositionEncoding, SourceLocation};
22+
use ruff_source_file::{LineIndex, OneIndexed, SourceLocation};
2323
use ruff_text_size::{Ranged, TextSize};
2424
use wasm_bindgen::prelude::*;
2525

@@ -42,13 +42,18 @@ pub fn run() {
4242
#[wasm_bindgen]
4343
pub struct Workspace {
4444
db: ProjectDatabase,
45+
position_encoding: PositionEncoding,
4546
system: WasmSystem,
4647
}
4748

4849
#[wasm_bindgen]
4950
impl Workspace {
5051
#[wasm_bindgen(constructor)]
51-
pub fn new(root: &str, options: JsValue) -> Result<Workspace, Error> {
52+
pub fn new(
53+
root: &str,
54+
position_encoding: PositionEncoding,
55+
options: JsValue,
56+
) -> Result<Workspace, Error> {
5257
let options = Options::deserialize_with(
5358
ValueSource::Cli,
5459
serde_wasm_bindgen::Deserializer::from(options),
@@ -62,7 +67,11 @@ impl Workspace {
6267

6368
let db = ProjectDatabase::new(project, system.clone()).map_err(into_error)?;
6469

65-
Ok(Self { db, system })
70+
Ok(Self {
71+
db,
72+
position_encoding,
73+
system,
74+
})
6675
}
6776

6877
#[wasm_bindgen(js_name = "updateOptions")]
@@ -216,13 +225,18 @@ impl Workspace {
216225
let source = source_text(&self.db, file_id.file);
217226
let index = line_index(&self.db, file_id.file);
218227

219-
let offset = position.to_text_size(&source, &index)?;
228+
let offset = position.to_text_size(&source, &index, self.position_encoding)?;
220229

221230
let Some(targets) = goto_type_definition(&self.db, file_id.file, offset) else {
222231
return Ok(Vec::new());
223232
};
224233

225-
let source_range = Range::from_text_range(targets.file_range().range(), &index, &source);
234+
let source_range = Range::from_text_range(
235+
targets.file_range().range(),
236+
&index,
237+
&source,
238+
self.position_encoding,
239+
);
226240

227241
let links: Vec<_> = targets
228242
.into_iter()
@@ -231,10 +245,12 @@ impl Workspace {
231245
full_range: Range::from_file_range(
232246
&self.db,
233247
FileRange::new(target.file(), target.full_range()),
248+
self.position_encoding,
234249
),
235250
selection_range: Some(Range::from_file_range(
236251
&self.db,
237252
FileRange::new(target.file(), target.focus_range()),
253+
self.position_encoding,
238254
)),
239255
origin_selection_range: Some(source_range),
240256
})
@@ -248,13 +264,18 @@ impl Workspace {
248264
let source = source_text(&self.db, file_id.file);
249265
let index = line_index(&self.db, file_id.file);
250266

251-
let offset = position.to_text_size(&source, &index)?;
267+
let offset = position.to_text_size(&source, &index, self.position_encoding)?;
252268

253269
let Some(range_info) = hover(&self.db, file_id.file, offset) else {
254270
return Ok(None);
255271
};
256272

257-
let source_range = Range::from_text_range(range_info.file_range().range(), &index, &source);
273+
let source_range = Range::from_text_range(
274+
range_info.file_range().range(),
275+
&index,
276+
&source,
277+
self.position_encoding,
278+
);
258279

259280
Ok(Some(Hover {
260281
markdown: range_info
@@ -272,14 +293,19 @@ impl Workspace {
272293
let result = inlay_hints(
273294
&self.db,
274295
file_id.file,
275-
range.to_text_range(&index, &source)?,
296+
range.to_text_range(&index, &source, self.position_encoding)?,
276297
);
277298

278299
Ok(result
279300
.into_iter()
280301
.map(|hint| InlayHint {
281302
markdown: hint.display(&self.db).to_string(),
282-
position: Position::from_text_size(hint.position, &index, &source),
303+
position: Position::from_text_size(
304+
hint.position,
305+
&index,
306+
&source,
307+
self.position_encoding,
308+
),
283309
})
284310
.collect())
285311
}
@@ -348,6 +374,7 @@ impl Diagnostic {
348374
Some(Range::from_file_range(
349375
&workspace.db,
350376
FileRange::new(span.file(), span.range()?),
377+
workspace.position_encoding,
351378
))
352379
})
353380
}
@@ -378,45 +405,51 @@ impl Range {
378405
}
379406

380407
impl Range {
381-
fn from_file_range(db: &dyn Db, file_range: FileRange) -> Self {
408+
fn from_file_range(
409+
db: &dyn Db,
410+
file_range: FileRange,
411+
position_encoding: PositionEncoding,
412+
) -> Self {
382413
let index = line_index(db.upcast(), file_range.file());
383414
let source = source_text(db.upcast(), file_range.file());
384415

385-
Self::from_text_range(file_range.range(), &index, &source)
416+
Self::from_text_range(file_range.range(), &index, &source, position_encoding)
386417
}
387418

388419
fn from_text_range(
389420
text_range: ruff_text_size::TextRange,
390421
line_index: &LineIndex,
391422
source: &str,
423+
position_encoding: PositionEncoding,
392424
) -> Self {
393425
Self {
394-
start: Position::from_text_size(text_range.start(), line_index, source),
395-
end: Position::from_text_size(text_range.end(), line_index, source),
426+
start: Position::from_text_size(
427+
text_range.start(),
428+
line_index,
429+
source,
430+
position_encoding,
431+
),
432+
end: Position::from_text_size(text_range.end(), line_index, source, position_encoding),
396433
}
397434
}
398435

399436
fn to_text_range(
400437
self,
401438
line_index: &LineIndex,
402439
source: &str,
440+
position_encoding: PositionEncoding,
403441
) -> Result<ruff_text_size::TextRange, Error> {
404-
let start = self.start.to_text_size(source, line_index)?;
405-
let end = self.end.to_text_size(source, line_index)?;
442+
let start = self
443+
.start
444+
.to_text_size(source, line_index, position_encoding)?;
445+
let end = self
446+
.end
447+
.to_text_size(source, line_index, position_encoding)?;
406448

407449
Ok(ruff_text_size::TextRange::new(start, end))
408450
}
409451
}
410452

411-
impl From<(LineColumn, LineColumn)> for Range {
412-
fn from((start, end): (LineColumn, LineColumn)) -> Self {
413-
Self {
414-
start: start.into(),
415-
end: end.into(),
416-
}
417-
}
418-
}
419-
420453
#[wasm_bindgen]
421454
#[derive(Copy, Clone, Eq, PartialEq, Debug)]
422455
pub struct Position {
@@ -436,7 +469,12 @@ impl Position {
436469
}
437470

438471
impl Position {
439-
fn to_text_size(self, text: &str, index: &LineIndex) -> Result<TextSize, Error> {
472+
fn to_text_size(
473+
self,
474+
text: &str,
475+
index: &LineIndex,
476+
position_encoding: PositionEncoding,
477+
) -> Result<TextSize, Error> {
440478
let text_size = index.offset(
441479
SourceLocation {
442480
line: OneIndexed::new(self.line).ok_or_else(|| {
@@ -451,22 +489,22 @@ impl Position {
451489
})?,
452490
},
453491
text,
454-
PositionEncoding::Utf32,
492+
position_encoding.into(),
455493
);
456494

457495
Ok(text_size)
458496
}
459497

460-
fn from_text_size(offset: TextSize, line_index: &LineIndex, source: &str) -> Self {
461-
line_index.line_column(offset, source).into()
462-
}
463-
}
464-
465-
impl From<LineColumn> for Position {
466-
fn from(location: LineColumn) -> Self {
498+
fn from_text_size(
499+
offset: TextSize,
500+
line_index: &LineIndex,
501+
source: &str,
502+
position_encoding: PositionEncoding,
503+
) -> Self {
504+
let location = line_index.source_location(offset, source, position_encoding.into());
467505
Self {
468506
line: location.line.get(),
469-
column: location.column.get(),
507+
column: location.character_offset.get(),
470508
}
471509
}
472510
}
@@ -506,6 +544,25 @@ impl From<ruff_text_size::TextRange> for TextRange {
506544
}
507545
}
508546

547+
#[derive(Default, Copy, Clone)]
548+
#[wasm_bindgen]
549+
pub enum PositionEncoding {
550+
#[default]
551+
Utf8,
552+
Utf16,
553+
Utf32,
554+
}
555+
556+
impl From<PositionEncoding> for ruff_source_file::PositionEncoding {
557+
fn from(value: PositionEncoding) -> Self {
558+
match value {
559+
PositionEncoding::Utf8 => Self::Utf8,
560+
PositionEncoding::Utf16 => Self::Utf16,
561+
PositionEncoding::Utf32 => Self::Utf32,
562+
}
563+
}
564+
}
565+
509566
#[wasm_bindgen]
510567
pub struct LocationLink {
511568
/// The target file path

‎crates/red_knot_wasm/tests/api.rs‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,16 @@
11
#![cfg(target_arch = "wasm32")]
22

3-
use red_knot_wasm::{Position, Workspace};
3+
use red_knot_wasm::{Position, PositionEncoding, Workspace};
44
use wasm_bindgen_test::wasm_bindgen_test;
55

66
#[wasm_bindgen_test]
77
fn check() {
8-
let mut workspace =
9-
Workspace::new("/", js_sys::JSON::parse("{}").unwrap()).expect("Workspace to be created");
8+
let mut workspace = Workspace::new(
9+
"/",
10+
PositionEncoding::Utf32,
11+
js_sys::JSON::parse("{}").unwrap(),
12+
)
13+
.expect("Workspace to be created");
1014

1115
workspace
1216
.open_file("test.py", "import random22\n")

‎crates/ruff_source_file/src/line_index.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -643,10 +643,9 @@ impl FromStr for OneIndexed {
643643
}
644644
}
645645

646-
#[derive(Default, Copy, Clone, Debug)]
646+
#[derive(Copy, Clone, Debug)]
647647
pub enum PositionEncoding {
648648
/// Character offsets count the number of bytes from the start of the line.
649-
#[default]
650649
Utf8,
651650

652651
/// Character offsets count the number of UTF-16 code units from the start of the line.

‎playground/knot/src/Playground.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
useState,
1111
} from "react";
1212
import { ErrorMessage, Header, setupMonaco, useTheme } from "shared";
13-
import { FileHandle, Workspace } from "red_knot_wasm";
13+
import { FileHandle, PositionEncoding, Workspace } from "red_knot_wasm";
1414
import { persist, persistLocal, restore } from "./Editor/persist";
1515
import { loader } from "@monaco-editor/react";
1616
import knotSchema from "../../../knot.schema.json";
@@ -30,7 +30,7 @@ export default function Playground() {
3030
workspacePromiseRef.current = workspacePromise = startPlayground().then(
3131
(fetched) => {
3232
setVersion(fetched.version);
33-
const workspace = new Workspace("/", {});
33+
const workspace = new Workspace("/", PositionEncoding.Utf16, {});
3434
restoreWorkspace(workspace, fetched.workspace, dispatchFiles, setError);
3535
setWorkspace(workspace);
3636
return workspace;

0 commit comments

Comments
 (0)