Skip to content

Commit

Permalink
Use ResolvedPos in ResolvedSpan
Browse files Browse the repository at this point in the history
Summary:
{F1077630638}

Obvious thing to do.

Also may need it later.

Reviewed By: JakobDegen

Differential Revision: D48726302

fbshipit-source-id: 02073e33893f02aaeb25bb967aff63d67574511b
  • Loading branch information
stepancheg authored and facebook-github-bot committed Aug 29, 2023
1 parent d38b039 commit e32b50b
Show file tree
Hide file tree
Showing 12 changed files with 79 additions and 87 deletions.
7 changes: 3 additions & 4 deletions starlark/src/analysis/find_call_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ impl AstModule {
#[cfg(test)]
mod test {

use crate::codemap::ResolvedPos;
use crate::codemap::ResolvedSpan;
use crate::syntax::AstModule;
use crate::syntax::Dialect;
Expand All @@ -89,10 +90,8 @@ def x(name = "foo_name"):

assert_eq!(
Some(ResolvedSpan {
begin_line: 1,
begin_column: 0,
end_line: 1,
end_column: 3
begin: ResolvedPos { line: 1, column: 0 },
end: ResolvedPos { line: 1, column: 3 }
}),
module
.find_function_call_with_name("foo_name")
Expand Down
4 changes: 2 additions & 2 deletions starlark/src/analysis/flow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ def test7():
let mut res = Vec::new();
redundant(&m.codemap, &m.statement, &mut res);
assert_eq!(
res.map(|x| x.location.resolve_span().begin_line),
res.map(|x| x.location.resolve_span().begin.line),
&[3, 9, 19]
);
}
Expand Down Expand Up @@ -520,6 +520,6 @@ def foo():
.collect::<Vec<_>>();
let mut res = Vec::new();
no_effect(&m.codemap, &m.statement, &mut res);
assert_eq!(res.map(|x| x.location.resolve_span().begin_line), bad);
assert_eq!(res.map(|x| x.location.resolve_span().begin.line), bad);
}
}
4 changes: 2 additions & 2 deletions starlark/src/analysis/lint_message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ impl LintMessage {
pub fn new(x: EvalMessage) -> Self {
Self {
path: x.path,
line: x.span.map(|x| x.begin_line + 1),
char: x.span.map(|x| x.begin_column + 1),
line: x.span.map(|x| x.begin.line + 1),
char: x.span.map(|x| x.begin.column + 1),
code: "STARLARK".to_owned(),
severity: x.severity,
name: x.name,
Expand Down
2 changes: 1 addition & 1 deletion starlark/src/assert/conformance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ impl ConformanceTest {
match err.downcast_ref::<Diagnostic>() {
Some(Diagnostic {
span: Some(span), ..
}) => Some(span.resolve_span().begin_line + 1),
}) => Some(span.resolve_span().begin.line + 1),
_ => None,
}
}
Expand Down
10 changes: 5 additions & 5 deletions starlark/src/debug/adapter/implementation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,10 +242,10 @@ fn convert_frame(id: usize, name: String, location: Option<FileSpan>) -> StackFr
};
if let Some(loc) = location {
let span = loc.resolve_span();
s.line = span.begin_line as i64 + 1;
s.column = span.begin_column as i64 + 1;
s.end_line = Some(span.end_line as i64 + 1);
s.end_column = Some(span.end_column as i64 + 1);
s.line = span.begin.line as i64 + 1;
s.column = span.begin.column as i64 + 1;
s.end_line = Some(span.end.line as i64 + 1);
s.end_column = Some(span.end.column as i64 + 1);
s.source = Some(Source {
path: Some(loc.filename().to_owned()),
..Source::default()
Expand Down Expand Up @@ -400,7 +400,7 @@ pub(crate) fn resolve_breakpoints(
let poss: HashMap<usize, FileSpan> = ast
.stmt_locations()
.iter()
.map(|span| (span.resolve_span().begin_line, span.dupe()))
.map(|span| (span.resolve_span().begin.line, span.dupe()))
.collect();
Ok(ResolvedBreakpoints(args.breakpoints.as_ref().map_or(
Vec::new(),
Expand Down
8 changes: 4 additions & 4 deletions starlark/src/lsp/completion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,8 @@ impl<T: LspContext> Backend<T> {
workspace_root: Option<&Path>,
) -> impl Iterator<Item = CompletionItem> {
match document.find_definition_at_location(
function_name_span.begin_line as u32,
function_name_span.begin_column as u32,
function_name_span.begin.line as u32,
function_name_span.begin.column as u32,
) {
Definition::Identifier(identifier) => self
.parameter_name_options_for_identifier_definition(
Expand Down Expand Up @@ -225,8 +225,8 @@ impl<T: LspContext> Backend<T> {
&document.ast.codemap,
&document.ast.statement,
ResolvedPos {
line: destination.begin_line,
column: destination.begin_column,
line: destination.begin.line,
column: destination.begin.column,
},
)
.remove(name)
Expand Down
23 changes: 15 additions & 8 deletions starlark/src/lsp/definition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -526,8 +526,8 @@ impl LspModule {
.and_then(|span| {
let resolved = self.ast.codemap.resolve_span(span);
self.find_definition_at_location(
resolved.begin_line as u32,
resolved.begin_column as u32,
resolved.begin.line as u32,
resolved.begin.column as u32,
)
.local_destination()
})
Expand Down Expand Up @@ -607,6 +607,7 @@ pub(crate) mod helpers {
use super::*;
use crate::codemap::CodeMap;
use crate::codemap::Pos;
use crate::codemap::ResolvedPos;
use crate::codemap::ResolvedSpan;
use crate::codemap::Span;
use crate::syntax::AstModule;
Expand Down Expand Up @@ -706,14 +707,16 @@ pub(crate) mod helpers {
self.ranges
.get(identifier)
.expect("identifier to be present")
.begin_line as u32
.begin
.line as u32
}

pub(crate) fn begin_column(&self, identifier: &str) -> u32 {
self.ranges
.get(identifier)
.expect("identifier to be present")
.begin_column as u32
.begin
.column as u32
}

pub(crate) fn resolved_span(&self, identifier: &str) -> ResolvedSpan {
Expand Down Expand Up @@ -770,10 +773,14 @@ pub(crate) mod helpers {
.into_iter()
.map(|(id, begin_line, begin_column, end_line, end_column)| {
let span = ResolvedSpan {
begin_line,
begin_column,
end_line,
end_column,
begin: ResolvedPos {
line: begin_line,
column: begin_column,
},
end: ResolvedPos {
line: end_line,
column: end_column,
},
};
(id.to_owned(), span)
})
Expand Down
4 changes: 2 additions & 2 deletions starlark/src/lsp/inspect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ impl AstModule {
// Utility function to get the span of a string literal without the quotes.
fn string_span_without_quotes(codemap: &CodeMap, span: Span) -> ResolvedSpan {
let mut span = codemap.resolve_span(span);
span.begin_column += 1;
span.end_column -= 1;
span.begin.column += 1;
span.end.column -= 1;
span
}

Expand Down
14 changes: 7 additions & 7 deletions starlark/src/lsp/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -926,8 +926,8 @@ impl<T: LspContext> Backend<T> {

TextEdit::new(
Range::new(
Position::new(load_span.begin_line as u32, load_span.begin_column as u32),
Position::new(load_span.end_line as u32, load_span.end_column as u32),
Position::new(load_span.begin.line as u32, load_span.begin.column as u32),
Position::new(load_span.end.line as u32, load_span.end.column as u32),
),
format!(
"load(\"{module}\", {})",
Expand All @@ -950,8 +950,8 @@ impl<T: LspContext> Backend<T> {
TextEdit::new(
match last_load {
Some(span) => Range::new(
Position::new(span.end_line as u32, span.end_column as u32),
Position::new(span.end_line as u32, span.end_column as u32),
Position::new(span.end.line as u32, span.end.column as u32),
Position::new(span.end.line as u32, span.end.column as u32),
),
None => Range::new(Position::new(0, 0), Position::new(0, 0)),
},
Expand Down Expand Up @@ -1056,8 +1056,8 @@ impl<T: LspContext> Backend<T> {
&document.ast.codemap,
&document.ast.statement,
ResolvedPos {
line: destination.begin_line,
column: destination.begin_column,
line: destination.begin.line,
column: destination.begin.column,
},
)
.remove(&name)
Expand Down Expand Up @@ -1938,7 +1938,7 @@ mod test {
let bar_resolved_span = bar.resolved_span("bar");
let bar_span_str = format!(
"{}:{}",
bar_resolved_span.begin_column, bar_resolved_span.end_column
bar_resolved_span.begin.column, bar_resolved_span.end.column
);

let foo_contents = dedent(
Expand Down
2 changes: 1 addition & 1 deletion starlark/src/syntax/grammar_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ fail(2)
let ast = assert::parse_ast(content);
match &ast.statement.node {
Stmt::Statements(xs) => {
let lines = xs.map(|x| ast.codemap.resolve_span(x.span).begin_line);
let lines = xs.map(|x| ast.codemap.resolve_span(x.span).begin.line);
assert_eq!(lines, vec![0, 3, 5])
}
_ => panic!("Expected to parse as statements"),
Expand Down
80 changes: 33 additions & 47 deletions starlark_syntax/src/codemap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ impl CodeMap {
}

/// All are 0-based, but print out with 1-based.
#[derive(Copy, Clone, Dupe, Hash, Eq, PartialEq, Debug)]
#[derive(Copy, Clone, Dupe, Hash, Eq, PartialEq, Debug, Default)]
pub struct ResolvedPos {
/// The line number within the file (0-indexed).
pub line: usize,
Expand All @@ -404,6 +404,12 @@ pub struct ResolvedPos {
pub column: usize,
}

impl Display for ResolvedPos {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}:{}", self.line + 1, self.column + 1)
}
}

impl ResolvedPos {
fn _testing_parse(line_col: &str) -> ResolvedPos {
let (line, col) = line_col.split_once(':').unwrap();
Expand Down Expand Up @@ -508,49 +514,32 @@ impl FileSpan {
/// All are 0-based, but print out with 1-based.
#[derive(Debug, Dupe, Clone, Copy, PartialEq, Eq, Hash, Default)]
pub struct ResolvedSpan {
/// 0-based line number of the beginning of the span.
pub begin_line: usize,
/// 0-based character column number of the beginning of the span.
pub begin_column: usize,
/// 0-based line number of the end of the span.
pub end_line: usize,
/// 0-based character column number of the end of the span.
pub end_column: usize,
/// Beginning of the span.
pub begin: ResolvedPos,
/// End of the span.
pub end: ResolvedPos,
}

impl Display for ResolvedSpan {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let single_line = self.begin_line == self.end_line;
let is_empty = single_line && self.begin_column == self.end_column;
let single_line = self.begin.line == self.end.line;
let is_empty = single_line && self.begin.column == self.end.column;

if is_empty {
write!(f, "{}:{}", self.begin_line + 1, self.begin_column + 1)
write!(f, "{}:{}", self.begin.line + 1, self.begin.column + 1)
} else if single_line {
write!(
f,
"{}:{}-{}",
self.begin_line + 1,
self.begin_column + 1,
self.end_column + 1
)
write!(f, "{}-{}", self.begin, self.end.column + 1)
} else {
write!(
f,
"{}:{}-{}:{}",
self.begin_line + 1,
self.begin_column + 1,
self.end_line + 1,
self.end_column + 1
)
write!(f, "{}-{}", self.begin, self.end,)
}
}
}

impl From<ResolvedSpan> for lsp_types::Range {
fn from(span: ResolvedSpan) -> Self {
lsp_types::Range::new(
lsp_types::Position::new(span.begin_line as u32, span.begin_column as u32),
lsp_types::Position::new(span.end_line as u32, span.end_column as u32),
lsp_types::Position::new(span.begin.line as u32, span.begin.column as u32),
lsp_types::Position::new(span.end.line as u32, span.end.column as u32),
)
}
}
Expand All @@ -559,19 +548,14 @@ impl ResolvedSpan {
/// Check that the given position is contained within this span.
/// Includes positions both at the beginning and the end of the range.
pub fn contains(&self, pos: ResolvedPos) -> bool {
(self.begin_line < pos.line
|| (self.begin_line == pos.line && self.begin_column <= pos.column))
&& (self.end_line > pos.line
|| (self.end_line == pos.line && self.end_column >= pos.column))
(self.begin.line < pos.line
|| (self.begin.line == pos.line && self.begin.column <= pos.column))
&& (self.end.line > pos.line
|| (self.end.line == pos.line && self.end.column >= pos.column))
}

fn from_span(begin: ResolvedPos, end: ResolvedPos) -> Self {
Self {
begin_line: begin.line,
begin_column: begin.column,
end_line: end.line,
end_column: end.column,
}
ResolvedSpan { begin, end }
}

fn _testing_parse(span: &str) -> ResolvedSpan {
Expand Down Expand Up @@ -754,10 +738,14 @@ mod tests {
assert_eq!(NativeCodeMap::SOURCE, CODEMAP.source_line(100));
assert_eq!(
ResolvedSpan {
begin_line: 100,
begin_column: 200,
end_line: 100,
end_column: 200 + NativeCodeMap::SOURCE.len(),
begin: ResolvedPos {
line: 100,
column: 200,
},
end: ResolvedPos {
line: 100,
column: 200 + NativeCodeMap::SOURCE.len(),
}
},
CODEMAP.resolve_span(CODEMAP.full_span())
);
Expand All @@ -766,10 +754,8 @@ mod tests {
#[test]
fn test_resolved_span_contains() {
let span = ResolvedSpan {
begin_line: 2,
begin_column: 3,
end_line: 4,
end_column: 5,
begin: ResolvedPos { line: 2, column: 3 },
end: ResolvedPos { line: 4, column: 5 },
};
assert!(!span.contains(ResolvedPos { line: 0, column: 7 }));
assert!(!span.contains(ResolvedPos { line: 2, column: 2 }));
Expand Down
8 changes: 4 additions & 4 deletions starlark_syntax/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,20 +133,20 @@ impl Diagnostic {
// we want the source_span to capture any whitespace ahead of the diagnostic span to
// get the column numbers correct in the DisplayList, and any trailing source code
// on the last line for context.
let first_line_span = span.file.line_span(region.begin_line);
let last_line_span = span.file.line_span(region.end_line);
let first_line_span = span.file.line_span(region.begin.line);
let last_line_span = span.file.line_span(region.end.line);
let source_span = span.span.merge(first_line_span).merge(last_line_span);
let source = span.file.source_span(source_span);

// We want to highlight the span, which needs to be relative to source, and in
// characters.
// Our spans are in terms of bytes, but our resolved spans in terms of characters.
let range_start_chars = region.begin_column;
let range_start_chars = region.begin.column;
let range_len_chars = fast_string::len(span.source_span()).0;

Slice {
source,
line_start: 1 + region.begin_line,
line_start: 1 + region.begin.line,
origin: Some(span.file.filename()),
fold: false,
annotations: vec![SourceAnnotation {
Expand Down

0 comments on commit e32b50b

Please sign in to comment.