Skip to content

Commit

Permalink
JSON Emitter: Use one indexed column numbers for edits (#4007)
Browse files Browse the repository at this point in the history
 I noticed in the byte-offsets refactor that the `JsonEmitter` uses one indexed column numbers for the diagnostic start and end locations but not for `edits`.

This PR changes the `JsonEmitter` to emit one-indexed column numbers for edits, as we already do for `Message::location` and `Message::end_location`.

## Open questions

~We'll need to change the LSP to subtract 1 from the columns in `_parse_fix`~

https://github.com/charliermarsh/ruff-lsp/blob/6e44fadf8a5954adaf3c21af196fa195708ff912/ruff_lsp/server.py#L129-L150

~@charliermarsh is there a way to get the ruff version in that method? If not, then I recommend adding a `version` that we increment whenever we make incompatible changes to the serialized message. We can then use it in the LSP to correctly compute the column offset.~

I'll use the presence of the `Fix::applicability` field to detect if the Ruff version uses one or zero-based column indices.

See astral-sh/ruff-lsp#103
  • Loading branch information
MichaReiser authored May 10, 2023
1 parent 5f64d23 commit 853d835
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 19 deletions.
17 changes: 4 additions & 13 deletions crates/ruff/src/message/json.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use crate::message::{Emitter, EmitterContext, Message};
use crate::registry::AsRule;
use ruff_diagnostics::Edit;
use ruff_python_ast::source_code::{SourceCode, SourceLocation};
use ruff_python_ast::source_code::SourceCode;
use serde::ser::SerializeSeq;
use serde::{Serialize, Serializer};
use serde_json::{json, Value};
use serde_json::json;
use std::io::Write;

#[derive(Default)]
Expand Down Expand Up @@ -79,12 +79,10 @@ impl Serialize for ExpandedEdits<'_> {
let mut s = serializer.serialize_seq(Some(self.edits.len()))?;

for edit in self.edits {
let start_location = self.source_code.source_location(edit.start());
let end_location = self.source_code.source_location(edit.end());
let value = json!({
"content": edit.content().unwrap_or_default(),
"location": to_zero_indexed_column(&start_location),
"end_location": to_zero_indexed_column(&end_location)
"location": self.source_code.source_location(edit.start()),
"end_location": self.source_code.source_location(edit.end())
});

s.serialize_element(&value)?;
Expand All @@ -94,13 +92,6 @@ impl Serialize for ExpandedEdits<'_> {
}
}

fn to_zero_indexed_column(location: &SourceLocation) -> Value {
json!({
"row": location.row,
"column": location.column.to_zero_indexed()
})
}

#[cfg(test)]
mod tests {
use crate::message::tests::{capture_emitter_output, create_messages};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ expression: content
"content": "",
"location": {
"row": 1,
"column": 0
"column": 1
},
"end_location": {
"row": 2,
"column": 0
"column": 1
}
}
]
Expand All @@ -45,11 +45,11 @@ expression: content
"content": "",
"location": {
"row": 6,
"column": 4
"column": 5
},
"end_location": {
"row": 6,
"column": 9
"column": 10
}
}
]
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_cli/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,11 @@ fn test_stdin_json() -> Result<()> {
"content": "",
"location": {{
"row": 1,
"column": 0
"column": 1
}},
"end_location": {{
"row": 2,
"column": 0
"column": 1
}}
}}
]
Expand Down

0 comments on commit 853d835

Please sign in to comment.